Skip to content

Commit

Permalink
upcoming: [M3-7527] - Improve Login History restricted and child user…
Browse files Browse the repository at this point in the history
… experience (#10125)

* Show warning and hide table for restricted users

* Factory, mock endpoint, and WIP integration test for Login History

* Finish tests; add aria labels to table and status icon

* Add changesets

* Fix typo in factory

* Address feedback: fix restricted access to match prod (any restricted user)
  • Loading branch information
mjac0bs authored Feb 9, 2024
1 parent e68bbb0 commit a53f63b
Show file tree
Hide file tree
Showing 9 changed files with 293 additions and 4 deletions.
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10125-tests-1707232650553.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tests
---

Add integration test coverage for Account Login History ([#10125](https://github.com/linode/manager/pull/10125))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Improve restricted access Login History experience for child and restricted users ([#10125](https://github.com/linode/manager/pull/10125))
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
/**
* @file Integration tests for Cloud Manager account login history flows.
*/

import { accountFactory, profileFactory } from 'src/factories';
import { accountLoginFactory } from 'src/factories/accountLogin';
import { formatDate } from 'src/utilities/formatDate';
import {
mockGetAccount,
mockGetAccountLogins,
} from 'support/intercepts/account';
import {
mockAppendFeatureFlags,
mockGetFeatureFlagClientstream,
} from 'support/intercepts/feature-flags';
import { mockGetProfile } from 'support/intercepts/profile';
import { makeFeatureFlagData } from 'support/util/feature-flags';

describe('Account login history', () => {
/*
* - Confirms that a user can navigate to and view the login history page.
* - Confirms that login table displays the expected column headers.
* - Confirms that the login table displays a mocked failed restricted user login.
* - Confirm that the login table displays a mocked successful unrestricted user login.
*/
it('users can view the login history table', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-user',
restricted: false,
user_type: null,
});
const mockFailedLogin = accountLoginFactory.build({
status: 'failed',
username: 'mock-restricted-user',
restricted: true,
});
const mockSuccessfulLogin = accountLoginFactory.build({
status: 'successful',
restricted: false,
});

mockGetAccount(mockAccount).as('getAccount');
mockGetProfile(mockProfile).as('getProfile');
mockGetAccountLogins([mockFailedLogin, mockSuccessfulLogin]).as(
'getAccountLogins'
);

// TODO: Parent/Child - M3-7559 clean up when feature is live in prod and feature flag is removed.
mockAppendFeatureFlags({
parentChildAccountAccess: makeFeatureFlagData(false),
}).as('getFeatureFlags');
mockGetFeatureFlagClientstream().as('getClientStream');

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);

// Confirm helper text above table is visible.
cy.findByText(
'Logins across all users on your account over the last 90 days.'
).should('be.visible');

// Confirm the login table includes the expected column headers and mocked logins are visible in table.
cy.findByLabelText('Account Logins').within(() => {
cy.get('thead').findByText('Date').should('be.visible');
cy.get('thead').findByText('Username').should('be.visible');
cy.get('thead').findByText('IP').should('be.visible');
cy.get('thead').findByText('Permission Level').should('be.visible');
cy.get('thead').findByText('Access').should('be.visible');

// Confirm that restricted user's failed login and status icon display in table.
cy.findByText(mockFailedLogin.username)
.should('be.visible')
.closest('tr')
.within(() => {
cy.findByText(mockFailedLogin.status, { exact: false }).should(
'be.visible'
);
cy.findAllByLabelText(`Status is ${mockFailedLogin.status}`);
cy.findByText('Restricted').should('be.visible');
});

// Confirm that unrestricted user login displays in table.
cy.findByText(mockSuccessfulLogin.username)
.should('be.visible')
.closest('tr')
.within(() => {
// Confirm that successful login and status icon display in table.
cy.findByText(mockSuccessfulLogin.status, { exact: false }).should(
'be.visible'
);
cy.findAllByLabelText(`Status is ${mockSuccessfulLogin.status}`);

// Confirm all other fields display in table.
cy.findByText(
formatDate(mockSuccessfulLogin.datetime, {
timezone: mockProfile.timezone,
})
).should('be.visible');
cy.findByText(mockSuccessfulLogin.ip).should('be.visible');
cy.findByText('Unrestricted').should('be.visible');
});
});
});

/**
* - Confirms that a child user can navigate to the Login History page.
* - Confirms that a child user cannot see login history data.
* - Confirms that a child user sees a notice instead.
*/
it('child users cannot view login history', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-child-user',
restricted: false,
user_type: 'child',
});

