Add E2E tests for device code grant flow.

Handle more errors for device code.
Dispatch a BSOD for an any unhandled exception from auth flow state
This commit is contained in:
ErickSkrauch 2024-12-17 23:11:39 +01:00
parent b0975f0b0f
commit be08857edc
No known key found for this signature in database
GPG Key ID: 669339FCBB30EE0E
20 changed files with 267 additions and 176 deletions

View File

@ -19,7 +19,6 @@ import {
activate as activateEndpoint,
resendActivation as resendActivationEndpoint,
} from 'app/services/api/signup';
import dispatchBsod from 'app/components/ui/bsod/dispatchBsod';
import { create as createPopup } from 'app/components/ui/popup/actions';
import ContactForm from 'app/components/contact';
import { Account } from 'app/components/accounts/reducer';
@ -455,11 +454,6 @@ function handleOauthParamsValidation(
userMessage?: string;
} = {},
) {
// TODO: it would be better to dispatch BSOD from the initial request performers
if (resp.error !== 'invalid_user_code') {
dispatchBsod();
}
localStorage.removeItem('oauthData');
// eslint-disable-next-line no-alert

View File

@ -31,7 +31,7 @@ const AuthError: ComponentType<Props> = ({ error, onClose }) => {
}, [error, onClose]);
return (
<PanelBodyHeader type="error" onClose={onClose}>
<PanelBodyHeader type="error" onClose={onClose} data-testid="auth-error">
{resolveError(error)}
</PanelBodyHeader>
);

View File

