Skip to content

Commit

Permalink
Add DELETE endpoint for UserChangemakerPermission
Browse files Browse the repository at this point in the history
This endpoint makes it possible for a user with appropriate permissions
to remove a changemaker permission for a given user.

Issue #1250 Support associations between users and organizational entities
  • Loading branch information
slifty committed Dec 17, 2024
1 parent 42b7738 commit 254f7ca
Show file tree
Hide file tree
Showing 13 changed files with 482 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Added

- Manage user changemaker permissions using `GET`, `PUT`, and `DELETE` on `/user/{keycloakUserId}/changemakers/{changemakerId}/permissions/{permission}`.

### Changed

- Upgraded to use OpenAPI Specification 3.1.
Expand Down
189 changes: 189 additions & 0 deletions src/__tests__/userChangemakerPermissions.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import {
createChangemaker,
createOrUpdateUserChangemakerPermission,
loadSystemUser,
loadUserChangemakerPermission,
removeUserChangemakerPermission,
} from '../database';
import { expectTimestamp, loadTestUser } from '../test/utils';
import {
mockJwt as authHeader,
mockJwtWithAdminRole as authHeaderWithAdminRole,
} from '../test/mockJwt';
import { keycloakUserIdToString, Permission } from '../types';
import { NotFoundError } from '../errors';

describe('/users/changemakers/:changemakerId/permissions/:permission', () => {
describe('PUT /', () => {
Expand Down Expand Up @@ -153,4 +156,190 @@ describe('/users/changemakers/:changemakerId/permissions/:permission', () => {
});
});
});

describe('DELETE /', () => {
it('returns 401 if the request lacks authentication', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.MANAGE}`,
)
.send()
.expect(401);
});

it('returns 401 if the authenticated user lacks permission', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.MANAGE}`,
)
.set(authHeader)
.send()
.expect(401);
});

it('returns 400 if the userId is not a valid keycloak user ID', async () => {
await request(app)
.delete(
`/users/notaguid/changemakers/1/permissions/${Permission.MANAGE}`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(400);
});

it('returns 400 if the changemaker ID is not a valid ID', async () => {
const user = await loadTestUser();
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/notanId/permissions/${Permission.MANAGE}`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(400);
});

it('returns 400 if the permission is not a valid permission', async () => {
const user = await loadTestUser();
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/1/permissions/notAPermission`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(400);
});

it('returns 404 if the permission does not exist', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.MANAGE}`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(404);
});

it('returns 404 if the permission had existed and previously been deleted', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await createOrUpdateUserChangemakerPermission({
userKeycloakUserId: user.keycloakUserId,
changemakerId: changemaker.id,
permission: Permission.EDIT,
createdBy: user.keycloakUserId,
});
await removeUserChangemakerPermission(
user.keycloakUserId,
changemaker.id,
Permission.EDIT,
);
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.EDIT}`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(404);
});

