Skip to content

Commit

Permalink
chore(auth): fix unnecessary network calls in getCurrentUser API (#12052
Browse files Browse the repository at this point in the history
)

* chore: fix api network calls in getCurrentUser

* chore: add getTokens method in internal auth class

* chore: fix tests

* chore: fix build

* chore: remove input types

* chore: fix build issue

* fix build issue

* fix build issue again

* chore: fix failing unit test
  • Loading branch information
israx authored Sep 15, 2023
1 parent c7dfaf2 commit a96ade9
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 75 deletions.
55 changes: 22 additions & 33 deletions packages/auth/__tests__/providers/cognito/getCurrentUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@
// SPDX-License-Identifier: Apache-2.0

import { Amplify } from 'aws-amplify';
import { decodeJWT, fetchAuthSession } from '@aws-amplify/core/internals/utils';
import { decodeJWT } from '@aws-amplify/core/internals/utils';
import { AuthError } from '../../../src/errors/AuthError';
import { getCurrentUser } from '../../../src/providers/cognito';
import { InitiateAuthException } from '../../../src/providers/cognito/types/errors';
import { fetchTransferHandler } from '@aws-amplify/core/internals/aws-client-utils';
import { buildMockErrorResponse, mockJsonResponse } from './testUtils/data';

import { Amplify as AmplifyV6 } from '@aws-amplify/core';
import { USER_UNAUTHENTICATED_EXCEPTION } from '../../../src/errors/constants';
jest.mock('@aws-amplify/core/lib/clients/handlers/fetch');
jest.mock('@aws-amplify/core/internals/utils', () => ({
...jest.requireActual('@aws-amplify/core/internals/utils'),
fetchAuthSession: jest.fn(),
}));

