feat(server): move authentication to tokens stored in the database (i…

…mmich-app#1381) * chore: add typeorm commands to npm and set default database config values * feat: move to server side authentication tokens * fix: websocket should emit error and disconnect on error thrown by the server * refactor: rename cookie-auth-strategy to user-auth-strategy * feat: user tokens and API keys now use SHA256 hash for performance improvements * test: album e2e test remove unneeded module import * infra: truncate api key table as old keys will no longer work with new hash algorithm * fix(server): e2e tests (immich-app#1435) * fix: root module paths * chore: linting * chore: rename user-auth to strategy.ts and make validate return AuthUserDto * fix: we should always send HttpOnly for our auth cookies * chore: remove now unused crypto functions and jwt dependencies * fix: return the extra fields for AuthUserDto in auth service validate --------- Co-authored-by: Jason Rasmussen <[email protected]>
shiqustudio · Jan 27, 2023 · 3f2513a · 3f2513a
1 parent 9be71f6
commit 3f2513a
Show file tree

Hide file tree

Showing 61 changed files with 372 additions and 516 deletions.
diff --git a/docker/.env.test b/docker/.env.test
@@ -10,9 +10,6 @@ REDIS_HOSTNAME=immich-redis-test
 # Upload File Config
 UPLOAD_LOCATION=./upload
 
-# JWT SECRET
-JWT_SECRET=randomstringthatissolongandpowerfulthatnoonecanguess
-
 # MAPBOX
 ## ENABLE_MAPBOX is either true of false -> if true, you have to provide MAPBOX_KEY
 ENABLE_MAPBOX=false

diff --git a/docker/example.env b/docker/example.env
@@ -30,16 +30,6 @@ REDIS_HOSTNAME=immich_redis
 
 UPLOAD_LOCATION=absolute_location_on_your_machine_where_you_want_to_store_the_backup
 
-###################################################################################
-# JWT SECRET
-#
-# This JWT_SECRET is used to sign the authentication keys for user login
-# You should set it to a long randomly generated value
-# You can use this command to generate one: openssl rand -base64 128
-###################################################################################
-
-JWT_SECRET=
-
 ###################################################################################
 # Reverse Geocoding
 #

diff --git a/docs/docs/developer/setup.md b/docs/docs/developer/setup.md
@@ -24,7 +24,7 @@ All the services are packaged to run as with single Docker Compose command.
 
 1. Clone the project repo.
 2. Run `cp docker/example.env docker/.env`.
-3. Edit `docker/.env` to provide values for the required variables `UPLOAD_LOCATION` and `JWT_SECRET`.
+3. Edit `docker/.env` to provide values for the required variable `UPLOAD_LOCATION`.
 4. From the root directory, run:
 
 ```bash title="Start development server"

diff --git a/docs/docs/install/docker-compose.md b/docs/docs/install/docker-compose.md
@@ -63,15 +63,6 @@ UPLOAD_LOCATION=absolute_location_on_your_machine_where_you_want_to_store_the_ba
 
 LOG_LEVEL=simple
 
-###################################################################################
-# JWT SECRET
-###################################################################################
-
-# This JWT_SECRET is used to sign the authentication keys for user login
-# You should set it to a long randomly generated value
-# You can use this command to generate one: openssl rand -base64 128
-JWT_SECRET=
-
 ###################################################################################
 # Reverse Geocoding
 ####################################################################################
@@ -102,11 +93,6 @@ PUBLIC_LOGIN_PAGE_MESSAGE="My Family Photos and Videos Backup Server"
 
 - Populate custom database information if necessary.
 - Populate `UPLOAD_LOCATION` with your preferred location for storing backup assets.
-- Populate a secret value for `JWT_SECRET`. You can use the command below to generate a secure key:
-
-```bash title="Command to generate secure JWT_SECRET key"
-openssl rand -base64 128
-```
 
 ### Step 3 - Start the containers
 

diff --git a/docs/docs/install/portainer.md b/docs/docs/install/portainer.md
@@ -40,11 +40,6 @@ Install Immich using Portainer's Stack feature.
 
 * Populate custom database information if necessary.
 * Populate `UPLOAD_LOCATION` with your preferred location for storing backup assets.
-* Populate a secret value for `JWT_SECRET`. You can use the command below to generate a secure key:
-
-```bash title="Generate secure JWT_SECRET key"
-openssl rand -base64 128
-```
 
 11. Click on "**Deploy the stack**".
 

diff --git a/docs/docs/install/unraid.md b/docs/docs/install/unraid.md
@@ -55,7 +55,6 @@ alt="Select Plugins > Compose.Manager > Add New Stack > Label it Immich"
 6.  Select the cog ⚙️ next to Immich, click "**Edit Stack**", then click "**Env File**"
 7.  Past the entire contents of the [Immich example.env](https://raw.githubusercontent.com/immich-app/immich/main/docker/example.env) file into the Unraid editor, then **before saving** edit the following:
 
-    - `JWT_SECRET`: Generate a unique secret and paste the value here > Can be generated by either typing `openssl rand -base64 128` in your terminal or copying from [uuidgenerator](https://www.uuidgenerator.net/version1)
     - `UPLOAD_LOCATION`: Create a folder in your Images Unraid share and place the **absolute** location here > For example my _"images"_ share has a folder within it called _"immich"_. If I browse to this directory in the terminal and type `pwd` the output is `/mnt/user/images/immich`. This is the exact value I need to enter as my `UPLOAD_LOCATION`
 
       <img

diff --git a/install.sh b/install.sh
@@ -45,12 +45,6 @@ populate_upload_location() {
   replace_env_value "UPLOAD_LOCATION" $upload_location
 }
 
-generate_jwt_secret() {
-  echo "Generating JWT_SECRET value..."
-  jwt_secret=$(openssl rand -base64 128)
-  replace_env_value "JWT_SECRET" $jwt_secret
-}
-
 start_docker_compose() {
   echo "Starting Immich's docker containers"
 
@@ -92,5 +86,4 @@ create_immich_directory
 download_docker_compose_file
 download_dot_env_file
 populate_upload_location
-generate_jwt_secret
 start_docker_compose
diff --git a/server/apps/immich/src/api-v1/communication/communication.gateway.ts b/server/apps/immich/src/api-v1/communication/communication.gateway.ts
@@ -19,16 +19,16 @@ export class CommunicationGateway implements OnGatewayConnection, OnGatewayDisco
   async handleConnection(client: Socket) {
     try {
       this.logger.log(`New websocket connection: ${client.id}`);
-
-      const user = await this.authService.validateSocket(client);
+      const user = await this.authService.validate(client.request.headers);
       if (user) {
         client.join(user.id);
       } else {
         client.emit('error', 'unauthorized');
         client.disconnect();
       }
     } catch (e) {
-      // Logger.error(`Error establish websocket conneciton ${e}`, 'HandleWebscoketConnection');
+      client.emit('error', 'unauthorized');
+      client.disconnect();
     }
   }
 }
diff --git a/server/apps/immich/src/app.module.ts b/server/apps/immich/src/app.module.ts
@@ -1,7 +1,6 @@
 import { immichAppConfig } from '@app/common/config';
 import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common';
 import { AssetModule } from './api-v1/asset/asset.module';
-import { ImmichJwtModule } from './modules/immich-jwt/immich-jwt.module';
 import { DeviceInfoModule } from './api-v1/device-info/device-info.module';
 import { ConfigModule } from '@nestjs/config';
 import { ServerInfoModule } from './api-v1/server-info/server-info.module';
@@ -23,6 +22,9 @@ import {
   SystemConfigController,
   UserController,
 } from './controllers';
+import { PublicShareStrategy } from './modules/immich-auth/strategies/public-share.strategy';
+import { APIKeyStrategy } from './modules/immich-auth/strategies/api-key.strategy';
+import { UserAuthStrategy } from './modules/immich-auth/strategies/user-auth.strategy';
 
 @Module({
   imports: [
@@ -34,8 +36,6 @@ import {
 
     AssetModule,
 
-    ImmichJwtModule,
-
     DeviceInfoModule,
 
     ServerInfoModule,
@@ -64,7 +64,7 @@ import {
     SystemConfigController,
     UserController,
   ],
-  providers: [],
+  providers: [UserAuthStrategy, APIKeyStrategy, PublicShareStrategy],
 })
 export class AppModule implements NestModule {
   // TODO: check if consumer is needed or remove

diff --git a/server/apps/immich/src/decorators/authenticated.decorator.ts b/server/apps/immich/src/decorators/authenticated.decorator.ts
@@ -1,7 +1,7 @@
 import { UseGuards } from '@nestjs/common';
 import { AdminRolesGuard } from '../middlewares/admin-role-guard.middleware';
 import { RouteNotSharedGuard } from '../middlewares/route-not-shared-guard.middleware';
-import { AuthGuard } from '../modules/immich-jwt/guards/auth.guard';
+import { AuthGuard } from '../modules/immich-auth/guards/auth.guard';
 
 interface AuthenticatedOptions {
   admin?: boolean;

diff --git a/server/apps/immich/src/global.d.ts b/server/apps/immich/src/global.d.ts
@@ -4,5 +4,8 @@ declare global {
   namespace Express {
     // eslint-disable-next-line @typescript-eslint/no-empty-interface
     interface User extends AuthUserDto {}
+    export interface Request {
+      user: AuthUserDto;
+    }
   }
 }
diff --git a/server/apps/immich/src/main.ts b/server/apps/immich/src/main.ts
@@ -40,9 +40,6 @@ async function bootstrap() {
     .addBearerAuth({
       type: 'http',
       scheme: 'Bearer',
-      bearerFormat: 'JWT',
-      name: 'JWT',
-      description: 'Enter JWT token',
       in: 'header',
     })
     .addServer('/api')

diff --git a/...c/modules/immich-jwt/guards/auth.guard.ts → .../modules/immich-auth/guards/auth.guard.ts b/...c/modules/immich-jwt/guards/auth.guard.ts → .../modules/immich-auth/guards/auth.guard.ts
@@ -1,8 +1,8 @@
 import { Injectable } from '@nestjs/common';
 import { AuthGuard as PassportAuthGuard } from '@nestjs/passport';
 import { API_KEY_STRATEGY } from '../strategies/api-key.strategy';
-import { JWT_STRATEGY } from '../strategies/jwt.strategy';
+import { AUTH_COOKIE_STRATEGY } from '../strategies/user-auth.strategy';
 import { PUBLIC_SHARE_STRATEGY } from '../strategies/public-share.strategy';
 
 @Injectable()
-export class AuthGuard extends PassportAuthGuard([PUBLIC_SHARE_STRATEGY, JWT_STRATEGY, API_KEY_STRATEGY]) {}
+export class AuthGuard extends PassportAuthGuard([PUBLIC_SHARE_STRATEGY, AUTH_COOKIE_STRATEGY, API_KEY_STRATEGY]) {}
diff --git a/...immich-jwt/strategies/api-key.strategy.ts → ...mmich-auth/strategies/api-key.strategy.ts b/...immich-jwt/strategies/api-key.strategy.ts → ...mmich-auth/strategies/api-key.strategy.ts
diff --git a/...h-jwt/strategies/public-share.strategy.ts → ...-auth/strategies/public-share.strategy.ts b/...h-jwt/strategies/public-share.strategy.ts → ...-auth/strategies/public-share.strategy.ts
diff --git a/server/apps/immich/src/modules/immich-auth/strategies/user-auth.strategy.ts b/server/apps/immich/src/modules/immich-auth/strategies/user-auth.strategy.ts
@@ -0,0 +1,24 @@
+import { Injectable, UnauthorizedException } from '@nestjs/common';
+import { PassportStrategy } from '@nestjs/passport';
+import { AuthService, AuthUserDto, UserService } from '@app/domain';
+import { Strategy } from 'passport-custom';
+import { Request } from 'express';
+
+export const AUTH_COOKIE_STRATEGY = 'auth-cookie';
+
+@Injectable()
+export class UserAuthStrategy extends PassportStrategy(Strategy, AUTH_COOKIE_STRATEGY) {
+  constructor(private userService: UserService, private authService: AuthService) {
+    super();
+  }
+
+  async validate(request: Request): Promise<AuthUserDto> {
+    const authUser = await this.authService.validate(request.headers);
+
+    if (!authUser) {
+      throw new UnauthorizedException('Incorrect token provided');
+    }
+
+    return authUser;
+  }
+}
diff --git a/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts b/server/apps/immich/src/modules/immich-jwt/immich-jwt.module.ts
diff --git a/server/apps/immich/src/modules/immich-jwt/strategies/jwt.strategy.ts b/server/apps/immich/src/modules/immich-jwt/strategies/jwt.strategy.ts
diff --git a/server/apps/immich/test/album.e2e-spec.ts b/server/apps/immich/test/album.e2e-spec.ts
@@ -5,10 +5,10 @@ import { clearDb, getAuthUser, authCustom } from './test-utils';
 import { InfraModule } from '@app/infra';
 import { AlbumModule } from '../src/api-v1/album/album.module';
 import { CreateAlbumDto } from '../src/api-v1/album/dto/create-album.dto';
-import { ImmichJwtModule } from '../src/modules/immich-jwt/immich-jwt.module';
 import { AuthUserDto } from '../src/decorators/auth-user.decorator';
 import { AuthService, DomainModule, UserService } from '@app/domain';
 import { DataSource } from 'typeorm';
+import { AppModule } from '../src/app.module';
 
 function _createAlbum(app: INestApplication, data: CreateAlbumDto) {
   return request(app.getHttpServer()).post('/album').send(data);
@@ -21,7 +21,7 @@ describe('Album', () => {
   describe('without auth', () => {
     beforeAll(async () => {
       const moduleFixture: TestingModule = await Test.createTestingModule({
-        imports: [DomainModule.register({ imports: [InfraModule] }), AlbumModule, ImmichJwtModule],
+        imports: [DomainModule.register({ imports: [InfraModule] }), AppModule],
       }).compile();
 
       app = moduleFixture.createNestApplication();

diff --git a/server/apps/immich/test/jest-e2e.json b/server/apps/immich/test/jest-e2e.json
@@ -1,5 +1,6 @@
 {
   "moduleFileExtensions": ["js", "json", "ts"],
+  "modulePaths": ["<rootDir>", "<rootDir>../../../"],
   "rootDir": ".",
   "testEnvironment": "node",
   "testRegex": ".e2e-spec.ts$",

diff --git a/server/apps/immich/test/test-utils.ts b/server/apps/immich/test/test-utils.ts
@@ -2,7 +2,7 @@ import { CanActivate, ExecutionContext } from '@nestjs/common';
 import { TestingModuleBuilder } from '@nestjs/testing';
 import { DataSource } from 'typeorm';
 import { AuthUserDto } from '../src/decorators/auth-user.decorator';
-import { AuthGuard } from '../src/modules/immich-jwt/guards/auth.guard';
+import { AuthGuard } from '../src/modules/immich-auth/guards/auth.guard';
 
 type CustomAuthCallback = () => AuthUserDto;
 

diff --git a/server/apps/immich/test/user.e2e-spec.ts b/server/apps/immich/test/user.e2e-spec.ts
@@ -3,11 +3,11 @@ import { INestApplication } from '@nestjs/common';
 import request from 'supertest';
 import { clearDb, authCustom } from './test-utils';
 import { InfraModule } from '@app/infra';
-import { ImmichJwtModule } from '../src/modules/immich-jwt/immich-jwt.module';
 import { DomainModule, CreateUserDto, UserService, AuthUserDto } from '@app/domain';
 import { DataSource } from 'typeorm';
 import { UserController } from '../src/controllers';
 import { AuthService } from '@app/domain';
+import { AppModule } from '../src/app.module';
 
 function _createUser(userService: UserService, data: CreateUserDto) {
   return userService.createUser(data);
@@ -25,7 +25,7 @@ describe('User', () => {
   describe('without auth', () => {
     beforeAll(async () => {
       const moduleFixture: TestingModule = await Test.createTestingModule({
-        imports: [DomainModule.register({ imports: [InfraModule] }), ImmichJwtModule],
+        imports: [DomainModule.register({ imports: [InfraModule] }), AppModule],
         controllers: [UserController],
       }).compile();
 

diff --git a/server/immich-openapi-specs.json b/server/immich-openapi-specs.json
@@ -2722,8 +2722,6 @@
         "scheme": "Bearer",
         "bearerFormat": "JWT",
         "type": "http",
-        "name": "JWT",
-        "description": "Enter JWT token",
         "in": "header"
       }
     },

diff --git a/server/libs/common/src/config/app.config.ts b/server/libs/common/src/config/app.config.ts
@@ -1,20 +1,5 @@
-import { Logger } from '@nestjs/common';
 import { ConfigModuleOptions } from '@nestjs/config';
 import Joi from 'joi';
-import { createSecretKey, generateKeySync } from 'node:crypto';
-
-const jwtSecretValidator: Joi.CustomValidator<string> = (value) => {
-  const key = createSecretKey(value, 'base64');
-  const keySizeBits = (key.symmetricKeySize ?? 0) * 8;
-
-  if (keySizeBits < 128) {
-    const newKey = generateKeySync('hmac', { length: 256 }).export().toString('base64');
-    Logger.warn('The current JWT_SECRET key is insecure. It should be at least 128 bits long!');
-    Logger.warn(`Here is a new, securely generated key that you can use instead: ${newKey}`);
-  }
-
-  return value;
-};
 
 const WHEN_DB_URL_SET = Joi.when('DB_URL', {
   is: Joi.exist(),
@@ -31,7 +16,6 @@ export const immichAppConfig: ConfigModuleOptions = {
     DB_PASSWORD: WHEN_DB_URL_SET,
     DB_DATABASE_NAME: WHEN_DB_URL_SET,
     DB_URL: Joi.string().optional(),
-    JWT_SECRET: Joi.string().required().custom(jwtSecretValidator),
     DISABLE_REVERSE_GEOCODING: Joi.boolean().optional().valid(true, false).default(false),
     REVERSE_GEOCODING_PRECISION: Joi.number().optional().valid(0, 1, 2, 3).default(3),
     LOG_LEVEL: Joi.string().optional().valid('simple', 'verbose', 'debug', 'log', 'warn', 'error').default('log'),

diff --git a/server/libs/domain/src/api-key/api-key.repository.ts b/server/libs/domain/src/api-key/api-key.repository.ts
@@ -10,7 +10,7 @@ export interface IKeyRepository {
    * Includes the hashed `key` for verification
    * @param id
    */
-  getKey(id: number): Promise<APIKeyEntity | null>;
+  getKey(hashedToken: string): Promise<APIKeyEntity | null>;
   getById(userId: string, id: number): Promise<APIKeyEntity | null>;
   getByUserId(userId: string): Promise<APIKeyEntity[]>;
 }