Skip to content

Commit

Permalink
fix(security): validate bot access (#365)
Browse files Browse the repository at this point in the history
* fix(security): validate bot access

* account for all bots as well
  • Loading branch information
EFF committed Jul 15, 2022
1 parent 323c5ee commit fd6bc0c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
21 changes: 10 additions & 11 deletions packages/studio-be/src/core/security/router-security.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
import { checkRule, CSRF_TOKEN_HEADER_LC, JWT_COOKIE_NAME, STANDALONE_USER } from 'common/auth'
import { RequestWithUser } from 'common/typings'
import { ConfigProvider } from 'core/config'
import {
InvalidOperationError,
ForbiddenError,
InternalServerError,
NotFoundError,
UnauthorizedError
} from 'core/routers'
import { InvalidOperationError, ForbiddenError, NotFoundError, UnauthorizedError } from 'core/routers'
import { WorkspaceService } from 'core/users'
import { NextFunction, Request, RequestHandler, Response } from 'express'
import { NextFunction, RequestHandler, Response } from 'express'
import { AuthService, WORKSPACE_HEADER, SERVER_USER } from './auth-service'

const debugFailure = DEBUG('audit:collab:fail')
const debugSuccess = DEBUG('audit:collab:success')
const debugSuperSuccess = DEBUG('audit:admin:success')
const debugSuperFailure = DEBUG('audit:admin:fail')

export const ALL_BOTS = '___'

export const checkTokenHeader =
(authService: AuthService, audience?: string) => async (req: RequestWithUser, res: Response, next: NextFunction) => {
Expand Down Expand Up @@ -141,6 +135,11 @@ const checkPermissions =
return new ForbiddenError(`User "${email}" doesn't have access to workspace "${req.workspace}"`)
}

const isBotIdValid = req.params.botId && req.params.botId !== ALL_BOTS
if (isBotIdValid && !(await workspaceService.isBotInWorkspace(req.workspace, req.params.botId))) {
return new NotFoundError(`Bot "${req.params.botId}" doesn't exist in workspace "${req.workspace}"`)
}

const role = await workspaceService.getRoleForUser(email, strategy, req.workspace)

if (!role || !checkRule(role.rules, operation, resource)) {
Expand All @@ -157,7 +156,7 @@ export const checkBotVisibility =
// '___' is a non-valid botId, but here acts as for "all bots"
// This is used in modules when they setup routes that work on a global level (they are not tied to a specific bot)
// Check the 'sso-login' module for an example
if (req.params.botId === '___' || req.originalUrl.endsWith('env.js')) {

This comment has been minimized.

Copy link
@slvnperron

slvnperron Jul 18, 2022

Member

why remove .js ?

This comment has been minimized.

Copy link
@slvnperron

slvnperron Jul 18, 2022

Member

nvm, I saw this changed recently

if (req.params.botId === '___' || req.originalUrl.endsWith('env')) {
return next()
}

Expand Down
9 changes: 9 additions & 0 deletions packages/studio-be/src/core/users/workspace-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,13 @@ export class WorkspaceService {
const user = await this.findUser(email, strategy, workspace)!
return user && this.findRole(user.role!, workspace)
}

async isBotInWorkspace(workspaceId: string, botId: string): Promise<boolean> {
try {
const workspace = await this.findWorkspace(workspaceId)
return workspace.bots.includes(botId)
} catch (err) {
return false
}
}
}

0 comments on commit fd6bc0c

Please sign in to comment.