Skip to content

Commit

Permalink
feat: collaborators (#510)
Browse files Browse the repository at this point in the history
* build(devdeps): add lodash types

* fix(model): rectify db model definitions

* refactor: add message param to ForbiddenError

* feat: add CollaboratorsService

* test: add tests for CollaboratorsService

* feat: use CollaboratorService in authorization middleware

* test: add tests for authorization middleware

* feat: add CollaboratorsRouter

* test: add tests for CollaboratorsRouter

* feat(db-migration): change site_members role enum in the database

* feat: modify authzMiddlewareService tests

* fix: error in mock collaborators fixture
  • Loading branch information
prestonlimlianjie authored and alexanderleegs committed Mar 16, 2023
1 parent 04dbb39 commit f46d58a
Show file tree
Hide file tree
Showing 22 changed files with 1,502 additions and 52 deletions.
6 changes: 6 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
"@types/aws-lambda": "^8.10.106",
"@types/express": "^4.17.13",
"@types/jest": "^27.4.1",
"@types/lodash": "^4.14.186",
"@types/node": "^17.0.21",
"@types/supertest": "^2.0.11",
"@types/validator": "^13.7.1",
Expand Down
6 changes: 6 additions & 0 deletions src/constants/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ export enum RedirectionTypes {
CNAME = "CNAME",
A = "A",
}

export enum CollaboratorRoles {
Admin = "ADMIN",
Contributor = "CONTRIBUTOR",
}

export const E2E_ISOMER_ID = "-1"
export const E2E_TEST_EMAIL = "test@e2e"
export const E2E_TEST_CONTACT = "12345678"
59 changes: 59 additions & 0 deletions src/database/migrations/20220811070630-change-role-enum.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
module.exports = {
async up(queryInterface, Sequelize) {
// Change the role enum values in the site_members table
await queryInterface.sequelize.transaction(async (transaction) => {
// 1. Change column type to TEXT
await queryInterface.changeColumn(
"site_members", // name of Source model
"role", // name of column we're modifying
{
type: Sequelize.TEXT,
},
{ transaction }
)
// 2. Discard enum type
await queryInterface.sequelize.query(
"drop type enum_site_members_role;",
{ transaction }
)
// 3. Change column type to new enum type (fails if inconsistent with existing data)
await queryInterface.changeColumn(
"site_members", // name of Source model
"role", // name of column we're modifying
{
type: Sequelize.ENUM("ADMIN", "CONTRIBUTOR"),
},
{ transaction }
)
})
},

async down(queryInterface, Sequelize) {
// Change the role enum values in the site_members table
await queryInterface.sequelize.transaction(async (transaction) => {
// 1. Change column type to TEXT
await queryInterface.changeColumn(
"site_members", // name of Source model
"role", // name of column we're modifying
{
type: Sequelize.TEXT,
},
{ transaction }
)
// 2. Discard enum type
await queryInterface.sequelize.query(
"drop type enum_site_members_role;",
{ transaction }
)
// 3. Change column type to new enum type (fails if inconsistent with existing data)
await queryInterface.changeColumn(
"site_members", // name of Source model
"role", // name of column we're modifying
{
type: Sequelize.ENUM("ADMIN", "USER"),
},
{ transaction }
)
})
},
}
3 changes: 2 additions & 1 deletion src/database/models/Site.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class Site extends Model {
@Column({
allowNull: false,
type: DataType.TEXT,
unique: true,
})
name!: string

Expand Down Expand Up @@ -71,7 +72,7 @@ export class Site extends Model {
through: () => SiteMember,
as: "site_members",
})
users!: User[]
site_members!: Array<User & { SiteMember: SiteMember }>

@HasOne(() => Repo)
repo?: Repo
Expand Down
6 changes: 4 additions & 2 deletions src/database/models/SiteMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
UpdatedAt,
} from "sequelize-typescript"

import { CollaboratorRoles } from "@constants/index"

import { Site } from "@database/models/Site"
import { User } from "@database/models/User"

Expand All @@ -23,9 +25,9 @@ export class SiteMember extends Model {

@Column({
allowNull: false,
type: DataType.ENUM("ADMIN", "USER"),
type: DataType.ENUM("ADMIN", "CONTRIBUTOR"),
})
role!: boolean
role!: CollaboratorRoles

@CreatedAt
createdAt!: Date
Expand Down
2 changes: 1 addition & 1 deletion src/database/models/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class User extends Model {
through: () => SiteMember,
as: "site_members",
})
sites!: Site[]
sites!: Array<Site & { SiteMember: SiteMember }>

