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.

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

Add view_identities permission #1787

Merged
merged 9 commits into from
Jan 3, 2023
Merged

Conversation

kyle-ssg
Copy link
Member

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?

Changes

Assuming the view_identities_permission feature is enabled:

  • Users with VIEW_IDENTITIES can view the users aside nav
  • Users with MANAGE_IDENTITIES can delete an identity, it will be disabled for VIEW_IDENTITIES permissions
  • Users with VIEW_IDENTITIES can see a read only view of the identity detail page
  • Users with MANAGE_IDENTITIES can edit a users feature states, this used to be based on UPDATE_FEATURE_STATE

@kyle-ssg kyle-ssg linked an issue Dec 19, 2022 that may be closed by this pull request
@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2022

Uffizzi Preview deployment-9339 was deleted.

matthewelwell
matthewelwell previously approved these changes Dec 22, 2022
@matthewelwell
Copy link
Contributor

It seems like this has had an effect on users with VIEW_ENVIRONMENT permissions from viewing Change Requests?

image

To reproduce:

  • Create a user with VIEW_PROJECT and VIEW_ENVIRONMENT permissions
  • Login with that user and try to view the Change Requests page

This might be unrelated to this PR but I've tested and it doesn't happen in production or staging.

@matthewelwell
Copy link
Contributor

The above is unrelated to this PR - it's caused by the fact that the preview environment is built without the change request views (as they are contained in a separate private repository and bundled into the application at build time for SaaS / private cloud build workflows).

@matthewelwell
Copy link
Contributor

matthewelwell commented Dec 22, 2022

In testing this, I've noticed a few issues:

  • Tooltips for delete identity and 'Add New Trait' buttons show that the user needs the 'update feature state' permission, rather than 'manage identities' (screenshots below)
  • Tooltip for 'Create Users' button shows that the user needs 'Admin' for the environment but should show 'manage identities' (screenshot below)
  • When an identity has no traits, I am able to click on the 'Add New Trait' button (note: this is not the case when the identity already has traits). It looks like this works too because the FE is using the SDK endpoint rather than the admin endpoint.
  • Delete trait button is clickable and tries to delete the trait but fails silently with a 403 error on the API (which is correct)

image

image

image

# Conflicts:
#	frontend/web/components/modals/CreateFlag.js
#	frontend/web/components/pages/UserPage.js
#	frontend/web/components/pages/UsersPage.js
…ith/flagsmith into chores/view_identities_permission
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

Approved but I've created #1819 to track issue related to point 3 above.

@kyle-ssg kyle-ssg merged commit 8b8c6da into main Jan 3, 2023
@kyle-ssg kyle-ssg deleted the chores/view_identities_permission branch January 3, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have read-only permission for 'users'
2 participants