0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2025-01-27 21:39:16 -05:00

fix(schemas): fix the get interation/consent api bug (#5503)

* fix(schemas): fix the get interation/consent api bug

fix the get interation/consent api bug

* chore: update changeset

update changeset

* fix: update changeset

update changeset

* refactor(schemas, console): alter the resource scopes description field to nullable (#5504)

* refactor(schemas, console): alter the resoruce scopes description field nullable

make the resourec scopes description nullable

* fix(test): fix the type issue in the integration test

fix the type issue in the integration test

* fix(console): add the field register

add the field register

* fix: update the changeset

update the changeset

* fix(console,test): update comments and rebase

update comments and rebase the master
This commit is contained in:
simeng-li 2024-03-20 14:31:35 +08:00 committed by GitHub
parent 0cb3116e86
commit 9518658595
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 211 additions and 9 deletions

View file

@ -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.

View file

@ -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<Scope, 'name' | 'description'>;
type CreatePermissionFormData = Pick<CreateScope, 'name' | 'description'>;
function CreatePermissionModal({ resourceId, totalResourceCount, onClose }: Props) {
const { currentPlan } = useContext(SubscriptionDataContext);
@ -118,10 +118,10 @@ function CreatePermissionModal({ resourceId, totalResourceCount, onClose }: Prop
error={errors.name?.message}
/>
</FormField>
<FormField isRequired title="api_resource_details.permission.description">
<FormField title="api_resource_details.permission.description">
<TextInput
placeholder={t('api_resource_details.permission.description_placeholder')}
{...register('description', { required: true })}
{...register('description')}
error={Boolean(errors.description)}
/>
</FormField>

View file

@ -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<string>; // Organization scope description cloud be `null`
}>;
};

View file

@ -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<RedirectResponse>();
export const getConsentInfo = async (cookie: string) =>
api
.get('interaction/consent', {
headers: { cookie },
followRedirect: false,
})
.json<ConsentInfoResponse>();
export const createSingleSignOnAuthorizationUri = async (
cookie: string,
payload: SocialAuthorizationUriPayload

View file

@ -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);
}

View file

@ -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<string, Application>();
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);
}
});
});

View file

@ -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,

View file

@ -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;

View file

@ -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.

View file

@ -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