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 1 commit
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
40 changes: 33 additions & 7 deletions src/middleware/authorization.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import autoBind from "auto-bind"
import { NextFunction, Request, Response } from "express"

import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"

import { ForbiddenError } from "@errors/ForbiddenError"
import { RequestHandler } from "@root/types"
import AuthorizationMiddlewareService from "@services/middlewareServices/AuthorizationMiddlewareService"

Expand All @@ -19,8 +18,30 @@ export class AuthorizationMiddleware {
autoBind(this)
}

// Check whether a user is a site admin
verifySiteAdmin: RequestHandler<
never,
unknown,
unknown,
never,
{ userWithSiteSessionData: UserWithSiteSessionData }
> = async (req, res, next) => {
const { userWithSiteSessionData } = res.locals

try {
const result = await this.authorizationMiddlewareService.checkIsSiteAdmin(
userWithSiteSessionData
)
if (result instanceof ForbiddenError) return next(new ForbiddenError())

return next()
} catch (err) {
return next(err)
}
}

// Check whether a user is a site member
checkIsSiteMember: RequestHandler<
verifySiteMember: RequestHandler<
never,
unknown,
unknown,
Expand All @@ -29,10 +50,15 @@ export class AuthorizationMiddleware {
> = async (req, res, next) => {
const { userWithSiteSessionData } = res.locals

await this.authorizationMiddlewareService.checkIsSiteMember(
userWithSiteSessionData
)
try {
const result = await this.authorizationMiddlewareService.checkIsSiteMember(
userWithSiteSessionData
)
if (result instanceof ForbiddenError) return next(new ForbiddenError())

return next()
return next()
} catch (err) {
return next(err)
}
}
}
4 changes: 4 additions & 0 deletions src/middleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"

