Skip to content

Commit ffc9317

Browse files
authored
Merge branch 'main' into security/118-domain-bound-signatures
2 parents 33e8a71 + ed6eacc commit ffc9317

26 files changed

Lines changed: 1406 additions & 208 deletions

context/progress-tracker.md

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,53 +6,33 @@ pure chore/docs commits). Direct pushes to main must also be logged here.
66

77
---
88

9-
## 2026-08-27
10-
11-
- Closed the audit gaps on the #118 PR (#124): the legacy raw-nonce migration
12-
window is now enforced at runtime, not just documented.
13-
- Added `AUTH_LEGACY_SIGNATURES_SUNSET` (default `2026-10-31`): after the
14-
cutoff, `verifyLegacyRawSignature()` rejects legacy raw-nonce signatures
15-
with `AUTH_LEGACY_SIGNATURE_DISABLED` even while
16-
`AUTH_ALLOW_LEGACY_RAW_SIGNATURES` is still true, so the replayable
17-
scheme closes automatically on the sunset date — no manual ops action
18-
required. Malformed sunset values fall back to the default rather than
19-
silently disabling the cutoff.
20-
- Strengthened the unit suite: the legacy-disabled regression test now
21-
proves rejection happens before any signature verification (mock verify
22-
returns true, assert it is never called), and new tests cover the
23-
sunset cutoff (past sunset + flag true → rejected; future sunset +
24-
flag true → accepted). Expired-envelope rejection unit test retained.
25-
- Resolved the unresolved conflict markers (leftover wrong-repo
26-
StepFi-Contracts content) that had been committed into
27-
`context/progress-tracker.md`, which the PR audit flagged as merge
28-
conflicts with the base branch.
29-
30-
## 2026-08-25
31-
32-
- Fixed cross-service signature replay (#118): `verifySignature()` now accepts
33-
exactly one scheme per request and every accepted signature provably signs a
34-
StepFi-bound challenge.
35-
- `generateNonce()` issues a canonical challenge envelope (domain, address,
36-
statement, uri, version, nonce, issuedAt, expirationTime,
37-
networkPassphrase) and stores a SHA-256 digest of the exact message on the
38-
nonce row (`issued_at`, `message_hash` columns via migration
39-
`20260825000000_add_nonce_message_binding.sql`).
40-
- Verification runs only against a message whose digest matches the stored
41-
challenge hash (`AUTH_CHALLENGE_MISMATCH` otherwise), with strict
42-
domain/URI/network/expiry checks (`AUTH_CHALLENGE_DOMAIN_MISMATCH`,
43-
`AUTH_CHALLENGE_URI_MISMATCH`, `AUTH_CHALLENGE_NETWORK_MISMATCH`,
44-
`AUTH_NONCE_EXPIRED`). The old "try raw, then 'Stellar Signing Key: '"
45-
fallback is gone — the weakest format no longer defines the security floor.
46-
- Browser wallets verify per SEP-53 (SHA-256 of
47-
"Stellar Signed Message:\n" + envelope, `signatureType: 'sep0043'`);
48-
native clients sign the envelope with raw Ed25519
49-
(`signatureType: 'envelope'`).
50-
- The legacy raw-nonce scheme is deprecated behind
51-
`AUTH_ALLOW_LEGACY_RAW_SIGNATURES` (default true for mobile-client
52-
compatibility) with a documented sunset date of **2026-10-31**; when
53-
disabled, legacy requests fail with `AUTH_LEGACY_SIGNATURE_DISABLED`.
54-
- Added `AUTH_CHALLENGE_DOMAIN` env (defaults to `API_URL` host); envelope
55-
`uri` is derived from `API_URL` + `API_PREFIX`.
9+
## 2026-08-24
10+
11+
- **Session families + refresh-token replay detection** (`sessions.family_id`
12+
migration, `fam` claim in refresh JWTs). Replaying an already-rotated
13+
refresh token now revokes every session in the family and writes a
14+
`auth.refresh_token_reuse` audit log entry — previously the first
15+
presenter of a stolen token won silently. Legacy tokens without a `fam`
16+
claim keep the old `AUTH_SESSION_NOT_FOUND` response.
17+
- **Blocked-user enforcement on every request**: new
18+
`UserStatusService` (in-memory TTL cache) consulted by `JwtStrategy`.
19+
Documented staleness bound: **30 seconds** — a blocked wallet loses API
20+
access within ~30s of being blocked instead of retaining access until its
21+
access token expires (up to 15 minutes). Cache is per-instance and fails
22+
open on DB errors to avoid locking out all users during a DB blip.
23+
- **Session cleanup cron** (`src/jobs/session-cleanup/`, hourly,
24+
mirrors nonce-cleanup): deletes only rows with `expires_at` older than
25+
1 hour; sessions no longer accumulate forever.
26+
- Tests: refresh-family rotation, replay → family-wide revocation + audit
27+
event, blocked-user denial within TTL bound, cache expiry re-query,
28+
cleanup job deletes-only-expired.
29+
30+
## 2026-08-26
31+
32+
- Fixed registration race conditions in `AuthService.register()` by eliminating application-side pre-checks (`findByWallet`, `checkUsernameExists`) and relying directly on DB-level UNIQUE constraints (`users.wallet_address`, `users.username`).
33+
- Added idempotent migration `20260826130000_ensure_users_unique_constraints.sql` to ensure unique indexes exist on `users.wallet_address` and `users.username`.
34+
- Updated `UsersRepository.createProfile()` to catch PostgreSQL unique constraint violation error `23505` and map to structured 409 `ConflictException` (`AUTH_WALLET_EXISTS`, `AUTH_USERNAME_TAKEN`).
35+
- Added cleanup handlers (`deleteAvatar`, `deleteUserById`) in `AuthService.register()` and `UsersRepository` to ensure failed registrations do not leave orphaned avatar files or partial user records.
5636

5737
## 2026-07-23
5838

src/app.module.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { IndexerModule } from './indexer/indexer.module';
2121
import { LoanPaymentReminderModule } from './jobs/loan-payment-reminder/loan-payment-reminder.module';
2222
import { TransactionStatusCheckerModule } from './jobs/transaction-status-checker/transaction-status-checker.module';
2323
import { NonceCleanupModule } from './jobs/nonce-cleanup/nonce-cleanup.module';
24+
import { SessionCleanupModule } from './jobs/session-cleanup/session-cleanup.module';
2425
import { SupabaseKeepAliveModule } from './jobs/supabase-keepalive/supabase-keepalive.module';
2526
import { StellarModule } from './stellar/stellar.module';
2627
import { LoggerModule } from './common/logger/logger.module';
@@ -62,6 +63,7 @@ import { AuditInterceptor } from './common/interceptors/audit.interceptor';
6263
LoanPaymentReminderModule,
6364
TransactionStatusCheckerModule,
6465
NonceCleanupModule,
66+
SessionCleanupModule,
6567
SupabaseKeepAliveModule,
6668
StateReconciliationModule,
6769
CreditScoringModule,

src/auth/guards/roles.guard.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import {
44
ExecutionContext,
55
ForbiddenException,
66
SetMetadata,
7+
Optional,
78
} from '@nestjs/common';
89
import { Reflector } from '@nestjs/core';
10+
import { UserStatusService } from '../../modules/auth/user-status.service';
911

1012
export const ROLES_KEY = 'roles';
1113

@@ -19,18 +21,22 @@ export const ROLES_KEY = 'roles';
1921
export const Roles = (...roles: string[]) => SetMetadata(ROLES_KEY, roles);
2022

2123
/**
22-
* Enforces the role claim carried in the JWT (set by JwtStrategy.validate).
24+
* Enforces user role authorization based on server truth in the datastore
25+
* (resolved via short-TTL cached UserStatusService).
2326
*
2427
* - Routes without @Roles metadata are unaffected.
25-
* - Tokens without a role claim (role not chosen yet, or token issued
26-
* before the role was set) are rejected with 403; the client must call
27-
* POST /auth/refresh after setting a role to obtain the claim.
28+
* - The JWT role claim is treated as a hint only; live datastore role is enforced.
29+
* - Roles revoked or changed server-side take effect within USER_STATUS_CACHE_TTL_MS (30s)
30+
* or immediately upon cache invalidation.
2831
*/
2932
@Injectable()
3033
export class RolesGuard implements CanActivate {
31-
constructor(private readonly reflector: Reflector) {}
34+
constructor(
35+
private readonly reflector: Reflector,
36+
@Optional() private readonly userStatusService?: UserStatusService,
37+
) {}
3238

33-
canActivate(context: ExecutionContext): boolean {
39+
async canActivate(context: ExecutionContext): Promise<boolean> {
3440
const requiredRoles = this.reflector.getAllAndOverride<string[]>(ROLES_KEY, [
3541
context.getHandler(),
3642
context.getClass(),
@@ -41,7 +47,21 @@ export class RolesGuard implements CanActivate {
4147
.switchToHttp()
4248
.getRequest<{ user?: { wallet: string; role?: string | null } }>();
4349

44-
if (!user?.role || !requiredRoles.includes(user.role)) {
50+
if (!user) {
51+
throw new ForbiddenException({
52+
code: 'AUTH_ROLE_FORBIDDEN',
53+
message: `This action requires one of the following roles: ${requiredRoles.join(', ')}.`,
54+
});
55+
}
56+
57+
let currentRole: string | null = null;
58+
if (this.userStatusService && user.wallet) {
59+
currentRole = await this.userStatusService.getRole(user.wallet);
60+
} else {
61+
currentRole = user.role ?? null;
62+
}
63+
64+
if (!currentRole || !requiredRoles.includes(currentRole)) {
4565
throw new ForbiddenException({
4666
code: 'AUTH_ROLE_FORBIDDEN',
4767
message: `This action requires one of the following roles: ${requiredRoles.join(', ')}. If you just selected your role, refresh your access token.`,
@@ -50,3 +70,5 @@ export class RolesGuard implements CanActivate {
5070
return true;
5171
}
5272
}
73+
74+

src/database/repositories/users.repository.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, InternalServerErrorException } from '@nestjs/common';
1+
import { Injectable, InternalServerErrorException, ConflictException } from '@nestjs/common';
22
import { SupabaseService } from '../supabase.client';
33
import { UpdateUserDto } from '../../modules/users/dto/update-user.dto';
44

@@ -226,6 +226,31 @@ export class UsersRepository {
226226
return (data as { wallet_address: string; role: UserRole } | null) ?? null;
227227
}
228228

229+
/**
230+
* Admin override: sets or resets the user's role regardless of whether a role was set previously.
231+
* Returns the updated user row, or null if the user does not exist.
232+
*/
233+
async forceSetRole(
234+
wallet: string,
235+
role: UserRole | null,
236+
): Promise<{ wallet_address: string; role: UserRole | null } | null> {
237+
const { data, error } = await this.supabaseService
238+
.getServiceRoleClient()
239+
.from('users')
240+
.update({ role })
241+
.eq('wallet_address', wallet)
242+
.select('wallet_address, role')
243+
.maybeSingle();
244+
245+
if (error) {
246+
throw new InternalServerErrorException({
247+
code: 'DATABASE_ROLE_UPDATE_FAILED',
248+
message: 'Failed to update user role.',
249+
});
250+
}
251+
return (data as { wallet_address: string; role: UserRole | null } | null) ?? null;
252+
}
253+
229254
// --- REGISTRATION METHODS ---
230255

231256
async checkUsernameExists(username: string): Promise<boolean> {
@@ -260,6 +285,19 @@ export class UsersRepository {
260285
.single();
261286

262287
if (error) {
288+
const combinedErr = `${error.code || ''} ${error.message || ''} ${error.details || ''} ${error.hint || ''}`;
289+
if (error.code === '23505' || combinedErr.includes('duplicate key') || combinedErr.includes('unique constraint')) {
290+
if (combinedErr.includes('username')) {
291+
throw new ConflictException({
292+
code: 'AUTH_USERNAME_TAKEN',
293+
message: 'Username is already taken.',
294+
});
295+
}
296+
throw new ConflictException({
297+
code: 'AUTH_WALLET_EXISTS',
298+
message: 'Wallet address is already registered.',
299+
});
300+
}
263301
throw new InternalServerErrorException({
264302
code: 'DATABASE_INSERT_ERROR',
265303
message: `Failed to create user profile: ${error.message}`,
@@ -292,4 +330,25 @@ export class UsersRepository {
292330
const { data } = client.storage.from('avatars').getPublicUrl(fileName);
293331
return data.publicUrl;
294332
}
333+
334+
async deleteAvatar(avatarUrl: string): Promise<void> {
335+
try {
336+
const fileName = avatarUrl.substring(avatarUrl.lastIndexOf('/') + 1);
337+
if (!fileName) return;
338+
const client = this.supabaseService.getServiceRoleClient();
339+
await client.storage.from('avatars').remove([fileName]);
340+
} catch {
341+
// Ignore cleanup failures
342+
}
343+
}
344+
345+
async deleteUserById(id: string): Promise<void> {
346+
try {
347+
const client = this.supabaseService.getServiceRoleClient();
348+
await client.from('users').delete().eq('id', id);
349+
} catch {
350+
// Ignore cleanup failures
351+
}
352+
}
295353
}
354+
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { Module } from '@nestjs/common';
2+
import { SessionCleanupService } from './session-cleanup.service';
3+
import { SupabaseService } from '../../database/supabase.client';
4+
5+
@Module({
6+
providers: [SessionCleanupService, SupabaseService],
7+
})
8+
export class SessionCleanupModule {}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { Injectable, Logger } from '@nestjs/common';
2+
import { Cron, CronExpression } from '@nestjs/schedule';
3+
import { SupabaseService } from '../../database/supabase.client';
4+
5+
@Injectable()
6+
export class SessionCleanupService {
7+
private readonly logger = new Logger(SessionCleanupService.name);
8+
9+
constructor(private readonly supabaseService: SupabaseService) {}
10+
11+
@Cron(CronExpression.EVERY_HOUR)
12+
async cleanupExpiredSessions(): Promise<void> {
13+
try {
14+
const client = this.supabaseService.getServiceRoleClient();
15+
16+
// Delete only rows already past their expiry; the 1h grace window
17+
// mirrors the nonce-cleanup pattern and keeps rows around long enough
18+
// that an "expired" response (instead of "not found") is still
19+
// possible for borderline requests.
20+
const cutoff = new Date(Date.now() - 60 * 60 * 1000).toISOString();
21+
22+
const { error, count } = await client
23+
.from('sessions')
24+
.delete({ count: 'exact' })
25+
.lt('expires_at', cutoff);
26+
27+
if (error) {
28+
this.logger.error(`Failed to delete expired sessions: ${error.message}`);
29+
throw error;
30+
}
31+
32+
this.logger.log(`Deleted ${count ?? 0} expired sessions`);
33+
} catch (error) {
34+
this.logger.error('Session cleanup failed', error);
35+
}
36+
}
37+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import {
2+
Controller,
3+
Post,
4+
Param,
5+
Body,
6+
NotFoundException,
7+
UseGuards,
8+
UseInterceptors,
9+
HttpCode,
10+
HttpStatus,
11+
} from '@nestjs/common';
12+
import { ApiTags, ApiOperation, ApiResponse, ApiBearerAuth, ApiParam } from '@nestjs/swagger';
13+
import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard';
14+
import { AdminGuard } from '../../auth/guards/admin.guard';
15+
import { AuditInterceptor } from '../../common/interceptors/audit.interceptor';
16+
import { AuditAction } from '../../common/decorators/audit-action.decorator';
17+
import { UsersRepository } from '../../database/repositories/users.repository';
18+
import { UserStatusService } from '../auth/user-status.service';
19+
import { AdminResetRoleDto } from './dto/admin-reset-role.dto';
20+
21+
@ApiTags('admin')
22+
@Controller('admin')
23+
@UseGuards(JwtAuthGuard, AdminGuard)
24+
@ApiBearerAuth()
25+
@UseInterceptors(AuditInterceptor)
26+
export class AdminRolesController {
27+
constructor(
28+
private readonly usersRepository: UsersRepository,
29+
private readonly userStatusService: UserStatusService,
30+
) {}
31+
32+
@Post('users/:wallet/role/reset')
33+
@HttpCode(HttpStatus.OK)
34+
@AuditAction('admin_users', 'RESET_USER_ROLE')
35+
@ApiOperation({
36+
summary: 'Reset or override user role (Admin only)',
37+
description:
38+
'Allows admins to reset a user\'s permanent role back to null or override it with a specific role. ' +
39+
'Immediately invalidates the user\'s server-side status cache and logs an audit event.',
40+
})
41+
@ApiParam({ name: 'wallet', description: 'Target user wallet address' })
42+
@ApiResponse({ status: 200, description: 'Role reset/updated successfully' })
43+
@ApiResponse({ status: 401, description: 'Unauthorized — missing or invalid JWT' })
44+
@ApiResponse({ status: 403, description: 'Forbidden — wallet is not in ADMIN_WALLETS' })
45+
@ApiResponse({ status: 404, description: 'User not found' })
46+
async resetUserRole(
47+
@Param('wallet') wallet: string,
48+
@Body() dto?: AdminResetRoleDto,
49+
) {
50+
const targetRole = dto?.role ?? null;
51+
const updated = await this.usersRepository.forceSetRole(wallet, targetRole);
52+
53+
if (!updated) {
54+
throw new NotFoundException({
55+
code: 'USERS_NOT_FOUND',
56+
message: 'User not found.',
57+
});
58+
}
59+
60+
this.userStatusService.invalidate(wallet);
61+
62+
return {
63+
success: true,
64+
data: {
65+
wallet: updated.wallet_address,
66+
role: updated.role,
67+
},
68+
message: 'User role updated successfully.',
69+
};
70+
}
71+
}

src/modules/admin/admin.module.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { Module } from '@nestjs/common';
22
import { AuditController } from './audit.controller';
33
import { AuditService } from './audit.service';
4+
import { AdminRolesController } from './admin-roles.controller';
45
import { SupabaseService } from '../../database/supabase.client';
6+
import { UsersRepository } from '../../database/repositories/users.repository';
7+
import { UserStatusService } from '../auth/user-status.service';
8+
import { AdminGuard } from '../../auth/guards/admin.guard';
59

610
@Module({
7-
controllers: [AuditController],
8-
providers: [AuditService, SupabaseService],
9-
exports: [AuditService],
11+
controllers: [AuditController, AdminRolesController],
12+
providers: [AuditService, SupabaseService, UsersRepository, UserStatusService, AdminGuard],
13+
exports: [AuditService, UserStatusService],
1014
})
1115
export class AdminModule {}

0 commit comments

Comments
 (0)