mockGetAccount(mockAccount).as('getAccount');
mockGetProfile(mockProfile).as('getProfile');

// TODO: Parent/Child - M3-7559 clean up when feature is live in prod and feature flag is removed.
mockAppendFeatureFlags({
parentChildAccountAccess: makeFeatureFlagData(true),
}).as('getFeatureFlags');
mockGetFeatureFlagClientstream().as('getClientStream');

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);

// Confirm helper text above table and table are not visible.
cy.findByText(
'Logins across all users on your account over the last 90 days.'
).should('not.exist');
cy.findByLabelText('Account Logins').should('not.exist');

cy.findByText(
'Access restricted. Please contact your business partner to request the necessary permission.'
);
});

/**
* - Confirms that a restricted user can navigate to the Login History page.
* - Confirms that a restricted user cannot see login history data.
* - Confirms that a restricted user sees a notice instead.
*/
it('restricted users cannot view login history', () => {
const mockAccount = accountFactory.build();
const mockProfile = profileFactory.build({
username: 'mock-restricted-user',
restricted: true,
user_type: null,
});

mockGetProfile(mockProfile).as('getProfile');
mockGetAccount(mockAccount).as('getAccount');

// TODO: Parent/Child - M3-7559 clean up when feature is live in prod and feature flag is removed.
mockAppendFeatureFlags({
parentChildAccountAccess: makeFeatureFlagData(true),
}).as('getFeatureFlags');
mockGetFeatureFlagClientstream().as('getClientStream');

// Navigate to Account Login History page.
cy.visitWithLogin('/account/login-history');
cy.wait([
'@getAccount',
'@getClientStream',
'@getFeatureFlags',
'@getProfile',
]);

// Confirm helper text above table and table are not visible.
cy.findByText(
'Logins across all users on your account over the last 90 days.'
).should('not.exist');
cy.findByLabelText('Account Logins').should('not.exist');

cy.findByText(
'Access restricted. Please contact your account administrator to request the necessary permission.'
);
});
});
17 changes: 17 additions & 0 deletions packages/manager/cypress/support/intercepts/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { makeResponse } from 'support/util/response';

import type {
Account,
AccountLogin,
AccountSettings,
Agreements,
CancelAccount,
Expand Down Expand Up @@ -514,3 +515,19 @@ export const mockGetAccountAgreements = (
makeResponse(agreements)
);
};

/**
* Intercepts GET request to fetch the account logins and mocks the response.
*
*
* @returns Cypress chainable.
*/
export const mockGetAccountLogins = (
accountLogins: AccountLogin[]
): Cypress.Chainable<null> => {
return cy.intercept(
'GET',
apiMatcher(`account/logins*`),
paginateResponse(accountLogins)
);
};
11 changes: 11 additions & 0 deletions packages/manager/src/factories/accountLogin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { AccountLogin } from '@linode/api-v4';
import * as Factory from 'factory.ts';

export const accountLoginFactory = Factory.Sync.makeFactory<AccountLogin>({
datetime: '2021-05-21T14:27:51',
id: Factory.each((id) => id),
ip: '127.0.0.1',
restricted: false,
status: 'successful',
username: 'mock-user',
});
20 changes: 18 additions & 2 deletions packages/manager/src/features/Account/AccountLogins.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as React from 'react';
import { makeStyles } from 'tss-react/mui';