import { RequestHandler } from "@root/types"
import IdentityAuthService from "@services/identity/AuthService"
import CollaboratorsService from "@root/services/identity/CollaboratorsService"
import IsomerAdminsService from "@services/identity/IsomerAdminsService"
import UsersService from "@services/identity/UsersService"
import AuthenticationMiddlewareService from "@services/middlewareServices/AuthenticationMiddlewareService"
Expand All @@ -27,15 +28,18 @@ const getAuthorizationMiddleware = ({
identityAuthService,
usersService,
isomerAdminsService,
collaboratorsService,
}: {
identityAuthService: IdentityAuthService
usersService: UsersService
isomerAdminsService: IsomerAdminsService
collaboratorsService: CollaboratorsService
}) => {
const authorizationMiddlewareService = new AuthorizationMiddlewareService({
identityAuthService,
usersService,
isomerAdminsService,
collaboratorsService,
})
const authorizationMiddleware = new AuthorizationMiddleware({
authorizationMiddlewareService,
Expand Down
2 changes: 1 addition & 1 deletion src/routes/v1/authenticatedSites/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const getAuthenticatedSitesSubrouter = ({

authenticatedSitesSubrouter.use(authenticationMiddleware.verifyJwt)
authenticatedSitesSubrouter.use(attachSiteHandler)
authenticatedSitesSubrouter.use(authorizationMiddleware.checkIsSiteMember)
authenticatedSitesSubrouter.use(authorizationMiddleware.verifySiteAdmin)
dcshzj marked this conversation as resolved.
Show resolved Hide resolved

authenticatedSitesSubrouter.use("/pages", pagesRouter)
authenticatedSitesSubrouter.use("/collections", collectionsRouter)
Expand Down
2 changes: 1 addition & 1 deletion src/routes/v2/authenticatedSites/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ const getAuthenticatedSitesSubrouter = ({

authenticatedSitesSubrouter.use(authenticationMiddleware.verifyJwt)
authenticatedSitesSubrouter.use(attachSiteHandler)
authenticatedSitesSubrouter.use(authorizationMiddleware.checkIsSiteMember)
authenticatedSitesSubrouter.use(authorizationMiddleware.verifySiteAdmin)
dcshzj marked this conversation as resolved.
Show resolved Hide resolved

authenticatedSitesSubrouter.use(
"/collections/:collectionName",
Expand Down
12 changes: 12 additions & 0 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ import DeploymentsService from "@services/identity/DeploymentsService"
import ReposService from "@services/identity/ReposService"
import InfraService from "@services/infra/InfraService"

import { AuthorizationMiddleware } from "./middleware/authorization"
import getAuthenticatedSubrouterV1 from "./routes/v1/authenticated"
import getAuthenticatedSitesSubrouterV1 from "./routes/v1/authenticatedSites"
import getAuthenticatedSubrouter from "./routes/v2/authenticated"
import getAuthenticatedSitesSubrouter from "./routes/v2/authenticatedSites"
import CollaboratorsService from "./services/identity/CollaboratorsService"

const path = require("path")

Expand Down Expand Up @@ -81,6 +83,13 @@ const infraService = new InfraService({
reposService,
deploymentsService,
})
const collaboratorsService = new CollaboratorsService({
siteRepository: Site,
siteMemberRepository: SiteMember,
sitesService,
usersService,
whitelist: Whitelist,
})

const gitHubService = new GitHubService({
axiosInstance: isomerRepoAxiosInstance,
Expand All @@ -94,6 +103,7 @@ const authorizationMiddleware = getAuthorizationMiddleware({
identityAuthService,
usersService,
isomerAdminsService,
collaboratorsService,
})

const authenticatedSubrouterV1 = getAuthenticatedSubrouterV1({
Expand All @@ -113,6 +123,8 @@ const authenticatedSubrouterV2 = getAuthenticatedSubrouter({
reposService,
deploymentsService,
isomerAdminsService,
collaboratorsService,
authorizationMiddleware,
})
const authenticatedSitesSubrouterV2 = getAuthenticatedSitesSubrouter({
authorizationMiddleware,
Expand Down
50 changes: 44 additions & 6 deletions src/services/middlewareServices/AuthorizationMiddlewareService.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { NotFoundError } from "@errors/NotFoundError"

import { CollaboratorRoles } from "@root/constants"
import { ForbiddenError } from "@root/errors/ForbiddenError"
import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"

import { E2E_ISOMER_ID } from "@root/constants"
import AuthService from "@services/identity/AuthService"
import CollaboratorsService from "@services/identity/CollaboratorsService"
import IsomerAdminsService from "@services/identity/IsomerAdminsService"
import UsersService from "@services/identity/UsersService"

Expand All @@ -14,6 +16,7 @@ interface AuthorizationMiddlewareServiceProps {
identityAuthService: AuthService
usersService: UsersService
isomerAdminsService: IsomerAdminsService
collaboratorsService: CollaboratorsService
}

export default class AuthorizationMiddlewareService {
Expand All @@ -23,36 +26,71 @@ export default class AuthorizationMiddlewareService {

readonly isomerAdminsService: AuthorizationMiddlewareServiceProps["isomerAdminsService"]

readonly collaboratorsService: AuthorizationMiddlewareServiceProps["collaboratorsService"]

constructor({
identityAuthService,
usersService,
isomerAdminsService,
collaboratorsService,
}: AuthorizationMiddlewareServiceProps) {
this.identityAuthService = identityAuthService
this.usersService = usersService
this.isomerAdminsService = isomerAdminsService
this.collaboratorsService = collaboratorsService
}

async checkIsSiteMember(sessionData: UserWithSiteSessionData) {
// Check if user has access to site
const { siteName, isomerUserId: userId } = sessionData

// Should always be defined - authorization middleware only exists if siteName is defined
if (!siteName) throw Error("No site name in authorization middleware")
if (!siteName) {
logger.error("No site name in authorization middleware")
return new ForbiddenError()
}

logger.info(`Verifying user's access to ${siteName}`)

const isSiteMember = await (sessionData.isEmailUser()
? this.usersService.hasAccessToSite(userId, siteName)
? (await this.collaboratorsService.getRole(siteName, userId)) !== null
: this.identityAuthService.hasAccessToSite(sessionData))

const isAdminUser = await this.isomerAdminsService.getByUserId(userId)
const isIsomerCoreAdmin = await this.isomerAdminsService.getByUserId(userId)

const isE2EUser = userId === E2E_ISOMER_ID
if (!isSiteMember && !isAdminUser && !isE2EUser) {
throw new NotFoundError("Site does not exist")
if (!isSiteMember && !isIsomerCoreAdmin && !isE2EUser) {
logger.error("Site does not exist")
return new ForbiddenError()
}

logger.info(`User ${userId} has access to ${siteName}`)
}

async checkIsSiteAdmin(sessionData: UserWithSiteSessionData) {
dcshzj marked this conversation as resolved.
Show resolved Hide resolved
// Check if user has access to site
const { siteName, isomerUserId: userId } = sessionData

// Should always be defined - authorization middleware only exists if siteName is defined
if (!siteName) {
logger.error("No site name in authorization middleware")
return new ForbiddenError()
}

logger.info(`Verifying user's access to ${siteName}`)

const isSiteAdmin = await (sessionData.isEmailUser()
? (await this.collaboratorsService.getRole(siteName, userId)) ===
CollaboratorRoles.Admin
: this.identityAuthService.hasAccessToSite(sessionData))
const isIsomerCoreAdmin = await this.isomerAdminsService.getByUserId(userId)

const isE2EUser = userId === E2E_ISOMER_ID
if (!isSiteAdmin && !isIsomerCoreAdmin && !isE2EUser) {
logger.error("Site does not exist")
return new ForbiddenError()
}

logger.info(`User ${userId} has admin access to ${siteName}`)
}
}