diff --git a/apps/backend/src/auth/jwt.strategy.ts b/apps/backend/src/auth/jwt.strategy.ts index 906773f4..b7670f13 100644 --- a/apps/backend/src/auth/jwt.strategy.ts +++ b/apps/backend/src/auth/jwt.strategy.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotFoundException } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { passportJwtSecret } from 'jwks-rsa'; import { ExtractJwt, Strategy } from 'passport-jwt'; @@ -31,6 +31,12 @@ export class JwtStrategy extends PassportStrategy(Strategy) { // we use the sub field in the payload to find the user in our database async validate(payload: CognitoJwtPayload): Promise { const dbUser = await this.usersService.findUserByCognitoId(payload.sub); + // If an exception is thrown, throw something here + if (!dbUser) { + throw new NotFoundException( + `User with payload sub ${payload.sub} not found`, + ); + } return dbUser; } } diff --git a/apps/backend/src/auth/ownership.decorator.ts b/apps/backend/src/auth/ownership.decorator.ts new file mode 100644 index 00000000..87101845 --- /dev/null +++ b/apps/backend/src/auth/ownership.decorator.ts @@ -0,0 +1,25 @@ +import { SetMetadata, Type } from '@nestjs/common'; + +// Resolver function type to get the owner user ID for a given entity ID +export type OwnerIdResolver = (params: { + entityId: number; + services: ServiceRegistry; +}) => Promise; + +// Registry of services that can be easily resolved +// Eliminates the issues with circular dependencies +// allowing the lambdas to resolve only the services they need +export interface ServiceRegistry { + get(serviceClass: Type): T; +} + +// Configuration for ownership check +export interface OwnershipConfig { + idParam: string; + resolver: OwnerIdResolver; +} + +export const OWNERSHIP_CHECK_KEY = 'ownership_check'; + +export const CheckOwnership = (config: OwnershipConfig) => + SetMetadata(OWNERSHIP_CHECK_KEY, config); diff --git a/apps/backend/src/auth/ownership.guard.ts b/apps/backend/src/auth/ownership.guard.ts new file mode 100644 index 00000000..0b21e154 --- /dev/null +++ b/apps/backend/src/auth/ownership.guard.ts @@ -0,0 +1,98 @@ +import { + Injectable, + CanActivate, + ExecutionContext, + ForbiddenException, + NotFoundException, + Type, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { ModuleRef } from '@nestjs/core'; +import { + OWNERSHIP_CHECK_KEY, + OwnershipConfig, + ServiceRegistry, +} from './ownership.decorator'; + +@Injectable() +export class OwnershipGuard implements CanActivate { + constructor(private reflector: Reflector, private moduleRef: ModuleRef) {} + + async canActivate(context: ExecutionContext): Promise { + const config = this.reflector.get( + OWNERSHIP_CHECK_KEY, + context.getHandler(), + ); + + if (!config) { + return true; + } + + // Process all request information and the logged in user + const req = context.switchToHttp().getRequest(); + const user = req.user; + + // Admins bypass ownership checks + if (user.role === 'ADMIN') { + return true; + } + + if (!user) { + throw new ForbiddenException('Not authenticated'); + } + + // Get the id from the parameters + const entityId = Number(req.params[config.idParam]); + + if (isNaN(entityId)) { + throw new ForbiddenException(`Invalid ${config.idParam}`); + } + + // Create a service registry that easily resolves services + const services = this.createServiceRegistry(); + + try { + // Execute the lambda function to get the owner user ID + const ownerId = await config.resolver({ + entityId, + services, + }); + + if (ownerId === null || ownerId === undefined) { + throw new ForbiddenException('Unable to determine resource ownership'); + } + + if (ownerId !== user.id) { + throw new ForbiddenException('Access denied'); + } + + return true; + } catch (error) { + throw new ForbiddenException('Error verifying resource ownership'); + } + } + + // Use a service registry for easy service resolution and caching + private createServiceRegistry(): ServiceRegistry { + const cache = new Map, unknown>(); + const moduleRef = this.moduleRef; + + return { + get(serviceClass: Type): T { + // Return cached service if already resolved before + if (cache.has(serviceClass)) { + return cache.get(serviceClass) as T; + } + + // Resolve and cache the service + try { + const service = moduleRef.get(serviceClass, { strict: false }); + cache.set(serviceClass, service); + return service; + } catch (error) { + throw new Error(`Could not resolve service: ${serviceClass.name}`); + } + }, + }; + } +} diff --git a/apps/backend/src/auth/sharedAuth.module.ts b/apps/backend/src/auth/sharedAuth.module.ts new file mode 100644 index 00000000..757fb974 --- /dev/null +++ b/apps/backend/src/auth/sharedAuth.module.ts @@ -0,0 +1,8 @@ +import { Module } from '@nestjs/common'; +import { OwnershipGuard } from './ownership.guard'; + +@Module({ + providers: [OwnershipGuard], + exports: [OwnershipGuard], +}) +export class SharedAuthModule {} diff --git a/apps/backend/src/donationItems/donationItems.controller.ts b/apps/backend/src/donationItems/donationItems.controller.ts index 6435dd9c..075fb145 100644 --- a/apps/backend/src/donationItems/donationItems.controller.ts +++ b/apps/backend/src/donationItems/donationItems.controller.ts @@ -5,17 +5,18 @@ import { Param, Get, Patch, + UseGuards, ParseIntPipe, - BadRequestException, } from '@nestjs/common'; import { ApiBody } from '@nestjs/swagger'; import { DonationItemsService } from './donationItems.service'; import { DonationItem } from './donationItems.entity'; +import { AuthGuard } from '@nestjs/passport'; import { FoodType } from './types'; import { CreateMultipleDonationItemsDto } from './dtos/create-donation-items.dto'; @Controller('donation-items') -//@UseInterceptors() +@UseGuards(AuthGuard('jwt')) export class DonationItemsController { constructor(private donationItemsService: DonationItemsService) {} diff --git a/apps/backend/src/donationItems/donationItems.service.ts b/apps/backend/src/donationItems/donationItems.service.ts index 740a2650..ef9f112d 100644 --- a/apps/backend/src/donationItems/donationItems.service.ts +++ b/apps/backend/src/donationItems/donationItems.service.ts @@ -13,6 +13,16 @@ export class DonationItemsService { @InjectRepository(Donation) private donationRepo: Repository, ) {} + async findOne(itemId: number): Promise { + validateId(itemId, 'Donation Item'); + + const donationItem = await this.repo.findOneBy({ itemId }); + if (!donationItem) { + throw new NotFoundException(`Donation item ${itemId} not found`); + } + return donationItem; + } + async getAllDonationItems(donationId: number): Promise { validateId(donationId, 'Donation'); return this.repo.find({ where: { donation: { donationId } } }); diff --git a/apps/backend/src/foodRequests/request.module.ts b/apps/backend/src/foodRequests/request.module.ts index 0e5dc280..d1c19680 100644 --- a/apps/backend/src/foodRequests/request.module.ts +++ b/apps/backend/src/foodRequests/request.module.ts @@ -9,6 +9,7 @@ import { AuthModule } from '../auth/auth.module'; import { OrdersService } from '../orders/order.service'; import { Order } from '../orders/order.entity'; import { Pantry } from '../pantries/pantries.entity'; +import { SharedAuthModule } from '../auth/sharedAuth.module'; @Module({ imports: [ @@ -16,6 +17,7 @@ import { Pantry } from '../pantries/pantries.entity'; MulterModule.register({ dest: './uploads' }), TypeOrmModule.forFeature([FoodRequest, Order, Pantry]), AuthModule, + SharedAuthModule, ], controllers: [RequestsController], providers: [RequestsService, OrdersService], diff --git a/apps/backend/src/orders/order.controller.ts b/apps/backend/src/orders/order.controller.ts index a3ec003a..78cb2eb5 100644 --- a/apps/backend/src/orders/order.controller.ts +++ b/apps/backend/src/orders/order.controller.ts @@ -7,6 +7,7 @@ import { Body, Query, BadRequestException, + UseGuards, ValidationPipe, } from '@nestjs/common'; import { OrdersService } from './order.service'; @@ -16,6 +17,10 @@ import { FoodManufacturer } from '../foodManufacturers/manufacturers.entity'; import { FoodRequest } from '../foodRequests/request.entity'; import { AllocationsService } from '../allocations/allocations.service'; import { OrderStatus } from './types'; +import { AuthGuard } from '@nestjs/passport'; +import { OwnershipGuard } from '../auth/ownership.guard'; +import { CheckOwnership } from '../auth/ownership.decorator'; +import { PantriesService } from '../pantries/pantries.service'; import { TrackingCostDto } from './dtos/tracking-cost.dto'; @Controller('orders') @@ -56,6 +61,29 @@ export class OrdersController { return this.ordersService.findOrderPantry(orderId); } + // Test endpoint for right now + @UseGuards(AuthGuard('jwt'), OwnershipGuard) + @CheckOwnership({ + idParam: 'orderId', + resolver: async ({ entityId, services }) => { + const request = await services + .get(OrdersService) + .findOrderFoodRequest(entityId); + + if (!request) { + return null; + } + + const pantry = await services + .get(PantriesService) + .findOne(request.pantryId); + + if (!pantry) { + return null; + } + return pantry?.pantryUser?.id ?? null; + }, + }) @Get('/:orderId/request') async getRequestFromOrder( @Param('orderId', ParseIntPipe) orderId: number, diff --git a/apps/backend/src/orders/order.module.ts b/apps/backend/src/orders/order.module.ts index 0734f876..1f435f43 100644 --- a/apps/backend/src/orders/order.module.ts +++ b/apps/backend/src/orders/order.module.ts @@ -3,8 +3,6 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { OrdersController } from './order.controller'; import { Order } from './order.entity'; import { OrdersService } from './order.service'; -import { JwtStrategy } from '../auth/jwt.strategy'; -import { AuthService } from '../auth/auth.service'; import { Pantry } from '../pantries/pantries.entity'; import { AllocationModule } from '../allocations/allocations.module'; import { AuthModule } from '../auth/auth.module'; diff --git a/apps/backend/src/pantries/pantries.controller.ts b/apps/backend/src/pantries/pantries.controller.ts index 8ed72be2..a47bdd6a 100644 --- a/apps/backend/src/pantries/pantries.controller.ts +++ b/apps/backend/src/pantries/pantries.controller.ts @@ -6,6 +6,7 @@ import { ParseIntPipe, Patch, Post, + UseGuards, Req, UnauthorizedException, } from '@nestjs/common'; @@ -13,6 +14,7 @@ import { Pantry } from './pantries.entity'; import { PantriesService } from './pantries.service'; import { Role } from '../users/types'; import { Roles } from '../auth/roles.decorator'; +import { AuthGuard } from '@nestjs/passport'; import { ValidationPipe } from '@nestjs/common'; import { PantryApplicationDto } from './dtos/pantry-application.dto'; import { ApiBody } from '@nestjs/swagger'; @@ -26,6 +28,8 @@ import { } from './types'; import { Order } from '../orders/order.entity'; import { OrdersService } from '../orders/order.service'; +import { OwnershipGuard } from '../auth/ownership.guard'; +import { CheckOwnership } from '../auth/ownership.decorator'; import { Public } from '../auth/public.decorator'; @Controller('pantries') @@ -53,6 +57,14 @@ export class PantriesController { return this.pantriesService.getPendingPantries(); } + @UseGuards(AuthGuard('jwt'), OwnershipGuard) + @CheckOwnership({ + idParam: 'pantryId', + resolver: async ({ entityId, services }) => { + const pantry = await services.get(PantriesService).findOne(entityId); + return pantry?.pantryUser?.id ?? null; + }, + }) @Roles(Role.PANTRY, Role.ADMIN) @Get('/:pantryId') async getPantry( diff --git a/apps/backend/src/pantries/pantries.module.ts b/apps/backend/src/pantries/pantries.module.ts index 5e60b78d..a60ffbf7 100644 --- a/apps/backend/src/pantries/pantries.module.ts +++ b/apps/backend/src/pantries/pantries.module.ts @@ -6,12 +6,14 @@ import { Pantry } from './pantries.entity'; import { AuthModule } from '../auth/auth.module'; import { OrdersModule } from '../orders/order.module'; import { User } from '../users/user.entity'; +import { SharedAuthModule } from '../auth/sharedAuth.module'; @Module({ imports: [ TypeOrmModule.forFeature([Pantry, User]), OrdersModule, forwardRef(() => AuthModule), + SharedAuthModule, ], controllers: [PantriesController], providers: [PantriesService], diff --git a/apps/frontend/src/api/apiClient.ts b/apps/frontend/src/api/apiClient.ts index 0378afb9..58f2ba59 100644 --- a/apps/frontend/src/api/apiClient.ts +++ b/apps/frontend/src/api/apiClient.ts @@ -4,6 +4,7 @@ import axios, { type AxiosInstance, type InternalAxiosRequestConfig, } from 'axios'; +import { NavigateFunction } from 'react-router-dom'; import { User, Order, @@ -27,6 +28,7 @@ const defaultBaseUrl = export class ApiClient { private axiosInstance: AxiosInstance; private accessToken: string | undefined; + private navigate: NavigateFunction | null = null; constructor() { this.axiosInstance = axios.create({ baseURL: defaultBaseUrl }); @@ -48,15 +50,24 @@ export class ApiClient { this.axiosInstance.interceptors.response.use( (response) => response, (error: AxiosError) => { - if (error.response?.status === 403) { - // TODO: For a future ticket, figure out a better method than renavigation on failure (or a better place to check than in the api requests) - window.location.replace('/unauthorized'); + if (error.response?.status === 403 && this.navigate) { + const errorData = error.response?.data as { message?: string }; + this.navigate('/unauthorized', { + replace: true, + state: { + errorMessage: errorData?.message || 'Access forbidden', + }, + }); } return Promise.reject(error); }, ); } + public setNavigate(navigate: NavigateFunction) { + this.navigate = navigate; + } + public setAccessToken(token: string | undefined) { this.accessToken = token; } diff --git a/apps/frontend/src/components/forms/deliveryConfirmationModal.tsx b/apps/frontend/src/components/forms/deliveryConfirmationModal.tsx index 03a6a66e..299e6fcb 100644 --- a/apps/frontend/src/components/forms/deliveryConfirmationModal.tsx +++ b/apps/frontend/src/components/forms/deliveryConfirmationModal.tsx @@ -8,7 +8,12 @@ import { Text, Dialog, } from '@chakra-ui/react'; -import { Form, ActionFunction, ActionFunctionArgs } from 'react-router-dom'; +import { + Form, + ActionFunction, + ActionFunctionArgs, + redirect, +} from 'react-router-dom'; import ApiClient from '@api/apiClient'; interface DeliveryConfirmationModalProps { @@ -151,7 +156,7 @@ export const submitDeliveryConfirmationFormModal: ActionFunction = async ({ const form = await request.formData(); const confirmDeliveryData = new FormData(); - const pantryId = form.get('pantryId'); + const pantryId = form.get('pantryId') as string; const requestId = form.get('requestId') as string; confirmDeliveryData.append('requestId', requestId); @@ -161,6 +166,7 @@ export const submitDeliveryConfirmationFormModal: ActionFunction = async ({ confirmDeliveryData.append('dateReceived', formattedDate); } else { alert('Delivery date is missing or invalid.'); + return redirect(`/request-form/${pantryId}`); } confirmDeliveryData.append('feedback', form.get('feedback') as string); diff --git a/apps/frontend/src/containers/root.tsx b/apps/frontend/src/containers/root.tsx index d179353f..b6963d35 100644 --- a/apps/frontend/src/containers/root.tsx +++ b/apps/frontend/src/containers/root.tsx @@ -1,10 +1,18 @@ -import { Outlet } from 'react-router-dom'; +import { Outlet, useNavigate } from 'react-router-dom'; +import { useEffect } from 'react'; import Header from '../components/Header'; import { useAuth } from '../hooks/useAuth'; +import apiClient from '@api/apiClient'; const Root: React.FC = () => { + const navigate = useNavigate(); + useAuth(); + useEffect(() => { + apiClient.setNavigate(navigate); + }, [navigate]); + return (