import { Hidden } from 'src/components/Hidden';
import { Notice } from 'src/components/Notice/Notice';
import { PaginationFooter } from 'src/components/PaginationFooter/PaginationFooter';
import { Table } from 'src/components/Table';
import { TableBody } from 'src/components/TableBody';
Expand All @@ -15,11 +16,14 @@ import { TableRowError } from 'src/components/TableRowError/TableRowError';
import { TableRowLoading } from 'src/components/TableRowLoading/TableRowLoading';
import { TableSortCell } from 'src/components/TableSortCell';
import { Typography } from 'src/components/Typography';
import { useFlags } from 'src/hooks/useFlags';
import { useOrder } from 'src/hooks/useOrder';
import { usePagination } from 'src/hooks/usePagination';
import { useAccountLoginsQuery } from 'src/queries/accountLogins';
import { useProfile } from 'src/queries/profile';

import AccountLoginsTableRow from './AccountLoginsTableRow';
import { getAccessRestrictedText } from './utils';

const preferenceKey = 'account-logins';

Expand All @@ -40,6 +44,7 @@ const useStyles = makeStyles()((theme: Theme) => ({
const AccountLogins = () => {
const { classes } = useStyles();
const pagination = usePagination(1, preferenceKey);
const flags = useFlags();

const { handleOrderChange, order, orderBy } = useOrder(
{
Expand All @@ -61,6 +66,13 @@ const AccountLogins = () => {
},
filter
);
const { data: profile } = useProfile();

const isRestrictedChildUser = Boolean(
flags.parentChildAccountAccess && profile?.user_type === 'child'
);
const isAccountAccessRestricted =
isRestrictedChildUser || profile?.restricted;

const renderTableContent = () => {
if (isLoading) {
Expand Down Expand Up @@ -90,12 +102,12 @@ const AccountLogins = () => {
return null;
};

return (
return !isAccountAccessRestricted ? (
<>
<Typography className={classes.copy} variant="body1">
Logins across all users on your account over the last 90 days.
</Typography>
<Table>
<Table aria-label="Account Logins">
<TableHead>
<TableRow>
<TableSortCell
Expand Down Expand Up @@ -152,6 +164,10 @@ const AccountLogins = () => {
pageSize={pagination.pageSize}
/>
</>
) : (
<Notice important variant="warning">
{getAccessRestrictedText(profile?.user_type ?? null)}
</Notice>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ const AccountLoginsTableRow = (props: AccountLogin) => {
</TableCell>
</Hidden>
<TableCell statusCell>
<StatusIcon pulse={false} status={accessIconMap[status] ?? 'other'} />
<StatusIcon
ariaLabel={`Status is ${status}`}
pulse={false}
status={accessIconMap[status] ?? 'other'}
/>
{capitalize(status)}
</TableCell>
</TableRow>
Expand Down
21 changes: 20 additions & 1 deletion packages/manager/src/features/Account/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { getStorage, setStorage } from 'src/utilities/storage';

import type { GlobalGrantTypes, Grants, Profile, Token } from '@linode/api-v4';
import type {
GlobalGrantTypes,
Grants,
Profile,
Token,
UserType,
} from '@linode/api-v4';
import type { GrantTypeMap } from 'src/features/Account/types';

type ActionType = 'create' | 'delete' | 'edit' | 'view';
Expand All @@ -10,6 +16,10 @@ interface GetRestrictedResourceText {
isSingular?: boolean;
resourceType: GrantTypeMap;
}

/**
* Get a resource restricted message based on action and resource type.
*/
export const getRestrictedResourceText = ({
action = 'edit',
isSingular = true,
Expand All @@ -22,6 +32,15 @@ export const getRestrictedResourceText = ({
return `You don't have permissions to ${action} ${resource}. Please contact your account administrator to request the necessary permissions.`;
};

/**
* Get an 'access restricted' message based on user type.
*/
export const getAccessRestrictedText = (userType: UserType | null) => {
return `Access restricted. Please contact your ${
userType === 'child' ? 'business partner' : 'account administrator'
} to request the necessary permission.`;
};

export const isRestrictedGlobalGrantType = ({
globalGrantType,
grants,
Expand Down
Loading

0 comments on commit a53f63b

Please sign in to comment.