diff --git a/.changeset/honest-knives-sleep.md b/.changeset/honest-knives-sleep.md new file mode 100644 index 000000000..319ede4e0 --- /dev/null +++ b/.changeset/honest-knives-sleep.md @@ -0,0 +1,5 @@ +--- +'hive': patch +--- + +Propagate updated email address from OIDC provider. This fixes a bug where a user was locked out of the Hive account after the email of the user on the OIDC provider side changed. diff --git a/.github/workflows/tests-integration.yaml b/.github/workflows/tests-integration.yaml index b8b7b8500..29c2682dc 100644 --- a/.github/workflows/tests-integration.yaml +++ b/.github/workflows/tests-integration.yaml @@ -54,6 +54,13 @@ jobs: uses: mikefarah/yq@4839dbbf80445070a31c7a9c1055da527db2d5ee # v4.44.6 with: cmd: yq -i 'del(.services.*.volumes)' docker/docker-compose.community.yml + # tests need to access host machine for OIDC stuff + - name: make host machine accessible to server container + uses: mikefarah/yq@4839dbbf80445070a31c7a9c1055da527db2d5ee # v4.44.6 + with: + cmd: + yq -i '.services.server.extra_hosts[] |= sub("host-gateway"; "172.17.0.1")' + ./integration-tests/docker-compose.integration.yaml - name: get cpu count for vitest id: cpu-cores diff --git a/cypress.config.ts b/cypress.config.ts index 739672eda..cd246b86d 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -61,40 +61,8 @@ export default defineConfig({ }; }, async getEmailConfirmationLink(input: string | { email: string; now: number }) { - const email = typeof input === 'string' ? input : input.email; - const now = new Date( - typeof input === 'string' ? Date.now() - 10_000 : input.now, - ).toISOString(); - const url = new URL('http://localhost:3014/_history'); - url.searchParams.set('after', now); - - return await asyncRetry( - async () => { - const emails = await fetch(url.toString()) - .then(res => res.json()) - .then(emails => - emails.filter(e => e.to === email && e.subject === 'Verify your email'), - ); - - if (emails.length === 0) { - throw new Error('Could not find email'); - } - - // take the latest one - const result = emails[emails.length - 1]; - - const urlMatch = result.body.match(/href=\"(http:\/\/[^\s"]+)/); - if (!urlMatch) throw new Error('No URL found in email'); - - const confirmUrl = new URL(urlMatch[1]); - return confirmUrl.pathname + confirmUrl.search; - }, - { - retries: 10, - minTimeout: 1000, - maxTimeout: 10000, - }, - ); + const url = await seed.pollForEmailVerificationLink(input); + return url.pathname + url.search; }, }); diff --git a/integration-tests/docker-compose.integration.yaml b/integration-tests/docker-compose.integration.yaml index 4e6ad78b9..b8cec1a9f 100644 --- a/integration-tests/docker-compose.integration.yaml +++ b/integration-tests/docker-compose.integration.yaml @@ -164,10 +164,12 @@ services: # Auth WEB_APP_URL: '${HIVE_APP_BASE_URL}' AUTH_ORGANIZATION_OIDC: '1' - AUTH_REQUIRE_EMAIL_VERIFICATION: '0' + AUTH_REQUIRE_EMAIL_VERIFICATION: '1' SUPERTOKENS_CONNECTION_URI: http://supertokens:3567 SUPERTOKENS_API_KEY: '${SUPERTOKENS_API_KEY}' GRAPHQL_PUBLIC_ORIGIN: http://localhost:8082 + extra_hosts: + - 'host.docker.internal:host-gateway' broker: image: redpandadata/redpanda:latest diff --git a/integration-tests/package.json b/integration-tests/package.json index 9759bcaae..9ac3f3f3f 100644 --- a/integration-tests/package.json +++ b/integration-tests/package.json @@ -21,12 +21,14 @@ "@hive/commerce": "workspace:*", "@hive/schema": "workspace:*", "@hive/server": "workspace:*", + "@hive/service-common": "workspace:*", "@hive/storage": "workspace:*", "@theguild/federation-composition": "0.21.3", "@trpc/client": "10.45.3", "@trpc/server": "10.45.3", "@types/async-retry": "1.4.8", "@types/dockerode": "3.3.43", + "@types/set-cookie-parser": "2.4.10", "async-retry": "1.3.3", "bcryptjs": "2.4.3", "csv-parse": "5.6.0", @@ -37,6 +39,7 @@ "graphql-sse": "2.6.0", "human-id": "4.1.1", "ioredis": "5.8.2", + "set-cookie-parser": "2.7.1", "slonik": "30.4.4", "strip-ansi": "7.1.2", "tslib": "2.8.1", diff --git a/integration-tests/testkit/oidc-integration.ts b/integration-tests/testkit/oidc-integration.ts new file mode 100644 index 000000000..902a0a927 --- /dev/null +++ b/integration-tests/testkit/oidc-integration.ts @@ -0,0 +1,368 @@ +import type { AddressInfo } from 'node:net'; +import humanId from 'human-id'; +import setCookie from 'set-cookie-parser'; +import { sql, type DatabasePool } from 'slonik'; +import z from 'zod'; +import formDataPlugin from '@fastify/formbody'; +import { createServer, type FastifyReply, type FastifyRequest } from '@hive/service-common'; +import { graphql } from './gql'; +import { execute } from './graphql'; +import { getServiceHost, pollForEmailVerificationLink } from './utils'; + +const apiAddress = await getServiceHost('server', 8082); + +async function createMockOIDCServer() { + const host = + process.env.RUN_AGAINST_LOCAL_SERVICES === '1' ? 'localhost' : 'host.docker.internal'; + const server = await createServer({ + sentryErrorHandler: false, + log: { + requests: false, + level: 'silent', + }, + name: '', + }); + await server.register(formDataPlugin); + + let registeredHandler: typeof handler; + + async function handler(request: FastifyRequest, reply: FastifyReply): Promise { + if (!handler) { + throw new Error('No handler registered'); + } + return await registeredHandler(request, reply); + } + + server.route({ + method: 'POST', + url: '/token', + handler, + }); + + server.route({ + method: 'GET', + url: '/userinfo', + handler, + }); + + await server.listen({ + port: 0, + host: '0.0.0.0', + }); + + return { + url: 'http://' + host + ':' + (server.server.address() as AddressInfo).port, + setHandler(newHandler: typeof handler) { + registeredHandler = newHandler; + }, + [Symbol.asyncDispose]: () => { + server.close(); + }, + }; +} + +const CreateOIDCIntegrationMutation = graphql(` + mutation TestKit_OIDCIntegration_CreateOIDCIntegrationMutation( + $input: CreateOIDCIntegrationInput! + ) { + createOIDCIntegration(input: $input) { + ok { + createdOIDCIntegration { + id + clientId + clientSecretPreview + tokenEndpoint + userinfoEndpoint + authorizationEndpoint + additionalScopes + oidcUserJoinOnly + oidcUserAccessOnly + } + } + error { + message + details { + clientId + clientSecret + tokenEndpoint + userinfoEndpoint + authorizationEndpoint + additionalScopes + } + } + } + } +`); + +const UpdateOIDCIntegrationMutation = graphql(` + mutation TestKit_OIDCIntegration_UpdateOIDCIntegrationMutation( + $input: UpdateOIDCIntegrationInput! + ) { + updateOIDCIntegration(input: $input) { + ok { + updatedOIDCIntegration { + id + tokenEndpoint + userinfoEndpoint + authorizationEndpoint + clientId + clientSecretPreview + additionalScopes + } + } + error { + message + details { + clientId + clientSecret + tokenEndpoint + userinfoEndpoint + authorizationEndpoint + additionalScopes + } + } + } + } +`); + +const SendVerificationEmailMutation = graphql(` + mutation TestKit_OIDCIntegration_SendVerificationEmailMutation( + $input: SendVerificationEmailInput! + ) { + sendVerificationEmail(input: $input) { + ok { + expiresAt + } + error { + message + emailAlreadyVerified + } + } + } +`); + +const VerifyEmailMutation = graphql(` + mutation TestKit_OIDCIntegration_VerifyEmailMutation($input: VerifyEmailInput!) { + verifyEmail(input: $input) { + ok { + verified + } + error { + message + } + } + } +`); + +export async function createOIDCIntegration(args: { + organizationId: string; + accessToken: string; + getPool: () => Promise; +}) { + const { accessToken: authToken, getPool } = args; + const result = await execute({ + document: CreateOIDCIntegrationMutation, + variables: { + input: { + organizationId: args.organizationId, + additionalScopes: [], + authorizationEndpoint: 'http://localhost:6666/noop/authoriation', + tokenEndpoint: 'http://localhost:6666/noop/token', + userinfoEndpoint: 'http://localhost:666/noop/userinfo', + clientId: 'noop', + clientSecret: 'noop', + }, + }, + authToken, + }).then(r => r.expectNoGraphQLErrors()); + + if (!result.createOIDCIntegration.ok) { + throw new Error(result.createOIDCIntegration.error?.message ?? 'Unexpected error.'); + } + + const oidcIntegration = result.createOIDCIntegration.ok.createdOIDCIntegration; + + return { + oidcIntegration, + async registerFakeDomain() { + const randomDomain = + humanId({ + separator: '', + capitalize: false, + }) + '.local'; + + const pool = await getPool(); + const query = sql` + INSERT INTO "oidc_integration_domains" ( + "organization_id" + , "oidc_integration_id" + , "domain_name" + , "verified_at" + ) VALUES ( + ${args.organizationId} + , ${oidcIntegration.id} + , ${randomDomain} + , NOW() + ) + `; + + await pool.query(query); + return randomDomain; + }, + async createMockServerAndUpdateIntegrationEndpoints(args?: { + additionalScopes?: Array; + clientId?: string; + clientSecret?: string; + }) { + const server = await createMockOIDCServer(); + + const result = await execute({ + document: UpdateOIDCIntegrationMutation, + variables: { + input: { + oidcIntegrationId: oidcIntegration.id, + authorizationEndpoint: server.url + '/authorize', + tokenEndpoint: server.url + '/token', + userinfoEndpoint: server.url + '/userinfo', + additionalScopes: args?.additionalScopes, + clientId: args?.clientId, + clientSecret: args?.clientSecret, + }, + }, + authToken, + }).then(r => r.expectNoGraphQLErrors()); + + if (!result.updateOIDCIntegration.ok) { + throw new Error(result.updateOIDCIntegration.error?.message ?? 'Unexpected error.'); + } + + return { + setHandler: server.setHandler, + setUser(args: { email: string; sub: string }) { + server.setHandler(async (req, res) => { + if (req.routeOptions.url === '/token') { + return res.status(200).send({ + access_token: 'yolo', + }); + } + + if (req.routeOptions.url === '/userinfo') { + return res.status(200).send({ + sub: args.sub, + email: args.email, + }); + } + + console.log('unhandled', req.routeOptions.url); + return res.status(404).send(); + }); + }, + async runGetAuthorizationUrl() { + const baseUrl = 'http://' + apiAddress; + const url = new URL('http://' + apiAddress + '/auth-api/authorisationurl'); + url.searchParams.set('thirdPartyId', 'oidc'); + url.searchParams.set('redirectURIOnProviderDashboard', baseUrl + '/'); + url.searchParams.set('oidc_id', oidcIntegration.id); + const result = await fetch(url).then(res => res.json()); + + const urlWithQueryParams = new URL(result.urlWithQueryParams); + return { + codeChallenge: urlWithQueryParams.searchParams.get('code_challenge') ?? '', + state: urlWithQueryParams.searchParams.get('state') ?? '', + }; + }, + async runSignInUp(args: { state: string; code?: string }) { + const url = new URL('http://' + apiAddress + '/auth-api/signinup'); + url.searchParams.set('oidc_id', oidcIntegration.id); + + const result = await fetch(url, { + method: 'POST', + body: JSON.stringify({ + thirdPartyId: 'oidc', + redirectURIInfo: { + redirectURIOnProviderDashboard: '/', + redirectURIQueryParams: { + state: args.state, + code: args.code ?? 'noop', + }, + }, + }), + headers: { + 'content-type': 'application/json', + 'st-auth-mode': 'cookie', + }, + }); + + if (result.status !== 200) { + throw new Error('Failed ' + result.status + (await result.text())); + } + + const rawBody = await result.json(); + + const body = z + .object({ + user: z.object({ + id: z.string(), + emails: z.array(z.string()), + loginMethods: z.array( + z.object({ + recipeUserId: z.string(), + }), + ), + }), + }) + .parse(rawBody); + const cookies = setCookie.parse(result.headers.getSetCookie()); + return { + accessToken: cookies.find(c => c.name === 'sAccessToken')?.value ?? ('' as string), + user: { + id: body.user.id, + email: body.user.emails[0], + userIdentityId: body.user.loginMethods[0]?.recipeUserId, + }, + }; + }, + async confirmEmail(args: { userIdentityId: string; email: string }) { + const now = Date.now(); + const sendMail = await execute({ + document: SendVerificationEmailMutation, + variables: { + input: { + userIdentityId: args.userIdentityId, + resend: true, + }, + }, + authToken, + }).then(e => e.expectNoGraphQLErrors()); + + if (!sendMail.sendVerificationEmail.ok) { + throw new Error(sendMail.sendVerificationEmail.error?.message ?? 'Unknown error.'); + } + + const url = await pollForEmailVerificationLink({ + email: args.email, + now, + }); + + const token = url.searchParams.get('token') ?? ''; + + const confirmMail = await execute({ + document: VerifyEmailMutation, + variables: { + input: { + userIdentityId: args.userIdentityId, + email: args.email, + token, + }, + }, + authToken, + }).then(e => e.expectNoGraphQLErrors()); + + if (!confirmMail.verifyEmail.ok) { + throw new Error(confirmMail.verifyEmail.error?.message ?? 'Unknown error.'); + } + }, + }; + }, + }; +} diff --git a/integration-tests/testkit/seed.ts b/integration-tests/testkit/seed.ts index e4d4cdf6f..52ad22ac7 100644 --- a/integration-tests/testkit/seed.ts +++ b/integration-tests/testkit/seed.ts @@ -53,13 +53,9 @@ import { updateTargetValidationSettings, } from './flow'; import * as GraphQLSchema from './gql/graphql'; -import { - BreakingChangeFormulaType, - ProjectType, - SchemaPolicyInput, - TargetAccessScope, -} from './gql/graphql'; +import { ProjectType, SchemaPolicyInput, TargetAccessScope } from './gql/graphql'; import { execute } from './graphql'; +import { createOIDCIntegration } from './oidc-integration.js'; import { CreateSavedFilterMutation, DeleteSavedFilterMutation, @@ -70,7 +66,7 @@ import { } from './saved-filters'; import { UpdateSchemaPolicyForOrganization, UpdateSchemaPolicyForProject } from './schema-policy'; import { collect, CollectedOperation, legacyCollect } from './usage'; -import { generateUnique, getServiceHost } from './utils'; +import { generateUnique, getServiceHost, pollForEmailVerificationLink } from './utils'; function createConnectionPool() { const pg = { @@ -106,11 +102,28 @@ export function initSeed() { return sharedDBPoolPromise.then(res => res.pool); } - async function doAuthenticate(email: string, oidcIntegrationId?: string) { - return await authenticate(await getPool(), email, oidcIntegrationId); + async function doAuthenticate( + email: string, + opts?: { + oidcIntegrationId?: string; + verifyEmail?: boolean; + }, + ) { + const auth = await authenticate(await getPool(), email, opts?.oidcIntegrationId); + + if (opts?.verifyEmail ?? true) { + const pool = await getPool(); + await pool.query(sql` + INSERT INTO "email_verifications" ("user_identity_id", "email", "verified_at") + VALUES (${auth.supertokensUserId}, ${email}, NOW()) + `); + } + + return auth; } return { + pollForEmailVerificationLink, async purgeOIDCDomains() { const pool = await getPool(); await pool.query(sql` @@ -162,15 +175,9 @@ export function initSeed() { }, async createOwner(verifyEmail: boolean = true) { const ownerEmail = userEmail(generateUnique()); - const auth = await doAuthenticate(ownerEmail); - - if (verifyEmail) { - const pool = await getPool(); - await pool.query(sql` - INSERT INTO "email_verifications" ("user_identity_id", "email", "verified_at") - VALUES (${auth.supertokensUserId}, ${ownerEmail}, NOW()) - `); - } + const auth = await doAuthenticate(ownerEmail, { + verifyEmail, + }); const ownerRefreshToken = auth.refresh_token; const ownerToken = auth.access_token; @@ -1159,9 +1166,10 @@ export function initSeed() { }, ); const memberEmail = userEmail(generateUnique()); - const memberToken = await doAuthenticate(memberEmail, oidcIntegrationId).then( - r => r.access_token, - ); + const memberToken = await doAuthenticate(memberEmail, { + oidcIntegrationId, + verifyEmail: true, + }).then(r => r.access_token); if (!oidcIntegrationId) { const invitationResult = await inviteToOrganization( @@ -1375,6 +1383,13 @@ export function initSeed() { }, }; }, + createOIDCIntegration() { + return createOIDCIntegration({ + organizationId: organization.id, + accessToken: ownerToken, + getPool: getPool, + }); + }, }; }, }; diff --git a/integration-tests/testkit/utils.ts b/integration-tests/testkit/utils.ts index 8fd3aa7bb..e96c9df1b 100644 --- a/integration-tests/testkit/utils.ts +++ b/integration-tests/testkit/utils.ts @@ -1,3 +1,4 @@ +import asyncRetry from 'async-retry'; import Docker from 'dockerode'; import { humanId } from 'human-id'; @@ -122,3 +123,37 @@ export function assertNonNullish( throw new Error(message); } } + +export async function pollForEmailVerificationLink(input: string | { email: string; now: number }) { + const email = typeof input === 'string' ? input : input.email; + const now = new Date(typeof input === 'string' ? Date.now() - 10_000 : input.now).toISOString(); + const url = new URL('http://localhost:3014/_history'); + url.searchParams.set('after', now); + + return await asyncRetry( + async () => { + const emails = await fetch(url.toString()) + .then(res => res.json()) + .then(emails => + emails.filter((e: any) => e.to === email && e.subject === 'Verify your email'), + ); + + if (emails.length === 0) { + throw new Error('Could not find email'); + } + + // take the latest one + const result = emails[emails.length - 1]; + + const urlMatch = result.body.match(/href=\"(http:\/\/[^\s"]+)/); + if (!urlMatch) throw new Error('No URL found in email'); + + return new URL(urlMatch[1]); + }, + { + retries: 10, + minTimeout: 1000, + maxTimeout: 10000, + }, + ); +} diff --git a/integration-tests/tests/api/auth/oidc.spec.ts b/integration-tests/tests/api/auth/oidc.spec.ts new file mode 100644 index 000000000..562bffcc5 --- /dev/null +++ b/integration-tests/tests/api/auth/oidc.spec.ts @@ -0,0 +1,157 @@ +import { graphql } from 'testkit/gql'; +import { execute } from 'testkit/graphql'; +import { initSeed } from 'testkit/seed'; + +const TestMeQuery = graphql(` + query OIDC_TestMeQuery { + me { + id + email + } + } +`); + +test.concurrent( + 'User can sign in/up with OIDC provider and confirm their email', + async ({ expect }) => { + const seed = initSeed(); + const email = seed.generateEmail(); + const { createOrg } = await seed.createOwner(); + const { createOIDCIntegration } = await createOrg(); + + const { createMockServerAndUpdateIntegrationEndpoints } = await createOIDCIntegration(); + const oidc = await createMockServerAndUpdateIntegrationEndpoints(); + + const auth = await oidc.runGetAuthorizationUrl(); + + oidc.setUser({ + sub: 'test-user', + email, + }); + + const result = await oidc.runSignInUp({ + state: auth.state, + }); + + const [error] = await execute({ + document: TestMeQuery, + authToken: result.accessToken, + }).then(r => r.expectGraphQLErrors()); + + expect(error).toMatchObject({ + extensions: { + code: 'VERIFY_EMAIL', + }, + message: 'Your account is not verified. Please verify your email address.', + }); + + await oidc.confirmEmail(result.user); + const meResult = await execute({ + document: TestMeQuery, + authToken: result.accessToken, + }).then(r => r.expectNoGraphQLErrors()); + expect(meResult).toMatchObject({ + me: { + email, + id: expect.any(String), + }, + }); + }, +); + +test.concurrent( + 'If the OIDC provider users email changes, the users email is updated upon login', + async ({ expect }) => { + const seed = initSeed(); + const oldEmail = seed.generateEmail(); + const newEmail = seed.generateEmail(); + const { createOrg } = await seed.createOwner(); + const { createOIDCIntegration } = await createOrg(); + + const { createMockServerAndUpdateIntegrationEndpoints } = await createOIDCIntegration(); + const oidc = await createMockServerAndUpdateIntegrationEndpoints(); + + let auth = await oidc.runGetAuthorizationUrl(); + + oidc.setUser({ + sub: 'test-user', + email: oldEmail, + }); + + let result = await oidc.runSignInUp({ + state: auth.state, + }); + + await oidc.confirmEmail(result.user); + + let meResult = await execute({ + document: TestMeQuery, + authToken: result.accessToken, + }).then(r => r.expectNoGraphQLErrors()); + expect(meResult).toMatchObject({ + me: { + email: oldEmail, + id: expect.any(String), + }, + }); + + auth = await oidc.runGetAuthorizationUrl(); + + oidc.setUser({ + sub: 'test-user', + email: newEmail, + }); + + result = await oidc.runSignInUp({ + state: auth.state, + }); + await oidc.confirmEmail(result.user); + meResult = await execute({ + document: TestMeQuery, + authToken: result.accessToken, + }).then(r => r.expectNoGraphQLErrors()); + expect(meResult).toMatchObject({ + me: { + email: newEmail, + id: expect.any(String), + }, + }); + }, +); + +test.concurrent( + 'User does not need to confirm their email if the domain is verified with the origanization', + async ({ expect }) => { + const seed = initSeed(); + const { createOrg } = await seed.createOwner(); + const { createOIDCIntegration } = await createOrg(); + + const { createMockServerAndUpdateIntegrationEndpoints, registerFakeDomain: registerDomain } = + await createOIDCIntegration(); + const domain = await registerDomain(); + const oidc = await createMockServerAndUpdateIntegrationEndpoints(); + + const email = 'foo@' + domain; + + let auth = await oidc.runGetAuthorizationUrl(); + + oidc.setUser({ + sub: 'test-user', + email, + }); + + const result = await oidc.runSignInUp({ + state: auth.state, + }); + const meResult = await execute({ + document: TestMeQuery, + authToken: result.accessToken, + }).then(r => r.expectNoGraphQLErrors()); + expect(meResult).toMatchObject({ + me: { + email, + id: expect.any(String), + }, + }); + }, +); diff --git a/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts b/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts index 39bc049e6..6289ba5b2 100644 --- a/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts +++ b/packages/services/api/src/modules/auth/lib/supertokens-strategy.ts @@ -430,6 +430,8 @@ export class SuperTokensUserAuthNStrategy extends AuthNStrategy