Skip to content

Commit

Permalink
First phase of authorization: simple rules
Browse files Browse the repository at this point in the history
next step:
- casl + prisma (or other supported orms) to avoid weird logic,
and to filter the resource for the rows that are accessible to the current user
- oauth0 service to verify that some fields match the user's data on user creation
  • Loading branch information
friedbyalice committed Dec 13, 2022
1 parent a96ccac commit e0d1da1
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 43 deletions.
2 changes: 2 additions & 0 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"@nestjs/typeorm": "^9.0.1",
"@types/js-yaml": "^4.0.5",
"@types/passport-jwt": "^3.0.7",
"extra": "link:@casl/ability/extra",
"joi": "^17.7.0",
"js-yaml": "^4.1.0",
"jwks-rsa": "^3.0.0",
Expand Down Expand Up @@ -59,6 +60,7 @@
"eslint-plugin-prettier": "^4.0.0",
"jest": "28.1.3",
"prettier": "^2.3.2",
"prisma": "^4.7.1",
"source-map-support": "^0.5.20",
"supertest": "^6.1.3",
"ts-jest": "28.0.8",
Expand Down
8 changes: 3 additions & 5 deletions api/src/authorization/authenticated-request.types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Role } from '@hkrecruitment/shared';
import { AppAbility, UserAuth } from '@hkrecruitment/shared';

export type AuthenticatedRequest = Request & {
user: {
sub: string;
role: Role;
};
user: UserAuth;
ability: AppAbility;
};
34 changes: 26 additions & 8 deletions api/src/authorization/authorization.guard.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,36 @@
import { CanActivate, ExecutionContext, Injectable, Logger } from '@nestjs/common';
import { abilityForUser } from '@hkrecruitment/shared';
import {
CanActivate,
ExecutionContext,
Injectable,
Logger,
} from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { UsersService } from 'src/users/users.service';
import { CHECK_POLICIES_KEY, PolicyHandler } from './check-policies.decorator';

