Skip to content

Commit 35a1bda

Browse files
authored
Typechecking fixes (#77)
2 parents 4c2d780 + b25030b commit 35a1bda

16 files changed

Lines changed: 100 additions & 121 deletions

apps/backend/src/auth/auth.module.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { DynamicModule, Logger, Module } from '@nestjs/common';
1+
import { DynamicModule, Module } from '@nestjs/common';
22
import { ConfigModule, ConfigService } from '@nestjs/config';
33
import { JwtModule } from '@nestjs/jwt';
4+
import ms from 'ms';
45

56
import { MailingModule } from '@server/mailing/mailing.module';
67
import { UserModule } from '@server/user/user.module';
@@ -26,21 +27,13 @@ export class AuthModule {
2627
inject: [ConfigService],
2728
imports: [ConfigModule],
2829
useFactory: async (config: ConfigService) => {
29-
const JWT_SECRET = config.get('JWT_SECRET');
30-
const JWT_EXPIRES_IN = config.get('JWT_EXPIRES_IN');
31-
32-
if (!JWT_SECRET) {
33-
Logger.error('JWT_SECRET is not set');
34-
throw new Error('JWT_SECRET is not set');
35-
}
36-
37-
if (!JWT_EXPIRES_IN) {
38-
Logger.warn('JWT_EXPIRES_IN is not set, using default of 60s');
39-
}
30+
const JWT_SECRET = config.getOrThrow<ms.StringValue>('JWT_SECRET');
31+
const JWT_EXPIRES_IN =
32+
config.getOrThrow<ms.StringValue>('JWT_EXPIRES_IN');
4033

4134
return {
4235
secret: JWT_SECRET,
43-
signOptions: { expiresIn: JWT_EXPIRES_IN || '60s' },
36+
signOptions: { expiresIn: JWT_EXPIRES_IN },
4437
};
4538
},
4639
}),
@@ -58,7 +51,7 @@ export class AuthModule {
5851
inject: [ConfigService],
5952
provide: 'COOKIE_EXPIRES_IN',
6053
useFactory: (configService: ConfigService) =>
61-
configService.getOrThrow<string>('COOKIE_EXPIRES_IN'),
54+
configService.getOrThrow<ms.StringValue>('COOKIE_EXPIRES_IN'),
6255
},
6356
{
6457
inject: [ConfigService],
@@ -82,7 +75,7 @@ export class AuthModule {
8275
inject: [ConfigService],
8376
provide: 'JWT_EXPIRES_IN',
8477
useFactory: (configService: ConfigService) =>
85-
configService.getOrThrow<string>('JWT_EXPIRES_IN'),
78+
configService.getOrThrow<ms.StringValue>('JWT_EXPIRES_IN'),
8679
},
8780
{
8881
inject: [ConfigService],
@@ -94,7 +87,7 @@ export class AuthModule {
9487
inject: [ConfigService],
9588
provide: 'JWT_REFRESH_EXPIRES_IN',
9689
useFactory: (configService: ConfigService) =>
97-
configService.getOrThrow<string>('JWT_REFRESH_EXPIRES_IN'),
90+
configService.getOrThrow<ms.StringValue>('JWT_REFRESH_EXPIRES_IN'),
9891
},
9992
{
10093
inject: [ConfigService],

apps/backend/src/auth/auth.service.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ describe('AuthService', () => {
192192
const refreshToken = 'refresh-token';
193193

194194
spyOn(jwtService, 'signAsync').mockImplementation(
195-
(payload, options: any) => {
195+
(payload: any, options: any) => {
196196
if (options.secret === 'test-jwt-secret') {
197197
return Promise.resolve(accessToken);
198198
} else if (options.secret === 'test-jwt-refresh-secret') {

apps/backend/src/auth/auth.service.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Inject, Injectable, Logger } from '@nestjs/common';
22
import { JwtService } from '@nestjs/jwt';
33
import axios from 'axios';
44
import type { Request, Response } from 'express';
5+
import ms from 'ms';
56

67
import { CreateUser } from '@nbw/database';
78
import type { UserDocument } from '@nbw/database';
@@ -22,18 +23,18 @@ export class AuthService {
2223
@Inject(JwtService)
2324
private readonly jwtService: JwtService,
2425
@Inject('COOKIE_EXPIRES_IN')
25-
private readonly COOKIE_EXPIRES_IN: string,
26+
private readonly COOKIE_EXPIRES_IN: ms.StringValue,
2627
@Inject('FRONTEND_URL')
2728
private readonly FRONTEND_URL: string,
2829

2930
@Inject('JWT_SECRET')
3031
private readonly JWT_SECRET: string,
3132
@Inject('JWT_EXPIRES_IN')
32-
private readonly JWT_EXPIRES_IN: string,
33+
private readonly JWT_EXPIRES_IN: ms.StringValue,
3334
@Inject('JWT_REFRESH_SECRET')
3435
private readonly JWT_REFRESH_SECRET: string,
3536
@Inject('JWT_REFRESH_EXPIRES_IN')
36-
private readonly JWT_REFRESH_EXPIRES_IN: string,
37+
private readonly JWT_REFRESH_EXPIRES_IN: ms.StringValue,
3738
@Inject('APP_DOMAIN')
3839
private readonly APP_DOMAIN?: string,
3940
) {}
@@ -171,11 +172,11 @@ export class AuthService {
171172

172173
public async createJwtPayload(payload: TokenPayload): Promise<Tokens> {
173174
const [accessToken, refreshToken] = await Promise.all([
174-
this.jwtService.signAsync(payload, {
175+
this.jwtService.signAsync<TokenPayload>(payload, {
175176
secret: this.JWT_SECRET,
176177
expiresIn: this.JWT_EXPIRES_IN,
177178
}),
178-
this.jwtService.signAsync(payload, {
179+
this.jwtService.signAsync<TokenPayload>(payload, {
179180
secret: this.JWT_REFRESH_SECRET,
180181
expiresIn: this.JWT_REFRESH_EXPIRES_IN,
181182
}),
@@ -199,7 +200,7 @@ export class AuthService {
199200

200201
const frontEndURL = this.FRONTEND_URL;
201202
const domain = this.APP_DOMAIN;
202-
const maxAge = parseInt(this.COOKIE_EXPIRES_IN) * 1000;
203+
const maxAge = ms(this.COOKIE_EXPIRES_IN) * 1000;
203204

204205
res.cookie('token', token.access_token, {
205206
domain: domain,

apps/backend/src/auth/strategies/JWT.strategy.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('JwtStrategy', () => {
3333
it('should throw an error if JWT_SECRET is not set', () => {
3434
jest.spyOn(configService, 'getOrThrow').mockReturnValue(null);
3535

36-
expect(() => new JwtStrategy(configService)).toThrowError(
36+
expect(() => new JwtStrategy(configService)).toThrow(
3737
'JwtStrategy requires a secret or key',
3838
);
3939
});
@@ -84,7 +84,7 @@ describe('JwtStrategy', () => {
8484

8585
const payload = { userId: 'test-user-id' };
8686

87-
expect(() => jwtStrategy.validate(req, payload)).toThrowError(
87+
expect(() => jwtStrategy.validate(req, payload)).toThrow(
8888
'No refresh token',
8989
);
9090
});

apps/backend/src/auth/strategies/discord.strategy/discord.strategy.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('DiscordStrategy', () => {
4343
it('should throw an error if Discord config is missing', () => {
4444
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);
4545

46-
expect(() => new DiscordStrategy(configService)).toThrowError(
46+
expect(() => new DiscordStrategy(configService)).toThrow(
4747
'OAuth2Strategy requires a clientID option',
4848
);
4949
});

apps/backend/src/auth/strategies/discord.strategy/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class DiscordStrategy extends PassportStrategy(strategy, 'discord') {
2727
callbackUrl: `${SERVER_URL}/v1/auth/discord/callback`,
2828
scope: [DiscordPermissionScope.Email, DiscordPermissionScope.Identify],
2929
fetchScope: true,
30-
prompt: 'none',
30+
prompt: 'none' as const,
3131
};
3232

3333
super(config);

apps/backend/src/auth/strategies/github.strategy.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('GithubStrategy', () => {
4343
it('should throw an error if GitHub config is missing', () => {
4444
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);
4545

46-
expect(() => new GithubStrategy(configService)).toThrowError(
46+
expect(() => new GithubStrategy(configService)).toThrow(
4747
'OAuth2Strategy requires a clientID option',
4848
);
4949
});

apps/backend/src/auth/strategies/github.strategy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ export class GithubStrategy extends PassportStrategy(strategy, 'github') {
2222
super({
2323
clientID: GITHUB_CLIENT_ID,
2424
clientSecret: GITHUB_CLIENT_SECRET,
25-
redirect_uri: `${SERVER_URL}/v1/auth/github/callback`,
25+
callbackURL: `${SERVER_URL}/v1/auth/github/callback`,
2626
scope: 'user:read,user:email',
2727
state: false,
28-
});
28+
} as any); // TODO: Fix types
2929
}
3030

3131
async validate(accessToken: string, refreshToken: string, profile: any) {

apps/backend/src/auth/strategies/google.strategy.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe('GoogleStrategy', () => {
4444
it('should throw an error if Google config is missing', () => {
4545
jest.spyOn(configService, 'getOrThrow').mockReturnValueOnce(null);
4646

47-
expect(() => new GoogleStrategy(configService)).toThrowError(
47+
expect(() => new GoogleStrategy(configService)).toThrow(
4848
'OAuth2Strategy requires a clientID option',
4949
);
5050
});

apps/backend/src/config/EnvironmentVariables.ts

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,35 @@
11
import { plainToInstance } from 'class-transformer';
2-
import { IsEnum, IsOptional, IsString, validateSync } from 'class-validator';
2+
import {
3+
IsEnum,
4+
IsOptional,
5+
IsString,
6+
registerDecorator,
7+
validateSync,
8+
ValidationArguments,
9+
ValidationOptions,
10+
} from 'class-validator';
11+
import ms from 'ms';
12+
13+
// Validate if the value is a valid duration string from the 'ms' library
14+
function IsDuration(validationOptions?: ValidationOptions) {
15+
return function (object: object, propertyName: string) {
16+
registerDecorator({
17+
name: 'isDuration',
18+
target: object.constructor,
19+
propertyName: propertyName,
20+
options: validationOptions,
21+
validator: {
22+
validate(value: unknown) {
23+
if (typeof value !== 'string') return false;
24+
return typeof ms(value as ms.StringValue) === 'number';
25+
},
26+
defaultMessage(args: ValidationArguments) {
27+
return `${args.property} must be a valid duration string (e.g., "1h", "30m", "7d")`;
28+
},
29+
},
30+
});
31+
};
32+
}
333

434
enum Environment {
535
Development = 'development',
@@ -38,14 +68,14 @@ export class EnvironmentVariables {
3868
@IsString()
3969
JWT_SECRET: string;
4070

41-
@IsString()
42-
JWT_EXPIRES_IN: string;
71+
@IsDuration()
72+
JWT_EXPIRES_IN: ms.StringValue;
4373

4474
@IsString()
4575
JWT_REFRESH_SECRET: string;
4676

47-
@IsString()
48-
JWT_REFRESH_EXPIRES_IN: string;
77+
@IsDuration()
78+
JWT_REFRESH_EXPIRES_IN: ms.StringValue;
4979

5080
// database
5181
@IsString()
@@ -91,8 +121,8 @@ export class EnvironmentVariables {
91121
@IsString()
92122
DISCORD_WEBHOOK_URL: string;
93123

94-
@IsString()
95-
COOKIE_EXPIRES_IN: string;
124+
@IsDuration()
125+
COOKIE_EXPIRES_IN: ms.StringValue;
96126
}
97127

98128
export function validate(config: Record<string, unknown>) {
@@ -105,7 +135,13 @@ export function validate(config: Record<string, unknown>) {
105135
});
106136

107137
if (errors.length > 0) {
108-
throw new Error(errors.toString());
138+
const messages = errors
139+
.map((error) => {
140+
const constraints = Object.values(error.constraints || {});
141+
return ` - ${error.property}: ${constraints.join(', ')}`;
142+
})
143+
.join('\n');
144+
throw new Error(`Environment validation failed:\n${messages}`);
109145
}
110146

111147
return validatedConfig;

0 commit comments

Comments
 (0)