Skip to content

Commit

Permalink
Store ephemeral user group associations
Browse files Browse the repository at this point in the history
Users are associated with user groups via the "organizations" attribute
of their authentication JWT.  We're going to want our queries and
authentication checks to account for those group associations, but we
don't want to have to constantly pass an unpredictably long list of
associations every time we make a query.

This entity, and the middleware that populates it, allows a given API
call to have those associations accounted for in a given HTTP request
context.  The table is labeled "ephemeral" to try to convey the fact
that the data it contains should not be considered accurate outside of
that context.

The implementation of the middleware right now is intentionally simple
and does not attempt to be efficient.  For example, it does not attempt
to re-use any past data in the table (e.g. associations that may have
existed from a past transaction and wouldn't necessarily have to be
re-created).

We may find that having O(n) insert queries for every authenticated API
call is too slow; if that ends up becoming a problem we may determine
that our model for using KeyCloak as the group management and JWTs as
the communication mechanism isn't correct.

These associations are not actually used when making auth decisions yet.
This commit is, however, a prerequisite for that feature.

Issue #Create user-group associations "cache" middleware
  • Loading branch information
slifty committed Feb 10, 2025
1 parent 0adf194 commit 918bb05
Show file tree
Hide file tree
Showing 14 changed files with 248 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
SELECT drop_function('ephemeral_user_group_association_to_json');

CREATE FUNCTION ephemeral_user_group_association_to_json(
ephemeral_user_group_association ephemeral_user_group_associations
)
RETURNS jsonb AS $$
BEGIN
RETURN jsonb_build_object(
'userKeycloakUserId', ephemeral_user_group_association.user_keycloak_user_id,
'userGroupKeycloakOrganizationId', ephemeral_user_group_association.user_group_keycloak_organization_id,
'createdAt', ephemeral_user_group_association.created_at
);
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
CREATE TABLE ephemeral_user_group_associations (
user_keycloak_user_id uuid NOT NULL REFERENCES users (
keycloak_user_id
) ON DELETE CASCADE,
user_group_keycloak_organization_id uuid NOT NULL,
created_at timestamp with time zone NOT NULL DEFAULT now(),
not_after timestamp with time zone NOT NULL,
PRIMARY KEY (user_keycloak_user_id, user_group_keycloak_organization_id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { generateCreateOrUpdateItemOperation } from '../generators';
import type {
EphemeralUserGroupAssociation,
InternallyWritableEphemeralUserGroupAssociation,
} from '../../../types';

const createEphemeralUserGroupAssociation = generateCreateOrUpdateItemOperation<
EphemeralUserGroupAssociation,
InternallyWritableEphemeralUserGroupAssociation,
[]
>(
'ephemeralUserGroupAssociations.insertOne',
['userKeycloakUserId', 'userGroupKeycloakOrganizationId'],
[],
);

export { createEphemeralUserGroupAssociation };
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './createEphemeralUserGroupAssociation';
export * from './loadEphemeralUserGroupAssociationsByUserKeycloakUserId';
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { generateLoadBundleOperation } from '../generators';
import { EphemeralUserGroupAssociation, type KeycloakId } from '../../../types';

const loadEphemeralUserGroupAssociationsByUserKeycloakUserId =
generateLoadBundleOperation<
EphemeralUserGroupAssociation,
[userKeycloakUserId: KeycloakId]
>(
'ephemeralUserGroupAssociations.selectByUserKeycloakUserId',
'ephemeral_user_group_associations',
['userKeycloakUserId'],
);

export { loadEphemeralUserGroupAssociationsByUserKeycloakUserId };
1 change: 1 addition & 0 deletions src/database/operations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export * from './bulkUploadTasks';
export * from './changemakerProposals';
export * from './changemakers';
export * from './dataProviders';
export * from './ephemeralUserGroupAssociations';
export * from './funders';
export * from './generic';
export * from './opportunities';
Expand Down
17 changes: 17 additions & 0 deletions src/database/queries/ephemeralUserGroupAssociations/insertOne.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
INSERT INTO ephemeral_user_group_associations (
user_keycloak_user_id,
user_group_keycloak_organization_id,
not_after
) VALUES (
:userKeycloakUserId,
:userGroupKeycloakOrganizationId,
now() + interval '1 hour'
)
ON CONFLICT (
user_keycloak_user_id, user_group_keycloak_organization_id
) DO UPDATE
SET not_after = now() + interval '1 hour'
RETURNING
ephemeral_user_group_association_to_json(
ephemeral_user_group_associations
) AS object;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
SELECT
ephemeral_user_group_association_to_json(
ephemeral_user_group_associations.*
) AS object
FROM ephemeral_user_group_associations
WHERE user_keycloak_user_id = :userKeycloakUserId::uuid;
65 changes: 63 additions & 2 deletions src/middleware/__tests__/addUserContext.int.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { addUserContext } from '../addUserContext';
import { db, loadUserByKeycloakUserId, loadTableMetrics } from '../../database';
import { generateNextWithAssertions } from '../../test/utils';
import {
db,
loadUserByKeycloakUserId,
loadTableMetrics,
loadEphemeralUserGroupAssociationsByUserKeycloakUserId,
} from '../../database';
import { expectTimestamp, generateNextWithAssertions } from '../../test/utils';
import { stringToKeycloakId } from '../../types';
import { InputValidationError } from '../../errors';
import type { AuthenticatedRequest } from '../../types';
Expand Down Expand Up @@ -58,6 +63,62 @@ describe('addUserContext', () => {
});
});

