From e00f40537bc5d7ba5606119b570aaec257697731 Mon Sep 17 00:00:00 2001 From: Peng Xiao Date: Sat, 9 Sep 2023 07:02:01 +0800 Subject: [PATCH] fix: allow login with credentials on production (#4288) --- .github/workflows/build-server.yml | 15 +++---- apps/server/src/modules/auth/guard.ts | 4 ++ .../src/modules/auth/next-auth-options.ts | 2 +- .../src/modules/auth/next-auth.controller.ts | 34 ++++++++++++++- ...invite.spec.ts => workspace-invite.e2e.ts} | 42 +++++++++++-------- .../{workspace.spec.ts => workspace.e2e.ts} | 33 +++++++++------ 6 files changed, 90 insertions(+), 40 deletions(-) rename apps/server/src/tests/{workspace-invite.spec.ts => workspace-invite.e2e.ts} (90%) rename apps/server/src/tests/{workspace.spec.ts => workspace.e2e.ts} (91%) diff --git a/.github/workflows/build-server.yml b/.github/workflows/build-server.yml index 1a13cb62aa..c2d3441abf 100644 --- a/.github/workflows/build-server.yml +++ b/.github/workflows/build-server.yml @@ -118,6 +118,8 @@ jobs: CARGO_TARGET_DIR: '${{ github.workspace }}/target' DATABASE_URL: postgresql://affine:affine@localhost:5432/affine ENABLE_LOCAL_EMAIL: true + OAUTH_EMAIL_LOGIN: affine + OAUTH_EMAIL_PASSWORD: affine - name: Upload server test coverage results uses: codecov/codecov-action@v3 @@ -168,15 +170,13 @@ jobs: - name: Generate prisma client run: | - yarn exec prisma generate - yarn exec prisma db push - working-directory: apps/server + yarn workspace @affine/server exec prisma generate + yarn workspace @affine/server exec prisma db push env: DATABASE_URL: postgresql://affine:affine@localhost:5432/affine - name: Run init-db script - run: yarn exec ts-node-esm ./scripts/init-db.ts - working-directory: apps/server + run: yarn workspace @affine/server exec ts-node-esm ./scripts/init-db.ts env: DATABASE_URL: postgresql://affine:affine@localhost:5432/affine @@ -187,12 +187,13 @@ jobs: path: ./apps/server - name: Run playwright tests - run: yarn e2e --forbid-only - working-directory: tests/affine-cloud + run: yarn workspace @affine-test/affine-cloud e2e --forbid-only env: COVERAGE: true DATABASE_URL: postgresql://affine:affine@localhost:5432/affine ENABLE_LOCAL_EMAIL: true + OAUTH_EMAIL_LOGIN: affine + OAUTH_EMAIL_PASSWORD: affine - name: Collect code coverage report run: yarn exec nyc report -t .nyc_output --report-dir .coverage --reporter=lcov diff --git a/apps/server/src/modules/auth/guard.ts b/apps/server/src/modules/auth/guard.ts index 8abb414010..f2f31cbb35 100644 --- a/apps/server/src/modules/auth/guard.ts +++ b/apps/server/src/modules/auth/guard.ts @@ -77,6 +77,10 @@ class AuthGuard implements CanActivate { if (isPublic) { return true; } else if (!token) { + if (!req.cookies) { + return isPublicable; + } + const session = await AuthHandler({ req: { cookies: req.cookies, diff --git a/apps/server/src/modules/auth/next-auth-options.ts b/apps/server/src/modules/auth/next-auth-options.ts index e0d40b4e76..fc122c3dad 100644 --- a/apps/server/src/modules/auth/next-auth-options.ts +++ b/apps/server/src/modules/auth/next-auth-options.ts @@ -85,7 +85,7 @@ export const NextAuthOptionsProvider: FactoryProvider = { adapter: prismaAdapter, debug: !config.node.prod, session: { - strategy: config.node.prod ? 'database' : 'jwt', + strategy: 'database', }, logger: { debug(code, metadata) { diff --git a/apps/server/src/modules/auth/next-auth.controller.ts b/apps/server/src/modules/auth/next-auth.controller.ts index b71c30d150..6119439e9a 100644 --- a/apps/server/src/modules/auth/next-auth.controller.ts +++ b/apps/server/src/modules/auth/next-auth.controller.ts @@ -17,7 +17,7 @@ import { hash, verify } from '@node-rs/argon2'; import type { User } from '@prisma/client'; import type { NextFunction, Request, Response } from 'express'; import { pick } from 'lodash-es'; -import type { AuthAction, NextAuthOptions } from 'next-auth'; +import type { AuthAction, CookieOption, NextAuthOptions } from 'next-auth'; import { AuthHandler } from 'next-auth/core'; import { Config } from '../../config'; @@ -29,6 +29,8 @@ import { AuthService } from './service'; const BASE_URL = '/api/auth/'; +const DEFAULT_SESSION_EXPIRE_DATE = 2592000 * 1000; // 30 days + @Controller(BASE_URL) export class NextAuthController { private readonly callbackSession; @@ -69,7 +71,11 @@ export class NextAuthController { .slice(BASE_URL.length) // make relative to baseUrl .replace(/\?.*/, '') // remove query part, use only path part .split('/') as [AuthAction, string]; // as array of strings; - if (providerId === 'credentials') { + + const credentialsSignIn = + req.method === 'POST' && providerId === 'credentials'; + let userId: string | undefined; + if (credentialsSignIn) { const { email } = req.body; if (email) { const user = await this.prisma.user.findFirst({ @@ -83,6 +89,7 @@ export class NextAuthController { req.body = null; throw new NotFoundException(`User not found`); } else { + userId = user.id; req.body = { ...req.body, name: user.name, @@ -140,6 +147,29 @@ export class NextAuthController { } } + let nextAuthTokenCookie: (CookieOption & { value: string }) | undefined; + // next-auth credentials login only support JWT strategy + // https://next-auth.js.org/configuration/providers/credentials + // let's store the session token in the database + if ( + credentialsSignIn && + (nextAuthTokenCookie = cookies?.find( + ({ name }) => name === 'next-auth.session-token' + )) + ) { + const cookieExpires = new Date(); + cookieExpires.setTime( + cookieExpires.getTime() + DEFAULT_SESSION_EXPIRE_DATE + ); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + await this.nextAuthOptions.adapter!.createSession!({ + sessionToken: nextAuthTokenCookie.value, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + userId: userId!, + expires: cookieExpires, + }); + } + if (redirect?.endsWith('api/auth/error?error=AccessDenied')) { this.logger.log(`Early access redirect headers: ${req.headers}`); this.metrics.authFailCounter(1, { diff --git a/apps/server/src/tests/workspace-invite.spec.ts b/apps/server/src/tests/workspace-invite.e2e.ts similarity index 90% rename from apps/server/src/tests/workspace-invite.spec.ts rename to apps/server/src/tests/workspace-invite.e2e.ts index 9299a980bb..8a93698734 100644 --- a/apps/server/src/tests/workspace-invite.spec.ts +++ b/apps/server/src/tests/workspace-invite.e2e.ts @@ -1,7 +1,7 @@ import type { INestApplication } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { PrismaClient } from '@prisma/client'; -import test from 'ava'; +import ava, { TestFn } from 'ava'; // @ts-expect-error graphql-upload is not typed import graphqlUploadExpress from 'graphql-upload/graphqlUploadExpress.mjs'; @@ -21,28 +21,26 @@ import { signUp, } from './utils'; -let app: INestApplication; +const test = ava as TestFn<{ + app: INestApplication; + client: PrismaClient; + auth: AuthService; + mail: MailService; +}>; -const client = new PrismaClient(); - -let auth: AuthService; -let mail: MailService; - -// cleanup database before each test -test.beforeEach(async () => { +test.beforeEach(async t => { + const client = new PrismaClient(); + t.context.client = client; await client.$connect(); await client.user.deleteMany({}); await client.snapshot.deleteMany({}); await client.update.deleteMany({}); await client.workspace.deleteMany({}); await client.$disconnect(); -}); - -test.beforeEach(async () => { const module = await Test.createTestingModule({ imports: [AppModule], }).compile(); - app = module.createNestApplication(); + const app = module.createNestApplication(); app.use( graphqlUploadExpress({ maxFileSize: 10 * 1024 * 1024, @@ -51,15 +49,19 @@ test.beforeEach(async () => { ); await app.init(); - auth = module.get(AuthService); - mail = module.get(MailService); + const auth = module.get(AuthService); + const mail = module.get(MailService); + t.context.app = app; + t.context.auth = auth; + t.context.mail = mail; }); -test.afterEach(async () => { - await app.close(); +test.afterEach(async t => { + await t.context.app.close(); }); test('should invite a user', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -76,6 +78,7 @@ test('should invite a user', async t => { }); test('should accept an invite', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -94,6 +97,7 @@ test('should accept an invite', async t => { }); test('should leave a workspace', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -108,6 +112,7 @@ test('should leave a workspace', async t => { }); test('should revoke a user', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -122,6 +127,7 @@ test('should revoke a user', async t => { }); test('should create user if not exist', async t => { + const { app, auth } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const workspace = await createWorkspace(app, u1.token.token); @@ -134,6 +140,7 @@ test('should create user if not exist', async t => { }); test('should invite a user by link', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -167,6 +174,7 @@ test('should invite a user by link', async t => { }); test('should send email', async t => { + const { mail, app } = t.context; if (mail.hasConfigured()) { const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'test', 'production@toeverything.info', '1'); diff --git a/apps/server/src/tests/workspace.spec.ts b/apps/server/src/tests/workspace.e2e.ts similarity index 91% rename from apps/server/src/tests/workspace.spec.ts rename to apps/server/src/tests/workspace.e2e.ts index c40989d6cc..2c764fb339 100644 --- a/apps/server/src/tests/workspace.spec.ts +++ b/apps/server/src/tests/workspace.e2e.ts @@ -1,7 +1,7 @@ import type { INestApplication } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { PrismaClient } from '@prisma/client'; -import test from 'ava'; +import ava, { TestFn } from 'ava'; // @ts-expect-error graphql-upload is not typed import graphqlUploadExpress from 'graphql-upload/graphqlUploadExpress.mjs'; import request from 'supertest'; @@ -20,25 +20,23 @@ import { updateWorkspace, } from './utils'; -let app: INestApplication; +const test = ava as TestFn<{ + app: INestApplication; + client: PrismaClient; +}>; -const client = new PrismaClient(); - -// cleanup database before each test -test.beforeEach(async () => { +test.beforeEach(async t => { + const client = new PrismaClient(); await client.$connect(); await client.user.deleteMany({}); await client.update.deleteMany({}); await client.snapshot.deleteMany({}); await client.workspace.deleteMany({}); await client.$disconnect(); -}); - -test.beforeEach(async () => { const module = await Test.createTestingModule({ imports: [AppModule], }).compile(); - app = module.createNestApplication(); + const app = module.createNestApplication(); app.use( graphqlUploadExpress({ maxFileSize: 10 * 1024 * 1024, @@ -46,20 +44,23 @@ test.beforeEach(async () => { }) ); await app.init(); + t.context.client = client; + t.context.app = app; }); -test.afterEach(async () => { - await app.close(); +test.afterEach(async t => { + await t.context.app.close(); }); test('should register a user', async t => { - const user = await signUp(app, 'u1', 'u1@affine.pro', '123456'); + const user = await signUp(t.context.app, 'u1', 'u1@affine.pro', '123456'); t.is(typeof user.id, 'string', 'user.id is not a string'); t.is(user.name, 'u1', 'user.name is not valid'); t.is(user.email, 'u1@affine.pro', 'user.email is not valid'); }); test.skip('should be throttled at call signUp', async t => { + const { app } = t.context; let token = ''; for (let i = 0; i < 10; i++) { token = (await signUp(app, `u${i}`, `u${i}@affine.pro`, `${i}`)).token @@ -72,6 +73,7 @@ test.skip('should be throttled at call signUp', async t => { }); test('should create a workspace', async t => { + const { app } = t.context; const user = await signUp(app, 'u1', 'u1@affine.pro', '1'); const workspace = await createWorkspace(app, user.token.token); @@ -79,6 +81,7 @@ test('should create a workspace', async t => { }); test('should can publish workspace', async t => { + const { app } = t.context; const user = await signUp(app, 'u1', 'u1@affine.pro', '1'); const workspace = await createWorkspace(app, user.token.token); @@ -100,6 +103,7 @@ test('should can publish workspace', async t => { }); test('should can read published workspace', async t => { + const { app } = t.context; const user = await signUp(app, 'u1', 'u1@affine.pro', '1'); const workspace = await createWorkspace(app, user.token.token); @@ -113,6 +117,7 @@ test('should can read published workspace', async t => { }); test('should share a page', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '1'); @@ -162,6 +167,7 @@ test('should share a page', async t => { }); test('should can get workspace doc', async t => { + const { app } = t.context; const u1 = await signUp(app, 'u1', 'u1@affine.pro', '1'); const u2 = await signUp(app, 'u2', 'u2@affine.pro', '2'); const workspace = await createWorkspace(app, u1.token.token); @@ -207,6 +213,7 @@ test('should can get workspace doc', async t => { }); test('should be able to get public workspace doc', async t => { + const { app } = t.context; const user = await signUp(app, 'u1', 'u1@affine.pro', '1'); const workspace = await createWorkspace(app, user.token.token);