refactor(server): auth guard (#1472)

* refactor: auth guard

* chore: move auth guard to middleware

* chore: tests

* chore: remove unused code

* fix: migration to uuid without dataloss

* chore: e2e tests

* chore: removed unused guards
This commit is contained in:
Jason Rasmussen
2023-01-31 13:11:49 -05:00
committed by GitHub
parent 68af4cd5ba
commit d2a9363fc5
40 changed files with 331 additions and 505 deletions

View File

@@ -19,7 +19,7 @@ export class CommunicationGateway implements OnGatewayConnection, OnGatewayDisco
async handleConnection(client: Socket) {
try {
this.logger.log(`New websocket connection: ${client.id}`);
const user = await this.authService.validate(client.request.headers);
const user = await this.authService.validate(client.request.headers, {});
if (user) {
client.join(user.id);
} else {

View File

@@ -21,9 +21,8 @@ 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';
import { APP_GUARD } from '@nestjs/core';
import { AuthGuard } from './middlewares/auth.guard';
@Module({
imports: [
@@ -61,7 +60,7 @@ import { UserAuthStrategy } from './modules/immich-auth/strategies/user-auth.str
SystemConfigController,
UserController,
],
providers: [UserAuthStrategy, APIKeyStrategy, PublicShareStrategy],
providers: [{ provide: APP_GUARD, useExisting: AuthGuard }, AuthGuard],
})
export class AppModule implements NestModule {
// TODO: check if consumer is needed or remove

View File

@@ -1,25 +1,28 @@
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-auth/guards/auth.guard';
import { applyDecorators, SetMetadata } from '@nestjs/common';
interface AuthenticatedOptions {
admin?: boolean;
isShared?: boolean;
}
export enum Metadata {
AUTH_ROUTE = 'auth_route',
ADMIN_ROUTE = 'admin_route',
SHARED_ROUTE = 'shared_route',
}
export const Authenticated = (options?: AuthenticatedOptions) => {
const guards: Parameters<typeof UseGuards> = [AuthGuard];
const decorators = [SetMetadata(Metadata.AUTH_ROUTE, true)];
options = options || {};
if (options.admin) {
guards.push(AdminRolesGuard);
decorators.push(SetMetadata(Metadata.ADMIN_ROUTE, true));
}
if (!options.isShared) {
guards.push(RouteNotSharedGuard);
if (options.isShared) {
decorators.push(SetMetadata(Metadata.SHARED_ROUTE, true));
}
return UseGuards(...guards);
return applyDecorators(...decorators);
};

View File

@@ -1,23 +0,0 @@
import { CanActivate, ExecutionContext, Injectable, Logger } from '@nestjs/common';
import { Request } from 'express';
import { UserResponseDto } from '@app/domain';
interface UserRequest extends Request {
user: UserResponseDto;
}
@Injectable()
export class AdminRolesGuard implements CanActivate {
logger = new Logger(AdminRolesGuard.name);
async canActivate(context: ExecutionContext): Promise<boolean> {
const request = context.switchToHttp().getRequest<UserRequest>();
const isAdmin = request.user?.isAdmin || false;
if (!isAdmin) {
this.logger.log(`Denied access to admin only route: ${request.path}`);
return false;
}
return true;
}
}

View File

@@ -0,0 +1,46 @@
import { AuthService } from '@app/domain';
import { CanActivate, ExecutionContext, Injectable, Logger } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { Request } from 'express';
import { Metadata } from '../decorators/authenticated.decorator';
@Injectable()
export class AuthGuard implements CanActivate {
private logger = new Logger(AuthGuard.name);
constructor(private reflector: Reflector, private authService: AuthService) {}
async canActivate(context: ExecutionContext): Promise<boolean> {
const targets = [context.getHandler(), context.getClass()];
const isAuthRoute = this.reflector.getAllAndOverride(Metadata.AUTH_ROUTE, targets);
const isAdminRoute = this.reflector.getAllAndOverride(Metadata.ADMIN_ROUTE, targets);
const isSharedRoute = this.reflector.getAllAndOverride(Metadata.SHARED_ROUTE, targets);
if (!isAuthRoute) {
return true;
}
const req = context.switchToHttp().getRequest<Request>();
const authDto = await this.authService.validate(req.headers, req.query as Record<string, string>);
if (!authDto) {
this.logger.warn(`Denied access to authenticated route: ${req.path}`);
return false;
}
if (authDto.isPublicUser && !isSharedRoute) {
this.logger.warn(`Denied access to non-shared route: ${req.path}`);
return false;
}
if (isAdminRoute && !authDto.isAdmin) {
this.logger.warn(`Denied access to admin only route: ${req.path}`);
return false;
}
req.user = authDto;
return true;
}
}

View File

@@ -1,21 +0,0 @@
import { CanActivate, ExecutionContext, Injectable, Logger } from '@nestjs/common';
import { Request } from 'express';
import { AuthUserDto } from '../decorators/auth-user.decorator';
@Injectable()
export class RouteNotSharedGuard implements CanActivate {
logger = new Logger(RouteNotSharedGuard.name);
async canActivate(context: ExecutionContext): Promise<boolean> {
const request = context.switchToHttp().getRequest<Request>();
const user = request.user as AuthUserDto;
// Inverse logic - I know it is weird
if (user.isPublicUser) {
this.logger.warn(`Denied attempt to access non-shared route: ${request.path}`);
return false;
}
return true;
}
}

View File

@@ -1,8 +0,0 @@
import { Injectable } from '@nestjs/common';
import { AuthGuard as PassportAuthGuard } from '@nestjs/passport';
import { API_KEY_STRATEGY } from '../strategies/api-key.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, AUTH_COOKIE_STRATEGY, API_KEY_STRATEGY]) {}

View File

@@ -1,21 +0,0 @@
import { APIKeyService, AuthUserDto } from '@app/domain';
import { Injectable } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { IStrategyOptions, Strategy } from 'passport-http-header-strategy';
export const API_KEY_STRATEGY = 'api-key';
const options: IStrategyOptions = {
header: 'x-api-key',
};
@Injectable()
export class APIKeyStrategy extends PassportStrategy(Strategy, API_KEY_STRATEGY) {
constructor(private apiKeyService: APIKeyService) {
super(options);
}
validate(token: string): Promise<AuthUserDto | null> {
return this.apiKeyService.validate(token);
}
}

View File

@@ -1,22 +0,0 @@
import { Injectable } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { IStrategyOptions, Strategy } from 'passport-http-header-strategy';
import { AuthUserDto, ShareService } from '@app/domain';
export const PUBLIC_SHARE_STRATEGY = 'public-share';
const options: IStrategyOptions = {
header: 'x-immich-share-key',
param: 'key',
};
@Injectable()
export class PublicShareStrategy extends PassportStrategy(Strategy, PUBLIC_SHARE_STRATEGY) {
constructor(private shareService: ShareService) {
super(options);
}
validate(key: string): Promise<AuthUserDto | null> {
return this.shareService.validate(key);
}
}

View File

@@ -1,18 +0,0 @@
import { AuthService, AuthUserDto } from '@app/domain';
import { Injectable } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { Request } from 'express';
import { Strategy } from 'passport-custom';
export const AUTH_COOKIE_STRATEGY = 'auth-cookie';
@Injectable()
export class UserAuthStrategy extends PassportStrategy(Strategy, AUTH_COOKIE_STRATEGY) {
constructor(private authService: AuthService) {
super();
}
validate(request: Request): Promise<AuthUserDto | null> {
return this.authService.validate(request.headers);
}
}

View File

@@ -2,11 +2,9 @@ import { Test, TestingModule } from '@nestjs/testing';
import { INestApplication } from '@nestjs/common';
import request from 'supertest';
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 { AuthUserDto } from '../src/decorators/auth-user.decorator';
import { AuthService, DomainModule, UserService } from '@app/domain';
import { AuthService, UserService } from '@app/domain';
import { DataSource } from 'typeorm';
import { AppModule } from '../src/app.module';
@@ -20,9 +18,7 @@ describe('Album', () => {
describe('without auth', () => {
beforeAll(async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [DomainModule.register({ imports: [InfraModule] }), AppModule],
}).compile();
const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule] }).compile();
app = moduleFixture.createNestApplication();
database = app.get(DataSource);
@@ -46,9 +42,7 @@ describe('Album', () => {
let authService: AuthService;
beforeAll(async () => {
const builder = Test.createTestingModule({
imports: [DomainModule.register({ imports: [InfraModule] }), AlbumModule],
});
const builder = Test.createTestingModule({ imports: [AppModule] });
authUser = getAuthUser(); // set default auth user
const moduleFixture: TestingModule = await authCustom(builder, () => authUser).compile();

View File

@@ -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-auth/guards/auth.guard';
import { AuthGuard } from '../src/middlewares/auth.guard';
type CustomAuthCallback = () => AuthUserDto;
@@ -34,5 +34,5 @@ export function authCustom(builder: TestingModuleBuilder, callback: CustomAuthCa
return true;
},
};
return builder.overrideGuard(AuthGuard).useValue(canActivate);
return builder.overrideProvider(AuthGuard).useValue(canActivate);
}

View File

@@ -2,10 +2,8 @@ import { Test, TestingModule } from '@nestjs/testing';
import { INestApplication } from '@nestjs/common';
import request from 'supertest';
import { clearDb, authCustom } from './test-utils';
import { InfraModule } from '@app/infra';
import { DomainModule, CreateUserDto, UserService, AuthUserDto } from '@app/domain';
import { 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';
@@ -24,10 +22,7 @@ describe('User', () => {
describe('without auth', () => {
beforeAll(async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [DomainModule.register({ imports: [InfraModule] }), AppModule],
controllers: [UserController],
}).compile();
const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [AppModule] }).compile();
app = moduleFixture.createNestApplication();
database = app.get(DataSource);
@@ -50,10 +45,7 @@ describe('User', () => {
let authUser: AuthUserDto;
beforeAll(async () => {
const builder = Test.createTestingModule({
imports: [DomainModule.register({ imports: [InfraModule] })],
controllers: [UserController],
});
const builder = Test.createTestingModule({ imports: [AppModule] });
const moduleFixture: TestingModule = await authCustom(builder, () => authUser).compile();
app = moduleFixture.createNestApplication();