-
Notifications
You must be signed in to change notification settings - Fork 370
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
fix: [M3-7967] - Returning proper scope when selecting all perms #10359
Conversation
} | ||
|
||
const scopeIsForVPC = scopeTup[0] === 'vpc'; | ||
{allPermissions.map((scopeTup) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scopeTup
- was wondering what it meant at first EDIT: This is whole thing is a bit hard to read because of naming conventions. naming it a tuple is ok-ish wondering if we can come up with something better.
also:
const something = renamedScopeTup[0]
const somethingElse = renamedScopeTup[1]
or similar to improve readability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it means "scope tuple", but agreed that this section of the code base isn't the most readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be a follow up since this is a hotfix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted for followup ✏️
(scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access' | ||
); | ||
isChildAccountAccessRestricted || | ||
!flags.parentChildAccountAccess; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same logic as CreateAPITokenDrawer https://github.com/linode/manager/pull/10359/files#diff-d179f0d0f9fc3644a265f97bc10ba57f5e177c4c96b9960e7b9f779bb7c724aeR204-R207
can we keep things dry and util this logic?
Coverage Report: ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on my normal personal account, a parent account, and a restricted parent account. Everything worked as expected! ✅
e53e629
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ In a normal development account (non-parent/child), create a PAT with all scopes, and observed response has scopes: "*"
✅ Observed that child account access is NOT visible
✅ In a parent user account (unrestricted access), create a PAT with all scopes, and observed response has scopes: "*"
✅ Observe that child account access is visible.
✅ In a parent user account (restricted access with no child account access grant), create a PAT with all scopes, and observe response has scopes: "*"
✅ Observe that child account access is NOT visible.
Confirmed that child_account_access is showing up in the View Account Access drawer as expected, as well.
Description 📝
The Cloud Manager isn't working right when making tokens that should have access to everything. Even when you choose 'all' for the token's access level, it's mistakenly giving limited access (even though the scopes are all included). Also, when making tokens using the API and setting access to * (which should mean everything), it should reply with * to show that. But, it's listing out each access area one by one instead of just using *, which is not what's supposed to happen.
Changes 🔄
child account access
since API is in prodTarget release date 🗓️
4/8-9/2024 (Hotfix)
Preview 📷
Before:
After:
How to test 🧪
Prerequisites
Reproduction steps
scopes: "*"
Verification steps
scopes: "*"
child account access
is NOT visiblescopes: "*"
child account access
is visible.scopes: "*"
child account access
is NOT visible.As an Author I have considered 🤔
Check all that apply