Amplify.configure({
Auth: {
Expand All @@ -26,27 +23,25 @@ Amplify.configure({
});
const mockedAccessToken =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c';
const mockFetchAuthSession = fetchAuthSession as jest.Mock;
const mockGetTokensFunction = jest.spyOn(AmplifyV6.Auth, 'getTokens');
const mockedSub = 'mockedSub';
const mockedUsername = 'XXXXXXXXXXXXXX';

describe('getUser API happy path cases', () => {
beforeEach(() => {
mockFetchAuthSession.mockResolvedValue({
tokens: {
accessToken: decodeJWT(mockedAccessToken),
idToken: {
payload: {
sub: mockedSub,
'cognito:username': mockedUsername,
},
mockGetTokensFunction.mockResolvedValue({
accessToken: decodeJWT(mockedAccessToken),
idToken: {
payload: {
sub: mockedSub,
'cognito:username': mockedUsername,
},
},
});
});

afterEach(() => {
mockFetchAuthSession.mockClear();
mockGetTokensFunction.mockClear();
});

test('get current user', async () => {
Expand All @@ -56,26 +51,20 @@ describe('getUser API happy path cases', () => {
});

describe('getUser API error path cases:', () => {
test('getUser API should raise service error', async () => {
expect.assertions(2);
mockFetchAuthSession.mockImplementationOnce(async () => {
throw new AuthError({
name: InitiateAuthException.InternalErrorException,
message: 'error at fetchAuthSession',
});
});
(fetchTransferHandler as jest.Mock).mockResolvedValue(
mockJsonResponse(
buildMockErrorResponse(InitiateAuthException.InternalErrorException)
)
);
beforeEach(() => {
mockGetTokensFunction.mockResolvedValue(null);
});

afterEach(() => {
mockGetTokensFunction.mockClear();
});
test('getUser API should raise a validation error when tokens are not found', async () => {
try {
await getCurrentUser({
recache: true,
});
const result = await getCurrentUser();
} catch (error) {
console.log(error);
expect(error).toBeInstanceOf(AuthError);
expect(error.name).toBe(InitiateAuthException.InternalErrorException);
expect(error.name).toBe(USER_UNAUTHENTICATED_EXCEPTION);
}
});
});
2 changes: 1 addition & 1 deletion packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"clean:size": "rimraf dual-publish-tmp tmp*",
"format": "echo \"Not implemented\"",
"lint": "tslint '{src}/**/*.ts' && npm run ts-coverage",
"ts-coverage": "typescript-coverage-report -p ./tsconfig.json -t 91.19"
"ts-coverage": "typescript-coverage-report -p ./tsconfig.json -t 91.18"
},
"typesVersions": {
">=3.8": {
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export {
} from './providers/cognito';

export {
GetCurrentUserInput,
ConfirmResetPasswordInput,
ConfirmSignInInput,
ConfirmSignUpInput,
Expand Down
8 changes: 3 additions & 5 deletions packages/auth/src/providers/cognito/apis/getCurrentUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import { Amplify } from '@aws-amplify/core';
import { GetCurrentUserInput, GetCurrentUserOutput } from '../types';
import { GetCurrentUserOutput } from '../types';
import { getCurrentUser as getCurrentUserInternal } from './internal/getCurrentUser';

/**
Expand All @@ -13,8 +13,6 @@ import { getCurrentUser as getCurrentUserInternal } from './internal/getCurrentU
* @throws - {@link InitiateAuthException} - Thrown when the service fails to refresh the tokens.
* @throws AuthTokenConfigException - Thrown when the token provider config is invalid.
*/
export const getCurrentUser = async (
input?: GetCurrentUserInput
): Promise<GetCurrentUserOutput> => {
return getCurrentUserInternal(Amplify, input);
export const getCurrentUser = async (): Promise<GetCurrentUserOutput> => {
return getCurrentUserInternal(Amplify);
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,16 @@
// SPDX-License-Identifier: Apache-2.0

import { AmplifyClassV6 } from '@aws-amplify/core';
import {
assertTokenProviderConfig,
fetchAuthSession,
} from '@aws-amplify/core/internals/utils';
import { GetCurrentUserInput, GetCurrentUserOutput } from '../../types';
import { assertTokenProviderConfig } from '@aws-amplify/core/internals/utils';
import { assertAuthTokens } from '../../utils/types';
import { GetCurrentUserOutput } from '../../types';

export const getCurrentUser = async (
amplify: AmplifyClassV6,
input?: GetCurrentUserInput
amplify: AmplifyClassV6
): Promise<GetCurrentUserOutput> => {
const authConfig = amplify.getConfig().Auth?.Cognito;
assertTokenProviderConfig(authConfig);
const { tokens } = await fetchAuthSession(amplify, {
forceRefresh: input?.recache ?? false,
});
const tokens = await amplify.Auth.getTokens({ forceRefresh: false });
assertAuthTokens(tokens);
const { 'cognito:username': username, sub } = tokens.idToken?.payload ?? {};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,18 @@ import {
AmplifyServer,
getAmplifyServerContext,
} from '@aws-amplify/core/internals/adapter-core';
import { GetCurrentUserOutput, GetCurrentUserInput } from '../../types';
import { GetCurrentUserOutput } from '../../types';
import { getCurrentUser as getCurrentUserInternal } from '../internal/getCurrentUser';

/**
* Gets the current user from the idToken.
*
* @param input - The GetCurrentUserInput object.
* @returns GetCurrentUserOutput
* @throws - {@link InitiateAuthException} - Thrown when the service fails to refresh the tokens.
* @throws AuthTokenConfigException - Thrown when the token provider config is invalid.
*/
export const getCurrentUser = async (
contextSpec: AmplifyServer.ContextSpec,
input?: GetCurrentUserInput
contextSpec: AmplifyServer.ContextSpec
): Promise<GetCurrentUserOutput> => {
return getCurrentUserInternal(
getAmplifyServerContext(contextSpec).amplify,
input
);
return getCurrentUserInternal(getAmplifyServerContext(contextSpec).amplify);
};
1 change: 0 additions & 1 deletion packages/auth/src/providers/cognito/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export { signInWithRedirect } from './apis/signInWithRedirect';
export { fetchUserAttributes } from './apis/fetchUserAttributes';
export { signOut } from './apis/signOut';
export {
GetCurrentUserInput,
ConfirmResetPasswordInput,
ConfirmSignInInput,
ConfirmSignUpInput,
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/providers/cognito/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export {
} from './options';

export {
GetCurrentUserInput,
ConfirmResetPasswordInput,
ConfirmSignInInput,
ConfirmSignUpInput,
Expand Down
6 changes: 0 additions & 6 deletions packages/auth/src/providers/cognito/types/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
VerifyTOTPSetupOptions,
} from '../types';
import {
AuthGetCurrentUserInput,
AuthConfirmResetPasswordInput,
AuthConfirmSignInInput,
AuthConfirmSignUpInput,
Expand All @@ -31,11 +30,6 @@ import {
AuthVerifyTOTPSetupInput,
} from '../../../types';

/**
* Input type for Cognito getCurrentUser API.
*/
export type GetCurrentUserInput = AuthGetCurrentUserInput;

/**
* Input type for Cognito confirmResetPassword API.
*/
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export {
AuthConfirmSignInInput,
AuthUpdatePasswordInput,
AuthUpdateUserAttributesInput,
AuthGetCurrentUserInput,
AuthConfirmUserAttributeInput,
AuthVerifyTOTPSetupInput,
AuthSignInWithRedirectInput,
Expand Down
6 changes: 0 additions & 6 deletions packages/auth/src/types/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,6 @@ export type AuthUpdateUserAttributesInput<
options?: { serviceOptions?: ServiceOptions };
};

/**
* Constructs a `GetCurrentUser` input.
* @param recache - whether to recache the user
*/
export type AuthGetCurrentUserInput = { recache: boolean };

/*
* Constructs a `verifyUserAttribute` input.
*
Expand Down
11 changes: 9 additions & 2 deletions packages/core/src/singleton/Auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ export class AuthClass {
let userSub: string | undefined;

// Get tokens will throw if session cannot be refreshed (network or service error) or return null if not available
tokens =
(await this.authOptions?.tokenProvider?.getTokens(options)) ?? undefined;
tokens = await this.getTokens(options);

if (tokens) {
userSub = tokens.accessToken?.payload?.sub;
Expand Down Expand Up @@ -93,4 +92,12 @@ export class AuthClass {
return await this.authOptions.credentialsProvider.clearCredentialsAndIdentityId();
}
}

async getTokens(
options: FetchAuthSessionOptions
): Promise<AuthTokens | undefined> {
return (
(await this.authOptions?.tokenProvider?.getTokens(options)) ?? undefined
);
}
}

0 comments on commit a96ade9

Please sign in to comment.