Skip to content

Commit 81538bf

Browse files
committed
refactor: fixup grant_types and response_types mismatch instead of rejecting
1 parent 969edba commit 81538bf

4 files changed

Lines changed: 41 additions & 35 deletions

File tree

docs/README.md

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4531,28 +4531,27 @@ const signedIn = !!session.accountId;
45314531
45324532
### Client Credentials only clients
45334533
4534-
The `redirect_uris is mandatory property` error occurs but Client Credential clients
4535-
don't need `redirect_uris` or `response_types`... This error appears
4536-
because they are required properties, but they can be empty...
4534+
Client Credential clients don't use the authorization endpoint, so set
4535+
`response_types` to an empty array. The `redirect_uris` will default to `[]`
4536+
automatically.
45374537
45384538
```js
45394539
{
45404540
// ... rest of the client configuration
4541-
redirect_uris: [],
45424541
response_types: [],
45434542
grant_types: ['client_credentials']
45444543
}
45454544
```
45464545
45474546
### Resource Server only clients (e.g. for token introspection)
45484547
4549-
The `redirect_uris is mandatory property` error occurs but the resource server needs
4550-
none. This error appears because they are required properties, but they can be empty...
4548+
Resource server clients don't use any grants or the authorization endpoint, so set
4549+
`response_types` to an empty array. The `redirect_uris` will default to `[]`
4550+
automatically.
45514551
45524552
```js
45534553
{
45544554
// ... rest of the client configuration
4555-
redirect_uris: [],
45564555
response_types: [],
45574556
grant_types: []
45584557
}

lib/helpers/client_schema.js

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,6 @@ export default function getSchema(provider) {
279279

280280
const responseTypes = new Set(this.response_types.map((rt) => rt.split(' ')).flat());
281281

282-
if (this.grant_types.some((type) => ['authorization_code', 'implicit'].includes(type)) && !this.response_types.length) {
283-
this.invalidate('response_types must contain members');
284-
}
285-
286282
if (responseTypes.size && !this.redirect_uris.length) {
287283
const { pushedAuthorizationRequests: par } = features;
288284
if (
@@ -301,13 +297,19 @@ export default function getSchema(provider) {
301297
}
302298

303299
if (responseTypes.has('code') && !this.grant_types.includes('authorization_code')) {
304-
this.invalidate("grant_types must contain 'authorization_code' when code is amongst response_types");
300+
this.grant_types.push('authorization_code');
305301
}
306302

307-
if (responseTypes.has('token') || responseTypes.has('id_token')) {
308-
if (!this.grant_types.includes('implicit')) {
309-
this.invalidate("grant_types must contain 'implicit' when 'id_token' or 'token' are amongst response_types");
310-
}
303+
if ((responseTypes.has('token') || responseTypes.has('id_token')) && !this.grant_types.includes('implicit')) {
304+
this.grant_types.push('implicit');
305+
}
306+
307+
if (this.grant_types.includes('implicit') && !responseTypes.has('id_token') && !responseTypes.has('token')) {
308+
this.grant_types.splice(this.grant_types.indexOf('implicit'), 1);
309+
}
310+
311+
if (this.grant_types.includes('authorization_code') && !responseTypes.has('code')) {
312+
this.grant_types.splice(this.grant_types.indexOf('authorization_code'), 1);
311313
}
312314

313315
{

test/configuration/client_metadata.test.js

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -373,20 +373,31 @@ describe('Client metadata validation', () => {
373373
defaultsTo(this.title, ['authorization_code']);
374374
mustBeArray(this.title);
375375
allows(this.title, ['authorization_code', 'refresh_token']);
376+
allows(this.title, [], { response_types: ['none'] });
376377
rejects(this.title, [123], /must only contain strings$/);
377-
rejects(this.title, []);
378378
rejects(this.title, ['not-a-type']);
379-
rejects(this.title, ['implicit'], undefined, {
379+
380+
// grant types auto-fixup
381+
allows(this.title, [], undefined, undefined, (client) => {
382+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql(['authorization_code']);
383+
});
384+
allows(this.title, ['implicit'], {
380385
// misses authorization_code
381386
response_types: ['id_token', 'code'],
387+
}, undefined, (client) => {
388+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql(['implicit', 'authorization_code']);
382389
});
383-
rejects(this.title, ['authorization_code'], undefined, {
384-
// misses implicit
390+
allows(this.title, ['authorization_code'], {
391+
// misses code in response_types
385392
response_types: ['id_token'],
393+
}, undefined, (client) => {
394+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql(['implicit']);
386395
});
387-
rejects(this.title, ['authorization_code'], undefined, {
388-
// misses implicit
389-
response_types: ['token'],
396+
allows(this.title, ['authorization_code', 'implicit'], {
397+
// misses token or id_token in response_types
398+
response_types: ['code'],
399+
}, undefined, (client) => {
400+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql(['authorization_code']);
390401
});
391402
});
392403

@@ -679,7 +690,9 @@ describe('Client metadata validation', () => {
679690
);
680691

681692
rejects(this.title, [123], /must only contain strings$/);
682-
rejects(this.title, [], /must contain members$/);
693+
allows(this.title, [], undefined, undefined, (client) => {
694+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql([]);
695+
});
683696
rejects(this.title, ['not-a-type']);
684697
rejects(this.title, ['not-a-type', 'none']);
685698
});

test/configuration/custom_client_metadata.test.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ describe('extraClientMetadata configuration', () => {
147147
expect(validator.calledWith(undefined, 'client_name', undefined)).to.be.true;
148148
});
149149

150-
it('should not allow customization to introduce grant_types and response_types mismatch', async () => {
150+
it('should allow customization to set grant_types that get auto-fixed based on response_types', async () => {
151151
const provider = new Provider('http://localhost:3000', {
152152
extraClientMetadata: {
153153
properties: ['foo'],
@@ -164,16 +164,8 @@ describe('extraClientMetadata configuration', () => {
164164
],
165165
});
166166

167-
try {
168-
await provider.Client.find('client');
169-
throw new Error('expected a throw from the above');
170-
} catch (err) {
171-
expect(err).to.have.property('message', 'invalid_client_metadata');
172-
expect(err).to.have.property(
173-
'error_description',
174-
"grant_types must contain 'authorization_code' when code is amongst response_types",
175-
);
176-
}
167+
const client = await provider.Client.find('client');
168+
expect(client.metadata()).to.have.property('grant_types').and.deep.eql(['authorization_code']);
177169
});
178170

179171
it('should not allow customization to remove redirect_uris when response_types need them', async () => {

0 commit comments

Comments
 (0)