@HasMany(() => Site, {
as: "sites_created",
Expand Down
4 changes: 2 additions & 2 deletions src/errors/ForbiddenError.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
const { BaseIsomerError } = require("@errors/BaseError")

class ForbiddenError extends BaseIsomerError {
constructor() {
super(403, "Access forbidden")
constructor(message) {
super(403, message || "Access forbidden")
}
}

Expand Down
120 changes: 120 additions & 0 deletions src/fixtures/identity.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { Attributes } from "sequelize/types"

import { User, SiteMember } from "@database/models"

export const mockRecipient = "[email protected]"
export const mockSubject = "mock subject"
export const mockBody = "somebody"
Expand All @@ -18,3 +22,119 @@ export const mockBearerTokenHeaders = {
Authorization: `Bearer ${process.env.POSTMAN_API_KEY}`,
},
}

const mockCollaboratorContributor1: Attributes<User> & {
SiteMember: Attributes<SiteMember>
} = {
id: 1,
email: "[email protected]",
githubId: "test1",
contactNumber: "12331231",
lastLoggedIn: new Date("2022-07-30T07:41:09.661Z"),
createdAt: new Date("2022-04-04T07:25:41.013Z"),
updatedAt: new Date("2022-07-30T07:41:09.662Z"),
deletedAt: undefined,
SiteMember: {
userId: 1,
siteId: "16",
role: "CONTRIBUTOR",
createdAt: new Date("2022-07-29T03:50:49.145Z"),
updatedAt: new Date("2022-07-29T03:50:49.145Z"),
},
sites: [],
}

const mockCollaboratorAdmin1: Attributes<User> & {
SiteMember: Attributes<SiteMember>
} = {
id: 2,
email: "[email protected]",
githubId: "test2",
contactNumber: "12331232",
lastLoggedIn: new Date("2022-07-30T07:41:09.661Z"),
createdAt: new Date("2022-04-04T07:25:41.013Z"),
updatedAt: new Date("2022-07-30T07:41:09.662Z"),
deletedAt: undefined,
SiteMember: {
userId: 2,
siteId: "16",
role: "ADMIN",
createdAt: new Date("2022-07-29T03:50:49.145Z"),
updatedAt: new Date("2022-07-29T03:50:49.145Z"),
},
sites: [],
}
const mockCollaboratorAdmin2: Attributes<User> & {
SiteMember: Attributes<SiteMember>
} = {
id: 3,
email: "[email protected]",
githubId: "test3",
contactNumber: "12331233",
lastLoggedIn: new Date("2022-06-30T07:41:09.661Z"),
createdAt: new Date("2022-04-04T07:25:41.013Z"),
updatedAt: new Date("2022-07-30T07:41:09.662Z"),
deletedAt: undefined,
SiteMember: {
userId: 3,
siteId: "16",
role: "ADMIN",
createdAt: new Date("2022-07-29T03:50:49.145Z"),
updatedAt: new Date("2022-07-29T03:50:49.145Z"),
},
sites: [],
}
const mockCollaboratorContributor2: Attributes<User> & {
SiteMember: Attributes<SiteMember>
} = {
id: 4,
email: "[email protected]",
githubId: "test4",
contactNumber: "12331234",
lastLoggedIn: new Date("2022-07-30T07:41:09.661Z"),
createdAt: new Date("2022-04-04T07:25:41.013Z"),
updatedAt: new Date("2022-07-30T07:41:09.662Z"),
deletedAt: undefined,
SiteMember: {
userId: 4,
siteId: "16",
role: "CONTRIBUTOR",
createdAt: new Date("2022-07-29T03:50:49.145Z"),
updatedAt: new Date("2022-07-29T03:50:49.145Z"),
},
sites: [],
}

export const unsortedMockCollaboratorsList = [
mockCollaboratorContributor1,
mockCollaboratorAdmin1,
mockCollaboratorAdmin2,
mockCollaboratorContributor2,
]

export const expectedSortedMockCollaboratorsList = [
mockCollaboratorAdmin2,
mockCollaboratorAdmin1,
mockCollaboratorContributor1,
mockCollaboratorContributor2,
]

export const mockSiteOrmResponseWithAllCollaborators = {
id: 1,
name: "",
site_members: unsortedMockCollaboratorsList,
}
export const mockSiteOrmResponseWithOneAdminCollaborator = {
id: 1,
name: "",
site_members: [mockCollaboratorAdmin1],
}
export const mockSiteOrmResponseWithOneContributorCollaborator = {
id: 1,
name: "",
site_members: [mockCollaboratorContributor2],
}
export const mockSiteOrmResponseWithNoCollaborators = {
id: 1,
site_members: "",
}
111 changes: 111 additions & 0 deletions src/middleware/__tests__/authorization.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { NextFunction, Request, Response } from "express"

import { ForbiddenError } from "@errors/ForbiddenError"

import { AuthorizationMiddleware } from "@middleware/authorization"

import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData"
import AuthorizationMiddlewareService from "@root/services/middlewareServices/AuthorizationMiddlewareService"

describe("Authorization middleware", () => {
const TEST_SITE_NAME = "sitename"
const TEST_ISOMER_USER_ID = "1"
const mockAuthorizationMiddlewareService = {
checkIsSiteAdmin: jest.fn(),
checkIsSiteMember: jest.fn(),
}
const mockReq = ({
params: { siteName: TEST_SITE_NAME },
} as unknown) as Request<
never,
unknown,
unknown,
never,
{ userWithSiteSessionData: UserWithSiteSessionData }
>
const mockRes = ({
locals: {
sessionData: { getIsomerUserId: jest.fn(() => TEST_ISOMER_USER_ID) },
},
} as unknown) as Response<
unknown,
{ userWithSiteSessionData: UserWithSiteSessionData }
>
const mockNext = jest.fn() as NextFunction

const authorizationMiddleware = new AuthorizationMiddleware({
authorizationMiddlewareService: (mockAuthorizationMiddlewareService as unknown) as AuthorizationMiddlewareService,
})

beforeEach(() => {
jest.clearAllMocks()
})

describe("verifySiteAdmin", () => {
it("correctly verifies that user is a site admin if no error is thrown in the authorization middleware service", async () => {
// Arrange
mockAuthorizationMiddlewareService.checkIsSiteAdmin.mockResolvedValue(
undefined
)

// Act
await authorizationMiddleware.verifySiteAdmin(mockReq, mockRes, mockNext)

// Assert
expect(
mockAuthorizationMiddlewareService.checkIsSiteAdmin
).toHaveBeenCalled()
expect(mockNext).toHaveBeenCalledWith()
})

it("correctly verifies that user is not site admin if an error is thrown in the authorization middleware service", async () => {
// Arrange
mockAuthorizationMiddlewareService.checkIsSiteAdmin.mockResolvedValue(
new ForbiddenError()
)

// Act
await authorizationMiddleware.verifySiteAdmin(mockReq, mockRes, mockNext)

// Assert
expect(
mockAuthorizationMiddlewareService.checkIsSiteAdmin
).toHaveBeenCalled()
expect(mockNext).toHaveBeenCalledWith(new ForbiddenError())
})
})

describe("verifySiteMember", () => {
it("correctly verifies that user is a site member if no error is thrown in the authorization middleware service", async () => {
// Arrange
mockAuthorizationMiddlewareService.checkIsSiteMember.mockResolvedValue(
undefined
)

// Act
await authorizationMiddleware.verifySiteMember(mockReq, mockRes, mockNext)

// Assert
expect(
mockAuthorizationMiddlewareService.checkIsSiteMember
).toHaveBeenCalled()
expect(mockNext).toHaveBeenCalledWith()
})

it("correctly verifies that user is not site member if an error is thrown in the authorization middleware service", async () => {
// Arrange
mockAuthorizationMiddlewareService.checkIsSiteMember.mockResolvedValue(
new ForbiddenError()
)

// Act
await authorizationMiddleware.verifySiteMember(mockReq, mockRes, mockNext)

// Assert
expect(
mockAuthorizationMiddlewareService.checkIsSiteMember
).toHaveBeenCalled()
expect(mockNext).toHaveBeenCalledWith(new ForbiddenError())
})
})
})
Loading

0 comments on commit f46d58a

Please sign in to comment.