@ -20,7 +20,6 @@ export default class DeviceCodeBody extends BaseAuthBody {
<Input
{...this.bindField('user_code')}
icon="key"
name="user_cide"
autoFocus
required
placeholder={nodes as string}

View File

@ -57,7 +57,7 @@ interface PanelBodyHeaderProps extends PropsWithChildren<any> {
onClose?: () => void;
}
export const PanelBodyHeader: FC<PanelBodyHeaderProps> = ({ type = 'default', onClose, children }) => {
export const PanelBodyHeader: FC<PanelBodyHeaderProps> = ({ type = 'default', onClose, children, ...props }) => {
const [isClosed, setIsClosed] = useState<boolean>(false);
const handleCloseClick = useCallback(() => {
setIsClosed(true);
@ -71,6 +71,7 @@ export const PanelBodyHeader: FC<PanelBodyHeaderProps> = ({ type = 'default', on
[styles.errorBodyHeader]: type === 'error',
[styles.isClosed]: isClosed,
})}
{...props}
>
{type === 'error' && <span className={styles.close} onClick={handleCloseClick} />}
{children}

View File

@ -13,6 +13,10 @@ const AuthFlowRouteContents: FC<Props> = ({ component: WantedComponent, location
const [component, setComponent] = useState<ReactElement | null>(null);
useEffect(() => {
// Promise that will be resolved after handleRequest might contain already non-actual component to render,
// so set it to false in the effect's clear function to prevent unwanted UI state
let isActual = true;
authFlow.handleRequest(
{
path: location.pathname,
@ -21,11 +25,15 @@ const AuthFlowRouteContents: FC<Props> = ({ component: WantedComponent, location
},
history.push,
() => {
if (isMounted()) {
if (isActual && isMounted()) {
setComponent(<WantedComponent history={history} location={location} match={match} />);
}
},
);
return () => {
isActual = false;
};
}, [location.pathname, location.search]);
return component;

View File

@ -1,5 +1,3 @@
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import { AuthContext } from './AuthFlow';
import CompleteState from './CompleteState';
@ -16,10 +14,7 @@ export default class AcceptRulesState extends AbstractState {
}
resolve(context: AuthContext): Promise<void> | void {
context
.run('acceptRules')
.then(() => context.setState(new CompleteState()))
.catch((err = {}) => err.errors || logger.warn('Error accepting rules', err));
return context.run('acceptRules').then(() => context.setState(new CompleteState()));
}
reject(context: AuthContext, payload: Record<string, any>): void {

View File

@ -1,4 +1,3 @@
import logger from 'app/services/logger';
import { AuthContext } from 'app/services/authFlow';
import AbstractState from './AbstractState';
@ -12,10 +11,7 @@ export default class ActivationState extends AbstractState {
}
resolve(context: AuthContext, payload: { key: string }): Promise<void> | void {
context
.run('activate', payload.key)
.then(() => context.setState(new CompleteState()))
.catch((err = {}) => err.errors || logger.warn('Error activating account', err));
return context.run('activate', payload.key).then(() => context.setState(new CompleteState()));
}
reject(context: AuthContext): void {

View File

@ -10,8 +10,9 @@ import {
} from 'app/components/accounts/actions';
import * as actions from 'app/components/auth/actions';
import { updateUser } from 'app/components/user/actions';
import FinishState from './FinishState';
import dispatchBsod from 'app/components/ui/bsod/dispatchBsod';
import FinishState from './FinishState';
import RegisterState from './RegisterState';
import LoginState from './LoginState';
import InitOAuthAuthCodeFlowState from './InitOAuthAuthCodeFlowState';
@ -121,11 +122,23 @@ export default class AuthFlow implements AuthContext {
}
resolve(payload: Record<string, any> = {}) {
this.state.resolve(this, payload);
const maybePromise = this.state.resolve(this, payload);
if (maybePromise && maybePromise.catch) {
maybePromise.catch((err) => {
dispatchBsod();
throw err;
});
}
}
reject(payload: Record<string, any> = {}) {
try {
this.state.reject(this, payload);
} catch (err) {
dispatchBsod();
throw err;
}
}
goBack() {
@ -133,11 +146,11 @@ export default class AuthFlow implements AuthContext {
}
run<T extends ActionId>(actionId: T, payload?: Parameters<typeof availableActions[T]>[0]): Promise<any> {
// @ts-ignore the extended version of redux with thunk will return the correct promise
// @ts-expect-error the extended version of redux with thunk will return the correct promise
return Promise.resolve(this.dispatch(this.actions[actionId](payload)));
}
setState(state: State) {
setState(state: State): Promise<void> | void {
this.state?.leave(this);
this.prevState = this.state;
this.state = state;
@ -256,8 +269,6 @@ export default class AuthFlow implements AuthContext {
/**
* Tries to restore last oauth request, if it was stored in localStorage
* in last 2 hours
*
* @returns {bool} - whether oauth state is being restored
*/
private restoreOAuthState(): boolean {
if (this.oAuthStateRestored) {

View File

@ -1,4 +1,5 @@
import type { Account } from 'app/components/accounts/reducer';
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import { AuthContext } from './AuthFlow';
@ -21,10 +22,16 @@ export default class ChooseAccountState extends AbstractState {
// So if there is no `id` property, it's an empty object
resolve(context: AuthContext, payload: Account): Promise<void> | void {
if (payload.id) {
return context
return (
context
.run('authenticate', payload)
.then(() => context.run('setAccountSwitcher', false))
.then(() => context.setState(new CompleteState()));
.then(() => context.setState(new CompleteState()))
// By default, this error must cause a BSOD. But by I don't know why reasons it shouldn't,
// because somebody somewhere catches an invalid authentication result and routes the user
// to the password entering form. To keep this behavior we catch all errors, log it and suppress
.catch((err) => err.errors || logger.warn('Error choosing an account', err))
);
}
// log in to another account

View File

@ -92,16 +92,21 @@ export default class CompleteState extends AbstractState {
user.isDeleted ||
hasPrompt(oauth.prompt, PROMPT_ACCOUNT_CHOOSE))
) {
context.setState(new ChooseAccountState());
} else if (user.isDeleted) {
return context.setState(new ChooseAccountState());
}
if (user.isDeleted) {
// you shall not pass
// if we are here, this means that user have already seen account
// switcher and now we should redirect him to his profile,
// because oauth is not available for deleted accounts
context.navigate('/');
} else if (oauth.code) {
context.setState(new FinishState());
} else {
return context.navigate('/');
}
if (oauth.code) {
return context.setState(new FinishState());
}
const data: Record<string, any> = {};
if (typeof this.isPermissionsAccepted !== 'undefined') {
@ -132,5 +137,4 @@ export default class CompleteState extends AbstractState {
}
});
}
}
}

View File

@ -6,7 +6,7 @@ export default class DeviceCodeState extends AbstractState {
async resolve(context: AuthContext, payload: { user_code: string }): Promise<void> {
const { query } = context.getRequest();
context
return context
.run('oAuthValidate', {
params: {
userCode: payload.user_code,
@ -16,7 +16,7 @@ export default class DeviceCodeState extends AbstractState {
})
.then(() => context.setState(new CompleteState()))
.catch((err) => {
if (err.error === 'invalid_user_code') {
if (['invalid_user_code', 'expired_token', 'used_user_code'].includes(err.error)) {
return context.run('setErrors', { [err.parameter]: err.error });
}

View File

@ -1,5 +1,3 @@
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import { AuthContext } from './AuthFlow';
import LoginState from './LoginState';
@ -11,13 +9,10 @@ export default class ForgotPasswordState extends AbstractState {
}
resolve(context: AuthContext, payload: { login: string; captcha: string }): Promise<void> | void {
context
.run('forgotPassword', payload)
.then(() => {
return context.run('forgotPassword', payload).then(() => {
context.run('setLogin', payload.login);
context.setState(new RecoverPasswordState());
})
.catch((err = {}) => err.errors || logger.warn('Error requesting password recoverage', err));
});
}
goBack(context: AuthContext): void {

View File

@ -16,7 +16,8 @@ export default class InitOAuthDeviceCodeFlowState extends AbstractState {
description: query.get('description')!,
prompt: query.get('prompt')!,
});
await context.setState(new CompleteState());
return context.setState(new CompleteState());
} catch {
// Ok, fallback to the default
}

View File

@ -30,7 +30,7 @@ export default class LoginState extends AbstractState {
login: string;
},
): Promise<void> | void {
context
return context
.run('login', payload)
.then(() => context.setState(new PasswordState()))
.catch((err = {}) => err.errors || logger.warn('Error validating login', err));

View File

@ -12,15 +12,15 @@ export default class PermissionsState extends AbstractState {
}
resolve(context: AuthContext): Promise<void> | void {
this.process(context, true);
return this.process(context, true);
}
reject(context: AuthContext): void {
this.process(context, false);
}
process(context: AuthContext, accept: boolean): void {
context.setState(
process(context: AuthContext, accept: boolean): Promise<void> | void {
return context.setState(
new CompleteState({
accept,
}),

View File

@ -1,5 +1,3 @@
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import { AuthContext } from './AuthFlow';
import LoginState from './LoginState';
@ -17,10 +15,7 @@ export default class RecoverPasswordState extends AbstractState {
context: AuthContext,
payload: { key: string; newPassword: string; newRePassword: string },
): Promise<void> | void {
context
.run('recoverPassword', payload)
.then(() => context.setState(new CompleteState()))
.catch((err = {}) => err.errors || logger.warn('Error recovering password', err));
return context.run('recoverPassword', payload).then(() => context.setState(new CompleteState()));
}
goBack(context: AuthContext): void {

View File

@ -1,5 +1,3 @@
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import { AuthContext } from './AuthFlow';
import CompleteState from './CompleteState';
@ -22,10 +20,7 @@ export default class RegisterState extends AbstractState {
rulesAgreement: boolean;
},
): Promise<void> | void {
context
.run('register', payload)
.then(() => context.setState(new CompleteState()))
.catch((err = {}) => err.errors || logger.warn('Error registering', err));
return context.run('register', payload).then(() => context.setState(new CompleteState()));
}
reject(context: AuthContext, payload: Record<string, any>): void {

View File

@ -1,5 +1,4 @@
import { AuthContext } from 'app/services/authFlow';
import logger from 'app/services/logger';
import AbstractState from './AbstractState';
import ActivationState from './ActivationState';
@ -11,10 +10,7 @@ export default class ResendActivationState extends AbstractState {
}
resolve(context: AuthContext, payload: { email: string; captcha: string }): Promise<void> | void {
context
.run('resendActivation', payload)
.then(() => context.setState(new ActivationState()))
.catch((err = {}) => err.errors || logger.warn('Error resending activation', err));
return context.run('resendActivation', payload).then(() => context.setState(new ActivationState()));
}
reject(context: AuthContext): void {

View File

@ -129,6 +129,13 @@ const errorsMap: Record<string, (props?: Record<string, any>) => ReactElement> =
'error.redirectUri_invalid': () => <Message key="redirectUriInvalid" defaultMessage="Redirect URI is invalid" />,
invalid_user_code: () => <Message key="invalidUserCode" defaultMessage="Invalid Device Code" />,
expired_token: () => (
<Message
key="expiredUserCode"
defaultMessage="The code has expired. Start the authorization flow in the application again."
/>
),
used_user_code: () => <Message key="usedUserCode" defaultMessage="This code has been already used" />,
};
interface ErrorLiteral {

View File

@ -10,6 +10,7 @@ const defaults = {
};
describe('OAuth', () => {
describe('AuthCode grant flow', () => {
it('should complete oauth', () => {
cy.login({ accounts: ['default'] });
@ -41,6 +42,145 @@ describe('OAuth', () => {
cy.url().should('equal', 'https://dev.ely.by/');
});
describe('static pages', () => {
it('should authenticate using static page', () => {
cy.server();
cy.route({
method: 'POST',
url: '/api/oauth2/v1/complete**',
}).as('complete');
cy.login({ accounts: ['default'] });
cy.visit(
`/oauth2/v1/ely?${new URLSearchParams({
...defaults,
client_id: 'tlauncher',
redirect_uri: 'static_page',
})}`,
);
cy.wait('@complete');
cy.url().should('include', 'oauth/finish#{%22auth_code%22:');
});
it('should authenticate using static page with code', () => {
cy.server();
cy.route({
method: 'POST',
url: '/api/oauth2/v1/complete**',
}).as('complete');
cy.login({ accounts: ['default'] });
cy.visit(
`/oauth2/v1/ely?${new URLSearchParams({
...defaults,
client_id: 'tlauncher',
redirect_uri: 'static_page_with_code',
})}`,
);
cy.wait('@complete');
cy.url().should('include', 'oauth/finish#{%22auth_code%22:');
cy.findByTestId('oauth-code-container').should('contain', 'provide the following code');
// just click on copy, but we won't assert if the string was copied
// because it is a little bit complicated
// https://github.com/cypress-io/cypress/issues/2752
cy.findByTestId('oauth-code-container').contains('Copy').click();
});
});
});
describe('DeviceCode grant flow', () => {
it('should complete flow by complete uri', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code?user_code=E2E-APPROVED');
cy.location('pathname').should('eq', '/oauth/finish');
cy.get('[data-e2e-content]').contains('successfully completed');
});
it('should complete flow with manual approve', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('E2E-UNAPPROVED{enter}');
cy.location('pathname').should('eq', '/oauth/permissions');
cy.findByTestId('auth-controls').contains('Approve').click();
cy.location('pathname').should('eq', '/oauth/finish');
cy.get('[data-e2e-content]').contains('successfully completed');
});
it('should complete flow with auto approve', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('E2E-APPROVED{enter}');
cy.location('pathname').should('eq', '/oauth/finish');
cy.get('[data-e2e-content]').contains('successfully completed');
});
it('should complete flow by declining the code', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('E2E-UNAPPROVED{enter}');
cy.location('pathname').should('eq', '/oauth/permissions');
cy.findByTestId('auth-controls-secondary').contains('Decline').click();
cy.location('pathname').should('eq', '/oauth/finish');
cy.get('[data-e2e-content]').contains('was failed');
});
it('should show an error for an unknown code', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('UNKNOWN-CODE{enter}');
cy.location('pathname').should('eq', '/code');
cy.findByTestId('auth-error').contains('Invalid Device Code');
});
it('should show an error for an expired code', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('E2E-EXPIRED{enter}');
cy.location('pathname').should('eq', '/code');
cy.findByTestId('auth-error').contains('The code has expired');
});
it('should show an error for an expired code', () => {
cy.login({ accounts: ['default'] });
cy.visit('/code');
cy.get('[name=user_code]').type('E2E-COMPLETED{enter}');
cy.location('pathname').should('eq', '/code');
cy.findByTestId('auth-error').contains('This code has been already used');
});
});
describe('AccountSwitcher', () => {
it('should ask to choose an account if user has multiple', () => {
cy.login({ accounts: ['default', 'default2'] }).then(({ accounts: [account] }) => {
@ -408,59 +548,6 @@ describe('OAuth', () => {
cy.url().should('match', /^http:\/\/localhost:8080\/?\?code=[^&]+$/);
});
});
describe('static pages', () => {
it('should authenticate using static page', () => {
cy.server();
cy.route({
method: 'POST',
url: '/api/oauth2/v1/complete**',
}).as('complete');
cy.login({ accounts: ['default'] });
cy.visit(
`/oauth2/v1/ely?${new URLSearchParams({
...defaults,
client_id: 'tlauncher',
redirect_uri: 'static_page',
})}`,
);
cy.wait('@complete');
cy.url().should('include', 'oauth/finish#{%22auth_code%22:');
});
it('should authenticate using static page with code', () => {
cy.server();
cy.route({
method: 'POST',
url: '/api/oauth2/v1/complete**',
}).as('complete');
cy.login({ accounts: ['default'] });
cy.visit(
`/oauth2/v1/ely?${new URLSearchParams({
...defaults,
client_id: 'tlauncher',
redirect_uri: 'static_page_with_code',
})}`,
);
cy.wait('@complete');
cy.url().should('include', 'oauth/finish#{%22auth_code%22:');
cy.findByTestId('oauth-code-container').should('contain', 'provide the following code');
// just click on copy, but we won't assert if the string was copied
// because it is a little bit complicated
// https://github.com/cypress-io/cypress/issues/2752
cy.findByTestId('oauth-code-container').contains('Copy').click();
});
});
});
function assertPermissions() {