Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): filter scopes for 3rd-party app #5845

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,8 +120,12 @@
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: {
getApplicationUserConsentOrganizationScopes,
Expand All @@ -144,7 +148,7 @@
)
);
}
// FIXME: @simeng double check if it's necessary

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[no-warning-comments] Unexpected 'fixme' comment: 'FIXME: @simeng double check if it's...'.
// Return all the scopes for the reserved resources
// eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check
default: {
Expand All @@ -154,12 +158,16 @@
}

// 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 @@
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 @@
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 @@
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 @@
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 { 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 @@
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 @@
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,
wangsijie marked this conversation as resolved.
Show resolved Hide resolved
}
);

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