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

fix: [M3-7967] - Returning proper scope when selecting all perms #10359

Merged
merged 3 commits into from
Apr 8, 2024
Merged

fix: [M3-7967] - Returning proper scope when selecting all perms #10359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53e6047
fix: [M3-7967] - Returning proper scope when selecting all perms
jaalah Apr 8, 2024
bb7ce02
Merge branch 'staging' of github.com:linode/manager into M3-7967
jaalah Apr 8, 2024
e53e629
Update changelog
jaalah Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/manager/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).

## [2024-04-05] - v1.116.1
## [2024-04-08] - v1.116.1

### Fixed:

- Search indefinitely loading on large accounts ([#10351](https://github.com/linode/manager/pull/10351))
- Returning proper scope when selecting all perms ([#10359](https://github.com/linode/manager/pull/10359))

## [2024-04-01] - v1.116.0

Expand Down
152 changes: 66 additions & 86 deletions packages/manager/src/features/Profile/APITokens/CreateAPITokenDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ import {
StyledSelectCell,
} from './APITokenDrawer.styles';
import {
basePermNameMap as _basePermNameMap,
Permission,
allScopesAreTheSame,
filterPermsNameMap,
basePermNameMap,
permTuplesToScopeString,
scopeStringToPermTuples,
} from './utils';
Expand Down Expand Up @@ -103,8 +102,6 @@ export const CreateAPITokenDrawer = (props: Props) => {

const { data: profile } = useProfile();

const isParentUser = profile?.user_type === 'parent';

const {
error,
isLoading,
Expand All @@ -115,17 +112,6 @@ export const CreateAPITokenDrawer = (props: Props) => {
globalGrantType: 'child_account_access',
});

const hasParentChildAccountAccess = Boolean(flags.parentChildAccountAccess);

// @TODO: Parent/Child - once in GA, remove _basePermNameMap logic and references.
// Just use the basePermNameMap import directly w/o any manipulation.
const basePermNameMap = filterPermsNameMap(_basePermNameMap, [
{
name: 'child_account',
shouldBeIncluded: hasParentChildAccountAccess,
},
]);

const form = useFormik<{
expiry: string;
label: string;
Expand Down Expand Up @@ -164,10 +150,7 @@ export const CreateAPITokenDrawer = (props: Props) => {
e: React.SyntheticEvent<RadioButton>
): void => {
const value = +e.currentTarget.value;
const newScopes = (showFilteredPermissions
? filteredPermissions
: allPermissions
).map(
const newScopes = form.values.scopes.map(
(scope): Permission => {
// Check the excluded scopes object to see if the current scope will have its own defaults.
const indexOfExcludedScope = excludedScopesFromSelectAll.findIndex(
Expand Down Expand Up @@ -217,16 +200,11 @@ export const CreateAPITokenDrawer = (props: Props) => {
// Filter permissions for all users except parent user accounts.
const allPermissions = form.values.scopes;

// Filter permissions for all users *except* parent user accounts with access to child accounts enabled.
const showFilteredPermissions =
!flags.parentChildAccountAccess ||
(flags.parentChildAccountAccess &&
(!isParentUser || isChildAccountAccessRestricted));

const filteredPermissions = allPermissions.filter(
// @TODO: Parent/Child - Once feature is released and all perms are always returned, use basePermNameMap[scopeTup[0]] !== 'Child Account Access'.
(scopeTup) => scopeTup[0] !== 'child_account'
);
// Visually hide the "Child Account Access" permission even though it's still part of the base perms.
const hideChildAccountAccessScope =
profile?.user_type !== 'parent' ||
isChildAccountAccessRestricted ||
!flags.parentChildAccountAccess;

return (
<Drawer onClose={onClose} open={open} title="Add Personal Access Token">
Expand Down Expand Up @@ -312,65 +290,67 @@ export const CreateAPITokenDrawer = (props: Props) => {
/>
</StyledPermissionsCell>
</TableRow>
{(showFilteredPermissions ? filteredPermissions : allPermissions).map(
(scopeTup) => {
if (!basePermNameMap[scopeTup[0]]) {
return null;
}

const scopeIsForVPC = scopeTup[0] === 'vpc';
{allPermissions.map((scopeTup) => {
Copy link
Contributor

@abailly-akamai abailly-akamai Apr 8, 2024 •
edited
Loading

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?

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted for followup ✏️

if (
!basePermNameMap[scopeTup[0]] ||
(hideChildAccountAccessScope &&
basePermNameMap[scopeTup[0]] === 'Child Account Access')
) {
return null;
}

return (
<TableRow
data-qa-row={basePermNameMap[scopeTup[0]]}
key={scopeTup[0]}
const scopeIsForVPC = scopeTup[0] === 'vpc';

return (
<TableRow
data-qa-row={basePermNameMap[scopeTup[0]]}
key={scopeTup[0]}
>
<StyledAccessCell padding="checkbox" parentColumn="Access">
{basePermNameMap[scopeTup[0]]}
</StyledAccessCell>
<StyledPermissionsCell padding="checkbox" parentColumn="None">
<AccessCell
active={scopeTup[1] === 0}
disabled={false}
onChange={handleScopeChange}
scope="0"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read Only"
>
<StyledAccessCell padding="checkbox" parentColumn="Access">
{basePermNameMap[scopeTup[0]]}
</StyledAccessCell>
<StyledPermissionsCell padding="checkbox" parentColumn="None">
<AccessCell
active={scopeTup[1] === 0}
disabled={false}
onChange={handleScopeChange}
scope="0"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read Only"
>
<AccessCell
tooltipText={
scopeIsForVPC ? VPC_READ_ONLY_TOOLTIP : undefined
}
active={scopeTup[1] === 1}
disabled={scopeIsForVPC} // "Read Only" is not a valid scope for VPC
onChange={handleScopeChange}
scope="1"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read/Write"
>
<AccessCell
active={scopeTup[1] === 2}
disabled={false}
onChange={handleScopeChange}
scope="2"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
</TableRow>
);
}
)}
<AccessCell
tooltipText={
scopeIsForVPC ? VPC_READ_ONLY_TOOLTIP : undefined
}
active={scopeTup[1] === 1}
disabled={scopeIsForVPC} // "Read Only" is not a valid scope for VPC
onChange={handleScopeChange}
scope="1"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read/Write"
>
<AccessCell
active={scopeTup[1] === 2}
disabled={false}
onChange={handleScopeChange}
scope="2"
scopeDisplay={scopeTup[0]}
viewOnly={false}
/>
</StyledPermissionsCell>
</TableRow>
);
})}
</TableBody>
</StyledPermsTable>
{errorMap.scopes && (
Expand Down
140 changes: 56 additions & 84 deletions packages/manager/src/features/Profile/APITokens/ViewAPITokenDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ import {
StyledPermissionsCell,
StyledPermsTable,
} from './APITokenDrawer.styles';
import {
basePermNameMap as _basePermNameMap,
filterPermsNameMap,
scopeStringToPermTuples,
} from './utils';
import { basePermNameMap, scopeStringToPermTuples } from './utils';

interface Props {
onClose: () => void;
Expand All @@ -39,39 +35,13 @@ export const ViewAPITokenDrawer = (props: Props) => {
globalGrantType: 'child_account_access',
});

const hasParentChildAccountAccess = Boolean(flags.parentChildAccountAccess);

// @TODO: Parent/Child - once in GA, remove _basePermNameMap logic and references.
// Just use the basePermNameMap import directly w/o any manipulation.
const basePermNameMap = filterPermsNameMap(_basePermNameMap, [
{
name: 'child_account',
shouldBeIncluded: hasParentChildAccountAccess,
},
]);

const allPermissions = scopeStringToPermTuples(token?.scopes ?? '');

/**
* A parent user with child_account_access can issue a PAT with child_account scope.
* If access is later revoked, we'll still display this scope for existing PATs, but not for new ones.
* */
const hideChildAccountAccessScope = allPermissions.some(
(scope) =>
scope[0] === 'child_account' &&
scope[1] === 0 &&
isChildAccountAccessRestricted
);

// Filter permissions for all users except parent user accounts.
const showFilteredPermissions =
!flags.parentChildAccountAccess ||
// Visually hide the "Child Account Access" permission even though it's still part of the base perms.
const hideChildAccountAccessScope =
profile?.user_type !== 'parent' ||
hideChildAccountAccessScope;

const filteredPermissions = allPermissions.filter(
(scopeTup) => basePermNameMap[scopeTup[0]] !== 'Child Account Access'
);
isChildAccountAccessRestricted ||
!flags.parentChildAccountAccess;
Copy link
Contributor

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?


return (
<Drawer onClose={onClose} open={open} title={token?.label ?? 'Token'}>
Expand All @@ -95,56 +65,58 @@ export const ViewAPITokenDrawer = (props: Props) => {
</TableRow>
</TableHead>
<TableBody>
{(showFilteredPermissions ? filteredPermissions : allPermissions).map(
(scopeTup) => {
if (!basePermNameMap[scopeTup[0]]) {
return null;
}
return (
<TableRow
data-qa-row={basePermNameMap[scopeTup[0]]}
key={scopeTup[0]}
>
<StyledAccessCell padding="checkbox" parentColumn="Access">
{basePermNameMap[scopeTup[0]]}
</StyledAccessCell>
<StyledPermissionsCell padding="checkbox" parentColumn="None">
<AccessCell
active={scopeTup[1] === 0}
disabled={false}
onChange={() => null}
scope="0"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read Only"
>
<AccessCell
active={scopeTup[1] === 1}
disabled={false}
onChange={() => null}
scope="1"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</StyledPermissionsCell>
<TableCell padding="checkbox" parentColumn="Read/Write">
<AccessCell
active={scopeTup[1] === 2}
disabled={false}
onChange={() => null}
scope="2"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</TableCell>
</TableRow>
);
{allPermissions.map((scopeTup) => {
if (
!basePermNameMap[scopeTup[0]] ||
(hideChildAccountAccessScope &&
basePermNameMap[scopeTup[0]] === 'Child Account Access')
) {
return null;
}
)}
return (
<TableRow
data-qa-row={basePermNameMap[scopeTup[0]]}
key={scopeTup[0]}
>
<StyledAccessCell padding="checkbox" parentColumn="Access">
{basePermNameMap[scopeTup[0]]}
</StyledAccessCell>
<StyledPermissionsCell padding="checkbox" parentColumn="None">
<AccessCell
active={scopeTup[1] === 0}
disabled={false}
onChange={() => null}
scope="0"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</StyledPermissionsCell>
<StyledPermissionsCell
padding="checkbox"
parentColumn="Read Only"
>
<AccessCell
active={scopeTup[1] === 1}
disabled={false}
onChange={() => null}
scope="1"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</StyledPermissionsCell>
<TableCell padding="checkbox" parentColumn="Read/Write">
<AccessCell
active={scopeTup[1] === 2}
disabled={false}
onChange={() => null}
scope="2"
scopeDisplay={scopeTup[0]}
viewOnly={true}
/>
</TableCell>
</TableRow>
);
})}
</TableBody>
</StyledPermsTable>
</Drawer>
Expand Down
Loading