Skip to content

Commit 98dbead

Browse files
authored
Merge pull request #6312 from logto-io/gao-log-app-secret
refactor(core): log app secret name
2 parents 8ee1eb9 + f773957 commit 98dbead

File tree

4 files changed

+49
-5
lines changed

4 files changed

+49
-5
lines changed

packages/core/src/event-listeners/grant.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { GrantType, LogResult, token } from '@logto/schemas';
22
import type { errors, KoaContextWithOIDC } from 'oidc-provider';
33

4+
import { type WithAppSecretContext } from '#src/middleware/koa-app-secret-transpilation.js';
45
import type { WithLogContext } from '#src/middleware/koa-audit-log.js';
56

67
import { stringifyError } from '../utils/format.js';
@@ -13,7 +14,7 @@ import { extractInteractionContext } from './utils.js';
1314
* @see {@link https://github.com/panva/node-oidc-provider/blob/v7.x/lib/shared/error_handler.js OIDC Provider error handler}
1415
*/
1516
export const grantListener = (
16-
ctx: KoaContextWithOIDC & WithLogContext & { body: GrantBody },
17+
ctx: KoaContextWithOIDC & WithLogContext & WithAppSecretContext & { body: GrantBody },
1718
error?: errors.OIDCProviderError
1819
) => {
1920
const { params } = ctx.oidc;

packages/core/src/event-listeners/utils.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import type { IRouterParamContext } from 'koa-router';
22
import type { KoaContextWithOIDC } from 'oidc-provider';
33

4+
import { type WithAppSecretContext } from '#src/middleware/koa-app-secret-transpilation.js';
45
import type { LogPayload } from '#src/middleware/koa-audit-log.js';
56

67
export const extractInteractionContext = (
7-
ctx: IRouterParamContext & KoaContextWithOIDC
8+
ctx: IRouterParamContext & KoaContextWithOIDC & WithAppSecretContext
89
): LogPayload => {
910
const {
1011
entities: { Account, Session, Client, Interaction },
@@ -13,6 +14,7 @@ export const extractInteractionContext = (
1314

1415
return {
1516
applicationId: Client?.clientId,
17+
applicationSecret: ctx.appSecret,
1618
sessionId: Session?.jti,
1719
interactionId: Interaction?.jti,
1820
userId: Account?.accountId,

packages/core/src/middleware/koa-app-secret-transpilation.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Nullable } from '@silverhand/essentials';
2-
import type { MiddlewareType } from 'koa';
2+
import type { Context, MiddlewareType } from 'koa';
33
import { errors } from 'oidc-provider';
44

55
import type Queries from '#src/tenants/Queries.js';
@@ -14,6 +14,14 @@ function decodeAuthToken(token: string) {
1414
return authToken;
1515
}
1616

17+
export type WithAppSecretContext<ContextT = Context> = ContextT & {
18+
/** The application secret that has been transpiled during the middleware execution. */
19+
appSecret?: {
20+
/** The application secret name of the application (client). */
21+
name: string;
22+
};
23+
};
24+
1725
/** @import { getConstantClientMetadata } from '#src/oidc/utils.js'; */
1826
/**
1927
* Create a middleware function that reads the app secret from the request and check if it matches
@@ -39,7 +47,7 @@ function decodeAuthToken(token: string) {
3947
*/
4048
export default function koaAppSecretTranspilation<StateT, ContextT, ResponseBodyT>(
4149
queries: Queries
42-
): MiddlewareType<StateT, ContextT, Nullable<ResponseBodyT>> {
50+
): MiddlewareType<StateT, WithAppSecretContext<ContextT>, Nullable<ResponseBodyT>> {
4351
return async (ctx, next) => {
4452
type ClientCredentials = Partial<{
4553
clientId: string;
@@ -127,6 +135,7 @@ export default function koaAppSecretTranspilation<StateT, ContextT, ResponseBody
127135
ctx.query.client_secret = result.originalSecret;
128136
}
129137

138+
ctx.appSecret = { name: result.name };
130139
return next();
131140
};
132141
}

packages/integration-tests/src/tests/api/oidc/client-authentication.test.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import assert from 'node:assert';
1010

11-
import { ApplicationType } from '@logto/schemas';
11+
import { ApplicationType, token } from '@logto/schemas';
1212
import { noop, removeUndefinedKeys } from '@silverhand/essentials';
1313
import { HTTPError } from 'ky';
1414

@@ -18,6 +18,7 @@ import {
1818
createApplicationSecret,
1919
deleteApplication,
2020
} from '#src/api/application.js';
21+
import { getAuditLogs } from '#src/api/index.js';
2122
import { createResource } from '#src/api/resource.js';
2223
import { devFeatureTest, randomString, waitFor } from '#src/utils.js';
2324

@@ -33,6 +34,23 @@ const [application, resource] = await Promise.all([
3334
createResource(),
3435
]);
3536

37+
const getLogs = async () =>
38+
getAuditLogs(
39+
new URLSearchParams({
40+
logKey: `${token.Type.ExchangeTokenBy}.${token.ExchangeByType.ClientCredentials}`,
41+
})
42+
);
43+
44+
const expectLog = (applicationId: string, secretName: string) =>
45+
expect.objectContaining({
46+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
47+
payload: expect.objectContaining({
48+
applicationId,
49+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
50+
applicationSecret: expect.objectContaining({ name: secretName }),
51+
}),
52+
});
53+
3654
afterAll(async () => {
3755
await deleteApplication(application.id).catch(noop);
3856
});
@@ -155,11 +173,14 @@ devFeatureTest.describe('client authentication', () => {
155173
});
156174

157175
it('should pass when client credentials are valid in authorization header', async () => {
176+
const application = await createApplication('application', ApplicationType.MachineToMachine);
158177
const secret = await createApplicationSecret({
159178
applicationId: application.id,
160179
name: randomString(),
161180
});
181+
const beforeLogs = await getLogs();
162182

183+
expect(beforeLogs).not.toContainEqual(expectLog(application.id, secret.name));
163184
await expect(
164185
post({
165186
authorization: `Basic ${Buffer.from(`${application.id}:${secret.value}`).toString(
@@ -170,14 +191,21 @@ devFeatureTest.describe('client authentication', () => {
170191
).resolves.toMatchObject({
171192
token_type: 'Bearer',
172193
});
194+
195+
const logs = await getLogs();
196+
expect(logs).toContainEqual(expectLog(application.id, secret.name));
197+
await deleteApplication(application.id);
173198
});
174199

175200
it('should pass when client credentials are valid in body', async () => {
201+
const application = await createApplication('application', ApplicationType.MachineToMachine);
176202
const secret = await createApplicationSecret({
177203
applicationId: application.id,
178204
name: randomString(),
179205
});
206+
const beforeLogs = await getLogs();
180207

208+
expect(beforeLogs).not.toContainEqual(expectLog(application.id, secret.name));
181209
await expect(
182210
post({
183211
body: {
@@ -206,5 +234,9 @@ devFeatureTest.describe('client authentication', () => {
206234
token_type: 'Bearer',
207235
});
208236
}
237+
238+
const logs = await getLogs();
239+
expect(logs).toContainEqual(expectLog(application.id, secret.name));
240+
await deleteApplication(application.id);
209241
});
210242
});

0 commit comments

Comments
 (0)