it('creates ephemeral user group associations when organizations are provided', (done) => {
const mockSub = '123e4567-e89b-12d3-a456-426614174000';
const myOrganizationId = '47d406ad-5e50-42d4-88f1-f87947a3e314';
const myOtherOrganizationId = '95e06557-6141-4be5-81df-e7f1a14bff5d';
const mockRequest = {
auth: {
sub: mockSub,
organizations: {
myOrganization: { id: myOrganizationId },
myOtherOrganization: { id: myOtherOrganizationId },
},
},
} as unknown as AuthenticatedRequest;
const mockResponse = {} as unknown as Response;

loadTableMetrics('ephemeral_user_group_associations')
.then(({ count: baselineEphemeralUserGroupAssociationCount }) => {
const runAssertions = async (err: unknown) => {
expect(err).toBe(undefined);
const { count: ephemeralUserGroupAssociationCount } =
await loadTableMetrics('ephemeral_user_group_associations');
const ephemeralUserGroupAssociations =
await loadEphemeralUserGroupAssociationsByUserKeycloakUserId(
db,
null,
stringToKeycloakId('123e4567-e89b-12d3-a456-426614174000'),
undefined,
undefined,
);
expect(ephemeralUserGroupAssociations.entries).toEqual(
expect.arrayContaining([
{
createdAt: expectTimestamp,
userGroupKeycloakOrganizationId: myOrganizationId,
userKeycloakUserId: mockSub,
},
{
createdAt: expectTimestamp,
userGroupKeycloakOrganizationId: myOtherOrganizationId,
userKeycloakUserId: mockSub,
},
]),
);
expect(ephemeralUserGroupAssociationCount).toEqual(
baselineEphemeralUserGroupAssociationCount + 2,
);
};

const nextMock = generateNextWithAssertions(runAssertions, done);
addUserContext(mockRequest, mockResponse, nextMock);
})
.catch((e) => {
done(e);
});
});

