Skip to content

Commit

Permalink
fix: update according to CR
Browse files Browse the repository at this point in the history
  • Loading branch information
darcyYe committed Nov 29, 2024
1 parent 225bdf4 commit d55112b
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 23 deletions.
29 changes: 16 additions & 13 deletions packages/core/src/saml-applications/libraries/saml-applications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import { generateStandardId } from '@logto/shared';
import { removeUndefinedKeys } from '@silverhand/essentials';

import RequestError from '#src/errors/RequestError/index.js';
import type Queries from '#src/tenants/Queries.js';
import assertThat from '#src/utils/assert-that.js';

Expand Down Expand Up @@ -42,7 +43,13 @@ export const createSamlApplicationsLibrary = (queries: Queries) => {

const findSamlApplicationById = async (id: string): Promise<SamlApplicationResponse> => {
const application = await findApplicationById(id);
assertThat(application.type === ApplicationType.SAML, 'application.saml.saml_application_only');
assertThat(
application.type === ApplicationType.SAML,
new RequestError({
code: 'application.saml.saml_application_only',
status: 422,
})
);

const samlConfig = await findSamlApplicationConfigByApplicationId(application.id);

Expand All @@ -53,24 +60,20 @@ export const createSamlApplicationsLibrary = (queries: Queries) => {
id: string,
patchApplicationObject: PatchSamlApplication
): Promise<SamlApplicationResponse> => {
const { name, description, customData, config } = patchApplicationObject;
const { config, ...applicationData } = patchApplicationObject;
const originalApplication = await findApplicationById(id);

assertThat(
originalApplication.type === ApplicationType.SAML,
'application.saml.saml_application_only'
new RequestError({
code: 'application.saml.saml_application_only',
status: 422,
})
);

const [updatedApplication, upToDateSamlConfig] = await Promise.all([
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
name || description || customData
? updateApplicationById(
id,
removeUndefinedKeys({
name,
description,
customData,
})
)
Object.keys(applicationData).length > 0
? updateApplicationById(id, removeUndefinedKeys(applicationData))
: originalApplication,
config
? updateSamlApplicationConfig({
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/saml-applications/libraries/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,9 @@ export const ensembleSamlApplication = ({
* Only HTTP-POST binding is supported for receiving SAML assertions at the moment.
*/
export const validateAcsUrl = (acsUrl: SamlAcsUrl) => {
const { binding } = acsUrl;
assertThat(
acsUrl.binding === BindingType.POST,
binding === BindingType.POST,
new RequestError({
code: 'application.saml.acs_url_binding_not_supported',
status: 422,
Expand Down
18 changes: 13 additions & 5 deletions packages/core/src/saml-applications/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import { generateStandardId } from '@logto/shared';
import { removeUndefinedKeys } from '@silverhand/essentials';
import { z } from 'zod';

import RequestError from '#src/errors/RequestError/index.js';
import koaGuard from '#src/middleware/koa-guard.js';
import { buildOidcClientMetadata } from '#src/oidc/utils.js';
import { generateInternalSecret } from '#src/routes/applications/application-secret.js';
import type { ManagementApiRouter, RouterInitArgs } from '#src/routes/types.js';
import { ensembleSamlApplication, validateAcsUrl } from '#src/saml-applications/libraries/utils.js';
import assertThat from '#src/utils/assert-that.js';

import { ensembleSamlApplication, validateAcsUrl } from '../libraries/utils.js';

export default function samlApplicationRoutes<T extends ManagementApiRouter>(
...[router, { queries, libraries }]: RouterInitArgs<T>
) {
Expand Down Expand Up @@ -84,7 +86,7 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
id: z.string(),
}),
response: samlApplicationResponseGuard,
status: [200, 400, 404],
status: [200, 404, 422],
}),
async (ctx, next) => {
const { id } = ctx.guard.params;
Expand All @@ -104,7 +106,7 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
params: z.object({ id: z.string() }),
body: samlApplicationPatchGuard,
response: samlApplicationResponseGuard,
status: [200, 400, 404],
status: [200, 404, 422],
}),
async (ctx, next) => {
const { id } = ctx.guard.params;
Expand All @@ -122,13 +124,19 @@ export default function samlApplicationRoutes<T extends ManagementApiRouter>(
'/saml-applications/:id',
koaGuard({
params: z.object({ id: z.string() }),
status: [204, 400, 404],
status: [204, 422, 404],

Check warning on line 127 in packages/core/src/saml-applications/routes/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/routes/index.ts#L127

Added line #L127 was not covered by tests
}),
async (ctx, next) => {
const { id } = ctx.guard.params;

const { type } = await findApplicationById(id);
assertThat(type === ApplicationType.SAML, 'application.saml.saml_application_only');
assertThat(
type === ApplicationType.SAML,
new RequestError({
code: 'application.saml.saml_application_only',
status: 422,
})
);

Check warning on line 139 in packages/core/src/saml-applications/routes/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/saml-applications/routes/index.ts#L133-L139

Added lines #L133 - L139 were not covered by tests

await deleteApplicationById(id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,15 @@ describe('SAML application', () => {

await expectRejects(deleteSamlApplication(application.id), {
code: 'application.saml.saml_application_only',
status: 400,
status: 422,
});
await expectRejects(updateSamlApplication(application.id, { name: 'updated' }), {
code: 'application.saml.saml_application_only',
status: 400,
status: 422,
});
await expectRejects(getSamlApplication(application.id), {
code: 'application.saml.saml_application_only',
status: 400,
status: 422,
});
await deleteApplication(application.id);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ export type SamlAcsUrl = {

export const samlAcsUrlGuard = z.object({
binding: z.nativeEnum(BindingType),
url: z.string(),
url: z.string().url(),
}) satisfies ToZodObject<SamlAcsUrl>;

0 comments on commit d55112b

Please sign in to comment.