From 42a6f8a3958faa98bd7bbc15a4feb522c0d13c83 Mon Sep 17 00:00:00 2001 From: Danny Coates Date: Tue, 4 Aug 2020 13:48:16 -0700 Subject: [PATCH] feat(settings): add session verified state to graphql because the frontend needs to know about the verified state of the session --- packages/fxa-auth-client/lib/client.ts | 8 +- packages/fxa-graphql-api/src/lib/auth.ts | 5 +- .../src/lib/resolvers/account-resolver.ts | 4 +- .../src/lib/resolvers/session-resolver.ts | 25 ++++++ .../src/lib/resolvers/types/session.ts | 14 ++++ packages/fxa-graphql-api/src/lib/server.ts | 14 ++-- .../fxa-graphql-api/src/test/lib/auth.spec.ts | 15 ++-- .../test/lib/datasources/authServer.spec.ts | 2 +- .../lib/datasources/profileServer.spec.ts | 2 +- .../fxa-graphql-api/src/test/lib/mocks.ts | 5 +- .../lib/resolvers/account-resolver.spec.ts | 24 +++--- .../src/test/lib/schema.spec.ts | 15 ++-- .../src/test/lib/server.spec.ts | 12 ++- .../src/components/AccountDataHOC/index.tsx | 3 - .../VerifiedSessionGuard/index.test.tsx | 78 +++++++++++++++++++ .../components/VerifiedSessionGuard/index.tsx | 37 +++++++++ packages/fxa-settings/src/lib/auth.ts | 3 + packages/fxa-settings/src/lib/cache.ts | 10 +-- 18 files changed, 223 insertions(+), 53 deletions(-) create mode 100644 packages/fxa-graphql-api/src/lib/resolvers/session-resolver.ts create mode 100644 packages/fxa-graphql-api/src/lib/resolvers/types/session.ts create mode 100644 packages/fxa-settings/src/components/VerifiedSessionGuard/index.test.tsx create mode 100644 packages/fxa-settings/src/components/VerifiedSessionGuard/index.tsx diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts index b6aa2c1035..b4013bb546 100644 --- a/packages/fxa-auth-client/lib/client.ts +++ b/packages/fxa-auth-client/lib/client.ts @@ -531,7 +531,9 @@ export default class AuthClient { return this.sessionPost('/session/destroy', sessionToken, options); } - async sessionStatus(sessionToken: string) { + async sessionStatus( + sessionToken: string + ): Promise<{ state: 'verified' | 'unverified'; uid: string }> { return this.sessionGet('/session/status', sessionToken); } @@ -933,7 +935,9 @@ export default class AuthClient { return this.sessionPost('/totp/destroy', sessionToken, {}); } - async checkTotpTokenExists(sessionToken: string): Promise<{ exists: boolean, verified: boolean }> { + async checkTotpTokenExists( + sessionToken: string + ): Promise<{ exists: boolean; verified: boolean }> { return this.sessionGet('/totp/exists', sessionToken); } diff --git a/packages/fxa-graphql-api/src/lib/auth.ts b/packages/fxa-graphql-api/src/lib/auth.ts index 261901e8a2..edb7437473 100644 --- a/packages/fxa-graphql-api/src/lib/auth.ts +++ b/packages/fxa-graphql-api/src/lib/auth.ts @@ -13,10 +13,9 @@ export class SessionTokenAuth { @Inject(fxAccountClientToken) private authClient!: AuthClient; - public async lookupUserId(sessionToken: string): Promise { + public async getSessionStatus(sessionToken: string) { try { - const result = await this.authClient.sessionStatus(sessionToken); - return result.uid; + return await this.authClient.sessionStatus(sessionToken); } catch (err) { throw new AuthenticationError('Invalid session token'); } diff --git a/packages/fxa-graphql-api/src/lib/resolvers/account-resolver.ts b/packages/fxa-graphql-api/src/lib/resolvers/account-resolver.ts index 1b452bbc0a..27183ad8cd 100644 --- a/packages/fxa-graphql-api/src/lib/resolvers/account-resolver.ts +++ b/packages/fxa-graphql-api/src/lib/resolvers/account-resolver.ts @@ -215,7 +215,7 @@ export class AccountResolver { @Query((returns) => AccountType, { nullable: true }) public account(@Ctx() context: Context, @Info() info: GraphQLResolveInfo) { - context.logger.info('account', { uid: context.authUser }); + context.logger.info('account', { uid: context.session.uid }); // Introspect the query to determine if we should load the emails const parsed: any = parseResolveInfo(info); @@ -228,7 +228,7 @@ export class AccountResolver { const options: AccountOptions = includeEmails ? { include: ['emails'] } : {}; - return accountByUid(context.authUser, options); + return accountByUid(context.session.uid, options); } @FieldResolver() diff --git a/packages/fxa-graphql-api/src/lib/resolvers/session-resolver.ts b/packages/fxa-graphql-api/src/lib/resolvers/session-resolver.ts new file mode 100644 index 0000000000..42bd4970e0 --- /dev/null +++ b/packages/fxa-graphql-api/src/lib/resolvers/session-resolver.ts @@ -0,0 +1,25 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { GraphQLResolveInfo } from 'graphql'; +import { + Ctx, + Info, + Query, + Resolver, +} from 'type-graphql'; +import { Context } from '../server'; +import { Session as SessionType } from './types/session' + +@Resolver((of) => SessionType) +export class SessionResolver { + + @Query((returns) => SessionType) + session(@Ctx() context: Context, @Info() info: GraphQLResolveInfo) { + context.logger.info('session', { uid: context.session.uid }); + return { + verified: context.session.state === 'verified' + } as SessionType + } +} diff --git a/packages/fxa-graphql-api/src/lib/resolvers/types/session.ts b/packages/fxa-graphql-api/src/lib/resolvers/types/session.ts new file mode 100644 index 0000000000..543c26113c --- /dev/null +++ b/packages/fxa-graphql-api/src/lib/resolvers/types/session.ts @@ -0,0 +1,14 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + + import { Field, ObjectType } from 'type-graphql'; + + @ObjectType({ description: 'Session (token) info' }) + export class Session { + @Field({ + description: + 'Whether the current session is verified', + }) + public verified!: boolean; + } diff --git a/packages/fxa-graphql-api/src/lib/server.ts b/packages/fxa-graphql-api/src/lib/server.ts index 66a85958a9..04875f1fa1 100644 --- a/packages/fxa-graphql-api/src/lib/server.ts +++ b/packages/fxa-graphql-api/src/lib/server.ts @@ -12,6 +12,7 @@ import { SessionTokenAuth } from './auth'; import { AuthServerSource } from './datasources/authServer'; import { ProfileServerSource } from './datasources/profileServer'; import { AccountResolver } from './resolvers/account-resolver'; +import { SessionResolver } from './resolvers/session-resolver'; import { reportGraphQLError } from './sentry'; type ServerConfig = { @@ -39,7 +40,7 @@ export function formatError(debug: boolean, logger: Logger, err: GraphQLError) { * Context available to resolvers */ export type Context = { - authUser: string; + session: { uid: string; state: 'verified' | 'unverified' }; token: string; dataSources: DataSources; logger: Logger; @@ -57,11 +58,11 @@ export async function createServer( ): Promise { const schema = await TypeGraphQL.buildSchema({ container: Container, - resolvers: [AccountResolver], + resolvers: [AccountResolver, SessionResolver], validate: false, }); const authHeader = config.authHeader.toLowerCase(); - const authUser = Container.get(SessionTokenAuth); + const authToken = Container.get(SessionTokenAuth); const debugMode = config.env !== 'production'; const defaultContext = async ({ req }: { req: Request }) => { const bearerToken = req.headers[authHeader]; @@ -70,13 +71,12 @@ export async function createServer( 'Invalid authentcation header found at: ' + authHeader ); } - const userId = await authUser.lookupUserId(bearerToken); + const session = await authToken.getSessionStatus(bearerToken); return { - authUser: userId, token: bearerToken, + session, logger, - }; - ''; + } as Context; }; return new ApolloServer({ diff --git a/packages/fxa-graphql-api/src/test/lib/auth.spec.ts b/packages/fxa-graphql-api/src/test/lib/auth.spec.ts index fd1366ca4e..9cf03146ea 100644 --- a/packages/fxa-graphql-api/src/test/lib/auth.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/auth.spec.ts @@ -25,18 +25,21 @@ describe('SessionTokenAuth', () => { sandbox.resetHistory(); }); - describe('lookupUserId', async () => { + describe('getSessionStatus', async () => { it('looks up user successfully', async () => { - fxAccountClient.sessionStatus.resolves({ uid: '9001xyz', state: 'test' }); - const result = await sessionAuth.lookupUserId('token'); - assert.equal(result, '9001xyz'); + fxAccountClient.sessionStatus.resolves({ + uid: '9001xyz', + state: 'unverified', + }); + const result = await sessionAuth.getSessionStatus('token'); + assert.equal(result.uid, '9001xyz'); }); it('throws when the authClient throws', async () => { fxAccountClient.sessionStatus.rejects(new Error('boom')); try { - await sessionAuth.lookupUserId('token'); - assert.fail('lookupUserId should have thrown'); + await sessionAuth.getSessionStatus('token'); + assert.fail('getSessionStatus should have thrown'); } catch (e) { assert.instanceOf(e, AuthenticationError); } diff --git a/packages/fxa-graphql-api/src/test/lib/datasources/authServer.spec.ts b/packages/fxa-graphql-api/src/test/lib/datasources/authServer.spec.ts index d9ca8a4752..578befaa5f 100644 --- a/packages/fxa-graphql-api/src/test/lib/datasources/authServer.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/datasources/authServer.spec.ts @@ -84,7 +84,7 @@ describe('AuthServerSource', () => { recoveryEmailSecondaryResendCode: sandbox.stub(), }; Container.set(fxAccountClientToken, authClient); - context = mockContext(); + context = mockContext() as Context; authSource = new AuthServerSource(); const config = stubInterface>(); config.context = context; diff --git a/packages/fxa-graphql-api/src/test/lib/datasources/profileServer.spec.ts b/packages/fxa-graphql-api/src/test/lib/datasources/profileServer.spec.ts index 43e4d3f799..b9e86e7012 100644 --- a/packages/fxa-graphql-api/src/test/lib/datasources/profileServer.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/datasources/profileServer.spec.ts @@ -43,7 +43,7 @@ describe('ProfileServerSource', () => { }; Container.set(fxAccountClientToken, authClient); Container.set(configContainerToken, config); - context = mockContext(); + context = mockContext() as Context; profileSource = new ProfileServerSource(); const pluginConfig = stubInterface>(); pluginConfig.context = context; diff --git a/packages/fxa-graphql-api/src/test/lib/mocks.ts b/packages/fxa-graphql-api/src/test/lib/mocks.ts index feb45b9565..ea3387594e 100644 --- a/packages/fxa-graphql-api/src/test/lib/mocks.ts +++ b/packages/fxa-graphql-api/src/test/lib/mocks.ts @@ -13,7 +13,10 @@ export function mockContext() { profileAPI: stubInterface(), }; return { - authUser: '', + session: { + uid: '', + state: 'unverified', + }, token: 'test', logger: stubInterface(), dataSources: ds, diff --git a/packages/fxa-graphql-api/src/test/lib/resolvers/account-resolver.spec.ts b/packages/fxa-graphql-api/src/test/lib/resolvers/account-resolver.spec.ts index 9a6bfaa2cf..d2e9f00f49 100644 --- a/packages/fxa-graphql-api/src/test/lib/resolvers/account-resolver.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/resolvers/account-resolver.spec.ts @@ -56,7 +56,7 @@ describe('accountResolver', () => { accountCreated } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -78,7 +78,7 @@ describe('accountResolver', () => { uid } }`; - context.authUser = USER_2.uid; + context.session.uid = USER_2.uid; const result = (await graphql( schema, query, @@ -101,7 +101,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -125,7 +125,7 @@ describe('accountResolver', () => { displayName } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -155,7 +155,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -188,7 +188,7 @@ describe('accountResolver', () => { recoveryCodes } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -217,7 +217,7 @@ describe('accountResolver', () => { recoveryCodes } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -244,7 +244,7 @@ describe('accountResolver', () => { success } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -268,7 +268,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -291,7 +291,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -314,7 +314,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, @@ -339,7 +339,7 @@ describe('accountResolver', () => { clientMutationId } }`; - context.authUser = USER_1.uid; + context.session.uid = USER_1.uid; const result = (await graphql( schema, query, diff --git a/packages/fxa-graphql-api/src/test/lib/schema.spec.ts b/packages/fxa-graphql-api/src/test/lib/schema.spec.ts index 3ccb8e445f..2fdb7496f2 100644 --- a/packages/fxa-graphql-api/src/test/lib/schema.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/schema.spec.ts @@ -29,20 +29,22 @@ describe('Schema', () => { schemaIntrospection = builtSchema.data!.__schema as IntrospectionSchema; assert.isDefined(schemaIntrospection); queryType = schemaIntrospection.types.find( - type => type.name === (schemaIntrospection as IntrospectionSchema).queryType.name + (type) => + type.name === + (schemaIntrospection as IntrospectionSchema).queryType.name ) as IntrospectionObjectType; const mutationTypeNameRef = schemaIntrospection.mutationType; if (mutationTypeNameRef) { mutationType = schemaIntrospection.types.find( - type => type.name === mutationTypeNameRef.name + (type) => type.name === mutationTypeNameRef.name ) as IntrospectionObjectType; } }); function findTypeByName(name: string) { return schemaIntrospection.types.find( - type => type.kind === TypeKind.OBJECT && type.name === name + (type) => type.kind === TypeKind.OBJECT && type.name === name ) as IntrospectionObjectType; } @@ -50,13 +52,13 @@ describe('Schema', () => { const typ = findTypeByName(name); assert.isDefined(typ); assert.lengthOf(typ.fields, members.length); - const typNames = typ.fields.map(it => it.name); + const typNames = typ.fields.map((it) => it.name); assert.sameMembers(typNames, members); } it('is created with expected types', async () => { - const queryNames = queryType.fields.map(it => it.name); - assert.sameMembers(queryNames, ['account']); + const queryNames = queryType.fields.map((it) => it.name); + assert.sameMembers(queryNames, ['account', 'session']); verifyTypeMembers('Account', [ 'uid', @@ -106,5 +108,6 @@ describe('Schema', () => { 'os', ]); verifyTypeMembers('Location', ['city', 'country', 'state', 'stateCode']); + verifyTypeMembers('Session', ['verified']); }); }); diff --git a/packages/fxa-graphql-api/src/test/lib/server.spec.ts b/packages/fxa-graphql-api/src/test/lib/server.spec.ts index 72ca154aba..4c5873399e 100644 --- a/packages/fxa-graphql-api/src/test/lib/server.spec.ts +++ b/packages/fxa-graphql-api/src/test/lib/server.spec.ts @@ -20,7 +20,7 @@ import { createServer } from '../../lib/server'; const sandbox = sinon.createSandbox(); -const sessionAuth = { lookupUserId: sandbox.stub() }; +const sessionAuth = { getSessionStatus: sandbox.stub() }; const mockLogger = ({ info: () => {} } as unknown) as Logger; @@ -89,7 +89,7 @@ describe('createServer', () => { }); it('should throw an AuthenticationError when auth server has auth error', async () => { - sessionAuth.lookupUserId.rejects( + sessionAuth.getSessionStatus.rejects( new AuthenticationError('Invalid token') ); try { @@ -101,12 +101,16 @@ describe('createServer', () => { }); it('should return a user and the bearer token', async () => { - sessionAuth.lookupUserId.resolves('9001xyz'); + sessionAuth.getSessionStatus.resolves({ + uid: '9001xyz', + state: 'unverified', + }); try { const context = await (server as any).context({ req: { headers: { authorization: 'lolcatz' } }, }); - assert.equal(context.authUser, '9001xyz'); + assert.equal(context.session.uid, '9001xyz'); + assert.equal(context.session.state, 'unverified'); assert.equal(context.token, 'lolcatz'); } catch (e) { assert.fail('Should not have thrown an exception: ' + e); diff --git a/packages/fxa-settings/src/components/AccountDataHOC/index.tsx b/packages/fxa-settings/src/components/AccountDataHOC/index.tsx index d5ed3a24cb..15bee84ec4 100644 --- a/packages/fxa-settings/src/components/AccountDataHOC/index.tsx +++ b/packages/fxa-settings/src/components/AccountDataHOC/index.tsx @@ -15,9 +15,6 @@ type AccountDataHOCProps = { children: (props: AccountDataHOCChildrenProps) => React.ReactNode; }; -// A data extraction layer for initial load. -// TODO: If an account is unverified we'll need to -// requery with less requested properties. export const AccountDataHOC = ({ children }: AccountDataHOCProps) => { const { loading, error, data } = useQuery(GET_ACCOUNT); diff --git a/packages/fxa-settings/src/components/VerifiedSessionGuard/index.test.tsx b/packages/fxa-settings/src/components/VerifiedSessionGuard/index.test.tsx new file mode 100644 index 0000000000..ce853199d6 --- /dev/null +++ b/packages/fxa-settings/src/components/VerifiedSessionGuard/index.test.tsx @@ -0,0 +1,78 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { InMemoryCache } from '@apollo/client'; +import '@testing-library/jest-dom/extend-expect'; +import { render, act, wait, screen } from '@testing-library/react'; +import { MockedProvider, MockedResponse } from '@apollo/client/testing'; +import { VerifiedSessionGuard, GET_SESSION } from '.'; + +const verifiedCache = new InMemoryCache(); +verifiedCache.writeQuery({ + query: GET_SESSION, + data: { + session: { + verified: true, + }, + }, +}); + +const unverifiedCache = new InMemoryCache(); +unverifiedCache.writeQuery({ + query: GET_SESSION, + data: { + session: { + verified: false, + }, + }, +}); + +it('renders the content when verified', async () => { + render( + + oops}> +
Content
+
+
+ ); + + await wait(); + + expect(screen.getByTestId('children')).toBeInTheDocument(); +}); + +it('renders the guard when unverified', async () => { + async () => { + render( + + oops}> +
Content
+
+
+ ); + + await wait(); + + expect(screen.getByTestId('guard')).toBeInTheDocument(); + }; +}); + +it('renders the guard when loading fails', async () => { + // don't pollute the test output with console logs + const consoleError = console.error; + console.error = () => {}; + render( + + oops}> +
Content
+
+
+ ); + + await wait(); + + expect(screen.getByTestId('guard')).toBeInTheDocument(); + console.error = consoleError; +}); diff --git a/packages/fxa-settings/src/components/VerifiedSessionGuard/index.tsx b/packages/fxa-settings/src/components/VerifiedSessionGuard/index.tsx new file mode 100644 index 0000000000..1f812edaf2 --- /dev/null +++ b/packages/fxa-settings/src/components/VerifiedSessionGuard/index.tsx @@ -0,0 +1,37 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { gql, useQuery } from '@apollo/client'; +import sentryMetrics from 'fxa-shared/lib/sentry'; + +export const GET_SESSION = gql` + query GetSession { + session { + verified + } + } +`; + +export const VerifiedSessionGuard = ({ + guard, + children, +}: { + guard: React.ReactElement; + children: React.ReactElement; +}) => { + const { error, data } = useQuery(GET_SESSION, { fetchPolicy: 'cache-only' }); + + if (error || !data) { + // idk if this'll ever happen irl + const e = error || new Error('VerifiedSessionGuard missing data'); + console.error(e); + sentryMetrics.captureException(e); + return guard; + } + + return data.session.verified ? children : guard; +}; + +export default VerifiedSessionGuard; diff --git a/packages/fxa-settings/src/lib/auth.ts b/packages/fxa-settings/src/lib/auth.ts index 6cc2d484ef..fd7b2dc441 100644 --- a/packages/fxa-settings/src/lib/auth.ts +++ b/packages/fxa-settings/src/lib/auth.ts @@ -15,3 +15,6 @@ export function useAuth() { } return auth!; } + +// NOTE for future self +// In cases where the sessionToken changes we should also flush the session { verified } value from apollo cache diff --git a/packages/fxa-settings/src/lib/cache.ts b/packages/fxa-settings/src/lib/cache.ts index 22a2d09ab2..d5a487580e 100644 --- a/packages/fxa-settings/src/lib/cache.ts +++ b/packages/fxa-settings/src/lib/cache.ts @@ -17,18 +17,18 @@ export function sessionToken(newToken?: string) { } } -// sessionToken is added as a local field in the cache as an example. +// sessionToken is added as a local field as an example. export const typeDefs = gql` - extend type Query { - sessionToken: String! + extend type Session { + token: String! } `; export const cache = new InMemoryCache({ typePolicies: { - Query: { + Session: { fields: { - sessionToken: { + token: { read() { return sessionToken(); },