Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BC-8092 - implement config validation #21

Merged
merged 10 commits into from
Oct 16, 2024
6 changes: 1 addition & 5 deletions .env.default
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
REDIS=redis://172.18.0.5:6379
REDIS_PREFIX=y
REDIS=redis://localhost:6379
API_HOST=http://localhost:3030
ADMIN_API__ALLOWED_API_KEYS=randomString

S3_ENDPOINT=localhost
S3_BUCKET=ydocs
S3_PORT=9000
S3_SSL=false
S3_ACCESS_KEY=miniouser
S3_SECRET_KEY=miniouser

WS_PORT="3345"
9 changes: 9 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
REDIS=redis://localhost:6379
API_HOST=http://localhost:3030
ADMIN_API__ALLOWED_API_KEYS=randomString

S3_ENDPOINT=localhost
S3_PORT=9000
S3_SSL=false
S3_ACCESS_KEY=miniouser
S3_SECRET_KEY=miniouser
35 changes: 35 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
"@nestjs/passport": "^10.0.3",
"@nestjs/platform-express": "^10.4.1",
"@y/redis": "github:hpi-schul-cloud/y-redis#7d48e08d18ec78c9ab90063a7d867ec7f191319c",
"class-transformer": "^0.5.1",
"class-validator": "^0.14.1",
"ioredis": "^5.4.1",
"passport": "^0.7.0",
"passport-headerapikey": "^1.2.2",
Expand Down
4 changes: 3 additions & 1 deletion src/infra/auth-guard/auth-guard.module.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Module } from '@nestjs/common';
import { PassportModule } from '@nestjs/passport';
import { ConfigurationModule } from '../configuration/configuration.module.js';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configuration und config sind sich zu ähnlich um ausdrücken zu können was für was ist.

import { XApiKeyStrategy } from './strategy/index.js';
import { XApiKeyConfig } from './x-api-key.config.js';

@Module({
imports: [PassportModule],
imports: [PassportModule, ConfigurationModule.register(XApiKeyConfig)],
providers: [XApiKeyStrategy],
exports: [],
})
Expand Down
1 change: 0 additions & 1 deletion src/infra/auth-guard/config/index.ts

This file was deleted.

3 changes: 0 additions & 3 deletions src/infra/auth-guard/config/x-api-key.config.ts

This file was deleted.

21 changes: 10 additions & 11 deletions src/infra/auth-guard/strategy/x-api-key.strategy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,28 @@
import { createMock } from '@golevelup/ts-jest';
import { UnauthorizedException } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { Test, TestingModule } from '@nestjs/testing';
import { XApiKeyConfig } from '../config/x-api-key.config.js';
import { XApiKeyConfig } from '../x-api-key.config.js';
import { XApiKeyStrategy } from './x-api-key.strategy.js';

