Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: restricted permissions for permission management [WD-18906] #1113

Merged
merged 22 commits into from
Feb 24, 2025
Merged

feat: restricted permissions for permission management [WD-18906] #1113

merged 22 commits into from
Feb 24, 2025

Conversation

mas-who
Copy link
Contributor

@mas-who mas-who commented Feb 17, 2025

Done

  • Graceful handling of permission management in the UI
  • covered actions documented in the permissions sheet

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • Check through the list of actions documented here

@webteam-app
Copy link

Demo

Jenkins

demos.haus

@mas-who mas-who force-pushed the restricted-permissions-auth branch 3 times, most recently from ebe8130 to 3687eaf Compare February 20, 2025 08:05
@mas-who mas-who force-pushed the restricted-permissions-auth branch 2 times, most recently from c4a1f43 to 85efee8 Compare February 20, 2025 10:17
edlerd
edlerd reviewed Feb 20, 2025
View reviewed changes
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some preliminary review comments, read through about half the changes.

@mas-who mas-who force-pushed the restricted-permissions-auth branch 5 times, most recently from 1a7e717 to 5bc8f09 Compare February 21, 2025 08:14
edlerd
edlerd reviewed Feb 21, 2025
View reviewed changes
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of issues described below.

@mas-who mas-who force-pushed the restricted-permissions-auth branch 2 times, most recently from c6e0d52 to 654ed93 Compare February 21, 2025 14:37
@mas-who mas-who force-pushed the restricted-permissions-auth branch from 654ed93 to 83bf2f9 Compare February 21, 2025 14:57
@mas-who mas-who requested a review from edlerd February 21, 2025 14:57
@mas-who mas-who force-pushed the restricted-permissions-auth branch from 83bf2f9 to 24de60b Compare February 21, 2025 15:28
edlerd
edlerd requested changes Feb 21, 2025
View reviewed changes
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments below. Also found a couple of QA issues on IDP groups.

@MasWho
feat: disbale create tls identity button when user has no permission
c8ef858 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: enable entitlement queries for identities
845f76e 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable edit identity groups for restricted permissions
ef0613d 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable delete identity button when the identity is restricted
5a5fa52 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable bulk edit identity groups button when user has no permi…
773c8a9 
…ssion

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: show restricted identities in bulk delete confirmation modal
7a230e8 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: enable group entitlements query
5c1bf87 
Signed-off-by: Mason Hu <[email protected]>
MasWho added 12 commits February 24, 2025 09:33
@MasWho
feat: show warning about groups with restricted permissions when bulk…
eff8daa 
… deleting

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: don't allow group identities bulk edits if the user doesn't hav…
8e2ac2a 
…e the required permissions

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable bulk delete identities button when user has no permissi…
8361305 
…on to delete any of the selected identities

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable bulk delete group button if user does not have permissi…
79354c6 
…on to delete all selected groups

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable rows in bulk identity edit panel for groups with restri…
15e2c3e 
…cted permissions

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable rows in edit group identities panel for restricted iden…
5f5e843 
…tities

Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable group permission editing for restricted permissions
98b4b1d 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: enable idp group entitlement query
d80b4dd 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable create idp groups for restricted permissions
0f5afe6 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable bulk delete idp group button with restricted permissions
cd4e3c1 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable edit IDP group button for restricted permissions
2f21a12 
Signed-off-by: Mason Hu <[email protected]>
@MasWho
feat: disable delete idp group button for restricted permissions
445950c 
Signed-off-by: Mason Hu <[email protected]>
@mas-who mas-who force-pushed the restricted-permissions-auth branch 3 times, most recently from 60c08eb to 78fd8e1 Compare February 24, 2025 09:07
@mas-who mas-who requested a review from edlerd February 24, 2025 09:07
edlerd
edlerd requested changes Feb 24, 2025
View reviewed changes
@mas-who mas-who force-pushed the restricted-permissions-auth branch 2 times, most recently from d02797d to ab3fecb Compare February 24, 2025 15:45
@mas-who mas-who requested a review from edlerd February 24, 2025 15:47
edlerd
edlerd reviewed Feb 24, 2025
View reviewed changes
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA issues seem all resolved, thanks :)

Two tiny code improvement suggestions below, then this should be good to merge.

@MasWho
fix: misc fixes
01995eb 
Signed-off-by: Mason Hu <[email protected]>
@mas-who mas-who force-pushed the restricted-permissions-auth branch from ab3fecb to 01995eb Compare February 24, 2025 16:43
edlerd
edlerd approved these changes Feb 24, 2025
View reviewed changes
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff, thanks for fixing this and communicating the discovered bugs to get them fixed upstream.

@mas-who mas-who merged commit 8bc7e81 into canonical:main Feb 24, 2025
11 checks passed
@mas-who mas-who mentioned this pull request Feb 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edlerd edlerd edlerd approved these changes

@markylaing markylaing markylaing left review comments

Assignees
No one assigned
Labels
Review: Code +1 Review: QA +1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mas-who @webteam-app @edlerd @markylaing @MasWho