it('passes an error and does not creates or assign a user when an invalid keycloakUserId is provided', (done) => {
const mockRequest = {
auth: {
Expand Down
38 changes: 32 additions & 6 deletions src/middleware/addUserContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { db, createOrUpdateUser } from '../database';
import {
db,
createOrUpdateUser,
loadUserByKeycloakUserId,
createEphemeralUserGroupAssociation,
} from '../database';
import {
getAuthSubFromRequest,
getKeycloakOrganizationIdsFromRequest,
isKeycloakId,
keycloakIdToString,
stringToKeycloakId,
Expand All @@ -16,6 +22,7 @@ const addUserContext = (
next: NextFunction,
): void => {
const keycloakUserId = getAuthSubFromRequest(req);
const keycloakOrganizationIds = getKeycloakOrganizationIdsFromRequest(req);
const systemUser = getSystemUser();
if (
keycloakUserId === undefined ||
Expand All @@ -38,12 +45,31 @@ const addUserContext = (
createOrUpdateUser(db, null, {
keycloakUserId: stringToKeycloakId(keycloakUserId),
})
.then((user) => {
(req as AuthenticatedRequest).user = user;
next();
.then(() => {
const createEphemeralUserGroupAssociationPromises =
keycloakOrganizationIds.map((keycloakOrganizationId) =>
createEphemeralUserGroupAssociation(db, null, {
userKeycloakUserId: keycloakUserId,
userGroupKeycloakOrganizationId: keycloakOrganizationId,
}),
);
Promise.all(createEphemeralUserGroupAssociationPromises)
.then(() => {
loadUserByKeycloakUserId(db, null, keycloakUserId)
.then((user) => {
(req as AuthenticatedRequest).user = user;
next();
})
.catch((err) => {
next(err);

Check warning on line 64 in src/middleware/addUserContext.ts

View check run for this annotation

Codecov / codecov/patch

src/middleware/addUserContext.ts#L63-L64

Added lines #L63 - L64 were not covered by tests
});
})
.catch((err) => {
next(err);

Check warning on line 68 in src/middleware/addUserContext.ts

View check run for this annotation

Codecov / codecov/patch

src/middleware/addUserContext.ts#L67-L68

Added lines #L67 - L68 were not covered by tests
});
})
.catch((error: unknown) => {
next(error);
.catch((err) => {
next(err);
});
};

Expand Down
4 changes: 4 additions & 0 deletions src/test/mockJwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const getMockJwt = (
sub?: string;
roles?: string[];
iss?: string;
organizations?: Record<string, string>;
} = {},
getToken: (payload: JwtPayload) => string = mockJwks.token,
): { Authorization: string } => {
Expand All @@ -28,6 +29,9 @@ const getMockJwt = (
aud: 'account',
typ: 'Bearer',
azp: 'pdc-service',
organizations: settings.organizations ?? {
fooOrganization: { id: '47d406ad-5e50-42d4-88f1-f87947a3e314' },
},
realm_access: {
roles: settings.roles ?? ['default-roles-pdc'],
},
Expand Down
24 changes: 24 additions & 0 deletions src/types/EphemeralUserGroupAssociation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { KeycloakId } from './KeycloakId';
import { Writable } from './Writable';

interface EphemeralUserGroupAssociation {
readonly userKeycloakUserId: KeycloakId;
readonly userGroupKeycloakOrganizationId: KeycloakId;
readonly createdAt: string;
}

type WritableEphemeralUserGroupAssociation =
Writable<EphemeralUserGroupAssociation>;

type InternallyWritableEphemeralUserGroupAssociation =
WritableEphemeralUserGroupAssociation &
Pick<
EphemeralUserGroupAssociation,
'userKeycloakUserId' | 'userGroupKeycloakOrganizationId'
>;

export {
EphemeralUserGroupAssociation,
InternallyWritableEphemeralUserGroupAssociation,
WritableEphemeralUserGroupAssociation,
};
44 changes: 44 additions & 0 deletions src/types/express/AuthenticatedRequest.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Request as JwtRequest } from 'express-jwt';
import { ajv } from '../../ajv';
import { KeycloakId, keycloakIdSchema } from '../KeycloakId';
import type { JSONSchemaType } from 'ajv';
import type { Request } from 'express';
import type { AuthContext } from '../AuthContext';
Expand All @@ -20,6 +21,12 @@ interface ObjectWithAuthWithRealmAccessRoles {
};
}

interface ObjectWithAuthWithOrganizations {
auth: {
organizations: Record<string, { id: KeycloakId }>;
};
}

const objectWithAuthWithSubSchema: JSONSchemaType<ObjectWithAuthWithSub> = {
type: 'object',
properties: {
Expand Down Expand Up @@ -60,18 +67,54 @@ const objectWithAuthWithRealmAccessRolesSchema: JSONSchemaType<ObjectWithAuthWit
required: ['auth'],
};

const objectWithAuthWithOrganizationsSchema: JSONSchemaType<ObjectWithAuthWithOrganizations> =
{
type: 'object',
properties: {
auth: {
type: 'object',
properties: {
organizations: {
type: 'object',
additionalProperties: {
type: 'object',
properties: {
id: keycloakIdSchema,
},
required: ['id'],
},
required: [],
},
},
required: ['organizations'],
},
},
required: ['auth'],
};

const hasAuthWithSub = ajv.compile(objectWithAuthWithSubSchema);

const hasAuthWithRealmAccessRoles = ajv.compile(
objectWithAuthWithRealmAccessRolesSchema,
);

const isObjectWithAuthWithOrganizations = ajv.compile(
objectWithAuthWithOrganizationsSchema,
);

const getAuthSubFromRequest = (req: Request): string | undefined =>
hasAuthWithSub(req) ? req.auth.sub : undefined;

const getRealmAccessRolesFromRequest = (req: Request): string[] =>
hasAuthWithRealmAccessRoles(req) ? req.auth.realm_access.roles : [];

const getKeycloakOrganizationIdsFromRequest = (req: Request): KeycloakId[] =>
isObjectWithAuthWithOrganizations(req)
? Object.values(req.auth.organizations).map(
(organization) => organization.id,
)
: [];

const hasMeaningfulAuthSub = (req: Request): boolean => {
const authSub = getAuthSubFromRequest(req);
return authSub !== undefined && authSub !== '';
Expand All @@ -81,5 +124,6 @@ export {
AuthenticatedRequest,
getAuthSubFromRequest,
getRealmAccessRolesFromRequest,
getKeycloakOrganizationIdsFromRequest,
hasMeaningfulAuthSub,
};
1 change: 1 addition & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export * from './ChangemakerProposal';
export * from './CheckResult';
export * from './CopyBaseFieldsJobPayload';
export * from './DataProvider';
export * from './EphemeralUserGroupAssociation';
export * from './express/AuthenticatedRequest';
export * from './Funder';
export * from './Id';
Expand Down

0 comments on commit 918bb05

Please sign in to comment.