Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ PROOFDESK_TERMINAL_MODE=restricted
PROOFDESK_TERMINAL_RUNTIME=container
PROOFDESK_SHARED_STATE_BACKEND=redis
PROOFDESK_REDIS_URL=redis://redis:6379/0
# Comma-separated GitHub logins allowed to read /monitoring/events.
# Unset or empty means no one is an administrator (the endpoint returns 403).
PROOFDESK_ADMIN_LOGINS=
PROOFDESK_MONITORING_ENABLED=true
PROOFDESK_MONITORING_WEBHOOK_URL=

Expand Down
46 changes: 46 additions & 0 deletions backend/src/middleware/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,52 @@ export const requireAccessToken = async (req: Request, res: Response, next: Next
next();
};

/**
* Administrative allow-list, read from PROOFDESK_ADMIN_LOGINS as a
* comma-separated list of GitHub logins.
*
* Parsed per request rather than at module load so an operator can change the
* variable without a rebuild, and so tests can set it per case.
*/
const getAdminLogins = (): Set<string> =>
new Set(
String(process.env.PROOFDESK_ADMIN_LOGINS || '')
.split(',')
.map((entry) => entry.trim().toLowerCase())
.filter(Boolean)
);

/**
* Restricts a route to operators named in PROOFDESK_ADMIN_LOGINS.
*
* Must be mounted after `requireAccessToken`, which is what populates
* `req.authSession`.
*
* Denies in three cases, all deliberately fail-closed:
*
* - the allow-list is empty or unset. An unconfigured allow-list means "no
* administrators have been designated", never "everyone qualifies";
* - there is no authenticated session. `requireAccessToken` also accepts a
* raw bearer token, and in that path `req.authSession` is null — a bearer
* token on its own carries no verified identity to check against the list;
* - the session's login is not on the list.
*
* Logins are compared case-insensitively, matching GitHub's own treatment.
*/
export const requireAdmin = (req: Request, res: Response, next: NextFunction): any => {
const admins = getAdminLogins();
if (admins.size === 0) {
return res.status(403).json({ error: 'Administrative access is not configured' });
}

const login = req.authSession?.user?.login;
if (!login || !admins.has(String(login).toLowerCase())) {
return res.status(403).json({ error: 'Administrative access required' });
}

next();
};

export const checkWorkspaceOwner = (req: Request, res: Response, next: NextFunction): any => {
const { sessionId } = req.params;
const login = req.authSession?.user?.login;
Expand Down
7 changes: 5 additions & 2 deletions backend/src/routes/system.routes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Router, Request, Response } from 'express';
import { requireAccessToken } from '../middleware/auth.js';
import { requireAccessToken, requireAdmin } from '../middleware/auth.js';
import {
getMonitoringContextFromRequest,
readRecentMonitoringEvents,
Expand Down Expand Up @@ -76,7 +76,10 @@ export const createSystemRouter = (): Router => {
});
});

router.get('/monitoring/events', requireAccessToken, async (req: Request, res: Response) => {
// System-wide monitoring events carry backend and frontend stack traces,
// internal filesystem paths and request metadata for every user, so this is
// restricted to operators rather than to any authenticated caller.
router.get('/monitoring/events', requireAccessToken, requireAdmin, async (req: Request, res: Response) => {
const limitQuery = req.query.limit;
const limit = typeof limitQuery === 'string' || typeof limitQuery === 'number' ? Number(limitQuery) : undefined;
const events = await readRecentMonitoringEvents({
Expand Down
Loading