0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2025-02-17 22:04:19 -05:00

fix(core): fix some webhook api body status 404 bug (#6311)

* fix(core): fix some webhook api body status 404 bug

fix some webhook api body status 404 bug

* fix(core): improve the webhook trigger logic

improve the webhook trigger logic

* chore: add changeset

add changeset

* chore: update the changeset

update the changeset
This commit is contained in:
simeng-li 2024-07-25 17:34:59 +08:00 committed by GitHub
parent a2e457e8ef
commit f76252e0d2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 28 additions and 6 deletions

View file

@ -0,0 +1,14 @@
---
"@logto/core": patch
---
fix the status code 404 error in webhook events payload
Impact webhook events:
- `Role.Scopes.Updated`
- `Organizations.Membership.Updates`
Issue: These webhook event payloads were returning a API response status code of 404 when the webhook was triggered.
Expected: A status code of 200 should be returned, as we only trigger the webhook when the request is successful.
Fix: All webhook event contexts should be created and inserted into the webhook pipeline after the response body and status code are properly set.

View file

@ -174,7 +174,12 @@ export default function roleRoutes<T extends ManagementApiRouter>(
await insertRolesScopes(
scopeIds.map((scopeId) => ({ id: generateStandardId(), roleId: role.id, scopeId }))
);
}
ctx.body = role;
// Hook context must be triggered after the response is set.
if (scopeIds) {
// Trigger the `Role.Scopes.Updated` event if scopeIds are provided. Should not break the request
await trySafe(async () => {
// Align the response type with POST /roles/:id/scopes
@ -188,8 +193,6 @@ export default function roleRoutes<T extends ManagementApiRouter>(
});
}
ctx.body = role;
return next();
}
);

View file

@ -1,4 +1,4 @@
import { type SchemaLike, type GeneratedSchema, type DataHookEvent } from '@logto/schemas';
import { type DataHookEvent, type GeneratedSchema, type SchemaLike } from '@logto/schemas';
import { generateStandardId } from '@logto/shared';
import { type DeepPartial, isPlainObject } from '@silverhand/essentials';
import camelcase from 'camelcase';
@ -257,8 +257,8 @@ export default class SchemaRouter<
[columns.relationSchemaId]: relationId,
})) ?? [])
);
appendHookContext(ctx, id);
ctx.status = 201;
appendHookContext(ctx, id);
return next();
}
);
@ -277,8 +277,8 @@ export default class SchemaRouter<
} = ctx.guard;
await relationQueries.replace(id, relationIds ?? []);
appendHookContext(ctx, id);
ctx.status = 204;
appendHookContext(ctx, id);
return next();
}
);
@ -302,9 +302,10 @@ export default class SchemaRouter<
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- `koaGuard()` ensures the value is not `undefined`
[columns.relationSchemaId]: relationId!,
});
ctx.status = 204;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
appendHookContext(ctx, id!);
ctx.status = 204;
return next();
}
);

View file

@ -76,6 +76,7 @@ export const mockHookResponseGuard = z.object({
event: hookEventGuard,
createdAt: z.string(),
hookId: z.string(),
status: z.number().optional(),
})
.catchall(z.any()),
// Use the raw payload for signature verification

View file

@ -47,6 +47,7 @@ const mockHookResponseGuard = z.object({
.optional()
.transform((value) => value?.toUpperCase()),
matchedRoute: z.string().optional(),
status: z.number().optional(),
})
.catchall(z.any()),
// Keep the raw payload for signature verification
@ -364,6 +365,7 @@ describe('data hook events coverage and signature verification', () => {
const webhookResult = await getWebhookResult(key);
expect(webhookResult).toBeDefined();
expect(webhookResult?.payload.status).not.toBe(404);
assert(webhookResult, new Error('webhookResult is undefined'));
const { signature, rawPayload } = webhookResult;

View file

@ -46,6 +46,7 @@ export const assertHookLogResult = async (
const { body } = mockHookResponseGuard.parse(payload.response);
expect(verifySignature(body.rawPayload, signingKey, body.signature)).toBeTruthy();
expect(body.payload).toEqual(expect.objectContaining(assertions.hookPayload));
expect(body.payload.status).not.toBe(404);
}
if (assertions.errorMessage) {