Skip to content
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Endpoints restricted to administrators now return `403` (instead of `401`) when an authenticated user lacks the administrator role. A `401` is still returned when the request is not authenticated.
- The `changemaker` attached to a changemaker-backed `source` is now documented as a `ShallowChangemaker`. The API has always served a shallow changemaker there, but the schema described a full `Changemaker`, promising `fields` and `fiscalSponsors` attributes that were never present.

## 0.42.0 2026-08-25
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/baseFields.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ describe('/baseFields', () => {
});

it('requires administrator role', async () => {
await request(app).put('/baseFields/summary').set(authHeader).expect(401);
await request(app).put('/baseFields/summary').set(authHeader).expect(403);
});

it('creates exactly one base field', async () => {
Expand Down Expand Up @@ -865,7 +865,7 @@ describe('/baseFields', () => {
await request(app)
.put('/baseFields/summary/localizations/fr')
.set(authHeader)
.expect(401);
.expect(403);
});

it('creates the specified base field localization if it does not exist', async () => {
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/baseFieldsCopyTasks.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('/tasks/baseFieldsCopy', () => {
});

it('requires administrator role', async () => {
await agent.get('/tasks/baseFieldsCopy').set(authHeader).expect(401);
await agent.get('/tasks/baseFieldsCopy').set(authHeader).expect(403);
});

it('returns an empty Bundle when no data is present', async () => {
Expand Down Expand Up @@ -166,7 +166,7 @@ describe('/tasks/baseFieldsCopy', () => {
await request(app)
.post('/tasks/baseFieldsCopy')
.set(authHeader)
.expect(401);
.expect(403);
});

it('throws an error if a synchzronizationUrl is not provided', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/changemakers.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1771,7 +1771,7 @@ describe('/changemakers', () => {
.type('application/json')
.set(authHeader)
.send({ name: '7404171616851845629932178083729587457161' })
.expect(401);
.expect(403);
});

it('Requires integer changemaker ID', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/dataProviders.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('/dataProviders', () => {
});

it('requires administrator role', async () => {
await agent.put('/dataProviders/foo').set(authHeader).expect(401);
await agent.put('/dataProviders/foo').set(authHeader).expect(403);
});

it('creates and returns exactly one data provider', async () => {
Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/defaultPermissionGrants.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('/defaultPermissionGrants', () => {
});

it('requires administrator role', async () => {
await agent.get('/defaultPermissionGrants').set(authHeader).expect(401);
await agent.get('/defaultPermissionGrants').set(authHeader).expect(403);
});

it('returns an empty list when no default permission grants exist', async () => {
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('/defaultPermissionGrants', () => {
});

it('requires administrator role', async () => {
await agent.get('/defaultPermissionGrants/1').set(authHeader).expect(401);
await agent.get('/defaultPermissionGrants/1').set(authHeader).expect(403);
});

it('returns 404 when the default permission grant does not exist', async () => {
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('/defaultPermissionGrants', () => {
});

it('requires administrator role', async () => {
await agent.post('/defaultPermissionGrants').set(authHeader).expect(401);
await agent.post('/defaultPermissionGrants').set(authHeader).expect(403);
});

it('creates and returns a default permission grant for a user', async () => {
Expand Down Expand Up @@ -429,7 +429,7 @@ describe('/defaultPermissionGrants', () => {
});

it('requires administrator role', async () => {
await agent.put('/defaultPermissionGrants/1').set(authHeader).expect(401);
await agent.put('/defaultPermissionGrants/1').set(authHeader).expect(403);
});

it('returns 404 when the default permission grant does not exist', async () => {
Expand Down Expand Up @@ -512,7 +512,7 @@ describe('/defaultPermissionGrants', () => {
await agent
.delete('/defaultPermissionGrants/1')
.set(authHeader)
.expect(401);
.expect(403);
});

it('returns 404 when the default permission grant does not exist', async () => {
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/funders.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ describe('/funders', () => {
});

it('requires administrator role', async () => {
await agent.put('/funders/foo').set(authHeader).expect(401);
await agent.put('/funders/foo').set(authHeader).expect(403);
});

it('creates and returns exactly one funder', async () => {
Expand Down Expand Up @@ -1002,7 +1002,7 @@ describe('/funders', () => {
await agent
.post('/funders/theFundFund/members/theFoundationFoundation')
.set(authHeader)
.expect(401);
.expect(403);
});

it('throws a 400 error if the funder short code is invalid', async () => {
Expand Down Expand Up @@ -1100,7 +1100,7 @@ describe('/funders', () => {
await agent
.delete('/funders/theFundFund/members/theFoundationFoundation')
.set(authHeader)
.expect(401);
.expect(403);
});

it('throws a 400 error if the funder short code is invalid', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/sources.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ describe('/sources', () => {
});

it('requires administrator role', async () => {
await agent.delete('/sources/:sourceId').expect(401);
await agent.delete('/sources/1').set(authHeader).expect(403);
});

it('deletes exactly one source that has no proposals associated with it', async () => {
Expand Down
12 changes: 5 additions & 7 deletions src/__tests__/terminologySets.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ describe('/terminologySets', () => {
});
});

it('returns 401 when the user lacks edit | terminologySet on the funder', async () => {
it('returns 403 when the user lacks edit | terminologySet on the funder', async () => {
const db = getDatabase();
const systemFunder = await loadSystemFunder(db, null);
const before = await loadTableMetrics(db, 'terminology_sets');
Expand All @@ -278,7 +278,7 @@ describe('/terminologySets', () => {
funderShortCode: systemFunder.shortCode,
name: 'Unauthorized vocab',
})
.expect(401);
.expect(403);
const after = await loadTableMetrics(db, 'terminology_sets');
expect(after.count).toEqual(before.count);
});
Expand Down Expand Up @@ -433,7 +433,7 @@ describe('/terminologySets', () => {
});
});

it('returns 401 when a non-admin caller lacks edit permission', async () => {
it('returns 403 when a non-admin caller lacks edit permission', async () => {
const db = getDatabase();
const systemUser = await loadSystemUser(db, null);
const systemUserAuthContext = getAuthContext(systemUser);
Expand All @@ -458,7 +458,7 @@ describe('/terminologySets', () => {
.type('application/json')
.set(authHeader)
.send({ proposalLabel: 'Submission' })
.expect(401);
.expect(403);
});
});

Expand Down Expand Up @@ -501,8 +501,6 @@ describe('/terminologySets', () => {
},
);

// Caller can CREATE opportunities on funder, but has no reference
// permission on the terminology set.
await createPermissionGrant(db, systemUserAuthContext, {
granteeType: PermissionGrantGranteeType.USER,
granteeUserKeycloakUserId: testUser.keycloakUserId,
Expand All @@ -521,7 +519,7 @@ describe('/terminologySets', () => {
funderShortCode: funder.shortCode,
terminologySetId: terminologySet.id,
})
.expect(401);
.expect(403);
});

it('allows POST /opportunities with terminologySetId when caller has reference permission via funder cascade', async () => {
Expand Down
5 changes: 3 additions & 2 deletions src/handlers/opportunitiesHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
FailedMiddlewareError,
ForbiddenError,
InputValidationError,
UnauthorizedError,
} from '../errors';
import {
extractFunderParameters,
Expand Down Expand Up @@ -102,7 +101,9 @@ const postOpportunity = async (req: Request, res: Response): Promise<void> => {
scope: PermissionGrantEntityType.TERMINOLOGY_SET,
}))
) {
throw new UnauthorizedError();
throw new ForbiddenError(
'Authenticated user does not have permission to reference the specified terminology set.',
);
}
const committedOpportunity = await db.transaction(async (txDb) => {
const opportunity = await createOpportunity(txDb, req, body);
Expand Down
10 changes: 7 additions & 3 deletions src/handlers/terminologySetsHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import {
} from '../types';
import {
FailedMiddlewareError,
ForbiddenError,
InputValidationError,
UnauthorizedError,
} from '../errors';
import {
extractFunderParameters,
Expand Down Expand Up @@ -97,7 +97,9 @@ const postTerminologySet = async (
scope: PermissionGrantEntityType.TERMINOLOGY_SET,
}))
) {
throw new UnauthorizedError();
throw new ForbiddenError(
'Authenticated user does not have permission to create a terminology set for the specified funder.',
);
}
const committedTerminologySet = await db.transaction(async (txDb) => {
const terminologySet = await createTerminologySet(txDb, req, body);
Expand Down Expand Up @@ -146,7 +148,9 @@ const patchTerminologySet = async (
scope: PermissionGrantEntityType.TERMINOLOGY_SET,
}))
) {
throw new UnauthorizedError();
throw new ForbiddenError(
'Authenticated user does not have permission to edit this terminology set.',
);
}

const updatedTerminologySet = await updateTerminologySet(
Expand Down
14 changes: 7 additions & 7 deletions src/middleware/__tests__/requireAdministratorRole.int.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { requireAdministratorRole } from '../requireAdministratorRole';
import { UnauthorizedError } from '../../errors';
import { ForbiddenError, UnauthorizedError } from '../../errors';
import { getMockedUser } from '../../test/utils';
import { getMockRequest, getMockResponse } from '../../test/mockExpress';
import type { AuthenticatedRequest } from '../../types';

describe('requireAuthentication', () => {
describe('requireAdministratorRole', () => {
it('calls next with an UnauthorizedError when no roles value is provided', (done) => {
const req = getMockRequest() as AuthenticatedRequest;
const res = getMockResponse();
Expand All @@ -15,26 +15,26 @@ describe('requireAuthentication', () => {
* We have validated that error is an UnauthorizedError, but eslint doesn't recognize that fact.
*/
expect((error as UnauthorizedError).message).toEqual(
'Your account must have the administrator role.',
'The request lacks an AuthContext.',
);
done();
});
requireAdministratorRole(req, res, nextMock);
});

it('calls next with an UnauthorizedError when the user has an administrator role set to false', (done) => {
it('calls next with a ForbiddenError when the user has an administrator role set to false', (done) => {
const req = getMockRequest() as AuthenticatedRequest;
const res = getMockResponse();
req.user = getMockedUser();
req.role = {
isAdministrator: false,
};
const nextMock = jest.fn((error) => {
expect(error).toBeInstanceOf(UnauthorizedError);
expect(error).toBeInstanceOf(ForbiddenError);
/* eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion --
* We have validated that error is an UnauthorizedError, but eslint doesn't recognize that fact.
* We have validated that error is a ForbiddenError, but eslint doesn't recognize that fact.
*/
expect((error as UnauthorizedError).message).toEqual(
expect((error as ForbiddenError).message).toEqual(
'Your account must have the administrator role.',
);
done();
Expand Down
12 changes: 7 additions & 5 deletions src/middleware/requireAdministratorRole.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { UnauthorizedError } from '../errors';
import { ForbiddenError, UnauthorizedError } from '../errors';
import { isAuthContext } from '../types';
import type { Request, Response, NextFunction } from 'express';

Expand All @@ -7,10 +7,12 @@ const requireAdministratorRole = (
res: Response,
next: NextFunction,
): void => {
if (!isAuthContext(req) || !req.role.isAdministrator) {
next(
new UnauthorizedError('Your account must have the administrator role.'),
);
if (!isAuthContext(req)) {
next(new UnauthorizedError('The request lacks an AuthContext.'));
return;
}
if (!req.role.isAdministrator) {
next(new ForbiddenError('Your account must have the administrator role.'));
return;
}
next();
Expand Down
3 changes: 3 additions & 0 deletions src/openapi/paths/baseField.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"404": {
"$ref": "../components/responses/NotFound.json",
"description": "The base field could not be found."
Expand Down
3 changes: 3 additions & 0 deletions src/openapi/paths/baseFieldLocalization.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"404": {
"$ref": "../components/responses/NotFound.json",
"description": "The base field could not be found."
Expand Down
6 changes: 6 additions & 0 deletions src/openapi/paths/baseFieldsCopyTasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"500": {
"$ref": "../components/responses/InternalServerError.json"
}
Expand Down Expand Up @@ -71,6 +74,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"500": {
"$ref": "../components/responses/InternalServerError.json"
}
Expand Down
3 changes: 3 additions & 0 deletions src/openapi/paths/changemaker.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"404": {
"$ref": "../components/responses/NotFound.json",
"description": "The given changemaker ID was not found."
Expand Down
6 changes: 6 additions & 0 deletions src/openapi/paths/changemakerFiscalSponsor.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"422": {
"$ref": "../components/responses/UnprocessableEntity.json",
"description": "The call passed one or more changemakerIds that do not exist in the database."
Expand All @@ -81,6 +84,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"404": {
"$ref": "../components/responses/NotFound.json"
},
Expand Down
3 changes: 3 additions & 0 deletions src/openapi/paths/dataProvider.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@
"401": {
"$ref": "../components/responses/Unauthorized.json"
},
"403": {
"$ref": "../components/responses/Forbidden.json"
},
"500": {
"$ref": "../components/responses/InternalServerError.json"
}
Expand Down
Loading
Loading