From 387b951b1c9d05c36114bd1781e7d98093d87ffa Mon Sep 17 00:00:00 2001 From: dschom Date: Fri, 25 Aug 2023 11:06:01 -0700 Subject: [PATCH] bug(settings): Fix missing connected service name after password reset Because: - Service name was blank for connected device after going through password reset flow This Commit: - Makes sure user agent propagates from graphql to auth-server - Adds tests for our gql decorator class - Adds logic to functional test to ensure the service name isn't blank --- .../react-conversion/resetPassword.spec.ts | 5 +++ .../fxa-graphql-api/src/decorators.spec.ts | 32 +++++++++++++++++ packages/fxa-graphql-api/src/decorators.ts | 35 +++++++++++++------ 3 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 packages/fxa-graphql-api/src/decorators.spec.ts diff --git a/packages/functional-tests/tests/react-conversion/resetPassword.spec.ts b/packages/functional-tests/tests/react-conversion/resetPassword.spec.ts index 1aaed4da8c..5d898cd0de 100644 --- a/packages/functional-tests/tests/react-conversion/resetPassword.spec.ts +++ b/packages/functional-tests/tests/react-conversion/resetPassword.spec.ts @@ -82,6 +82,11 @@ test.describe('reset password react', () => { await page.getByRole('heading', { name: 'Settings', level: 2 }).waitFor(); + // Check that connected service name is not empty! + expect(await page.getByTestId('service-name').innerText()).toContain( + 'Firefox' + ); + // Cleanup requires setting this value to correct password credentials.password = NEW_PASSWORD; }); diff --git a/packages/fxa-graphql-api/src/decorators.spec.ts b/packages/fxa-graphql-api/src/decorators.spec.ts new file mode 100644 index 0000000000..c28c637cc6 --- /dev/null +++ b/packages/fxa-graphql-api/src/decorators.spec.ts @@ -0,0 +1,32 @@ +/* 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 { extractRequiredHeaders } from './decorators'; + +describe('gql decorators', () => { + describe('header extraction for auth server', () => { + const ip = '127.0.0.1'; + const agent = 'Mozilla/5.0'; + + it('forwards ip', () => { + const headers = extractRequiredHeaders({ ip, headers: {} }); + expect(headers.get('x-forwarded-for')).toEqual('127.0.0.1'); + }); + + it('forwards relevant headers', () => { + const headers = extractRequiredHeaders({ + ip, + headers: { 'user-agent': agent }, + }); + expect(headers.get('user-agent')).toEqual(agent); + }); + + it('does not forward irrelevant headers', () => { + const headers = extractRequiredHeaders({ + ip, + headers: { Accept: '*/*' }, + }); + expect(headers.get('Accept')).toBeNull(); + }); + }); +}); diff --git a/packages/fxa-graphql-api/src/decorators.ts b/packages/fxa-graphql-api/src/decorators.ts index de6f2c425c..fe219bf8ae 100644 --- a/packages/fxa-graphql-api/src/decorators.ts +++ b/packages/fxa-graphql-api/src/decorators.ts @@ -4,6 +4,8 @@ import { createParamDecorator, ExecutionContext } from '@nestjs/common'; import { GqlExecutionContext } from '@nestjs/graphql'; import { SessionTokenResult } from './auth/session-token.strategy'; +import { Request } from 'express'; +import { IncomingHttpHeaders } from 'http'; /** * Extracts the token from an authenticated user for a GraphQL request. @@ -39,16 +41,29 @@ export const GqlUserState = createParamDecorator( * Extracts headers to be sent to auth-server */ export const GqlXHeaders = createParamDecorator( - (data: unknown, context: ExecutionContext): Headers => { + (_data: unknown, context: ExecutionContext): Headers => { const ctx = GqlExecutionContext.create(context).getContext(); - const headers: Record = {}; - - // Set the x-forwarded-for header since the auth-server will use this - // to determine client geolocation - if (ctx.req?.ip) { - headers['x-forwarded-for'] = ctx.req?.ip; - } - - return new Headers(headers); + return extractRequiredHeaders(ctx.req); } ); + +export function extractRequiredHeaders(req?: { + ip: string; + headers: IncomingHttpHeaders; +}): Headers { + const headers: Record = {}; + + // Set the x-forwarded-for header since the auth-server will use this + // to determine client geolocation + if (req?.ip) { + headers['x-forwarded-for'] = req?.ip; + } + + // Set the user agent headers. Connected devices use this header for display name purposes + const ua = req?.headers['user-agent']; + if (ua) { + headers['user-agent'] = ua; + } + + return new Headers(headers); +}