Skip to content

Commit

Permalink
feat: update fapiRW draft feature
Browse files Browse the repository at this point in the history
Removes the check that all params outside of the Request Object are also
provided inside it. With Request Object mergingStrategy now
configurable fapiRW should be accompanied with
`requestObjects.mergingStrategy.name` set to `strict`
  • Loading branch information
panva committed Sep 6, 2019
1 parent ef55ef6 commit 8b927fc
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 66 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ jobs:
run: |
git clone https://gitlab.com/openid/conformance-suite.git
cd conformance-suite
git checkout release-v3.2.4
git checkout release-v3.2.5
sed -i -e 's/localhost/localhost.emobix.co.uk/g' src/main/resources/application.properties
docker-compose -f builder-compose.yml run builder
docker-compose -f docker-compose-dev.yml up -d
Expand Down
15 changes: 9 additions & 6 deletions certification/fapi/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const ms = require('ms');
const debug = require('./debug');

const FINISHED = new Set(['FINISHED']);
const RESULTS = new Set(['REVIEW', 'PASSED']);

class API {
constructor({ baseUrl, bearerToken } = {}) {
Expand Down Expand Up @@ -86,19 +87,21 @@ class API {
return response;
}

async waitForState({ moduleId, states = FINISHED, timeout = ms('4m') } = {}) {
async waitForState({ moduleId, timeout = ms('4m') } = {}) {
assert(moduleId, 'argument property "moduleId" missing');
assert(moduleId, 'argument property "states" missing');
assert(states instanceof Set, 'argument property "states" must be a Set instance');
assert(moduleId, 'argument property "timeout" missing');

const timeoutAt = Date.now() + timeout;

while (Date.now() < timeoutAt) {
const { status } = await this.getModuleInfo({ moduleId });
const { status, result } = await this.getModuleInfo({ moduleId });
debug('module id %s status is %s', moduleId, status);
if (states.has(status)) {
return status;
if (FINISHED.has(status)) {
if (!RESULTS.has(result)) {
throw new Error(`module id ${moduleId} is ${status} but ${result}`);
}
return [status, result];
}

if (status === 'INTERRUPTED') {
Expand All @@ -110,7 +113,7 @@ class API {
}

debug(`module id ${moduleId} expected state timeout`);
throw new Error(`Timed out waiting for test module ${moduleId} to be in one of states: ${[...states].join(', ')}`);
throw new Error(`Timed out waiting for test module ${moduleId} to be in one of states: ${[...FINISHED].join(', ')}`);
}
}

Expand Down
24 changes: 24 additions & 0 deletions certification/fapi/mtls.json
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,30 @@
}
]
},
"fapi-rw-id2-ensure-request-object-without-redirect-uri-fails": {
"browser": [
{
"comment": "expect an immediate error page",
"match": "https://fapi.panva.cz/auth*",
"tasks": [
{
"task": "Expect redirect_uri missing error page",
"match": "https://fapi.panva.cz/auth*",
"commands": [
[
"wait",
"xpath",
"//*",
10,
"oops! something went wrong",
"update-image-placeholder"
]
]
}
]
}
]
},
"fapi-rw-id2-user-rejects-authentication": {
"browser": [
{
Expand Down
24 changes: 24 additions & 0 deletions certification/fapi/pkjwt.json
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,30 @@
}
]
},
"fapi-rw-id2-ensure-request-object-without-redirect-uri-fails": {
"browser": [
{
"comment": "expect an immediate error page",
"match": "https://fapi.panva.cz/auth*",
"tasks": [
{
"task": "Expect redirect_uri missing error page",
"match": "https://fapi.panva.cz/auth*",
"commands": [
[
"wait",
"xpath",
"//*",
10,
"oops! something went wrong",
"update-image-placeholder"
]
]
}
]
}
]
},
"fapi-rw-id2-user-rejects-authentication": {
"browser": [
{
Expand Down
27 changes: 24 additions & 3 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -925,9 +925,9 @@ _**default value**_:
[Financial-grade API - Part 2: Read and Write API Security Profile](https://openid.net/specs/openid-financial-api-part-2-ID2.html)

Enables extra behaviours defined in FAPI Part 1 & 2 that cannot be achieved by other configuration options, namely:
- Request Object "exp" claim is required
- Request Object must contain all parameters that are also sent as regular parameters
- userinfo endpoint becomes a FAPI resource, echoing back the x-fapi-interaction-id header and disabling query string as a mechanism for providing access tokens
- Request Object `exp` claim is REQUIRED
- `userinfo_endpoint` becomes a FAPI resource, echoing back the x-fapi-interaction-id header and disabling query string as a mechanism for providing access tokens



_**default value**_:
Expand All @@ -936,6 +936,27 @@ _**default value**_:
enabled: false
}
```
<a name="features-fapi-rw-other-configuration-needed-to-reach-fapi-levels"></a><details>
<summary>(Click to expand) other configuration needed to reach FAPI levels
</summary>
<br>


- `clientDefaults` for setting different default client `token_endpoint_auth_method`
- `clientDefaults` for setting different default client `id_token_signed_response_alg`
- `clientDefaults` for setting different default client `response_types`
- `clientDefaults` for setting client `tls_client_certificate_bound_access_tokens` to true
- `features.mTLS` and enable `certificateBoundAccessTokens`
- `features.mTLS` and enable `selfSignedTlsClientAuth` and/or `tlsClientAuth`
- `features.claimsParameter`
- `features.requestObjects` and enable `request` and/or `request_uri`
- `features.requestObjects.mergingStrategy.name` set to `strict`
- `whitelistedJWA`
- (optional) `features.pushedRequestObjects`
- (optional) `features.jwtResponseModes`


</details>

### features.frontchannelLogout

Expand Down
20 changes: 0 additions & 20 deletions lib/actions/authorization/process_request_object.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const isPlainObject = require('lodash/isPlainObject');

const formatters = require('../../helpers/formatters');
const JWT = require('../../helpers/jwt');
const instance = require('../../helpers/weak_cache');
const { InvalidRequest, InvalidRequestObject, OIDCProviderError } = require('../../helpers/errors');
Expand Down Expand Up @@ -146,25 +145,6 @@ module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMidd
if (!('exp' in payload)) {
throw new InvalidRequestObject('Request Object is missing the "exp" claim');
}

const missing = Object.entries(ctx.oidc.params)
.filter(([key, value]) => {
if (key === 'request' || key === 'request_uri' || value === undefined) {
return false;
}

return true;
}).map(([key]) => {
if (!(key in payload)) {
return key;
}

return undefined;
}).filter(Boolean);

if (missing.length) {
throw new InvalidRequestObject(`missing ${formatters.pluralize('parameter', missing.length)} ${formatters.formatList(missing)} in the Request Object`);
}
}

try {
Expand Down
20 changes: 17 additions & 3 deletions lib/helpers/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -727,10 +727,24 @@ const DEFAULTS = {
* description: Enables extra behaviours defined in FAPI Part 1 & 2 that cannot be achieved by
* other configuration options, namely:
*
* - Request Object "exp" claim is required
* - Request Object must contain all parameters that are also sent as regular parameters
* - userinfo endpoint becomes a FAPI resource, echoing back the x-fapi-interaction-id header
* - Request Object `exp` claim is REQUIRED
* - `userinfo_endpoint` becomes a FAPI resource, echoing back the x-fapi-interaction-id header
* and disabling query string as a mechanism for providing access tokens
*
* example: other configuration needed to reach FAPI levels
*
* - `clientDefaults` for setting different default client `token_endpoint_auth_method`
* - `clientDefaults` for setting different default client `id_token_signed_response_alg`
* - `clientDefaults` for setting different default client `response_types`
* - `clientDefaults` for setting client `tls_client_certificate_bound_access_tokens` to true
* - `features.mTLS` and enable `certificateBoundAccessTokens`
* - `features.mTLS` and enable `selfSignedTlsClientAuth` and/or `tlsClientAuth`
* - `features.claimsParameter`
* - `features.requestObjects` and enable `request` and/or `request_uri`
* - `features.requestObjects.mergingStrategy.name` set to `strict`
* - `whitelistedJWA`
* - (optional) `features.pushedRequestObjects`
* - (optional) `features.jwtResponseModes`
*/
fapiRW: { enabled: false },

Expand Down
2 changes: 1 addition & 1 deletion lib/helpers/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const DRAFTS = new Map(Object.entries({
name: 'Financial-grade API - Part 2: Read and Write API Security Profile',
type: 'OIDF FAPI Working Group draft',
url: 'https://openid.net/specs/openid-financial-api-part-2-ID2.html',
version: 'id02',
version: 'id02-rev.2',
},
mTLS: {
name: 'OAuth 2.0 Mutual TLS Client Authentication and Certificate-Bound Access Tokens - draft 17',
Expand Down
5 changes: 4 additions & 1 deletion test/fapirw/fapirw.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ const config = clone(require('../default.config'));

config.features = {
fapiRW: { enabled: true },
request: { enabled: true },
requestObjects: {
request: true,
mergingStrategy: { name: 'strict' },
},
};
config.whitelistedJWA = {
requestObjectSigningAlgValues: ['none'],
Expand Down
31 changes: 0 additions & 31 deletions test/fapirw/fapirw.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,36 +98,5 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha
.expect(auth.validateError('invalid_request_object'))
.expect(auth.validateErrorDescription('Request Object is missing the "exp" claim'));
});

it('requires all parameters to be present inside the signed Request Object', function () {
const request = `${base64url.encode(JSON.stringify({ alg: 'none' }))}.${base64url.encode(JSON.stringify({
client_id: 'client',
scope: 'openid',
response_type: 'id_token',
exp: epochTime() + 60,
}))}.`;

const auth = new this.AuthorizationRequest({
request,
scope: 'openid',
client_id: 'client',
response_type: 'id_token',
nonce: 'foo',
});

return this.wrap({
agent: this.agent,
route: '/auth',
verb: 'get',
auth,
})
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['error', 'error_description', 'state']))
.expect(auth.validateState)
.expect(auth.validateClientLocation)
.expect(auth.validateError('invalid_request_object'))
.expect(auth.validateErrorDescription("missing parameters 'nonce', 'redirect_uri', and 'state' in the Request Object"));
});
});
});

0 comments on commit 8b927fc

Please sign in to comment.