From 645d4e2d41c76d2bd73180a4675f6dbf6a54fdd0 Mon Sep 17 00:00:00 2001 From: wangsijie Date: Sat, 11 May 2024 11:07:29 +0800 Subject: [PATCH] fix(core): filter scopes for 3rd-party app --- packages/core/src/oidc/resource.ts | 14 +- .../src/routes/interaction/consent/index.ts | 4 + .../src/routes/interaction/consent/utils.ts | 23 ++- .../third-party-sign-in/happy-path.test.ts | 145 ++++++++++++------ 4 files changed, 138 insertions(+), 48 deletions(-) diff --git a/packages/core/src/oidc/resource.ts b/packages/core/src/oidc/resource.ts index 91f3f5b5b00..37c43cf73c5 100644 --- a/packages/core/src/oidc/resource.ts +++ b/packages/core/src/oidc/resource.ts @@ -120,7 +120,11 @@ export const filterResourceScopesForTheThirdPartyApplication = async ( libraries: Libraries, applicationId: string, indicator: string, - scopes: ReadonlyArray<{ name: string; id: string }> + scopes: ReadonlyArray<{ name: string; id: string }>, + { + includeOrganizationResourceScopes = true, + includeResourceScopes = true, + }: { includeOrganizationResourceScopes?: boolean; includeResourceScopes?: boolean } = {} ) => { const { applications: { @@ -154,12 +158,16 @@ export const filterResourceScopesForTheThirdPartyApplication = async ( } // Get the API resource scopes that are enabled in the application - const userConsentResources = await getApplicationUserConsentResourceScopes(applicationId); + const userConsentResources = includeResourceScopes + ? await getApplicationUserConsentResourceScopes(applicationId) + : []; const userConsentResource = userConsentResources.find( ({ resource }) => resource.indicator === indicator ); const userConsentOrganizationResources = EnvSet.values.isDevFeaturesEnabled - ? await getApplicationUserConsentOrganizationResourceScopes(applicationId) + ? includeOrganizationResourceScopes + ? await getApplicationUserConsentOrganizationResourceScopes(applicationId) + : [] : []; const userConsentOrganizationResource = userConsentOrganizationResources.find( ({ resource }) => resource.indicator === indicator diff --git a/packages/core/src/routes/interaction/consent/index.ts b/packages/core/src/routes/interaction/consent/index.ts index d2f05f6773c..471c4b34ded 100644 --- a/packages/core/src/routes/interaction/consent/index.ts +++ b/packages/core/src/routes/interaction/consent/index.ts @@ -106,6 +106,7 @@ export default function consentRoutes( queries, libraries, userId, + applicationId, }); const organizationsWithMissingResourceScopes = await Promise.all( @@ -120,6 +121,7 @@ export default function consentRoutes( libraries, userId, organizationId: id, + applicationId, }); return { name, id, missingResourceScopes }; @@ -249,6 +251,7 @@ export default function consentRoutes( queries, libraries, userId: accountId, + applicationId: clientId, }); // Find the organizations if the application is requesting the organizations scope @@ -268,6 +271,7 @@ export default function consentRoutes( libraries, userId: accountId, organizationId: id, + applicationId: clientId, }); return { name, id, missingResourceScopes }; diff --git a/packages/core/src/routes/interaction/consent/utils.ts b/packages/core/src/routes/interaction/consent/utils.ts index eda88989721..d438bb43233 100644 --- a/packages/core/src/routes/interaction/consent/utils.ts +++ b/packages/core/src/routes/interaction/consent/utils.ts @@ -3,7 +3,10 @@ import { type MissingResourceScopes, type Scope, missingResourceScopesGuard } fr import { errors } from 'oidc-provider'; import { EnvSet } from '#src/env-set/index.js'; -import { findResourceScopes } from '#src/oidc/resource.js'; +import { + filterResourceScopesForTheThirdPartyApplication, + findResourceScopes, +} from '#src/oidc/resource.js'; import type Libraries from '#src/tenants/Libraries.js'; import type Queries from '#src/tenants/Queries.js'; import assertThat from '#src/utils/assert-that.js'; @@ -91,12 +94,14 @@ export const filterAndParseMissingResourceScopes = async ({ queries, libraries, userId, + applicationId, organizationId, }: { resourceScopes: Record; queries: Queries; libraries: Libraries; userId: string; + applicationId: string; organizationId?: string; }) => { const filteredResourceScopes = Object.fromEntries( @@ -118,9 +123,23 @@ export const filterAndParseMissingResourceScopes = async ({ organizationId, }); + // Filter the scopes for the third-party application. + // Although the "missingResourceScopes" from the prompt details are already filtered, + // there may be duplicated scopes from either resources or organization resources. + const filteredScopes = await filterResourceScopesForTheThirdPartyApplication( + libraries, + applicationId, + resourceIndicator, + scopes, + { + includeOrganizationResourceScopes: Boolean(organizationId), + includeResourceScopes: !organizationId, + } + ); + return [ resourceIndicator, - missingScopes.filter((scope) => scopes.some(({ name }) => name === scope)), + missingScopes.filter((scope) => filteredScopes.some(({ name }) => name === scope)), ]; } ) diff --git a/packages/integration-tests/src/tests/api/interaction/third-party-sign-in/happy-path.test.ts b/packages/integration-tests/src/tests/api/interaction/third-party-sign-in/happy-path.test.ts index 8c3e1bb453c..1d9ebea3121 100644 --- a/packages/integration-tests/src/tests/api/interaction/third-party-sign-in/happy-path.test.ts +++ b/packages/integration-tests/src/tests/api/interaction/third-party-sign-in/happy-path.test.ts @@ -136,61 +136,120 @@ describe('consent api', () => { await deleteUser(user.id); }); - it('get consent info with organization resource scopes', async () => { - const application = applications.get(thirdPartyApplicationName); - assert(application, new Error('application.not_found')); - - const resource = await createResource(generateResourceName(), generateResourceIndicator()); - const scope = await createScope(resource.id, generateScopeName()); - const scope2 = await createScope(resource.id, generateScopeName()); + describe('get consent info with organization resource scopes', () => { const roleApi = new OrganizationRoleApiTest(); - const role = await roleApi.create({ - name: generateRoleName(), - resourceScopeIds: [scope.id], - }); const organizationApi = new OrganizationApiTest(); - const organization = await organizationApi.create({ name: 'test_org' }); - const { userProfile, user } = await generateNewUser({ username: true, password: true }); - await organizationApi.addUsers(organization.id, [user.id]); - await organizationApi.addUserRoles(organization.id, user.id, [role.id]); - await assignUserConsentScopes(application.id, { - organizationResourceScopes: [scope.id], - userScopes: [UserScope.Organizations], + afterEach(async () => { + await roleApi.cleanUp(); + await organizationApi.cleanUp(); }); - const client = await initClient( - { - appId: application.id, - appSecret: application.secret, - scopes: [UserScope.Organizations, UserScope.Profile, scope.name, scope2.name], - resources: [resource.indicator], - }, - redirectUri - ); + it('should get scope list from orgniazation roles', async () => { + const application = applications.get(thirdPartyApplicationName); + assert(application, new Error('application.not_found')); - await client.successSend(putInteraction, { - event: InteractionEvent.SignIn, - identifier: { - username: userProfile.username, - password: userProfile.password, - }, + const resource = await createResource(generateResourceName(), generateResourceIndicator()); + const scope = await createScope(resource.id, generateScopeName()); + const scope2 = await createScope(resource.id, generateScopeName()); + const role = await roleApi.create({ + name: generateRoleName(), + resourceScopeIds: [scope.id], + }); + const organization = await organizationApi.create({ name: 'test_org' }); + const { userProfile, user } = await generateNewUser({ username: true, password: true }); + await organizationApi.addUsers(organization.id, [user.id]); + await organizationApi.addUserRoles(organization.id, user.id, [role.id]); + + await assignUserConsentScopes(application.id, { + organizationResourceScopes: [scope.id], + userScopes: [UserScope.Organizations], + }); + + const client = await initClient( + { + appId: application.id, + appSecret: application.secret, + scopes: [UserScope.Organizations, UserScope.Profile, scope.name, scope2.name], + resources: [resource.indicator], + }, + redirectUri + ); + + await client.successSend(putInteraction, { + event: InteractionEvent.SignIn, + identifier: { + username: userProfile.username, + password: userProfile.password, + }, + }); + + const { redirectTo } = await client.submitInteraction(); + + await client.processSession(redirectTo, false); + + const result = await client.send(getConsentInfo); + + expect(result.missingResourceScopes).toHaveLength(0); + // Only scope1, scope2 is removed + expect(result.organizations?.[0]?.missingResourceScopes).toHaveLength(1); + + await deleteResource(resource.id); + await deleteUser(user.id); }); - const { redirectTo } = await client.submitInteraction(); + it('should handle duplicated scopes which are assigned to either personal or organization', async () => { + const application = applications.get(thirdPartyApplicationName); + assert(application, new Error('application.not_found')); - await client.processSession(redirectTo, false); + const resource = await createResource(generateResourceName(), generateResourceIndicator()); + const scope = await createScope(resource.id, generateScopeName()); + const role = await roleApi.create({ + name: generateRoleName(), + resourceScopeIds: [scope.id], + }); + const organization = await organizationApi.create({ name: 'test_org' }); + const { userProfile, user } = await generateNewUser({ username: true, password: true }); + await organizationApi.addUsers(organization.id, [user.id]); + await organizationApi.addUserRoles(organization.id, user.id, [role.id]); - const result = await client.send(getConsentInfo); + // Assign the scope to resourceScopes but not to organizationResourceScopes + await assignUserConsentScopes(application.id, { + resourceScopes: [scope.id], + userScopes: [UserScope.Organizations], + }); - expect(result.missingResourceScopes).toHaveLength(0); - // Only scope1, scope2 is removed - expect(result.organizations?.[0]?.missingResourceScopes).toHaveLength(1); + const client = await initClient( + { + appId: application.id, + appSecret: application.secret, + scopes: [UserScope.Organizations, UserScope.Profile, scope.name], + resources: [resource.indicator], + }, + redirectUri + ); - await roleApi.cleanUp(); - await organizationApi.cleanUp(); - await deleteResource(resource.id); - await deleteUser(user.id); + await client.successSend(putInteraction, { + event: InteractionEvent.SignIn, + identifier: { + username: userProfile.username, + password: userProfile.password, + }, + }); + + const { redirectTo } = await client.submitInteraction(); + + await client.processSession(redirectTo, false); + + const result = await client.send(getConsentInfo); + + expect(result.missingResourceScopes).toHaveLength(0); + // No missing resource scopes, because the scope is only assigned to resourceScopes + expect(result.organizations?.[0]?.missingResourceScopes).toHaveLength(0); + + await deleteResource(resource.id); + await deleteUser(user.id); + }); }); describe('submit consent info', () => {