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

ref(token): Update create user token page to use dropdowns #54651

Merged
merged 7 commits into from
Sep 21, 2023
Merged

ref(token): Update create user token page to use dropdowns #54651

merged 7 commits into from
Sep 21, 2023

Conversation

schew2381
Copy link
Member

@schew2381 schew2381 commented Aug 11, 2023 •
edited
Loading

Requires #56537

Summary

Refactor the create user token page to use existing dropdowns (like org/integration tokens page). This enforces hierarchy automatically when creating user tokens through the UI.

Scopes

Originally you could select scopes granularly, so you could check project:admin but not project:read. The dropdowns will include all weaker scopes for the same resource you're selecting. Ideally we would call each option in the dropdown:

Ideal          Current
- read         - read
- write   vs.  - read + write
- admin        - admin 
Note: Admin implies access to everything so includes read + write + any other special scope for that resource
e.g. project:releases/org:integrations

and explain the hierarchy in the header description with a link to our scope docs. However we still need to overhaul our scope docs so this change will have to come after the docs are complete.

Before

Screenshot 2023-08-11 at 3 15 13 PM

After

Screen.Recording.2023-09-19.at.2.13.03.PM.mov

@schew2381 schew2381 self-assigned this Aug 11, 2023
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 11, 2023
@schew2381 schew2381 mentioned this pull request Aug 10, 2023
Merged
@schew2381 schew2381 changed the title Ref(api): Update create user token page to use dropdowns Ref(tokens): Update create user token page to use dropdowns Aug 11, 2023
@schew2381 schew2381 changed the title Ref(tokens): Update create user token page to use dropdowns Ref(token): Update create user token page to use dropdowns Aug 11, 2023
@schew2381 schew2381 changed the title Ref(token): Update create user token page to use dropdowns ref(token): Update create user token page to use dropdowns Aug 11, 2023
@schew2381 schew2381 force-pushed the seiji/ref/update-user-token-creation-page branch from d681355 to a10d361 Compare September 18, 2023 19:02
@schew2381 schew2381 force-pushed the seiji/ref/update-user-token-creation-page branch from a10d361 to 2c428b1 Compare September 19, 2023 20:04
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 19, 2023
@schew2381 schew2381 removed the Scope: Backend Automatically applied to PRs that change backend components label Sep 19, 2023
@getsentry getsentry deleted a comment from github-actions bot Sep 19, 2023
@schew2381 schew2381 requested a review from a team September 19, 2023 22:04
@schew2381 schew2381 marked this pull request as ready for review September 19, 2023 22:09
cathteng
cathteng reviewed Sep 19, 2023
View reviewed changes
static/app/views/settings/account/apiNewToken.tsx Outdated Show resolved Hide resolved
static/app/views/settings/account/apiNewToken.tsx Outdated Show resolved Hide resolved
tests/acceptance/test_api.py Outdated Show resolved Hide resolved
static/app/views/settings/account/apiNewToken.spec.tsx Outdated Show resolved Hide resolved
@schew2381 schew2381 mentioned this pull request Sep 20, 2023
Merged
@schew2381 schew2381 requested a review from a team as a code owner September 20, 2023 18:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 20, 2023
@schew2381 schew2381 force-pushed the seiji/ref/update-user-token-creation-page branch from 8a66fb7 to 872675f Compare September 20, 2023 18:18
@getsentry getsentry deleted a comment from github-actions bot Sep 20, 2023
@schew2381 schew2381 added Scope: Frontend Automatically applied to PRs that change frontend components and removed Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Sep 20, 2023
schew2381 added a commit that referenced this pull request Sep 20, 2023
@schew2381
chore(api): Delete acceptance test for user tokens (#56537)
26cd230 
This test must be deleted so
#54651 can pass. It's not
possible to click the Create Button anymore after switching to
dropdowns. Also, this is only testing that an API is called, so it's not
very useful.
@schew2381 schew2381 enabled auto-merge (squash) September 21, 2023 18:16
@schew2381 schew2381 merged commit 28ca6ff into master Sep 21, 2023
41 checks passed
@schew2381 schew2381 deleted the seiji/ref/update-user-token-creation-page branch September 21, 2023 18:38
@schew2381 schew2381 mentioned this pull request Sep 21, 2023
Merged
michellewzhang pushed a commit that referenced this pull request Sep 21, 2023
@schew2381 @michellewzhang
chore(api): Delete acceptance test for user tokens (#56537)
80a611b 
This test must be deleted so
#54651 can pass. It's not
possible to click the Create Button anymore after switching to
dropdowns. Also, this is only testing that an API is called, so it's not
very useful.
michellewzhang pushed a commit that referenced this pull request Sep 21, 2023
@schew2381 @michellewzhang
ref(token): Update create user token page to use dropdowns (#54651)
3d2111c 
Requires #56537

### Summary
Refactor the create user token page to use existing dropdowns (like
[org/integration tokens
page](https://santry.sentry.io/settings/developer-settings/new-internal/)).
This enforces hierarchy automatically when creating user tokens through
the UI.

### Scopes
Originally you could select scopes granularly, so you could check
`project:admin` but not `project:read`. The dropdowns will include all
weaker scopes for the same resource you're selecting. Ideally we would
call each option in the dropdown:
```mdx
Ideal          Current
- read         - read
- write   vs.  - read + write
- admin        - admin 
Note: Admin implies access to everything so includes read + write + any other special scope for that resource
e.g. project:releases/org:integrations
```
and explain the hierarchy in the header description with a link to our
scope docs. However we still need to overhaul our scope docs so this
change will have to come after the docs are complete.

### Before
<img width="1267" alt="Screenshot 2023-08-11 at 3 15 13 PM"
src="https://github.com/getsentry/sentry/assets/67301797/9e590eff-f839-4678-ab55-ac41610fdc8a">

### After


https://github.com/getsentry/sentry/assets/67301797/f0c1c730-ad51-4977-8b0c-0b026cf3d8b4
schew2381 added a commit to getsentry/sentry-docs that referenced this pull request Sep 21, 2023
@schew2381
fix(docs): Match user token update and prioritize internal integratio…
1586111 
…ns (#7918)

### Background
As part of the change to [Introduce Scope Hierarchy](https://www.notion.so/sentry/24Q3-Permission-Scope-Hierarchy-67620f99783345bbb8bc2828b1addc64), I edited the page where you create user auth tokens to use the same dropdowns as internal integrations in getsentry/sentry#54651. You can test this for yourself [here](https://sentry.io/settings/account/api/auth-tokens/new-token/).

### Changes
1. Clarify that user auth tokens are not editable after they are created. In order to change the scopes on them, you must create a new token.
2. Replace old picture of the create user auth page from box selectors to the new dropdowns
3. I moved user auth tokens to below integration tokens to hopefully have people favor integration > user tokens. In the future, we will probably look into eventually deprecating user auth tokens in favor of the other 2 tokens.
4. For the internal integration, I changed the wording from 
`See our docs on Internal Integrations to learn more.`
to
`To get started, see our docs on Internal Integrations`
because it wasn't immediately clear to me which link in that section leads to the steps to create the integration.
schew2381 added a commit that referenced this pull request Sep 26, 2023
@schew2381
feat(scopes): Enforce scope hierarchy (#54510)
647704b 
### Note
- User Token UI Changes should be merged beforehand
#54651
- See[ Notion
doc](https://www.notion.so/sentry/WIP-Permission-Scope-Hierarchy-67620f99783345bbb8bc2828b1addc64)
for detailed spec on this work

### Implementation
To model the hierarchy I used a simple dict of scope -> all granted
scopes. I chose this approach b/c it's very simple and easy to enforce
in the code.

We use a pre-save signal on the `ApiKey` and `ApiToken` models to
enforce scope hierarchy. The basic scope hierarchy for each resource is:
- read grants nothing
- write grants read
- admin grants read+write

When updating one of these models, there is no way to know if the scopes
are enforced without fetching it from the DB. To avoid this trip, we
always iterate through the hierarchy mapping.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cathteng cathteng cathteng approved these changes

@sentaur-athena sentaur-athena Awaiting requested review from sentaur-athena

Assignees

@schew2381 schew2381

Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schew2381 @cathteng