Skip to content

Commit 6698679

Browse files
authored
fix(core): fix enterprise SSO unknown field not synced issue (#6763)
* fix(core): fix enterprise SSO unknown field not synced issue fix enterprise SSO unknown field not synced issue * fix(core): cleanup undefined in unknown json cleanup undefined in unknown json * fix(core): should sync the sso identity detail on sign-in (#6764) should sync the sso identity detail on sign-in
1 parent 0c777c1 commit 6698679

File tree

9 files changed

+112
-20
lines changed

9 files changed

+112
-20
lines changed

packages/core/src/queries/user-sso-identities.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { sql, type CommonQueryMethods } from '@silverhand/slonik';
1010
import SchemaQueries from '#src/utils/SchemaQueries.js';
1111
import { manyRows } from '#src/utils/sql.js';
1212

13+
import { buildUpdateWhereWithPool } from '../database/update-where.js';
14+
1315
export default class UserSsoIdentityQueries extends SchemaQueries<
1416
UserSsoIdentityKeys,
1517
CreateUserSsoIdentity,
@@ -40,4 +42,16 @@ export default class UserSsoIdentityQueries extends SchemaQueries<
4042
`)
4143
);
4244
}
45+
46+
async updateUserSsoIdentityDetailByIdentityId(
47+
issuer: string,
48+
identityId: string,
49+
detail: UserSsoIdentity['detail']
50+
) {
51+
return buildUpdateWhereWithPool(this.pool)(this.schema, true)({
52+
set: { detail },
53+
where: { issuer, identityId },
54+
jsonbMode: 'replace',
55+
});
56+
}
4357
}

packages/core/src/routes/experience/classes/experience-interaction.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,10 @@ export default class ExperienceInteraction {
376376
* @throws {RequestError} with 422 if the profile data is not unique across users
377377
* @throws {RequestError} with 422 if the required profile fields are missing
378378
**/
379+
// eslint-disable-next-line complexity
379380
public async submit() {
380381
const {
381-
queries: { users: userQueries },
382+
queries: { users: userQueries, userSsoIdentities: userSsoIdentityQueries },
382383
} = this.tenant;
383384

384385
// Identified
@@ -425,7 +426,8 @@ export default class ExperienceInteraction {
425426
await this.mfa.assertUserMandatoryMfaFulfilled();
426427
}
427428

428-
const { socialIdentity, enterpriseSsoIdentity, ...rest } = this.profile.data;
429+
const { socialIdentity, enterpriseSsoIdentity, syncedEnterpriseSsoIdentity, ...rest } =
430+
this.profile.data;
429431
const { mfaSkipped, mfaVerifications } = this.mfa.toUserMfaVerifications();
430432

431433
// Update user profile
@@ -457,6 +459,16 @@ export default class ExperienceInteraction {
457459
lastSignInAt: Date.now(),
458460
});
459461

462+
// Sync SSO identity
463+
if (syncedEnterpriseSsoIdentity) {
464+
const { identityId, issuer, detail } = syncedEnterpriseSsoIdentity;
465+
await userSsoIdentityQueries.updateUserSsoIdentityDetailByIdentityId(
466+
issuer,
467+
identityId,
468+
detail
469+
);
470+
}
471+
460472
if (enterpriseSsoIdentity) {
461473
await this.provisionLibrary.addSsoIdentityToUser(user.id, enterpriseSsoIdentity);
462474
}

packages/core/src/routes/experience/classes/helpers.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export const identifyUserByVerificationRecord = async (
6868
*/
6969
syncedProfile?: Pick<
7070
InteractionProfile,
71-
'enterpriseSsoIdentity' | 'socialIdentity' | 'avatar' | 'name'
71+
'enterpriseSsoIdentity' | 'syncedEnterpriseSsoIdentity' | 'socialIdentity' | 'avatar' | 'name'
7272
>;
7373
}> => {
7474
// Check verification record can be used to identify a user using the `identifyUser` method.
@@ -99,7 +99,12 @@ export const identifyUserByVerificationRecord = async (
9999
case VerificationType.EnterpriseSso: {
100100
try {
101101
const user = await verificationRecord.identifyUser();
102-
const syncedProfile = await verificationRecord.toSyncedProfile();
102+
const { enterpriseSsoIdentity } = verificationRecord.toUserProfile();
103+
// Sync the enterprise SSO identity details
104+
const syncedProfile = {
105+
syncedEnterpriseSsoIdentity: enterpriseSsoIdentity,
106+
...(await verificationRecord.toSyncedProfile()),
107+
};
103108
return { user, syncedProfile };
104109
} catch (error: unknown) {
105110
// Auto fallback to identify the related user if the user does not exist for enterprise SSO.

packages/core/src/routes/experience/classes/libraries/provision-library.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class ProvisionLibrary {
5858
queries: { userSsoIdentities: userSsoIdentitiesQueries },
5959
} = this.tenantContext;
6060

61-
const { socialIdentity, enterpriseSsoIdentity, ...rest } = profile;
61+
const { socialIdentity, enterpriseSsoIdentity, syncedEnterpriseSsoIdentity, ...rest } = profile;
6262

6363
const { isCreatingFirstAdminUser, initialUserRoles, customData } =
6464
await this.getUserProvisionContext(profile);

packages/core/src/routes/experience/classes/verifications/enterprise-sso-verification.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { socialUserInfoGuard, type SocialUserInfo, type ToZodObject } from '@logto/connector-kit';
1+
import { type ToZodObject } from '@logto/connector-kit';
22
import {
33
VerificationType,
44
type JsonObject,
@@ -17,10 +17,12 @@ import {
1717
getSsoAuthorizationUrl,
1818
verifySsoIdentity,
1919
} from '#src/routes/interaction/utils/single-sign-on.js';
20+
import { extendedSocialUserInfoGuard, type ExtendedSocialUserInfo } from '#src/sso/types/saml.js';
2021
import type Libraries from '#src/tenants/Libraries.js';
2122
import type Queries from '#src/tenants/Queries.js';
2223
import type TenantContext from '#src/tenants/TenantContext.js';
2324
import assertThat from '#src/utils/assert-that.js';
25+
import { safeParseUnknownJson } from '#src/utils/json.js';
2426

2527
import type { InteractionProfile } from '../../types.js';
2628

@@ -34,15 +36,15 @@ export type EnterpriseSsoVerificationRecordData = {
3436
/**
3537
* The enterprise SSO identity returned by the connector.
3638
*/
37-
enterpriseSsoUserInfo?: SocialUserInfo;
39+
enterpriseSsoUserInfo?: ExtendedSocialUserInfo;
3840
issuer?: string;
3941
};
4042

4143
export const enterPriseSsoVerificationRecordDataGuard = z.object({
4244
id: z.string(),
4345
connectorId: z.string(),
4446
type: z.literal(VerificationType.EnterpriseSso),
45-
enterpriseSsoUserInfo: socialUserInfoGuard.optional(),
47+
enterpriseSsoUserInfo: extendedSocialUserInfoGuard.optional(),
4648
issuer: z.string().optional(),
4749
}) satisfies ToZodObject<EnterpriseSsoVerificationRecordData>;
4850

@@ -60,7 +62,7 @@ export class EnterpriseSsoVerification
6062
public readonly id: string;
6163
public readonly type = VerificationType.EnterpriseSso;
6264
public readonly connectorId: string;
63-
public enterpriseSsoUserInfo?: SocialUserInfo;
65+
public enterpriseSsoUserInfo?: ExtendedSocialUserInfo;
6466
public issuer?: string;
6567

6668
private connectorDataCache?: SupportedSsoConnector;
@@ -135,17 +137,14 @@ export class EnterpriseSsoVerification
135137
}
136138

137139
/**
138-
* Identify the user by the enterprise SSO identity.
139-
* If the user is not found, find the related user by the enterprise SSO identity and return the related user.
140+
* Identify the user by the enterprise SSO identity and sync the user SSO identity.
140141
*/
141142
async identifyUser(): Promise<User> {
142143
assertThat(
143144
this.isVerified,
144145
new RequestError({ code: 'session.verification_failed', status: 400 })
145146
);
146147

147-
// TODO: sync userInfo and link sso identity
148-
149148
const userSsoIdentityResult = await this.findUserSsoIdentityByEnterpriseSsoUserInfo();
150149

151150
if (userSsoIdentityResult) {
@@ -171,7 +170,7 @@ export class EnterpriseSsoVerification
171170
}
172171

173172
/**
174-
* Returns the use SSO identity as a new user profile.
173+
* Returns the user SSO identity as a new user profile.
175174
*/
176175
toUserProfile(): Required<Pick<InteractionProfile, 'enterpriseSsoIdentity'>> {
177176
assertThat(
@@ -184,7 +183,7 @@ export class EnterpriseSsoVerification
184183
issuer: this.issuer,
185184
ssoConnectorId: this.connectorId,
186185
identityId: this.enterpriseSsoUserInfo.id,
187-
detail: this.enterpriseSsoUserInfo,
186+
detail: safeParseUnknownJson(this.enterpriseSsoUserInfo),
188187
},
189188
};
190189
}

packages/core/src/routes/experience/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export type InteractionProfile = {
2828
UserSsoIdentity,
2929
'identityId' | 'ssoConnectorId' | 'issuer' | 'detail'
3030
>;
31+
// Syncing the existing enterprise SSO identity detail
32+
syncedEnterpriseSsoIdentity?: Pick<UserSsoIdentity, 'identityId' | 'issuer' | 'detail'>;
3133
} & Pick<
3234
CreateUser,
3335
| 'avatar'
@@ -64,6 +66,13 @@ export const interactionProfileGuard = Users.createGuard
6466
detail: true,
6567
})
6668
.optional(),
69+
syncedEnterpriseSsoIdentity: UserSsoIdentities.guard
70+
.pick({
71+
identityId: true,
72+
issuer: true,
73+
detail: true,
74+
})
75+
.optional(),
6776
}) satisfies ToZodObject<InteractionProfile>;
6877

6978
/**

packages/core/src/routes/interaction/utils/single-sign-on.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable max-lines -- will migrate this file to the latest experience APIs */
2-
import { ConnectorError, type SocialUserInfo } from '@logto/connector-kit';
2+
import { ConnectorError } from '@logto/connector-kit';
33
import { validateRedirectUrl } from '@logto/core-kit';
44
import {
55
InteractionEvent,
@@ -17,9 +17,11 @@ import RequestError from '#src/errors/RequestError/index.js';
1717
import { type WithLogContext } from '#src/middleware/koa-audit-log.js';
1818
import SamlConnector from '#src/sso/SamlConnector/index.js';
1919
import { ssoConnectorFactories, type SingleSignOnConnectorSession } from '#src/sso/index.js';
20+
import { type ExtendedSocialUserInfo } from '#src/sso/types/saml.js';
2021
import type Queries from '#src/tenants/Queries.js';
2122
import type TenantContext from '#src/tenants/TenantContext.js';
2223
import assertThat from '#src/utils/assert-that.js';
24+
import { safeParseUnknownJson } from '#src/utils/json.js';
2325

2426
import { type WithInteractionHooksContext } from '../middleware/koa-interaction-hooks.js';
2527

@@ -128,7 +130,7 @@ export const getSsoAuthorizationUrl = async (
128130
type SsoAuthenticationResult = {
129131
/** The issuer of the SSO provider, we need to store this in the user SSO identity to identify the provider. */
130132
issuer: string;
131-
userInfo: SocialUserInfo;
133+
userInfo: ExtendedSocialUserInfo;
132134
};
133135

134136
/**
@@ -268,7 +270,7 @@ const signInWithSsoAuthentication = async (
268270

269271
// Update the user's SSO identity details
270272
await userSsoIdentitiesQueries.updateById(id, {
271-
detail: userInfo,
273+
detail: safeParseUnknownJson(userInfo),
272274
});
273275

274276
const { name, avatar, id: identityId } = userInfo;
@@ -331,7 +333,7 @@ const signInAndLinkWithSsoAuthentication = async (
331333
userId,
332334
identityId,
333335
issuer,
334-
detail: userInfo,
336+
detail: safeParseUnknownJson(userInfo),
335337
});
336338

337339
// Sync the user name and avatar to the existing user if the connector has syncProfile enabled (sign-in)
@@ -413,7 +415,7 @@ export const registerWithSsoAuthentication = async (
413415
ssoConnectorId: connectorId,
414416
identityId: userInfo.id,
415417
issuer,
416-
detail: userInfo,
418+
detail: safeParseUnknownJson(userInfo),
417419
});
418420

419421
// JIT provision for new users signing up with SSO

packages/core/src/utils/json.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { safeParseUnknownJson } from './json.js';
2+
3+
describe('json utils test', () => {
4+
it('should parse unknown json object properly', () => {
5+
const unknownJson: Record<string, unknown> = {
6+
text: 'hello',
7+
null: null,
8+
number: 123,
9+
boolean: true,
10+
empty: undefined,
11+
array: [1, 2, 3],
12+
object: {
13+
key: 'value',
14+
number: 123,
15+
array: [1, 2, 3],
16+
empty: undefined,
17+
},
18+
};
19+
20+
const result = safeParseUnknownJson(unknownJson);
21+
22+
expect(result).toEqual({
23+
text: 'hello',
24+
number: 123,
25+
boolean: true,
26+
null: null,
27+
array: [1, 2, 3],
28+
object: {
29+
key: 'value',
30+
number: 123,
31+
array: [1, 2, 3],
32+
},
33+
});
34+
});
35+
});

packages/core/src/utils/json.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
1+
import { type JsonObject, jsonObjectGuard } from '@logto/schemas';
12
import { trySafe } from '@silverhand/essentials';
3+
import cleanDeep from 'clean-deep';
24

35
export const safeParseJson = (jsonString: string): unknown =>
46
// eslint-disable-next-line no-restricted-syntax
57
trySafe(() => JSON.parse(jsonString) as unknown);
8+
9+
// Safely parse Zod unknown JSON object to JsonObject
10+
export const safeParseUnknownJson = (unknownJson: Record<string, unknown>): JsonObject =>
11+
trySafe(
12+
() =>
13+
jsonObjectGuard.safeParse(
14+
cleanDeep(unknownJson, {
15+
emptyArrays: false,
16+
emptyObjects: false,
17+
emptyStrings: false,
18+
nullValues: false,
19+
})
20+
).data
21+
) ?? {};

0 commit comments

Comments
 (0)