Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: collaborators #485

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4592234
Chore: migrate authmiddlewareservice to typescript
alexanderleegs Jul 28, 2022
705d2d8
Feat: add site retrieval for email and admin users
alexanderleegs Jul 28, 2022
d94183a
build(devdeps): add lodash types
prestonlimlianjie Aug 11, 2022
00ec691
fix(model): rectify db model definitions
prestonlimlianjie Aug 11, 2022
7528b75
refactor: add message param to ForbiddenError
prestonlimlianjie Aug 11, 2022
0bdb767
fix(SitesService): fix type change from database model
prestonlimlianjie Aug 11, 2022
3262749
feat: add CollaboratorsService
prestonlimlianjie Aug 11, 2022
f2a1ae2
test: add tests for CollaboratorsService
prestonlimlianjie Aug 11, 2022
542d01f
feat: use CollaboratorService in authorization middleware
prestonlimlianjie Aug 11, 2022
a8e385c
test: add tests for authorization middleware
prestonlimlianjie Aug 11, 2022
e1b4889
feat: add CollaboratorsRouter
prestonlimlianjie Aug 11, 2022
eea564b
test: add tests for CollaboratorsRouter
prestonlimlianjie Aug 11, 2022
739f0bb
feat(db-migration): change site_members role enum in the database
prestonlimlianjie Aug 11, 2022
4f68c34
fix(models): undo erroneous changes
prestonlimlianjie Aug 11, 2022
4b700f0
fix: model field and failing tests
prestonlimlianjie Aug 15, 2022
bd377a0
fix: clean up painful rebase
prestonlimlianjie Aug 24, 2022
a41412d
chore: rename comment for clarity
prestonlimlianjie Sep 22, 2022
ea4e726
chore: remove unnecessary imports
prestonlimlianjie Sep 22, 2022
92575fc
style: rename var to be more boolean-like
prestonlimlianjie Sep 22, 2022
b1b6a00
style: define let vars as const
prestonlimlianjie Sep 22, 2022
cef758b
fix: use correct authZ method
prestonlimlianjie Sep 22, 2022
63bdf5d
refactor: authZ middleware methods
prestonlimlianjie Sep 22, 2022
22b28a7
chore: add types for collaborator mocks
prestonlimlianjie Sep 23, 2022
bd1649e
docs: add comment for deletion check
prestonlimlianjie Sep 28, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -83,6 +83,7 @@
"@tsconfig/recommended": "^1.0.1",
"@types/express": "^4.17.13",
"@types/jest": "^27.4.1",
"@types/lodash": "^4.14.182",
"@types/node": "^17.0.21",
"@types/supertest": "^2.0.11",
"@types/validator": "^13.7.1",
Expand Down
5 changes: 5 additions & 0 deletions src/constants/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,8 @@ export enum SiteStatus {
}

export const E2E_ISOMER_ID = "e2e-id"

export enum CollaboratorRoles {
Admin = "ADMIN",
Contributor = "CONTRIBUTOR",
}
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 @@ -32,6 +32,7 @@ export class Site extends Model {
@Column({
allowNull: false,
type: DataType.TEXT,
unique: true,
})
name!: string

Expand Down Expand Up @@ -70,7 +71,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
4 changes: 2 additions & 2 deletions src/database/models/SiteMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ export class SiteMember extends Model {

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

@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 @@ -67,7 +67,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 = [
mockCollaboratorAdmin1,
mockCollaboratorAdmin2,
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