update libnest to v8 #1226
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { accessibleQuery } from '../ability.utils'; | ||
|
|
||
| const accessibleBy = vi.hoisted(() => vi.fn()); | ||
|
|
||
| vi.mock('@casl/prisma/runtime', () => ({ | ||
| createAccessibleByFactory: () => accessibleBy | ||
| })); | ||
|
|
||
| describe('accessibleQuery', () => { | ||
| it('should return an empty object if ability is undefined', () => { | ||
| expect(accessibleQuery(undefined, 'manage', 'User')).toStrictEqual({}); | ||
| expect(accessibleBy).not.toHaveBeenCalled(); | ||
| }); | ||
| it('should call accessibleBy with the correct parameters and return the result of accessibleBy for the model', () => { | ||
| accessibleBy.mockReturnValueOnce({ | ||
| User: 'QUERY' | ||
| }); | ||
| const ability = vi.fn(); | ||
| expect(accessibleQuery(ability as any, 'manage', 'User')).toStrictEqual('QUERY'); | ||
| expect(accessibleBy).toHaveBeenCalledExactlyOnceWith(ability, 'manage'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { AbilityBuilder } from '@casl/ability'; | ||
| import { createPrismaAbility } from '@casl/prisma'; | ||
| import { LoggingService } from '@douglasneuroinformatics/libnest'; | ||
| import { Injectable } from '@nestjs/common'; | ||
| import type { TokenPayload } from '@opendatacapture/schemas/auth'; | ||
|
|
||
| import { createAppAbility, detectAppSubject } from './ability.utils'; | ||
|
|
||
| import type { AppAbility, Permission } from './auth.types'; | ||
|
|
||
| @Injectable() | ||
| export class AbilityFactory { | ||
| constructor(private readonly loggingService: LoggingService) {} | ||
|
|
||
| createForPayload(payload: Omit<TokenPayload, 'permissions'>): AppAbility { | ||
| this.loggingService.verbose({ | ||
| message: 'Creating Ability From Payload', | ||
| payload | ||
| }); | ||
| const ability = new AbilityBuilder<AppAbility>(createPrismaAbility); | ||
| const groupIds = payload.groups.map((group) => group.id); | ||
| switch (payload.basePermissionLevel) { | ||
| case 'ADMIN': | ||
| ability.can('manage', 'all'); | ||
| break; | ||
| case 'GROUP_MANAGER': | ||
| ability.can('manage', 'Assignment', { groupId: { in: groupIds } }); | ||
| ability.can('manage', 'Group', { id: { in: groupIds } }); | ||
| ability.can('read', 'Instrument'); | ||
| ability.can('create', 'InstrumentRecord'); | ||
| ability.can('read', 'InstrumentRecord', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Session'); | ||
| ability.can('read', 'Session', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Subject'); | ||
| ability.can('read', 'Subject', { groupIds: { hasSome: groupIds } }); | ||
| ability.can('read', 'User', { groupIds: { hasSome: groupIds } }); | ||
| break; | ||
| case 'STANDARD': | ||
| ability.can('read', 'Group', { id: { in: groupIds } }); | ||
| ability.can('read', 'Instrument'); | ||
| ability.can('create', 'InstrumentRecord'); | ||
| ability.can('read', 'Session', { groupId: { in: groupIds } }); | ||
| ability.can('create', 'Session'); | ||
| ability.can('create', 'Subject'); | ||
| ability.can('read', 'Subject', { groupIds: { hasSome: groupIds } }); | ||
| break; | ||
| } | ||
| payload.additionalPermissions?.forEach(({ action, subject }) => { | ||
| ability.can(action, subject); | ||
| }); | ||
| return ability.build({ | ||
| detectSubjectType: detectAppSubject | ||
| }); | ||
| } | ||
|
|
||
| createForPermissions(permissions: Permission[]): AppAbility { | ||
| this.loggingService.verbose({ | ||
| message: 'Creating Ability From Permissions', | ||
| permissions | ||
| }); | ||
| return createAppAbility(permissions); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { detectSubjectType } from '@casl/ability'; | ||
| import { createPrismaAbility } from '@casl/prisma'; | ||
| import type { PrismaQuery } from '@casl/prisma'; | ||
| import { createAccessibleByFactory } from '@casl/prisma/runtime'; | ||
| import type { AppSubject, Prisma } from '@prisma/client'; | ||
|
|
||
| import type { PrismaModelWhereInputMap } from '@/core/prisma'; | ||
|
|
||
| import type { AppAbilities, AppAbility, AppAction, Permission } from './auth.types'; | ||
|
|
||
| const accessibleBy = createAccessibleByFactory<PrismaModelWhereInputMap, PrismaQuery>(); | ||
|
|
||
| export function detectAppSubject(obj: { [key: string]: any }) { | ||
| if (typeof obj.__modelName === 'string') { | ||
| return obj.__modelName as AppSubject; | ||
| } | ||
| return detectSubjectType(obj) as AppSubject; | ||
| } | ||
|
|
||
| export function createAppAbility(permissions: Permission[]): AppAbility { | ||
| return createPrismaAbility<AppAbilities>(permissions, { | ||
| detectSubjectType: detectAppSubject | ||
| }); | ||
| } | ||
|
|
||
| export function accessibleQuery<T extends Prisma.ModelName>( | ||
| ability: AppAbility | undefined, | ||
| action: AppAction, | ||
| modelName: T | ||
| ): NonNullable<PrismaModelWhereInputMap[T]> { | ||
| if (!ability) { | ||
| return {}; | ||
| } | ||
| return accessibleBy(ability, action)[modelName]!; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { CurrentUser } from '@douglasneuroinformatics/libnest'; | ||
| import type { RequestUser } from '@douglasneuroinformatics/libnest'; | ||
| import { Body, Controller, Get, HttpCode, HttpStatus, Post } from '@nestjs/common'; | ||
| import { ApiOperation } from '@nestjs/swagger'; | ||
| import { $LoginCredentials } from '@opendatacapture/schemas/auth'; | ||
|
|
||
| import { RouteAccess } from '@/core/decorators/route-access.decorator.js'; | ||
|
|
||
| import { AuthService } from './auth.service.js'; | ||
|
|
||
| @Controller({ path: 'auth' }) | ||
| export class AuthController { | ||
| constructor(private readonly authService: AuthService) {} | ||
|
|
||
| @Get('create-instrument-token') | ||
| @HttpCode(HttpStatus.OK) | ||
| @RouteAccess({ action: 'create', subject: 'Instrument' }) | ||
| async getCreateInstrumentToken(@CurrentUser() currentUser: RequestUser): Promise<{ accessToken: string }> { | ||
| return this.authService.getCreateInstrumentToken(currentUser); | ||
| } | ||
|
|
||
| @ApiOperation({ summary: 'Login' }) | ||
| @HttpCode(HttpStatus.OK) | ||
| @Post('login') | ||
| @RouteAccess('public') | ||
| async login(@Body() credentials: $LoginCredentials): Promise<{ accessToken: string }> { | ||
| return this.authService.login(credentials); | ||
| } | ||
|
Comment on lines
+22
to
+28
Contributor
There was a problem hiding this comment. 🧩 Analysis chainConsider rate limiting and enhance API documentation. The login endpoint is publicly accessible and lacks visible rate limiting, making it vulnerable to brute-force attacks. Also consider documenting error responses (401, 400) using Verify rate limiting configuration: If rate limiting isn't configured, consider adding it: import { Throttle } from '@nestjs/throttler';
// Add above the login method:
@Throttle({ default: { limit: 5, ttl: 60000 } }) // 5 attempts per minute🏁 Script executed: #!/bin/bash
# Check if rate limiting is configured globally or on auth routes
rg -nP --type=ts -C5 'ThrottlerModule|RateLimit|@Throttle'Length of output: 0 🏁 Script executed: #!/bin/bash
# Check app.module.ts for ThrottlerModule import
fd -t f "app.module.ts" --exec cat {}
# Check main.ts for global guards or rate limiting setup
fd -t f "main.ts" --exec cat {}
# Search for any throttler-related imports in the codebase
rg -n "throttler|rate.limit" -i --type=tsLength of output: 6568 Rate limiting is not configured. Add throttling to the login endpoint to prevent brute-force attacks. Verification confirms no rate limiting is configured globally or on the auth routes. The login endpoint remains unprotected. Add the @Throttle({ default: { limit: 5, ttl: 60000 } })Also document error responses using 🤖 Prompt for AI Agents |
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { ConfigService } from '@douglasneuroinformatics/libnest'; | ||
| import { Module } from '@nestjs/common'; | ||
| import { APP_GUARD } from '@nestjs/core'; | ||
| import { JwtModule } from '@nestjs/jwt'; | ||
|
|
||
| import { UsersModule } from '@/users/users.module'; | ||
|
|
||
| import { AbilityFactory } from './ability.factory'; | ||
| import { AuthController } from './auth.controller'; | ||
| import { AuthService } from './auth.service'; | ||
| import { JwtAuthGuard } from './guards/jwt-auth.guard'; | ||
| import { JwtStrategy } from './strategies/jwt.strategy'; | ||
|
|
||
| @Module({ | ||
| controllers: [AuthController], | ||
| imports: [ | ||
| JwtModule.registerAsync({ | ||
| inject: [ConfigService], | ||
| useFactory: (configService: ConfigService) => ({ | ||
| secret: configService.get('SECRET_KEY') | ||
| }) | ||
| }), | ||
| UsersModule | ||
| ], | ||
| providers: [ | ||
| AbilityFactory, | ||
| AuthService, | ||
| JwtStrategy, | ||
| { | ||
| provide: APP_GUARD, | ||
| useClass: JwtAuthGuard | ||
| } | ||
| ] | ||
| }) | ||
| export class AuthModule {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import { CryptoService } from '@douglasneuroinformatics/libnest'; | ||
| import type { RequestUser } from '@douglasneuroinformatics/libnest'; | ||
| import { ForbiddenException, Injectable, NotFoundException, UnauthorizedException } from '@nestjs/common'; | ||
| import { JwtService } from '@nestjs/jwt'; | ||
| import type { $LoginCredentials, TokenPayload } from '@opendatacapture/schemas/auth'; | ||
| import type { Group, User } from '@prisma/client'; | ||
|
|
||
| import { UsersService } from '@/users/users.service'; | ||
|
|
||
| import { AbilityFactory } from './ability.factory'; | ||
|
|
||
| @Injectable() | ||
| export class AuthService { | ||
| constructor( | ||
| private readonly abilityFactory: AbilityFactory, | ||
| private readonly cryptoService: CryptoService, | ||
| private readonly jwtService: JwtService, | ||
| private readonly usersService: UsersService | ||
| ) {} | ||
|
|
||
| async getCreateInstrumentToken(currentUser: RequestUser) { | ||
| if (!currentUser.ability.can('create', 'Instrument')) { | ||
| throw new ForbiddenException(); | ||
| } | ||
|
|
||
| const limitedAbility = this.abilityFactory.createForPermissions([{ action: 'create', subject: 'Instrument' }]); | ||
|
|
||
| return { | ||
| accessToken: await this.jwtService.signAsync({ permissions: limitedAbility.rules }, { expiresIn: '1h' }) | ||
| }; | ||
| } | ||
|
|
||
| async login(credentials: $LoginCredentials): Promise<{ accessToken: string }> { | ||
| let user: User & { | ||
| groups: Group[]; | ||
| }; | ||
| try { | ||
| user = await this.usersService.findByUsername(credentials.username, { includeHashedPassword: true }); | ||
| } catch (err) { | ||
| if (err instanceof NotFoundException) { | ||
| throw new UnauthorizedException('Invalid Credentials'); | ||
| } | ||
| throw err; | ||
| } | ||
| const isCorrectPassword = await this.cryptoService.comparePassword(credentials.password, user.hashedPassword); | ||
| if (isCorrectPassword !== true) { | ||
| throw new UnauthorizedException('Invalid Credentials'); | ||
| } | ||
|
|
||
| const tokenPayload: Omit<TokenPayload, 'permissions'> = { | ||
| additionalPermissions: user.additionalPermissions, | ||
| basePermissionLevel: user.basePermissionLevel, | ||
| firstName: user.firstName, | ||
| groups: user.groups, | ||
| lastName: user.lastName, | ||
| username: user.username | ||
| }; | ||
|
|
||
| const ability = this.abilityFactory.createForPayload(tokenPayload); | ||
|
|
||
| return { | ||
| accessToken: await this.jwtService.signAsync( | ||
| { | ||
| ...tokenPayload, | ||
| permissions: ability.rules | ||
| }, | ||
| { | ||
| expiresIn: '1h' | ||
| } | ||
| ) | ||
| }; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that create permissions for InstrumentRecord, Session, and Subject are properly scoped at the service layer.
The permission rules allow GROUP_MANAGER and STANDARD users to create records without explicit ability-level constraints on which groups they can create for. Ensure that service methods enforce group membership checks when creating these entities.
🏁 Script executed:
Length of output: 8128
🏁 Script executed:
Length of output: 2843
🏁 Script executed:
Length of output: 1017
🏁 Script executed:
Length of output: 337
🏁 Script executed:
Length of output: 1348
🏁 Script executed:
Length of output: 647
Authorization bypass in Sessions and Subjects create endpoints.
Verification confirms the original concern: Session and Subject create operations bypass authorization checks.
abilityto service; service callsgroupsService.findById(groupId)withoutability, allowing any user to create sessions for any group.abilityto service; service.create() doesn't useaccessibleQueryat all, allowing any user to create subjects.abilitythrough the service chain and checks group access.Both Sessions and Subjects endpoints need ability enforcement at the service layer, following the InstrumentRecords pattern: pass
abilityfrom controller and useaccessibleQueryin service create methods.