Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.template
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@ SERVER_PRIVATE_KEY=
# Stalwart webhook (ingest events)
STALWART_WEBHOOK_USERNAME=stalwart
STALWART_WEBHOOK_SECRET=

# MTA hooks (RCPT)
MTA_HOOKS_USERNAME=stalwart
MTA_HOOKS_SECRET=
Comment on lines +35 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Do not leave the MTA hooks secret empty in the deployment template.

With MTA_HOOKS_SECRET= and the configuration fallback to '', deployments that copy this file unchanged will have unusable RCPT authentication because the guard requires non-empty credentials. Use an explicit placeholder and validate the secret at startup, or document that it must be replaced before deployment.

🧰 Tools
🪛 dotenv-linter (4.0.0)

[warning] 37-37: [UnorderedKey] The MTA_HOOKS_SECRET key should go before the MTA_HOOKS_USERNAME key

(UnorderedKey)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.template around lines 35 - 37, Replace the empty MTA_HOOKS_SECRET value
in the deployment template with an explicit placeholder, and ensure the startup
configuration validation rejects the placeholder or any missing secret before
enabling RCPT hooks. Update the relevant MTA hooks configuration/validation
symbols and document that operators must replace the placeholder before
deployment.

2 changes: 2 additions & 0 deletions src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { GatewayModule } from './modules/gateway/gateway.module';
import { HttpGlobalExceptionFilter } from './common/filters/http-global-exception.filter';
import { AddressesModule } from './modules/addresses/addresses.module';
import { StalwartEventsModule } from './modules/stalwart-events/stalwart-events.module';
import { MtaHooksModule } from './modules/mta-hooks/mta-hooks.module';

