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 647c988 commit 939d9e9
Show file tree
Hide file tree
Showing 12 changed files with 182 additions and 9 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', data_provider.created_at
);
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
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(),
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,
[]
>(
'ephemeralUserGroupAssociation.insertOne',
['userKeycloakUserId', 'userGroupKeycloakOrganizationId'],
[],
);

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

const removeEphemeralUserGroupAssociationsByUserKeycloakUserId =
generateRemoveOperation<[userKeycloakUserId: KeycloakId]>(
'ephemeralUserGroupAssociations.deleteByUserKeycloakUserId',
['userKeycloakUserId'],
);

export { removeEphemeralUserGroupAssociationsByUserKeycloakUserId };
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
DELETE FROM ephemeral_user_group_associations
WHERE user_keycloak_user_id = :userKeycloakUserId
RETURNING *;
15 changes: 15 additions & 0 deletions src/database/queries/ephemeralUserGroupAssociations/insertOne.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
INSERT INTO ephemeral_user_group_associations (
user_keycloak_user_id,
user_group_keycloak_organization_id
) VALUES (
:userKeycloakUserId,
:userGroupKeycloakOrganizationId
)
ON CONFLICT (
user_keycloak_user_id, permission, user_group_keycloak_organization_id
) DO UPDATE
SET user_keycloak_user_id = user_keycloak_user_id
RETURNING
ephemeral_user_group_association_to_json(
ephemeral_user_group_associations
) AS object;
52 changes: 43 additions & 9 deletions src/middleware/addUserContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { db, createOrUpdateUser } from '../database';
import {
db,
createOrUpdateUser,
loadUserByKeycloakUserId,
createEphemeralUserGroupAssociation,
removeEphemeralUserGroupAssociationsByUserKeycloakUserId,
} from '../database';
import {
getAuthSubFromRequest,
getKeycloakOrganizationIdsFromRequest,
isKeycloakId,
keycloakIdToString,
stringToKeycloakId,
Expand All @@ -16,6 +23,7 @@ const addUserContext = (
next: NextFunction,
): void => {
const keycloakUserId = getAuthSubFromRequest(req);
const keycloakOrganizationIds = getKeycloakOrganizationIdsFromRequest(req);
const systemUser = getSystemUser();
if (
keycloakUserId === undefined ||
Expand All @@ -35,15 +43,41 @@ const addUserContext = (
return;
}

createOrUpdateUser(db, null, {
keycloakUserId: stringToKeycloakId(keycloakUserId),
})
.then((user) => {
(req as AuthenticatedRequest).user = user;
next();
Promise.all([
createOrUpdateUser(db, null, {
keycloakUserId: stringToKeycloakId(keycloakUserId),
}),
removeEphemeralUserGroupAssociationsByUserKeycloakUserId(
db,
null,
keycloakUserId,
),
])
.then(() => {
const createEphemeralUserGroupAssociationPromises =
keycloakOrganizationIds.map((keycloakOrganizationId) =>
createEphemeralUserGroupAssociation(db, null, {

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

View check run for this annotation

Codecov / codecov/patch

src/middleware/addUserContext.ts#L59

Added line #L59 was not covered by tests
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 72 in src/middleware/addUserContext.ts

View check run for this annotation

Codecov / codecov/patch

src/middleware/addUserContext.ts#L71-L72

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

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

View check run for this annotation

Codecov / codecov/patch

src/middleware/addUserContext.ts#L75-L76

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

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,

Check warning on line 114 in src/types/express/AuthenticatedRequest.ts

View check run for this annotation

Codecov / codecov/patch

src/types/express/AuthenticatedRequest.ts#L113-L114

Added lines #L113 - L114 were not covered by tests
)
: [];

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 939d9e9

Please sign in to comment.