it('deletes the user changemaker permission when the user has administrative credentials', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await createOrUpdateUserChangemakerPermission({
userKeycloakUserId: user.keycloakUserId,
changemakerId: changemaker.id,
permission: Permission.EDIT,
createdBy: user.keycloakUserId,
});
const permission = await loadUserChangemakerPermission(
user.keycloakUserId,
changemaker.id,
Permission.EDIT,
);
expect(permission).toEqual({
changemakerId: changemaker.id,
createdAt: expectTimestamp,
createdBy: user.keycloakUserId,
permission: Permission.EDIT,
userKeycloakUserId: user.keycloakUserId,
});
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.EDIT}`,
)
.set(authHeaderWithAdminRole)
.send()
.expect(204);
await expect(loadUserChangemakerPermission(
user.keycloakUserId,
changemaker.id,
Permission.EDIT,
)).rejects.toThrow(NotFoundError);
});

it('deletes the user changemaker permission when the user has permission to manage the changemaker', async () => {
const user = await loadTestUser();
const changemaker = await createChangemaker({
taxId: '11-1111111',
name: 'Example Inc.',
});
await createOrUpdateUserChangemakerPermission({
userKeycloakUserId: user.keycloakUserId,
changemakerId: changemaker.id,
permission: Permission.MANAGE,
createdBy: user.keycloakUserId,
});
await createOrUpdateUserChangemakerPermission({
userKeycloakUserId: user.keycloakUserId,
changemakerId: changemaker.id,
permission: Permission.EDIT,
createdBy: user.keycloakUserId,
});
const permission = await loadUserChangemakerPermission(
user.keycloakUserId,
changemaker.id,
Permission.EDIT,
);
expect(permission).toEqual({
changemakerId: changemaker.id,
createdAt: expectTimestamp,
createdBy: user.keycloakUserId,
permission: Permission.EDIT,
userKeycloakUserId: user.keycloakUserId,
});
await request(app)
.delete(
`/users/${keycloakUserIdToString(user.keycloakUserId)}/changemakers/${changemaker.id}/permissions/${Permission.EDIT}`,
)
.set(authHeader)
.send()
.expect(204);
await expect(loadUserChangemakerPermission(
user.keycloakUserId,
changemaker.id,
Permission.EDIT,
)).rejects.toThrow(NotFoundError);
});
});
});
41 changes: 41 additions & 0 deletions src/__tests__/users.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createUser,
loadSystemUser,
loadTableMetrics,
removeUserChangemakerPermission,
} from '../database';
import { expectTimestamp, loadTestUser } from '../test/utils';
import {
Expand Down Expand Up @@ -122,6 +123,46 @@ describe('/users', () => {
});
});

it('does not return deleted permissions associated with a user', async () => {
const systemUser = await loadSystemUser();
const testUser = await loadTestUser();
const changemaker = await createChangemaker({
name: 'Test Changemaker',
taxId: '12-3456789',
});
await createOrUpdateUserChangemakerPermission({
userKeycloakUserId: testUser.keycloakUserId,
permission: Permission.VIEW,
changemakerId: changemaker.id,
createdBy: systemUser.keycloakUserId,
});
await removeUserChangemakerPermission(
testUser.keycloakUserId,
changemaker.id,
Permission.VIEW,
);
const { count: userCount } = await loadTableMetrics('users');

const response = await request(app)
.get('/users')
.set(authHeader)
.expect(200);
expect(response.body).toEqual({
total: userCount,
entries: [
{
keycloakUserId: testUser.keycloakUserId,
permissions: {
changemaker: {},
dataProvider: {},
funder: {},
},
createdAt: expectTimestamp,
},
],
});
});

it('returns all users when the user is an administrator', async () => {
const systemUser = await loadSystemUser();
const testUser = await loadTestUser();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { db } from '../../db';
import { NotFoundError } from '../../../errors';
import { keycloakUserIdToString } from '../../../types';
import type {
CheckResult,
Id,
KeycloakUserId,
Permission,
} from '../../../types';

const assertUserChangemakerPermissionExists = async (
userKeycloakUserId: KeycloakUserId,
changemakerId: Id,
permission: Permission,
): Promise<void> => {
const result = await db.sql<CheckResult>(
'userChangemakerPermissions.checkExistsByPrimaryKey',
{
userKeycloakUserId,
changemakerId,
permission,
},
);

if (result.rows[0]?.result !== true) {
throw new NotFoundError(`Entity not found`, {
entityType: 'UserChangemakerPermission',
entityPrimaryKey: {
userKeycloakUserId: keycloakUserIdToString(userKeycloakUserId),
changemakerId,
permission,
},
});
}
};

export { assertUserChangemakerPermissionExists };
3 changes: 3 additions & 0 deletions src/database/operations/userChangemakerPermissions/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
export * from './assertUserChangemakerPermissionExists';
export * from './createOrUpdateUserChangemakerPermission';
export * from './loadUserChangemakerPermission';
export * from './removeUserChangemakerPermission';
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { db } from '../../db';
import { NotFoundError } from '../../../errors';
import {
keycloakUserIdToString,
type Id,
type JsonResultSet,
type KeycloakUserId,
type Permission,
type UserChangemakerPermission,
} from '../../../types';

export const loadUserChangemakerPermission = async (
userKeycloakUserId: KeycloakUserId,
changemakerId: Id,
permission: Permission,
): Promise<UserChangemakerPermission> => {
const result = await db.sql<JsonResultSet<UserChangemakerPermission>>(
'userChangemakerPermissions.selectByPrimaryKey',
{
userKeycloakUserId,
changemakerId,
permission,
},
);
const object = result.rows[0]?.object;
if (object === undefined) {
throw new NotFoundError(`Entity not found`, {
entityType: 'UserchangemakerPermission',
entityPrimaryKey: {
userKeycloakUserId: keycloakUserIdToString(userKeycloakUserId),
changemakerId,
permission,
},
});
}
return object;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { NotFoundError } from '../../../errors';
import { keycloakUserIdToString } from '../../../types';
import { db } from '../../db';
import type { KeycloakUserId, Permission } from '../../../types';

const removeUserChangemakerPermission = async (
userKeycloakUserId: KeycloakUserId,
changemakerId: number,
permission: Permission,
): Promise<void> => {
const result = await db.sql('userChangemakerPermissions.deleteOne', {
userKeycloakUserId,
permission,
changemakerId,
});

if (result.row_count === 0) {
throw new NotFoundError(

Check warning on line 18 in src/database/operations/userChangemakerPermissions/removeUserChangemakerPermission.ts

View check run for this annotation

Codecov / codecov/patch

src/database/operations/userChangemakerPermissions/removeUserChangemakerPermission.ts#L18

Added line #L18 was not covered by tests
'The item did not exist and could not be deleted.',
{
entityType: 'UserChangemakerPermission',
entityPrimaryKey: {
userKeycloakUserId: keycloakUserIdToString(userKeycloakUserId),
permission,
changemakerId,
},
},
);
}
};

export { removeUserChangemakerPermission };
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
SELECT exists(
SELECT 1
FROM user_changemaker_permissions
WHERE user_keycloak_user_id = :userKeycloakUserId
AND changemaker_id = :changemakerId
AND permission = :permission
AND NOT is_expired(not_after)
) AS result;
Loading

0 comments on commit 254f7ca

Please sign in to comment.