Skip to content

Commit 969edba

Browse files
committed
refactor(CIMD): filter unrecognized array members before validating
closes #1398
1 parent 35fb736 commit 969edba

5 files changed

Lines changed: 156 additions & 9 deletions

File tree

lib/helpers/add_client.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1+
/* eslint-disable no-param-reassign */
12
import sectorValidate from './sector_validate.js';
23

3-
export default async function add(provider, metadata, { ctx, store = false } = {}) {
4-
const client = new provider.Client(metadata, ctx); // eslint-disable-line no-use-before-define
4+
export default async function add(provider, metadata, { ctx, store, cimd } = {}) {
5+
const client = new provider.Client(metadata, ctx, { cimd });
56

67
if (client.sectorIdentifierUri !== undefined) {
78
await sectorValidate(provider, client);
89
}
910

10-
if (store) {
11+
if (!cimd && store) {
1112
await provider.Client.adapter.upsert(client.clientId, client.metadata());
1213
}
1314
return client;

lib/helpers/client_id_metadata_document.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export async function resolveClientByMetadataDocument(provider, id) {
102102
// Check cache
103103
const cached = entries.get(id);
104104
if (cached && cached.freshUntil > Date.now()) {
105-
const client = await addClient(provider, cached.properties, { store: false });
105+
const client = await addClient(provider, cached.properties, { cimd: true });
106106
Object.defineProperty(client, 'clientIdMetadataDocument', { value: true });
107107

108108
if (!(await feature.allowClient(ctx, client))) {
@@ -180,7 +180,7 @@ export async function resolveClientByMetadataDocument(provider, id) {
180180
// Compute cache TTL
181181
const ttl = parseCacheDuration(response, feature.cacheDuration);
182182

183-
const client = await addClient(provider, properties, { store: false });
183+
const client = await addClient(provider, properties, { cimd: true });
184184

185185
Object.defineProperty(client, 'clientIdMetadataDocument', { value: true });
186186

lib/helpers/client_schema.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,16 @@ export default function getSchema(provider) {
211211
};
212212

213213
class Schema {
214+
#cimd = false;
215+
214216
constructor(
215217
metadata,
216218
ctx,
217219
processCustomMetadata = !!configuration.extraClientMetadata.properties.length,
220+
cimd = false,
218221
) {
222+
this.#cimd = cimd;
223+
219224
if (processCustomMetadata) {
220225
this.#assign(metadata);
221226
this.processCustomMetadata(ctx);
@@ -557,7 +562,12 @@ export default function getSchema(provider) {
557562
}
558563

559564
if (isAry && !this[prop].every((val) => only[method](val))) {
560-
if (length) {
565+
if (this.#cimd && length) {
566+
this[prop] = this[prop].filter((val) => only[method](val));
567+
if (!this[prop].length) {
568+
this.invalidate(`${prop} has no values supported by this authorization server`);
569+
}
570+
} else if (length) {
561571
this.invalidate(`${prop} can only contain ${formatters.formatList([...only], { type: 'disjunction' })}`);
562572
} else {
563573
this.invalidate(`${prop} must be empty (no values are allowed)`);

lib/models/client.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,8 @@ export default function getClient(provider) {
322322

323323
static #adapter;
324324

325-
constructor(metadata, ctx) {
326-
const schema = new Client.Schema(metadata, ctx);
325+
constructor(metadata, ctx, { cimd } = {}) {
326+
const schema = new Client.Schema(metadata, ctx, undefined, cimd);
327327

328328
Object.assign(this, mapKeys(schema, (value, key) => {
329329
if (!instance(provider).RECOGNIZED_METADATA.includes(key)) {
@@ -551,7 +551,7 @@ export default function getClient(provider) {
551551
let client = dynamicClients.get(propHash);
552552

553553
if (!client) {
554-
client = await addClient(provider, properties, { store: false });
554+
client = await addClient(provider, properties);
555555
dynamicClients.set(propHash, client);
556556
}
557557

test/client_id_metadata_document/client_id_metadata_document.test.js

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,142 @@ describe('Client ID Metadata Document', () => {
502502
});
503503
});
504504

505+
describe('unsupported metadata value filtering', () => {
506+
it('filters unsupported grant_types and keeps supported ones', function () {
507+
mock('https://filter-grants.example.com')
508+
.intercept({ path: '/client' })
509+
.reply(200, JSON.stringify({
510+
client_id: 'https://filter-grants.example.com/client',
511+
redirect_uris: ['https://filter-grants.example.com/cb'],
512+
token_endpoint_auth_method: 'none',
513+
grant_types: ['authorization_code', 'urn:ietf:params:oauth:grant-type:device_code'],
514+
}), {
515+
headers: { 'content-type': 'application/json' },
516+
});
517+
518+
return this.agent.get('/auth')
519+
.query({
520+
client_id: 'https://filter-grants.example.com/client',
521+
redirect_uri: 'https://filter-grants.example.com/cb',
522+
response_type: 'code',
523+
scope: 'openid',
524+
})
525+
.expect(303)
526+
.expect((response) => {
527+
const location = new URL(response.headers.location, this.provider.issuer);
528+
expect(location.pathname).to.match(/\/interaction\//);
529+
});
530+
});
531+
532+
it('rejects when all grant_types are unsupported', function () {
533+
mock('https://no-grants.example.com')
534+
.intercept({ path: '/client' })
535+
.reply(200, JSON.stringify({
536+
client_id: 'https://no-grants.example.com/client',
537+
redirect_uris: ['https://no-grants.example.com/cb'],
538+
token_endpoint_auth_method: 'none',
539+
grant_types: ['urn:ietf:params:oauth:grant-type:device_code'],
540+
}), {
541+
headers: { 'content-type': 'application/json' },
542+
});
543+
544+
return this.agent.get('/auth')
545+
.query({
546+
client_id: 'https://no-grants.example.com/client',
547+
redirect_uri: 'https://no-grants.example.com/cb',
548+
response_type: 'code',
549+
scope: 'openid',
550+
})
551+
.expect(400)
552+
.expect((response) => {
553+
expect(response.text).to.contain('invalid_client_metadata');
554+
});
555+
});
556+
557+
it('filters unsupported response_types and keeps supported ones', function () {
558+
mock('https://filter-rt.example.com')
559+
.intercept({ path: '/client' })
560+
.reply(200, JSON.stringify({
561+
client_id: 'https://filter-rt.example.com/client',
562+
redirect_uris: ['https://filter-rt.example.com/cb'],
563+
token_endpoint_auth_method: 'none',
564+
grant_types: ['authorization_code'],
565+
response_types: ['code', 'code token id_token unsupported'],
566+
}), {
567+
headers: { 'content-type': 'application/json' },
568+
});
569+
570+
return this.agent.get('/auth')
571+
.query({
572+
client_id: 'https://filter-rt.example.com/client',
573+
redirect_uri: 'https://filter-rt.example.com/cb',
574+
response_type: 'code',
575+
scope: 'openid',
576+
})
577+
.expect(303)
578+
.expect((response) => {
579+
const location = new URL(response.headers.location, this.provider.issuer);
580+
expect(location.pathname).to.match(/\/interaction\//);
581+
});
582+
});
583+
584+
it('filters unsupported response_modes and keeps supported ones', function () {
585+
mock('https://filter-rm.example.com')
586+
.intercept({ path: '/client' })
587+
.reply(200, JSON.stringify({
588+
client_id: 'https://filter-rm.example.com/client',
589+
redirect_uris: ['https://filter-rm.example.com/cb'],
590+
token_endpoint_auth_method: 'none',
591+
response_modes: ['query', 'unsupported_mode'],
592+
}), {
593+
headers: { 'content-type': 'application/json' },
594+
});
595+
596+
return this.agent.get('/auth')
597+
.query({
598+
client_id: 'https://filter-rm.example.com/client',
599+
redirect_uri: 'https://filter-rm.example.com/cb',
600+
response_type: 'code',
601+
scope: 'openid',
602+
})
603+
.expect(303)
604+
.expect((response) => {
605+
const location = new URL(response.headers.location, this.provider.issuer);
606+
expect(location.pathname).to.match(/\/interaction\//);
607+
});
608+
});
609+
610+
it('post-enum consistency validation still applies to filtered values', function () {
611+
mock('https://consistency.example.com')
612+
.intercept({ path: '/client' })
613+
.reply(200, JSON.stringify({
614+
client_id: 'https://consistency.example.com/client',
615+
redirect_uris: ['https://consistency.example.com/cb'],
616+
token_endpoint_auth_method: 'none',
617+
grant_types: ['urn:ietf:params:oauth:grant-type:device_code', 'implicit'],
618+
response_types: ['id_token'],
619+
}), {
620+
headers: { 'content-type': 'application/json' },
621+
});
622+
623+
// implicit is supported but device_code is not
624+
// after filtering grant_types becomes ['implicit'], response_types ['id_token'] is ok
625+
return this.agent.get('/auth')
626+
.query({
627+
client_id: 'https://consistency.example.com/client',
628+
redirect_uri: 'https://consistency.example.com/cb',
629+
response_type: 'id_token',
630+
scope: 'openid',
631+
nonce: 'nonce',
632+
})
633+
.expect(303)
634+
.expect((response) => {
635+
const location = new URL(response.headers.location, this.provider.issuer);
636+
expect(location.pathname).to.match(/\/interaction\//);
637+
});
638+
});
639+
});
640+
505641
describe('allowFetch hook', () => {
506642
it('rejects when allowFetch returns false', async function () {
507643
const { features } = i(this.provider); // eslint-disable-line no-undef

0 commit comments

Comments
 (0)