describe('XApiKeyStrategy', () => {
let module: TestingModule;
let strategy: XApiKeyStrategy;
let configService: ConfigService<XApiKeyConfig, true>;
let config: XApiKeyConfig;

beforeAll(async () => {
module = await Test.createTestingModule({
imports: [],
providers: [
XApiKeyStrategy,
{
provide: ConfigService,
useValue: createMock<ConfigService<XApiKeyConfig, true>>({
get: () => ['7ccd4e11-c6f6-48b0-81eb-cccf7922e7a4'],
}),
provide: XApiKeyConfig,
useValue: createMock<XApiKeyConfig>(),
},
],
}).compile();

strategy = module.get(XApiKeyStrategy);
configService = module.get(ConfigService<XApiKeyConfig, true>);
config = module.get(XApiKeyConfig);
});

afterAll(async () => {
Expand All @@ -41,33 +38,35 @@ describe('XApiKeyStrategy', () => {
describe('when a valid api key is provided', () => {
const setup = () => {
const CORRECT_API_KEY = '7ccd4e11-c6f6-48b0-81eb-cccf7922e7a4';
config.ADMIN_API__ALLOWED_API_KEYS = [CORRECT_API_KEY];

return { CORRECT_API_KEY, done };
};
it('should do nothing', () => {
const { CORRECT_API_KEY } = setup();
strategy.validate(CORRECT_API_KEY, done);
expect(done).toBeCalledWith(null, true);
expect(done).toHaveBeenCalledWith(null, true);
});
});

describe('when a invalid api key is provided', () => {
const setup = () => {
const INVALID_API_KEY = '7ccd4e11-c6f6-48b0-81eb-cccf7922e7a4BAD';
config.ADMIN_API__ALLOWED_API_KEYS = [INVALID_API_KEY];

return { INVALID_API_KEY, done };
};
it('should throw error', () => {
const { INVALID_API_KEY } = setup();
strategy.validate(INVALID_API_KEY, done);
expect(done).toBeCalledWith(new UnauthorizedException(), null);
expect(done).toHaveBeenCalledWith(new UnauthorizedException(), null);
});
});
});

describe('constructor', () => {
it('should create strategy', () => {
const ApiKeyStrategy = new XApiKeyStrategy(configService);
const ApiKeyStrategy = new XApiKeyStrategy(config);
expect(ApiKeyStrategy).toBeDefined();
expect(ApiKeyStrategy).toBeInstanceOf(XApiKeyStrategy);
});
Expand Down
7 changes: 3 additions & 4 deletions src/infra/auth-guard/strategy/x-api-key.strategy.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { Injectable, UnauthorizedException } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { PassportStrategy } from '@nestjs/passport';
import { Strategy } from 'passport-headerapikey/lib/Strategy.js';
import { XApiKeyConfig } from '../config/index.js';
import { StrategyType } from '../interface/index.js';
import { XApiKeyConfig } from '../x-api-key.config.js';

@Injectable()
export class XApiKeyStrategy extends PassportStrategy(Strategy, StrategyType.API_KEY) {
private readonly allowedApiKeys: string[];

public constructor(private readonly configService: ConfigService<XApiKeyConfig, true>) {
public constructor(private readonly config: XApiKeyConfig) {
super({ header: 'X-API-KEY' }, false);
this.allowedApiKeys = this.configService.get<string[]>('ADMIN_API__ALLOWED_API_KEYS');
this.allowedApiKeys = this.config.ADMIN_API__ALLOWED_API_KEYS;
}

public validate(apiKey: string, done: (error: Error | null, data: boolean | null) => void): void {
Expand Down
8 changes: 8 additions & 0 deletions src/infra/auth-guard/x-api-key.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Transform } from 'class-transformer';
import { IsArray } from 'class-validator';

export class XApiKeyConfig {
@Transform(({ value }) => value.split(',').map((part: string) => (part.split(':').pop() ?? '').trim()))
@IsArray()
public ADMIN_API__ALLOWED_API_KEYS!: string[];
}
6 changes: 6 additions & 0 deletions src/infra/authorization/authorization.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { IsUrl } from 'class-validator';

export class AuthorizationConfig {
@IsUrl({ require_tld: false })
public API_HOST!: string;
}
4 changes: 3 additions & 1 deletion src/infra/authorization/authorization.module.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Module } from '@nestjs/common';
import { ConfigurationModule } from '../configuration/configuration.module.js';
import { LoggerModule } from '../logging/logger.module.js';
import { AuthorizationConfig } from './authorization.config.js';
import { AuthorizationService } from './authorization.service.js';

@Module({
imports: [LoggerModule],
imports: [LoggerModule, ConfigurationModule.register(AuthorizationConfig)],
providers: [AuthorizationService],
exports: [AuthorizationService],
})
Expand Down
11 changes: 5 additions & 6 deletions src/infra/authorization/authorization.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { ConfigService } from '@nestjs/config';
import { Test, TestingModule } from '@nestjs/testing';
import { HttpRequest } from 'uws';
import { Logger } from '../logging/logger.js';
import { AuthorizationConfig } from './authorization.config.js';
import { AuthorizationService } from './authorization.service.js';

describe(AuthorizationService.name, () => {
let module: TestingModule;
let service: AuthorizationService;
let configService: DeepMocked<ConfigService>;

beforeAll(async () => {
module = await Test.createTestingModule({
providers: [
AuthorizationService,
{
provide: ConfigService,
useValue: createMock<ConfigService>(),
provide: AuthorizationConfig,
useValue: createMock<AuthorizationConfig>({
API_HOST: 'http://localhost:3000',
}),
},
{
provide: Logger,
Expand All @@ -26,7 +27,6 @@ describe(AuthorizationService.name, () => {
}).compile();

service = module.get<AuthorizationService>(AuthorizationService);
configService = module.get(ConfigService);
});

afterAll(async () => {
Expand All @@ -41,7 +41,6 @@ describe(AuthorizationService.name, () => {
const req: DeepMocked<HttpRequest> = createMock<HttpRequest>();
jest.spyOn(req, 'getParameter').mockReturnValue(roomId);
jest.spyOn(req, 'getHeader').mockReturnValue(cookies);
configService.getOrThrow.mockReturnValue('API_HOST');
const fetchSpy = jest.spyOn(global, 'fetch');

return { req, fetchSpy };
Expand Down
8 changes: 4 additions & 4 deletions src/infra/authorization/authorization.service.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { Injectable } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { HttpRequest } from 'uws';
import { Logger } from '../logging/logger.js';
import { AuthorizationConfig } from './authorization.config.js';
import { ResponsePayload } from './interfaces/response.payload.js';
import { ResponsePayloadBuilder } from './response.builder.js';

@Injectable()
export class AuthorizationService {
public constructor(
private configService: ConfigService,
private logger: Logger,
private readonly config: AuthorizationConfig,
private readonly logger: Logger,
) {
logger.setContext(AuthorizationService.name);
}
Expand Down Expand Up @@ -49,7 +49,7 @@ export class AuthorizationService {
}

private async fetchAuthorization(room: string, token: string): Promise<ResponsePayload> {
const apiHost = this.configService.getOrThrow<string>('API_HOST');
const apiHost = this.config.API_HOST;
const requestOptions = this.createAuthzRequestOptions(room, token);

const response = await fetch(`${apiHost}/api/v3/authorization/by-reference`, requestOptions);
Expand Down
39 changes: 39 additions & 0 deletions src/infra/configuration/configuration.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { DynamicModule, Module } from '@nestjs/common';
import { ConfigModule, ConfigModuleOptions } from '@nestjs/config';
import { Configuration } from './configuration.service.js';

const getEnvConfig = (): ConfigModuleOptions => {
const envConfig = {
envFilePath: '.env',
ignoreEnvFile: false,
};

if (process.env.NODE_ENV === 'test') {
envConfig.envFilePath = '.env.test';
}

if (process.env.NODE_ENV === 'production') {
envConfig.ignoreEnvFile = true;
}

return envConfig;
};

@Module({})
export class ConfigurationModule {
public static register<T extends object>(Constructor: new () => T): DynamicModule {
return {
imports: [ConfigModule.forRoot({ cache: true, ...getEnvConfig() })],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hier könnte man das cache: true auch noch nach getEnvConfig verschieben und einfach das ganze object den root übergeben.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das stimmt

providers: [
Configuration,
{
provide: Constructor,
useFactory: (config: Configuration): T => config.getAllValidConfigsByType(Constructor),
inject: [Configuration],
},
],
CeEv marked this conversation as resolved.
Show resolved Hide resolved
exports: [Configuration, Constructor],
module: ConfigurationModule,
};
}
}
Loading
Loading