@Injectable()
export class AuthorizationGuard implements CanActivate {
private readonly logger = new Logger(AuthorizationGuard.name);

constructor(private readonly usersService: UsersService) {}
constructor(
private reflector: Reflector,
private readonly usersService: UsersService,
) {}

async canActivate(
context: ExecutionContext,
): Promise<boolean> {
const role = await this.usersService.getRoleForOauthId(context.switchToHttp().getRequest().user.sub) ?? 'none';
async canActivate(context: ExecutionContext): Promise<boolean> {
const role =
(await this.usersService.getRoleForOauthId(
context.switchToHttp().getRequest().user.sub,
)) ?? 'none';
context.switchToHttp().getRequest().user.role = role;
this.logger.log(`user.sub: ${context.switchToHttp().getRequest().user.sub} -> ${role}}`);
return true;
this.logger.log(
`user.sub: ${context.switchToHttp().getRequest().user.sub} -> ${role}}`,
);
const ability = abilityForUser(context.switchToHttp().getRequest().user);
context.switchToHttp().getRequest().ability = ability;

const handlers = this.reflector.get<PolicyHandler[]>(CHECK_POLICIES_KEY, context.getHandler()) ?? [];
return handlers.every((handler) => handler(ability));
}
}
7 changes: 7 additions & 0 deletions api/src/authorization/check-policies.decorator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { AppAbility } from '@hkrecruitment/shared';
import { SetMetadata } from '@nestjs/common';

export type PolicyHandler = (ability: AppAbility) => boolean;

export const CHECK_POLICIES_KEY = 'check-policies';
export const CheckPolicies = (...args: PolicyHandler[]) => SetMetadata(CHECK_POLICIES_KEY, args);
6 changes: 3 additions & 3 deletions api/src/users/create-user.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ export class CreateUserDto implements Partial<Person> {
@ApiProperty()
email: string;

@ApiProperty()
@ApiProperty({ required: false })
phone_no?: string;

@ApiProperty()
@ApiProperty({ required: false })
telegramId?: string;

@ApiProperty({ enum: Role })
@ApiProperty({ enum: Role, required: false })
role?: Role
}
2 changes: 1 addition & 1 deletion api/src/users/user.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class User implements Person {
@Column()
sex: string;

@Column({ unique: true })
@Column()
email: string;

@Column({ nullable: true })
Expand Down
56 changes: 47 additions & 9 deletions api/src/users/users.controller.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,39 @@
import { Body, Controller, Delete, ForbiddenException, Get, NotFoundException, Param, Patch, Post, Req, UseGuards } from '@nestjs/common';
import {
Body,
Controller,
Delete,
ForbiddenException,
Get,
NotFoundException,
Param,
Patch,
Post,
Req,
UseGuards,
} from '@nestjs/common';
import { User } from './user.entity';
import { UsersService } from './users.service';
import { createUserSchema, Role, updateUserSchema } from '@hkrecruitment/shared';
import {
Action,
canContinue,
createUserSchema,
Role,
updateUserSchema,
} from '@hkrecruitment/shared';
import { CreateUserDto } from './create-user.dto';
import { UpdateUserDto } from './update-user.dto';
import { JoiValidate } from '../joi-validation/joi-validate.decorator';
import { ApiBadRequestResponse, ApiBearerAuth, ApiForbiddenResponse, ApiNotFoundResponse, ApiTags, ApiUnauthorizedResponse } from '@nestjs/swagger';
import {
ApiBadRequestResponse,
ApiBearerAuth,
ApiForbiddenResponse,
ApiNotFoundResponse,
ApiTags,
ApiUnauthorizedResponse,
} from '@nestjs/swagger';
import { AuthenticatedRequest } from 'src/authorization/authenticated-request.types';
import * as Joi from 'joi';
import { CheckPolicies } from 'src/authorization/check-policies.decorator';

@ApiBearerAuth()
@ApiTags('users')
Expand All @@ -24,7 +50,12 @@ export class UsersController {
@ApiNotFoundResponse()
@ApiUnauthorizedResponse()
@Get(':oauthId')
async findByOauthId(@Param('oauthId') oauthId: string): Promise<User> {
@CheckPolicies((ability) => ability.can(Action.Read, 'Person'))
async findByOauthId(
@Param('oauthId') oauthId: string,
@Req() req: AuthenticatedRequest,
): Promise<User> {
// TODO: filter using ability
const result = await this.usersService.findByOauthId(oauthId);
if (result === null) {
throw new NotFoundException();
Expand All @@ -36,19 +67,26 @@ export class UsersController {
@ApiForbiddenResponse()
@Post()
@JoiValidate(createUserSchema.append({ oauthId: Joi.string().required() }))
create(@Body() user: CreateUserDto, @Req() req: AuthenticatedRequest): Promise<User> {
if (req.user.sub !== user.oauthId) {
throw new ForbiddenException('User cannot create another user');
create(
@Body() user: CreateUserDto,
@Req() req: AuthenticatedRequest,
): Promise<User> {
const ability = req.ability;
if (!canContinue(ability, Action.Create, user, 'Person')) {
throw new ForbiddenException();
}
return this.usersService.create({...user, role: Role.None});
return this.usersService.create({ ...user, role: Role.None });
}

@ApiNotFoundResponse()
@ApiBadRequestResponse()
@ApiForbiddenResponse()
@Patch(':oauthId')
@JoiValidate(updateUserSchema)
async update(@Param('oauthId') oauthId: string, @Body() updateUser: UpdateUserDto): Promise<User> {
async update(
@Param('oauthId') oauthId: string,
@Body() updateUser: UpdateUserDto,
): Promise<User> {
const user = await this.usersService.findByOauthId(oauthId);
if (user === null) {
throw new NotFoundException();
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
"dev:web": "pnpm --filter frontend dev",
"dev:api": "pnpm --filter api start:dev",
"dev": "pnpm run --stream dev:web & pnpm run --stream dev:api",
"dev:tmux": "tmux new-session -d -s hkrecruitment 'pnpm run --stream dev:web' \\; split-window -v -p 50 'pnpm run --stream dev:api' \\; attach",
"dev:tmux": "tmux new-session -d -s hkrecruitment 'pnpm run dev:web' \\; split-window -h -p 50 'pnpm run dev:api' \\; attach",
"build:web": "pnpm --filter frontend build",
"build:api": "pnpm --filter api prebuild && pnpm --filter api build",
"build": "pnpm run build:web && pnpm run build:api",
"preview:web": "pnpm --filter frontend preview",
"preview:api": "pnpm --filter api start:prod",
"preview": "pnpm run --stream preview:web & pnpm run --stream preview:api",
"preview:tmux": "tmux new-session -d -s hkrecruitment 'pnpm run --stream preview:web' \\; split-window -v -p 50 'pnpm run --stream preview:api' \\; attach"
"preview:tmux": "tmux new-session -d -s hkrecruitment 'pnpm run preview:web' \\; split-window -h -p 50 'pnpm run preview:api' \\; attach"
},
"keywords": [],
"author": "",
Expand Down
18 changes: 18 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions shared/src/abilities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { AbilityBuilder, AbilityClass, createMongoAbility, PureAbility, subject } from "@casl/ability";
import { applyAbilitesOnPerson, Person, Role } from "./person";

export interface UserAuth {
sub: string;
role: Role;
}

export enum Action {
Manage = "manage",
Create = "create",
Read = "read",
Update = "update",
Delete = "delete",
}
type SubjectsTypes = Partial<Person>;
type SubjectNames = 'Person';
export type Subjects = SubjectsTypes | SubjectNames;

export type AppAbility = PureAbility<[Action, Subjects]>;
export const AppAbility = PureAbility as AbilityClass<AppAbility>;

export type ApplyAbilities = (
user: UserAuth,
{ can, cannot }: AbilityBuilder<AppAbility>
) => void;

export const abilityForUser = (user: UserAuth): AppAbility => {
const builder = new AbilityBuilder<AppAbility>(createMongoAbility);

applyAbilitesOnPerson(user, builder);

const { build } = builder;
return build();
};

export const canContinue = (ability: AppAbility, Action: Action, subjectObj: SubjectsTypes, subjectName: SubjectNames): boolean => {
const subj = subject(subjectName, subjectObj);
return ability.can(Action, subj) && Object.keys(subject).every(field => ability.can(Action, subj, field));
}
2 changes: 1 addition & 1 deletion shared/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export * from "./person";
export * from "./user-auth";
export * from "./abilities";
37 changes: 36 additions & 1 deletion shared/src/person.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import * as Joi from "joi";
import { Role } from "./user-auth"
import { Action, ApplyAbilities } from "./abilities";

export enum Role {
None = "none",
Applicant = "applicant",
Member = "member",
Clerk = "clerk",
Supervisor = "supervisor",
Admin = "admin",
}

export interface Person {
oauthId: string;
Expand Down Expand Up @@ -38,3 +47,29 @@ export const updateUserSchema = Joi.object<Person>({
.valid("none", "applicant", "member", "clerk", "supervisor", "admin")
.optional(),
});

export const applyAbilitesOnPerson: ApplyAbilities = (
user,
{ can, cannot }
) => {
can(Action.Read, "Person", { oauthId: user.sub });
can(
Action.Create,
"Person",
["oauthId", "firstName", "lastName", "sex", "email", "phone_no", "telegramId"],
{ oauthId: user.sub }
);
// can(
// Action.Update,
// "Person",
// ["firstName", "lastName", "sex", "phone_no", "telegramId"],
// { oauthId: user.sub }
// );
can(Action.Delete, "Person", { oauthId: user.sub });

if (user.role === Role.Admin) {
can(Action.Manage, "Person"); // Admin can do anything on any user
}

cannot(Action.Update, "Person", ["oauthId"]); // No one can update oauthId
};
13 changes: 0 additions & 13 deletions shared/src/user-auth.ts

This file was deleted.

1 comment on commit e0d1da1

@friedbyalice
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably using typeorm forever is enough. CASL author doesn't like typeorm, he likes MikroORM and said he would create an integration with it. However at the moment the only official integration is with Prisma (a framework that uses codegen, so it's a excessive for me).
https://casl.js.org/v6/en/advanced/ability-to-database-query could be the answer, hopefully it's possible to add rules to all typeorm queries using this helper functions

Please sign in to comment.