-
Notifications
You must be signed in to change notification settings - Fork 5
State‐Parameter‐Saga
We expect RelyingParties to URL-encode query parameters prior to sending them to the OIDC Authorization endpoint. The query parameters are decoded by the Spring Boot framework used by OIDC and are URL-encoded again in the redirect URL response back to RelyingParty.
In the 6.1.10 release prior to any changes in the decoding / encoding algorithm, the following behaviour was observed:
| Raw parameter | Parameter encoded by the RP | Decoded parameter in OIDC-NG | Encoded return parameter |
|---|---|---|---|
https://example.com |
https%3A%2F%2Fexample.com |
https://example.com |
https://example.com |
It appears the return parameter is not encoded, but this caused by the use of the Spring org.springframework.web.util.UriComponentsBuilder. The UriComponentsBuilder
does not encode : and / characters. UriComponentsBuilder is encoding the URI in accordance with RFC 3986 (see , in particular
section 3.4 which is about the 'query' component of a URI). Within the query component, the characters / and : are permitted, and do not need escaping.
UriComponentsBuilder.fromUriString("/").queryParam("state", "https://example.com?key=value").toUriString();
// returns /?state=https://example.com?key%3Dvalue
Note the URL-encoding of key=value, but not the https://example.com part.
This is however not very intuitive, as other URL encode / decode libraries do escape the / and : characters. See the following Node example:
$ node
> encodeURIComponent("https://example.com")
'https%3A%2F%2Fexample.com'
and Java default library example
jshell> URLEncoder.encode("https://example.com", Charset.defaultCharset());
$1 ==> "https%3A%2F%2Fexample.com"
The correct fix for State parameter with possible url encoding is decoded when the user is redirected to the RP would have been
to NOT use the UriComponentsBuilder, but the default URLEncoder to encode the query parameters when constructing the redirect URL in the Authorization endpoint.
Mistakenly the fix applied, was to skip the encoding of the state parameter overall in this commit.
Resulting in this behaviour:
| Test parameter | Parameter encoded by RestAssured test framework | Decoded parameter in OIDC-NG | Return parameter (not encoded) |
|---|---|---|---|
https%3A%2F%2Fexample.com |
https%253A%252F%252Fexample.com |
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
The fix looks ok, but was indeed not, as the RestAssured test framework encodes the query parameters also by default, resulting in double encoding, masking the bug. Re-running the integration test against this version of OICD-NG with single encoding reveales the error.
return given().redirects().follow(false)
.urlEncodingEnabled(false)
.when()
.header("Content-type", "application/json")
.queryParams(queryParams)
.get("oidc/authorize");
Resulting in:
| Test parameter | Parameter NOT encoded by RestAssured test framework | Decoded parameter in OIDC-NG | Return parameter (not encoded) |
|---|---|---|---|
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https://example.com |
https://example.com |
The above results were deployed in version 6.1.11 and this broke the login flow of some Relying Parties, as they send the parameters encoded - which is correct - but got them back not encoded again (e.g. still decoded).
The following fix was applied to ensure the state parameter was not decoded and also not encoded. See Do not decode / encode state parameter in redirectURI. This resulted in the following behaviour:
| Test parameter | Parameter NOT encoded by RestAssured test framework | Parameter not decoded in OIDC-NG | Return parameter (not encoded) |
|---|---|---|---|
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
Although technically correct, this provided a challenge to existing Relying Parties who send the state parameter encoded, but actually already worked around the old previous issue that it came back still decoded (e.g. before release 6.1.11.)
To revert and fix the situation in release 6.1.12, we introduced a feature toggle described in Undo state parameter encoding implemented in
this commit. Relying Parties can be configured with the option oidc:state_parameter_decoding_disabled. When this option is set to True then the behaviour of release 6.1.12 is applied, otherwise the behaviour of release 6.1.11 is applied (the default). However this did not solve the underlying problem of the 6.1.11 release, where the query parameters were not encoded when send back to the RP in the redirect URL response of the Authorization endpoint:
state_parameter_decoding_disabled |
Test parameter | Parameter NOT encoded by RestAssured test framework | Parameter in OIDC-NG | Return parameter |
|---|---|---|---|---|
| False (default) | https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https://example.com |
https://example.com |
| True | https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
The actual correct implementation is the old situation in release 6.1.10, but with the correct bug-fix applied. The Spring boot framework used in OIDC-NG decodes all query parameters and subsequently we use the default Java URLEncoding algorithm to encode the parameters before we redirect to the Relying Party
state_parameter_decoding_disabled |
Test parameter | Parameter NOT encoded by RestAssured test framework | Parameter in OIDC-NG | Return parameter standard encoding |
|---|---|---|---|---|
| Not Used | https%3A%2F%2Fexample.com |
https%3A%2F%2Fexample.com |
https://example.com |
https%3A%2F%2Fexample.com |
If a Relying Party does not encode the query parameters and the request is accepted (e.g. it does not contain forbidden characters like {), then the return paramaters are encode. As we do expect the RelyingParty to properly decode them, this should not pose a problem, but do note the input and output parameters are not equal then