Wrong JSONObject type when decoding claims from Signed JAR Authentication request
Description
Environment
Activity
Henri MikkonenMarch 11, 2022 at 7:20 AM
Updated the DefaultRequestedClaimsLookupFunction to check if the claims object is a Map. Also improved unit testing (including authorise flow test) to make sure that it now works as expected.
Thanks for the very detailed report!
Henri MikkonenMarch 10, 2022 at 11:31 AM
This problem could be reproduced also with the plugin’s 3.1 -codebase, involving newer version of Nimbus libraries.
As noted by Simone, JSONObject (shaded or not) extends a Map. One and possibly the simplest approach would be to change DefaultRequestedClaimsLookupFunction into checking if the claims object is a Map, and in that case build a non-shaded JSONObject (net.minidev.json.JSONObject) from it. There's an existing constructor in JSONObject for that. The newly constructed JSONObject could then be fed for the IDCClaimsRequest.parse(..) method.
Simone AvogadroMarch 10, 2022 at 9:25 AM
Makes sense, passing thru a String also other positive side-effects, maybe some additional considerations:
the current com.nimbusdds.jose.shaded.json.JSONObject is a Map<K,V>
.toString should properly serialize the object and this is usually true in other JSON implementations
using the standard Nimbusds libs to compose the signed request will place the claims inside a string (which is indeed a representation of the JSON) instead of using a JSONObject
"claims":"{\"id_token\":{\"given_name\":{\"essential\":true}},\"userinfo\":{\"family_name\":{\"essential\":true}}}"instead of an object as we’d expect
"claims":{"id_token":{"given_name":{"essential":true}},"userinfo":{"family_name":{"essential":true}}}
Scott CantorMarch 9, 2022 at 10:21 PM
It’s a problem that we have exposure to those objects at all because their API isn’t clean. If they’re exposing a shaded class, that would be tough to count on, I doubt any of that is well-defined or stable. We might need to somehow route it into a string form so we could control the JSON type involved.
Simone AvogadroMarch 9, 2022 at 10:14 PMEdited
the class does
whereas the claims are extracted as:
.getJWTClaimsSet() returns an object of type org.nimbus.jwt.JWTClaimsSet thus it might make sens that these object return com.nimbusds…JSONObject rather then net.minidev.json.JSONObject
whiel experimenting with a custom client which used signed JAR requests to start authentication the claims seems to be ignored with this message:
Debugging the code seems to drill this down to: net.shibboleth.idp.plugin.oidc.op.profile.context.navigate.DefaultRequestedClaimsLookupFunction:48
which tests “claims instanceof JSONObject” however:
the claims Object is of class: com.nimbusds.jose.shaded.json.JSONObject
whereas the JSONObject being tested is: net.minidev.json.JSONObject
here’s an example request (before being signed and incapsulated in threquest parameters):
{"iss":"demo_rp","response_type":"code","code_challenge_method":"S256","nonce":"-vFn_CJUfNC7FqJiXUI8Bj6YU16NhfRXZWqFfWsl6Vs","client_id":"demo_rp","aud":"https://op.mindmercatis.com","scope":"openid profile offline_access","acr_values":"https://www.spid.gov.it/SpidL1","claims":{"id_token":{"given_name":{"essential":true}},"userinfo":{"family_name":{"essential":true}}},"redirect_uri":"https://rp.mindmercatis.com/mockws/rp/postauth","state":"81c33d57-59c7-4b41-9a15-80e2ed1482e21646856869080","prompt":"consent","code_challenge":"UBkBqkYTEU0OIFf6T8F8dXWMWhEq_rWa-GXxKk1pLpY"}
here’s the actual auth request complete URL:
https://op.mindmercatis.com/idp/profile/oidc/authorize?response_type=code&request=eyJraWQiOiJkZWZhdWx0UlNBU2lnbiIsImFsZyI6IlJTMjU2In0.eyJpc3MiOiJkZW1vX3JwIiwicmVzcG9uc2VfdHlwZSI6ImNvZGUiLCJjb2RlX2NoYWxsZW5nZV9tZXRob2QiOiJTMjU2Iiwibm9uY2UiOiJrNXItVXdqdzBLS3IxOFhpS0QyVmJpTHREMmFkd3Q4NV9IaVN2ekJpOEZJIiwiY2xpZW50X2lkIjoiZGVtb19ycCIsImF1ZCI6Imh0dHBzOlwvXC9vcC5taW5kbWVyY2F0aXMuY29tIiwic2NvcGUiOiJvcGVuaWQgcHJvZmlsZSBvZmZsaW5lX2FjY2VzcyIsImFjcl92YWx1ZXMiOiJodHRwczpcL1wvd3d3LnNwaWQuZ292Lml0XC9TcGlkTDEiLCJjbGFpbXMiOnsiaWRfdG9rZW4iOnsiZ2l2ZW5fbmFtZSI6eyJlc3NlbnRpYWwiOnRydWV9fSwidXNlcmluZm8iOnsiZmFtaWx5X25hbWUiOnsiZXNzZW50aWFsIjp0cnVlfX19LCJyZWRpcmVjdF91cmkiOiJodHRwczpcL1wvcnAubWluZG1lcmNhdGlzLmNvbVwvbW9ja3dzXC9ycFwvcG9zdGF1dGgiLCJzdGF0ZSI6IjgxYzMzZDU3LTU5YzctNGI0MS05YTE1LTgwZTJlZDE0ODJlMjE2NDY4NTczNDk1MzciLCJwcm9tcHQiOiJjb25zZW50IiwiY29kZV9jaGFsbGVuZ2UiOiJNaUFSLVV4Q2o2b1Z5UGF0Y1VucmIzTUdFWmJ3TEtCbUlSU29PS0xMVGwwIn0.BLkQxZKCldZ3plZoCcZzSnhzyJRDrKJLN7q1hEy1eIdybKYdallVNxZa-OHGVzrJhlH-H1OO0erve-CYyIgLXl4uIiHg2PZ4o3S3sIlcADDrkd_1nrmrou6hQ5aq1G-joVO3Th5kNMhZd37JDdFLgKgyIzkr0T4elD-gNmkqDsy6xR-8zWHGIfRCSrzxgGnnex0ROV7EDcgtCWb3YCQEDDnJrMygvMQ1Xix-r7TO4mSIbqBBbx4oQEcvQVSMfE_mBLAgBg8T4GSRO1mZjaf4n9vAdd500Db5NoNnc4R0s61BSqkdQdc6CR7wW1nOglsGnYeICUJ8uendLkxMsOlWOw&redirect_uri=https%3A%2F%2Frp.mindmercatis.com%2Fmockws%2Frp%2Fpostauth&client_id=demo_rp&scope=openid+profile+offline_access