From 6feb5314352b72e6237eada7cc0a1bb78f793aa4 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Wed, 20 Mar 2024 13:16:23 +0800 Subject: [PATCH] feat: add profile api, update api and database, add tests --- packages/core/src/oidc/scope.ts | 9 +++++ packages/core/src/routes/admin-user/basics.ts | 32 +++++++++++++++++ .../integration-tests/src/api/admin-user.ts | 7 ++++ .../integration-tests/src/helpers/index.ts | 7 +++- .../src/tests/api/admin-user.test.ts | 34 +++++++++++++++++-- ...9622-add-oidc-standard-claim-properties.ts | 16 +++++++++ packages/schemas/tables/_functions.sql | 8 +++++ packages/schemas/tables/users.sql | 5 +++ 8 files changed, 115 insertions(+), 3 deletions(-) diff --git a/packages/core/src/oidc/scope.ts b/packages/core/src/oidc/scope.ts index 0532c4035..105a75b97 100644 --- a/packages/core/src/oidc/scope.ts +++ b/packages/core/src/oidc/scope.ts @@ -115,6 +115,15 @@ export const getUserClaimsData = async ( } default: { if (isUserProfileClaim(claim)) { + // Unlike other database fields (e.g. `name`), the claims stored in the `profile` field + // will fall back to `undefined` rather than `null`. We refrain from using `?? null` + // here to reduce the size of ID tokens, since `undefined` fields will be stripped in + // tokens. + // + // The only consideration here is the inconsistency for `name` and `picture`, which are + // also standard claims but they will fall back to `null`. While it's possible to align + // their behavior by having their fallback to `undefined`, it's better to maintain the + // current setup for now to prevent breaking changes. return [claim, user.profile[claimToUserProfileKey[claim]]]; } diff --git a/packages/core/src/routes/admin-user/basics.ts b/packages/core/src/routes/admin-user/basics.ts index e6b83166f..9ab6624e7 100644 --- a/packages/core/src/routes/admin-user/basics.ts +++ b/packages/core/src/routes/admin-user/basics.ts @@ -3,6 +3,7 @@ import { UsersPasswordEncryptionMethod, jsonObjectGuard, userInfoSelectFields, + userProfileGuard, userProfileResponseGuard, } from '@logto/schemas'; import { conditional, pick, yes } from '@silverhand/essentials'; @@ -108,6 +109,32 @@ export default function adminUserBasicsRoutes(...args: R } ); + router.patch( + '/users/:userId/profile', + koaGuard({ + params: object({ userId: string() }), + body: object({ profile: userProfileGuard }), + response: userProfileGuard, + status: [200, 404], + }), + async (ctx, next) => { + const { + params: { userId }, + body: { profile }, + } = ctx.guard; + + await findUserById(userId); + + const user = await updateUserById(userId, { + profile, + }); + + ctx.body = user.profile; + + return next(); + } + ); + router.post( '/users', koaGuard({ @@ -121,6 +148,7 @@ export default function adminUserBasicsRoutes(...args: R name: string(), avatar: string().url().or(literal('')).nullable(), customData: jsonObjectGuard, + profile: userProfileGuard, }).partial(), response: userProfileResponseGuard, status: [200, 404, 422], @@ -136,6 +164,7 @@ export default function adminUserBasicsRoutes(...args: R passwordAlgorithm, avatar, customData, + profile, } = ctx.guard.body; assertThat(!(password && passwordDigest), new RequestError('user.password_and_digest')); @@ -178,6 +207,7 @@ export default function adminUserBasicsRoutes(...args: R passwordEncryptionMethod: passwordAlgorithm, } ), + ...conditional(profile && { profile }), }, [] ); @@ -199,6 +229,7 @@ export default function adminUserBasicsRoutes(...args: R name: string().or(literal('')).nullable(), avatar: string().url().or(literal('')).nullable(), customData: jsonObjectGuard, + profile: userProfileGuard, }).partial(), response: userProfileResponseGuard, status: [200, 404, 422], @@ -317,6 +348,7 @@ export default function adminUserBasicsRoutes(...args: R return next(); } + // eslint-disable-next-line max-lines ); router.delete( diff --git a/packages/integration-tests/src/api/admin-user.ts b/packages/integration-tests/src/api/admin-user.ts index a052d53c8..f9dd9d371 100644 --- a/packages/integration-tests/src/api/admin-user.ts +++ b/packages/integration-tests/src/api/admin-user.ts @@ -48,6 +48,13 @@ export const updateUser = async (userId: string, payload: Partial) => }) .json(); +export const updateUserProfile = async (userId: string, profile: Partial) => + authedAdminApi + .patch(`users/${userId}/profile`, { + json: { profile }, + }) + .json(); + export const suspendUser = async (userId: string, isSuspended: boolean) => authedAdminApi.patch(`users/${userId}/is-suspended`, { json: { isSuspended } }).json(); diff --git a/packages/integration-tests/src/helpers/index.ts b/packages/integration-tests/src/helpers/index.ts index 13fb84e24..d31c5ddf6 100644 --- a/packages/integration-tests/src/helpers/index.ts +++ b/packages/integration-tests/src/helpers/index.ts @@ -2,7 +2,11 @@ import fs from 'node:fs/promises'; import { createServer, type RequestListener } from 'node:http'; import { mockConnectorFilePaths, type SendMessagePayload } from '@logto/connector-kit'; -import { type JsonObject, type UsersPasswordEncryptionMethod } from '@logto/schemas'; +import { + type UserProfile, + type JsonObject, + type UsersPasswordEncryptionMethod, +} from '@logto/schemas'; import { RequestError } from 'got'; import { createUser } from '#src/api/index.js'; @@ -18,6 +22,7 @@ export const createUserByAdmin = async ( passwordDigest?: string; passwordAlgorithm?: UsersPasswordEncryptionMethod; customData?: JsonObject; + profile?: UserProfile; } = {} ) => { const { username, name, ...rest } = payload; diff --git a/packages/integration-tests/src/tests/api/admin-user.test.ts b/packages/integration-tests/src/tests/api/admin-user.test.ts index 72fd002f7..d11369425 100644 --- a/packages/integration-tests/src/tests/api/admin-user.test.ts +++ b/packages/integration-tests/src/tests/api/admin-user.test.ts @@ -18,6 +18,7 @@ import { postUserIdentity, verifyUserPassword, putUserIdentity, + updateUserProfile, } from '#src/api/index.js'; import { createUserByAdmin, expectRejects } from '#src/helpers/index.js'; import { createNewSocialUserWithUsernameAndPassword } from '#src/helpers/interactions.js'; @@ -46,12 +47,14 @@ describe('admin console user management', () => { await expect(verifyUserPassword(user.id, 'password')).resolves.not.toThrow(); }); - it('should create user with custom data successfully', async () => { + it('should create user with custom data and profile successfully', async () => { const user = await createUserByAdmin({ customData: { foo: 'bar' }, + profile: { gender: 'neutral' }, }); - const { customData } = await getUser(user.id); + const { customData, profile } = await getUser(user.id); expect(customData).toStrictEqual({ foo: 'bar' }); + expect(profile).toStrictEqual({ gender: 'neutral' }); }); it('should fail when create user with conflict identifiers', async () => { @@ -95,11 +98,38 @@ describe('admin console user management', () => { customData: { level: 1, }, + profile: { + familyName: 'new family name', + address: { + formatted: 'new formatted address', + }, + }, }; const updatedUser = await updateUser(user.id, newUserData); expect(updatedUser).toMatchObject(newUserData); + expect(updatedUser.updatedAt).toBeGreaterThan(user.updatedAt); + }); + + it('should able to update profile partially', async () => { + const user = await createUserByAdmin(); + const profile = { + familyName: 'new family name', + address: { + formatted: 'new formatted address', + }, + }; + + const updatedProfile = await updateUserProfile(user.id, profile); + expect(updatedProfile).toStrictEqual(profile); + + const patchProfile = { + familyName: 'another name', + website: 'https://logto.io/', + }; + const updatedProfile2 = await updateUserProfile(user.id, patchProfile); + expect(updatedProfile2).toStrictEqual({ ...profile, ...patchProfile }); }); it('should respond 422 when no update data provided', async () => { diff --git a/packages/schemas/alterations/next-1710859622-add-oidc-standard-claim-properties.ts b/packages/schemas/alterations/next-1710859622-add-oidc-standard-claim-properties.ts index bee3f622d..1354c7c85 100644 --- a/packages/schemas/alterations/next-1710859622-add-oidc-standard-claim-properties.ts +++ b/packages/schemas/alterations/next-1710859622-add-oidc-standard-claim-properties.ts @@ -9,6 +9,18 @@ const alteration: AlterationScript = { add column profile jsonb not null default '{}'::jsonb, add column updated_at timestamptz not null default (now()); `); + await pool.query(sql` + create function set_updated_at() returns trigger as + $$ begin + new.updated_at = now(); + return new; + end; $$ language plpgsql; + + create trigger set_updated_at + before update on users + for each row + execute procedure set_updated_at(); + `); }, down: async (pool) => { await pool.query(sql` @@ -16,6 +28,10 @@ const alteration: AlterationScript = { drop column profile, drop column updated_at; `); + await pool.query(sql` + drop trigger set_updated_at on users; + drop function set_updated_at(); + `); }, }; diff --git a/packages/schemas/tables/_functions.sql b/packages/schemas/tables/_functions.sql index ccb7a9ef9..b65236866 100644 --- a/packages/schemas/tables/_functions.sql +++ b/packages/schemas/tables/_functions.sql @@ -1,5 +1,6 @@ /* init_order = 0.5 */ +/** A function to set the tenant_id column based on the current user. */ create function set_tenant_id() returns trigger as $$ begin if new.tenant_id is not null then @@ -13,4 +14,11 @@ $$ begin return new; end; $$ language plpgsql; +/** A function to set the created_at column to the current time. */ +create function set_updated_at() returns trigger as +$$ begin + new.updated_at = now(); + return new; +end; $$ language plpgsql; + /* no_after_each */ diff --git a/packages/schemas/tables/users.sql b/packages/schemas/tables/users.sql index 3981f1b45..66294bba2 100644 --- a/packages/schemas/tables/users.sql +++ b/packages/schemas/tables/users.sql @@ -39,3 +39,8 @@ create index users__id create index users__name on users (tenant_id, name); + +create trigger set_updated_at + before update on users + for each row + execute procedure set_updated_at();