Skip to content

Commit

Permalink
fix(core): filter scopes for 3rd-party app
Browse files Browse the repository at this point in the history
  • Loading branch information
wangsijie committed May 12, 2024
1 parent 3d4404f commit 645d4e2
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 48 deletions.
14 changes: 11 additions & 3 deletions packages/core/src/oidc/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {}

Check warning on line 127 in packages/core/src/oidc/resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/resource.ts#L123-L127

Added lines #L123 - L127 were not covered by tests
) => {

Check warning on line 128 in packages/core/src/oidc/resource.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/core/src/oidc/resource.ts#L128

[max-params] Async arrow function has too many parameters (5). Maximum allowed is 4.
const {
applications: {
Expand Down Expand Up @@ -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)
: [];

Check warning on line 163 in packages/core/src/oidc/resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/resource.ts#L161-L163

Added lines #L161 - L163 were not covered by tests
const userConsentResource = userConsentResources.find(
({ resource }) => resource.indicator === indicator
);
const userConsentOrganizationResources = EnvSet.values.isDevFeaturesEnabled
? await getApplicationUserConsentOrganizationResourceScopes(applicationId)
? includeOrganizationResourceScopes
? await getApplicationUserConsentOrganizationResourceScopes(applicationId)
: []

Check warning on line 170 in packages/core/src/oidc/resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/resource.ts#L168-L170

Added lines #L168 - L170 were not covered by tests
: [];
const userConsentOrganizationResource = userConsentOrganizationResources.find(
({ resource }) => resource.indicator === indicator
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/routes/interaction/consent/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export default function consentRoutes<T extends IRouterParamContext>(
queries,
libraries,
userId,
applicationId,

Check warning on line 109 in packages/core/src/routes/interaction/consent/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/index.ts#L109

Added line #L109 was not covered by tests
});

const organizationsWithMissingResourceScopes = await Promise.all(
Expand All @@ -120,6 +121,7 @@ export default function consentRoutes<T extends IRouterParamContext>(
libraries,
userId,
organizationId: id,
applicationId,

Check warning on line 124 in packages/core/src/routes/interaction/consent/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/index.ts#L124

Added line #L124 was not covered by tests
});

return { name, id, missingResourceScopes };
Expand Down Expand Up @@ -249,6 +251,7 @@ export default function consentRoutes<T extends IRouterParamContext>(
queries,
libraries,
userId: accountId,
applicationId: clientId,

Check warning on line 254 in packages/core/src/routes/interaction/consent/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/index.ts#L254

Added line #L254 was not covered by tests
});

// Find the organizations if the application is requesting the organizations scope
Expand All @@ -268,6 +271,7 @@ export default function consentRoutes<T extends IRouterParamContext>(
libraries,
userId: accountId,
organizationId: id,
applicationId: clientId,

Check warning on line 274 in packages/core/src/routes/interaction/consent/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/index.ts#L274

Added line #L274 was not covered by tests
});

return { name, id, missingResourceScopes };
Expand Down
23 changes: 21 additions & 2 deletions packages/core/src/routes/interaction/consent/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -91,12 +94,14 @@ export const filterAndParseMissingResourceScopes = async ({
queries,
libraries,
userId,
applicationId,

Check warning on line 97 in packages/core/src/routes/interaction/consent/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/utils.ts#L97

Added line #L97 was not covered by tests
organizationId,
}: {
resourceScopes: Record<string, string[]>;
queries: Queries;
libraries: Libraries;
userId: string;
applicationId: string;

Check warning on line 104 in packages/core/src/routes/interaction/consent/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/utils.ts#L104

Added line #L104 was not covered by tests
organizationId?: string;
}) => {
const filteredResourceScopes = Object.fromEntries(
Expand All @@ -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,
}
);

Check warning on line 139 in packages/core/src/routes/interaction/consent/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/utils.ts#L126-L139

Added lines #L126 - L139 were not covered by tests
return [
resourceIndicator,
missingScopes.filter((scope) => scopes.some(({ name }) => name === scope)),
missingScopes.filter((scope) => filteredScopes.some(({ name }) => name === scope)),

Check warning on line 142 in packages/core/src/routes/interaction/consent/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/routes/interaction/consent/utils.ts#L142

Added line #L142 was not covered by tests
];
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 645d4e2

Please sign in to comment.