@Module({
imports: [
Expand Down Expand Up @@ -89,6 +90,7 @@ import { StalwartEventsModule } from './modules/stalwart-events/stalwart-events.
AddressesModule,
GatewayModule,
StalwartEventsModule,
MtaHooksModule,
],
controllers: [],
providers: [
Expand Down
5 changes: 5 additions & 0 deletions src/config/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,9 @@ export default () => ({
username: process.env.STALWART_WEBHOOK_USERNAME ?? 'stalwart',
secret: process.env.STALWART_WEBHOOK_SECRET ?? '',
},

mtaHooks: {
username: process.env.MTA_HOOKS_USERNAME ?? 'stalwart',
secret: process.env.MTA_HOOKS_SECRET ?? '',
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
43 changes: 43 additions & 0 deletions src/modules/infrastructure/bridge/bridge.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,47 @@ describe('BridgeClient', () => {
expect(error.details).toBe('entry not found');
});
});

describe('getUserUsage', () => {
it('when Bridge returns 200, then signs a gateway token, GETs the usage, and returns the snapshot', async () => {
const snapshot = { maxSpaceBytes: 5000, totalUsedSpaceBytes: 1200 };
jwtService.sign.mockReturnValue('signed-jwt');
httpRequest.mockResolvedValue({
statusCode: 200,
body: { text: () => Promise.resolve(JSON.stringify(snapshot)) },
});

const result = await service.getUserUsage('user-1');

expect(result).toStrictEqual(snapshot);
expect(httpRequest).toHaveBeenCalledWith(
expect.objectContaining({
method: 'GET',
path: '/v2/gateway/users/user-1/usage',
headers: expect.objectContaining({
authorization: 'Bearer signed-jwt',
}) as unknown,
}),
);
});

it('when Bridge returns a non-200 status, then throws BridgeApiError with statusCode and details', async () => {
jwtService.sign.mockReturnValue('signed-jwt');
httpRequest.mockResolvedValue({
statusCode: 500,
body: { text: () => Promise.resolve('internal error') },
});

const error: unknown = await service
.getUserUsage('user-1')
.catch((e: unknown) => e);

expect(error).toBeInstanceOf(BridgeApiError);
if (!(error instanceof BridgeApiError)) {
throw new Error('expected BridgeApiError');
}
expect(error.statusCode).toBe(500);
expect(error.details).toBe('internal error');
});
});
});
31 changes: 30 additions & 1 deletion src/modules/infrastructure/bridge/bridge.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import {
import { ConfigService } from '@nestjs/config';
import { JwtService } from '@nestjs/jwt';
import { Client } from 'undici';
import type { BucketEntry, MailBucket } from './bridge.types.js';
import type {
BucketEntry,
MailBucket,
UserSpaceSnapshot,
} from './bridge.types.js';

@Injectable()
export class BridgeClient implements OnModuleInit, OnModuleDestroy {
Expand Down Expand Up @@ -157,6 +161,31 @@ export class BridgeClient implements OnModuleInit, OnModuleDestroy {
}
}

async getUserUsage(userUuid: string): Promise<UserSpaceSnapshot> {
const token = this.signGatewayToken(userUuid);

const { statusCode, body } = await this.httpClient.request({
method: 'GET',
path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/usage`,
headers: {
accept: 'application/json',
authorization: `Bearer ${token}`,
},
});

const text = await body.text();

if (statusCode !== 200) {
throw new BridgeApiError(
`Failed to fetch usage for user '${userUuid}': HTTP ${statusCode}`,
statusCode,
text,
);
}

return JSON.parse(text) as UserSpaceSnapshot;
}

private signGatewayToken(userUuid: string): string {
return this.jwtService.sign(
{ payload: { uuid: userUuid } },
Expand Down
89 changes: 89 additions & 0 deletions src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { createMock } from '@golevelup/ts-vitest';
import { type ConfigService } from '@nestjs/config';
import { UnauthorizedException, type ExecutionContext } from '@nestjs/common';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';

const username = 'stalwart';
const secret = 'secret';

describe('MtaHooksAuthGuard', () => {
let guard: MtaHooksAuthGuard;

const contextWithAuth = (header?: string): ExecutionContext =>
createMock<ExecutionContext>({
switchToHttp: () => ({
getRequest: () => ({
headers: header ? { authorization: header } : {},
}),
}),
});

const basic = (username: string, secret: string): string =>
`Basic ${Buffer.from(`${username}:${secret}`).toString('base64')}`;

beforeEach(() => {
const configService = createMock<ConfigService>();
configService.getOrThrow.mockImplementation((key: string) => {
if (key === 'mtaHooks.username') return username;
if (key === 'mtaHooks.secret') return secret;
throw new Error(`unknown key: ${key}`);
});
guard = new MtaHooksAuthGuard(configService);
});

it('when credentials match, then allows the request', () => {
const context = contextWithAuth(basic(username, secret));

expect(guard.canActivate(context)).toBe(true);
});

it('when the secret is wrong, then throws Unauthorized', () => {
const context = contextWithAuth(basic(username, 'wrong'));

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the username is wrong, then throws Unauthorized', () => {
const context = contextWithAuth(basic('wrong', secret));

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the Authorization header is missing, then throws Unauthorized', () => {
const context = contextWithAuth(undefined);

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the scheme is not Basic, then throws Unauthorized', () => {
const context = contextWithAuth('Bearer some-token');

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the decoded credentials lack a colon separator, then throws Unauthorized', () => {
const context = contextWithAuth(
`Basic ${Buffer.from('nocolon').toString('base64')}`,
);

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the secret is empty, then throws Unauthorized', () => {
const context = contextWithAuth(basic(username, ''));

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the configured secret is empty, then fails to construct', () => {
const configService = createMock<ConfigService>();
configService.getOrThrow.mockImplementation((key: string) => {
if (key === 'mtaHooks.username') return username;
if (key === 'mtaHooks.secret') return '';
throw new Error(`unknown key: ${key}`);
});

expect(() => new MtaHooksAuthGuard(configService)).toThrow();
});
});
69 changes: 69 additions & 0 deletions src/modules/mta-hooks/mta-hooks-auth.guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { timingSafeEqual } from 'node:crypto';
import {
CanActivate,
type ExecutionContext,
Injectable,
UnauthorizedException,
} from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import type { Request } from 'express';

@Injectable()
export class MtaHooksAuthGuard implements CanActivate {
private readonly expectedUsername: string;
private readonly expectedSecret: string;

constructor(configService: ConfigService) {
this.expectedUsername = this.requireNonEmpty(
configService.getOrThrow<string>('mtaHooks.username'),
'mtaHooks.username',
);
this.expectedSecret = this.requireNonEmpty(
configService.getOrThrow<string>('mtaHooks.secret'),
'mtaHooks.secret',
);
}

private requireNonEmpty(value: string, key: string): string {
if (value.length === 0) {
throw new Error(`Missing required configuration: ${key}`);
}
return value;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

canActivate(context: ExecutionContext): boolean {
const request = context.switchToHttp().getRequest<Request>();
const header = request.headers.authorization ?? '';

const [scheme, encoded] = header.split(' ');
if (scheme !== 'Basic' || !encoded) {
throw new UnauthorizedException('Missing or malformed Basic credentials');
}

const decoded = Buffer.from(encoded, 'base64').toString('utf8');
const separatorIndex = decoded.indexOf(':');
if (separatorIndex === -1) {
throw new UnauthorizedException('Malformed Basic credentials');
}

const username = decoded.slice(0, separatorIndex);
const secret = decoded.slice(separatorIndex + 1);

const usernameMatches = this.safeEqual(username, this.expectedUsername);
const secretMatches = this.safeEqual(secret, this.expectedSecret);
if (!usernameMatches || !secretMatches) {
throw new UnauthorizedException('Invalid MTA hook credentials');
}

return true;
}

private safeEqual(a: string, b: string): boolean {
const bufferA = Buffer.from(a, 'utf8');
const bufferB = Buffer.from(b, 'utf8');
if (bufferA.length !== bufferB.length) {
return false;
}
return timingSafeEqual(bufferA, bufferB);
}
}
23 changes: 23 additions & 0 deletions src/modules/mta-hooks/mta-hooks.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Body, Controller, Post, UseGuards } from '@nestjs/common';
import { ApiBasicAuth, ApiOperation, ApiTags } from '@nestjs/swagger';
import { Public } from '../auth/decorators/public.decorator.js';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';
import { MtaHooksService } from './mta-hooks.service.js';
import type { MtaHookRequest, MtaHookResponse } from './mta-hooks.types.js';

@ApiTags('MTA Hooks')
@ApiBasicAuth('mta-hooks')
@Public()
@UseGuards(MtaHooksAuthGuard)
@Controller('mta-hooks')
export class MtaHooksController {
constructor(private readonly mtaHooksService: MtaHooksService) {}

@Post('rcpt')
@ApiOperation({
summary: 'RCPT-stage hook',
})
rcpt(@Body() request: MtaHookRequest): Promise<MtaHookResponse> {
return this.mtaHooksService.handleRcpt(request);
}
}
13 changes: 13 additions & 0 deletions src/modules/mta-hooks/mta-hooks.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Module } from '@nestjs/common';
import { AccountModule } from '../account/account.module.js';
import { BridgeModule } from '../infrastructure/bridge/bridge.module.js';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';
import { MtaHooksController } from './mta-hooks.controller.js';
import { MtaHooksService } from './mta-hooks.service.js';

@Module({
imports: [AccountModule, BridgeModule],
controllers: [MtaHooksController],
providers: [MtaHooksService, MtaHooksAuthGuard],
})
export class MtaHooksModule {}
Loading
Loading