openid-certification / oidctest

THE CERTIFICATION TEST SUITE HAS BEEN MIGRATED TO A NEW SERVICE https://www.certificatinon.openid.net
Other
49 stars 15 forks source link

OP-redirect_uri-Query-Mismatch not registering with query component #189

Closed techdad24 closed 4 years ago

techdad24 commented 4 years ago

The OP-redirect_uri-Query-Mismatch is supposed to check that when a query component is part of the registered uri that it matches when a query component is used later on to request an access token. But during registration the query component is not included as part of the uri in the request_uris array.

zandbelt commented 4 years ago

Thanks for spotting this, it looks indeed due to a typo in https://github.com/openid-certification/oidctest/blob/master/test_tool/cp/test_op/flows/OP-redirect_uri-Query-Mismatch.json#L16 where

        "redirect_uri_with_query_component": {

should be

        "redirect_uris_with_query_component": {

as in https://github.com/openid-certification/oidctest/blob/master/test_tool/cp/test_op/flows/OP-redirect_uri-Query-OK.json#L34 since the syntax differers between the Registration and the Authorization phase.

zandbelt commented 4 years ago

for the record: this typo effectively makes OP-redirect_uri-Query-Mismatch a duplicate of OP-redirect_uri-Query-Added

techdad24 commented 4 years ago

Will this be fixed in the near term?

On Fri, Oct 4, 2019, 4:08 PM Hans Zandbelt notifications@github.com wrote:

for the record: this typo effectively makes OP-redirect_uri-Query-Mismatch a duplicate of OP-redirect_uri-Query-Added

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openid-certification/oidctest/issues/189?email_source=notifications&email_token=AEHEOJZ3WQDL7ITCFQG44ATQM7EHRA5CNFSM4I5TUE22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANDDTI#issuecomment-538587597, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHEOJ6T6MAVV37HECGUGJ3QM7EHRANCNFSM4I5TUE2Q .

zandbelt commented 4 years ago

I don't think it is breaking anything at this point: it is just a test that doesn't quite do what it does but also does not result in a spec violation or non-interoperability so I don't see a reason to expedite it; let us know if you see this differently

techdad24 commented 4 years ago

No, that's fine. I appreciate the tests, helped find some flaws in my OAuth engine that needed fixing.

On Sat, Oct 5, 2019, 7:39 AM Hans Zandbelt notifications@github.com wrote:

I don't think it is breaking anything at this point: it is just a test that doesn't quite do what it does but also does not result in a spec violation or non-interoperability so I don't see a reason to expedite it; let us know if you see this differently

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openid-certification/oidctest/issues/189?email_source=notifications&email_token=AEHEOJ6Y546C4GXMFZWKK63QNCRLHA5CNFSM4I5TUE22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANTWEY#issuecomment-538655507, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHEOJYGOOKICJBQW6FLZQTQNCRLHANCNFSM4I5TUE2Q .

selfissued commented 4 years ago

Yep - these tests are there to catch OAuth redirect_uri handling that doesn't always perform exact matching. One of the failure modes we wanted to check for was that the matching might not include the query parameters.

Glad these are helping you find flaws in your OAuth code.

selfissued commented 4 years ago

Hans will fix this, per the 11-Oct-19 call

zandbelt commented 4 years ago

hmm, we may have to rollback that change because things are apparently more messed up now [1]

  1. the tool tries to start registration with a bunch of parameters that look OK, i.e. redirect_uri set to https://op-test:60001/authz_cb?foo=bar' in phase 0
  2. the actual registration request JSON however, does not contain the query parameter, https://op-test:60001/authz_cb
  3. at authentication request time there's both a redirect_uri and a redirect_uris parameter in the request... redirect_uri=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb&redirect_uris=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb%3Fbar%3Dfoo

@rohe do you know what is going on? should we just rollback and forget about this fix for now?

0 phase <--<-- 2 --- Registration -->-->
0 register kwargs:{'response_types': ['code'], 'grant_types': ['authorization_code'], 'application_name': 'OIC test tool', 'application_type': 'web', 'redirect_uris': ['https://op-test:60001/authz_cb'], 'contacts': ['roland@example.com'], 'post_logout_redirect_uris': ['https://op-test:60001/logout'], 'url': 'https://op:4433/reg', 'jwks_uri': 'https://op-test:60001/static/jwks_60001.json', 'token_endpoint_auth_method': 'client_secret_basic', 'redirect_uri': ['https://op-test:60001/authz_cb?foo=bar']}
0 RegistrationRequest { "application_type": "web", "contacts": [ "roland@example.com" ], "grant_types": [ "authorization_code" ], "jwks_uri": "https://op-test:60001/static/jwks_60001.json", "post_logout_redirect_uris": [ "https://op-test:60001/logout" ], "redirect_uris": [ "https://op-test:60001/authz_cb" ], "request_uris": [ "https://op-test:60001/requests/1fd6f4901e6d91a9a7f30f782fb992c268a2075257c007c6bf0cf266061ff7db#yddwH1xc24kV6dGZ" ], "response_types": [ "code" ], "token_endpoint_auth_method": "client_secret_basic" }
0 http response url:https://op:4433/reg status_code:201
0 RegistrationResponse { "application_type": "web", "backchannel_logout_session_required": false, "client_id": "de9NTUCgXJP7k27daVzQ5", "client_id_issued_at": 1572858337, "client_secret": "-XSN7HXCNb6KAiWUyIHkb9wYXsl63vhk28eKW51XIN6xs1mmEp9hsBkCpKww4pB5BY-Gyt7t5lURr-fKGB2Lgw", "client_secret_expires_at": 0, "contacts": [ "roland@example.com" ], "frontchannel_logout_session_required": false, "grant_types": [ "authorization_code" ], "id_token_signed_response_alg": "RS256", "introspection_endpoint_auth_method": "client_secret_basic", "jwks_uri": "https://op-test:60001/static/jwks_60001.json", "post_logout_redirect_uris": [ "https://op-test:60001/logout" ], "redirect_uris": [ "https://op-test:60001/authz_cb" ], "registration_access_token": "qZtAu9OmTmOzCjW3YJc7yvjF4L153JF6NH0rcwQL8mY", "registration_client_uri": "https://op:4433/reg/de9NTUCgXJP7k27daVzQ5", "request_uris": [ "https://op-test:60001/requests/1fd6f4901e6d91a9a7f30f782fb992c268a2075257c007c6bf0cf266061ff7db#yddwH1xc24kV6dGZ" ], "require_auth_time": false, "response_types": [ "code" ], "revocation_endpoint_auth_method": "client_secret_basic", "subject_type": "public", "token_endpoint_auth_method": "client_secret_basic", "web_message_uris": [] }
0 phase <--<-- 3 --- Note -->-->
3 phase <--<-- 4 --- AsyncAuthn -->-->
3 AuthorizationRequest { "client_id": "de9NTUCgXJP7k27daVzQ5", "nonce": "6MXEOsSr8GQOyUK1", "redirect_uri": "https://op-test:60001/authz_cb", "redirect_uris": [ "https://op-test:60001/authz_cb?bar=foo" ], "response_type": "code", "scope": "openid", "state": "Pi1O1DCjIpVKNM7v" }
3 redirect url https://op:4433/auth?state=Pi1O1DCjIpVKNM7v&nonce=6MXEOsSr8GQOyUK1&response_type=code&scope=openid&redirect_uri=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb&redirect_uris=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb%3Fbar%3Dfoo&client_id=de9NTUCgXJP7k27daVzQ5
3 redirect https://op:4433/auth?state=Pi1O1DCjIpVKNM7v&nonce=6MXEOsSr8GQOyUK1&response_type=code&scope=openid&redirect_uri=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb&redirect_uris=https%3A%2F%2Fop-test%3A60001%2Fauthz_cb%3Fbar%3Dfoo&client_id=de9NTUCgXJP7k27daVzQ5
12 response Response URL with query part
12 response {'code': 'rk4Dzb9B9pPqfZKhcizjnzQDRgixqbChBi4WMU-Fv3C', 'state': 'Pi1O1DCjIpVKNM7v', 'session_state': '4fHwsmNimjaDnNQyGyxzbwe89IJcf7b4TXQ7cfjVzy0'}
12 response {'code': 'rk4Dzb9B9pPqfZKhcizjnzQDRgixqbChBi4WMU-Fv3C', 'state': 'Pi1O1DCjIpVKNM7v', 'session_state': '4fHwsmNimjaDnNQyGyxzbwe89IJcf7b4TXQ7cfjVzy0'}
12 AuthorizationResponse { "code": "rk4Dzb9B9pPqfZKhcizjnzQDRgixqbChBi4WMU-Fv3C", "session_state": "4fHwsmNimjaDnNQyGyxzbwe89IJcf7b4TXQ7cfjVzy0", "state": "Pi1O1DCjIpVKNM7v" }
12 phase <--<-- 5 --- Done -->-->
12 end  
12 assertion VerifyResponse
12 condition verify-response: status=ERROR, message=Got a AuthorizationResponse response !? [Checks that the last response was one of a possible set of OpenID Connect Responses]
12 condition Done: status=OK
rohe commented 4 years ago

Rolling back this specific fix would probably be a good thing. If you do that, I'll promise to look at the original issue later today.

rohe commented 4 years ago

If you use redirect_uris_with_query_component for the registration and redirect_uri_with_query_component for AsyncAuth then everything seems to work. Like this

{ "Registration": { "redirect_uris_with_query_component": { "foo": "bar" } } }, "Note", { "AsyncAuthn": { "redirect_uri_with_query_component": { "bar": "foo" }, "set_response_where": null } }

zandbelt commented 4 years ago

ok, thanks, I issued a PR for that

Hans.

On Mon, Nov 4, 2019 at 2:41 PM Roland Hedberg notifications@github.com wrote:

If you use redirect_uris_with_query_component for the registration and redirect_uri_with_query_component for AsyncAuth then everything seems to work. Like this

{ "Registration": { "redirect_uris_with_query_component": { "foo": "bar" } } }, "Note", { "AsyncAuthn": { "redirect_uri_with_query_component": { "bar": "foo" }, "set_response_where": null } }

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/openid-certification/oidctest/issues/189?email_source=notifications&email_token=AAR3IJ2WAG3XQICEEINFOFLQSARCLA5CNFSM4I5TUE22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7ISZY#issuecomment-549357927, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR3IJ472EBNJGYW2TXKIEDQSARCLANCNFSM4I5TUE2Q .

-- hans.zandbelt@zmartzone.eu ZmartZone IAM - www.zmartzone.eu

rohe commented 4 years ago

So this could be closed then ?!