diff --git a/.changeset/seven-socks-perform.md b/.changeset/seven-socks-perform.md new file mode 100644 index 000000000..957b5ca84 --- /dev/null +++ b/.changeset/seven-socks-perform.md @@ -0,0 +1,25 @@ +--- +"@logto/schemas": patch +--- + +## Resolve third-party app's /interaction/consent endpoint 500 error + +### Reproduction steps + +- Create an organization scope with an empty description and assign this scope to a third-party application. + +- Login to the third-party application and request the organization scope. + +- Proceed through the interaction flow until reaching the consent page. + +- An internal server error 500 is returned. + +### Root cause + +For the `/interaction/consent` endpoint, the organization scope is returned alongside other resource scopes in the `missingResourceScopes` property. + +In the `consentInfoResponseGuard`, we utilize the resource Scopes zod guard to validate the `missingResourceScopes` property. However, the description field in the resource scope is mandatory while organization scopes'description is optional. An organization scope with an empty description will not pass the validation. + +### Solution + +Alter the resource scopes table to make the description field nullable. Related Scope zod guard and the consentInfoResponseGuard will be updated to reflect this change. Align the resource scopes table with the organization scopes table to ensure consistency. diff --git a/packages/console/src/pages/ApiResourceDetails/ApiResourcePermissions/components/CreatePermissionModal/index.tsx b/packages/console/src/pages/ApiResourceDetails/ApiResourcePermissions/components/CreatePermissionModal/index.tsx index 4efd0b508..39326b11b 100644 --- a/packages/console/src/pages/ApiResourceDetails/ApiResourcePermissions/components/CreatePermissionModal/index.tsx +++ b/packages/console/src/pages/ApiResourceDetails/ApiResourcePermissions/components/CreatePermissionModal/index.tsx @@ -1,5 +1,5 @@ import { noSpaceRegEx } from '@logto/core-kit'; -import type { Scope } from '@logto/schemas'; +import type { Scope, CreateScope } from '@logto/schemas'; import { useContext } from 'react'; import { useForm } from 'react-hook-form'; import { Trans, useTranslation } from 'react-i18next'; @@ -24,7 +24,7 @@ type Props = { onClose: (scope?: Scope) => void; }; -type CreatePermissionFormData = Pick; +type CreatePermissionFormData = Pick; function CreatePermissionModal({ resourceId, totalResourceCount, onClose }: Props) { const { currentPlan } = useContext(SubscriptionDataContext); @@ -118,10 +118,10 @@ function CreatePermissionModal({ resourceId, totalResourceCount, onClose }: Prop error={errors.name?.message} /> - + diff --git a/packages/experience/src/pages/Consent/ScopesListCard/index.tsx b/packages/experience/src/pages/Consent/ScopesListCard/index.tsx index 15c20614f..e5bcbd606 100644 --- a/packages/experience/src/pages/Consent/ScopesListCard/index.tsx +++ b/packages/experience/src/pages/Consent/ScopesListCard/index.tsx @@ -1,5 +1,6 @@ import { ReservedResource, UserScope } from '@logto/core-kit'; import { type ConsentInfoResponse } from '@logto/schemas'; +import { type Nullable } from '@silverhand/essentials'; import classNames from 'classnames'; import { useCallback, useMemo, useState } from 'react'; import { Trans, useTranslation } from 'react-i18next'; @@ -20,7 +21,7 @@ type ScopeGroupProps = { scopes: Array<{ id: string; name: string; - description?: string; + description?: Nullable; // Organization scope description cloud be `null` }>; }; diff --git a/packages/integration-tests/src/api/interaction.ts b/packages/integration-tests/src/api/interaction.ts index a355b5e67..555067d96 100644 --- a/packages/integration-tests/src/api/interaction.ts +++ b/packages/integration-tests/src/api/interaction.ts @@ -5,6 +5,7 @@ import type { RequestVerificationCodePayload, BindMfaPayload, VerifyMfaPayload, + ConsentInfoResponse, } from '@logto/schemas'; import type { Got } from 'got'; @@ -154,6 +155,14 @@ export const consent = async (api: Got, cookie: string) => }) .json(); +export const getConsentInfo = async (cookie: string) => + api + .get('interaction/consent', { + headers: { cookie }, + followRedirect: false, + }) + .json(); + export const createSingleSignOnAuthorizationUri = async ( cookie: string, payload: SocialAuthorizationUriPayload diff --git a/packages/integration-tests/src/client/index.ts b/packages/integration-tests/src/client/index.ts index 4d71a2c47..802650759 100644 --- a/packages/integration-tests/src/client/index.ts +++ b/packages/integration-tests/src/client/index.ts @@ -84,7 +84,12 @@ export default class MockClient { assert(this.interactionCookie, new Error('Get cookie from authorization endpoint failed')); } - public async processSession(redirectTo: string) { + /** + * + * @param {string} redirectTo the sign-in or register redirect uri + * @param {boolean} [consent=true] whether to automatically consent. Need to manually handle the consent flow if set to false + */ + public async processSession(redirectTo: string, consent = true) { // Note: should redirect to OIDC auth endpoint assert( redirectTo.startsWith(`${this.config.endpoint}/oidc/auth`), @@ -106,6 +111,11 @@ export default class MockClient { this.rawCookies = authResponse.headers['set-cookie'] ?? []; + // Manually handle the consent flow + if (!consent) { + return; + } + const signInCallbackUri = await this.consent(); await this.logto.handleSignInCallback(signInCallbackUri); } 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 new file mode 100644 index 000000000..44d998e34 --- /dev/null +++ b/packages/integration-tests/src/tests/api/interaction/third-party-sign-in/happy-path.test.ts @@ -0,0 +1,134 @@ +import { ReservedResource, UserScope } from '@logto/core-kit'; +import { type Application, InteractionEvent, ApplicationType } from '@logto/schemas'; +import { assert } from '@silverhand/essentials'; + +import { deleteUser } from '#src/api/admin-user.js'; +import { assignUserConsentScopes } from '#src/api/application-user-consent-scope.js'; +import { createApplication, deleteApplication } from '#src/api/application.js'; +import { getConsentInfo, putInteraction } from '#src/api/interaction.js'; +import { OrganizationScopeApi } from '#src/api/organization-scope.js'; +import { initClient } from '#src/helpers/client.js'; +import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js'; +import { generateNewUser } from '#src/helpers/user.js'; + +describe('consent api', () => { + const applications = new Map(); + const thirdPartyApplicationName = 'consent-third-party-app'; + const redirectUri = 'http://example.com'; + + const bootStrapApplication = async () => { + const thirdPartyApplication = await createApplication( + thirdPartyApplicationName, + ApplicationType.Traditional, + { + isThirdParty: true, + oidcClientMetadata: { + redirectUris: [redirectUri], + postLogoutRedirectUris: [], + }, + } + ); + + applications.set(thirdPartyApplicationName, thirdPartyApplication); + + await assignUserConsentScopes(thirdPartyApplication.id, { + userScopes: [UserScope.Profile], + }); + }; + + beforeAll(async () => { + await Promise.all([enableAllPasswordSignInMethods(), bootStrapApplication()]); + }); + + it('get consent info', async () => { + const application = applications.get(thirdPartyApplicationName); + assert(application, new Error('application.not_found')); + + const { userProfile, user } = await generateNewUser({ username: true, password: true }); + + const client = await initClient( + { + appId: application.id, + appSecret: application.secret, + }, + 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.application.id).toBe(application.id); + expect(result.user.id).toBe(user.id); + expect(result.redirectUri).toBe(redirectUri); + expect(result.missingOIDCScope).toEqual([UserScope.Profile]); + + await deleteUser(user.id); + }); + + it('get consent info with organization scope', async () => { + const application = applications.get(thirdPartyApplicationName); + assert(application, new Error('application.not_found')); + + const organizationScopeApi = new OrganizationScopeApi(); + + const organizationScope = await organizationScopeApi.create({ + name: 'organization-scope', + }); + + await assignUserConsentScopes(application.id, { + organizationScopes: [organizationScope.id], + userScopes: [UserScope.Organizations], + }); + + const { userProfile, user } = await generateNewUser({ username: true, password: true }); + + const client = await initClient( + { + appId: application.id, + appSecret: application.secret, + scopes: [UserScope.Organizations, UserScope.Profile, organizationScope.name], + }, + 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?.find( + ({ resource }) => resource.name === ReservedResource.Organization + ) + ).not.toBeUndefined(); + + await organizationScopeApi.delete(organizationScope.id); + await deleteUser(user.id); + }); + + afterAll(async () => { + for (const application of applications.values()) { + void deleteApplication(application.id); + } + }); +}); diff --git a/packages/integration-tests/src/tests/api/resource.scope.test.ts b/packages/integration-tests/src/tests/api/resource.scope.test.ts index 2e65ddbdf..f098f9907 100644 --- a/packages/integration-tests/src/tests/api/resource.scope.test.ts +++ b/packages/integration-tests/src/tests/api/resource.scope.test.ts @@ -5,7 +5,7 @@ import { HTTPError } from 'got'; import { createResource } from '#src/api/index.js'; import { createScope, deleteScope, getScopes, updateScope } from '#src/api/scope.js'; -import { generateScopeName } from '#src/utils.js'; +import { generateName, generateScopeName } from '#src/utils.js'; describe('scopes', () => { it('should get management api resource scopes successfully', async () => { @@ -63,7 +63,7 @@ describe('scopes', () => { expect(scope).toBeTruthy(); const newScopeName = `new_${scope.name}`; - const newScopeDescription = `new_${scope.description}`; + const newScopeDescription = scope.description ? `new_${scope.description}` : generateName(); const updatesScope = await updateScope(resource.id, scope.id, { name: newScopeName, diff --git a/packages/schemas/alterations/next-1710408335-make-resource-scopes-description-nullable.ts b/packages/schemas/alterations/next-1710408335-make-resource-scopes-description-nullable.ts new file mode 100644 index 000000000..23c932556 --- /dev/null +++ b/packages/schemas/alterations/next-1710408335-make-resource-scopes-description-nullable.ts @@ -0,0 +1,22 @@ +import { sql } from '@silverhand/slonik'; + +import type { AlterationScript } from '../lib/types/alteration.js'; + +const alteration: AlterationScript = { + up: async (pool) => { + // Make the resource scopes description nullable + await pool.query(sql` + alter table scopes + alter column description drop not null; + `); + }, + down: async (pool) => { + // Revert the resource scopes description nullable + await pool.query(sql` + alter table scopes + alter column description set not null; + `); + }, +}; + +export default alteration; diff --git a/packages/schemas/src/types/consent.ts b/packages/schemas/src/types/consent.ts index a872fe5a3..d1e58468d 100644 --- a/packages/schemas/src/types/consent.ts +++ b/packages/schemas/src/types/consent.ts @@ -44,6 +44,7 @@ export const publicOrganizationGuard = Organizations.guard.pick({ id: true, name: true, }); + export const missingResourceScopesGuard = z.object({ // The original resource id has a maximum length of 21 restriction. We need to make it compatible with the logto reserved organization name. // use string here, as we do not care about the resource id length here. diff --git a/packages/schemas/tables/scopes.sql b/packages/schemas/tables/scopes.sql index 74f1d4a89..954009d2c 100644 --- a/packages/schemas/tables/scopes.sql +++ b/packages/schemas/tables/scopes.sql @@ -7,7 +7,7 @@ create table scopes ( resource_id varchar(21) not null references resources (id) on update cascade on delete cascade, name varchar(256) not null, - description text not null, + description text, created_at timestamptz not null default(now()), primary key (id), constraint scopes__resource_id_name