search

rohe / oidctest

Test framework for testing OpenID Connect entities
Other
29 stars 27 forks source link

add OP-Response-token_type test, see: openid-certification/oidctest#89 #108

Closed zandbelt closed 6 years ago

rohe commented 6 years ago

That the Travis build failed is that for you or Filip to fix ?

zandbelt commented 6 years ago

Yes, that's for us to fix: it is because I've added a new test for which no CI test exists yet in https://github.com/openid-certification/openid-client-conformance-tests

@panva : I guess adding the CI test before OP-Response-token_type is merged would also lead to errors?

panva commented 6 years ago

Nop. Having a test extra doesnt matter. Not having it does :)

panva commented 6 years ago

Why does this need to be a new test anyway? It's testing behaviour that should be present in every test hitting the authorization or token endpoints and getting an access_token.

Same as with strictly checking response_mode, getting the required parameters back should be a part of the core function that's e.g. parsing these responses.

edit: 🤦‍♂️ at myself

zandbelt commented 6 years ago

because somebody thought it was better to do so: https://github.com/openid-certification/oidctest/issues/89#issuecomment-389909152

...

panva commented 6 years ago

Well. Yeah :( hahaha

zandbelt commented 6 years ago

But I'm open to reconsider

panva commented 6 years ago

Up to you, really. I've merged the missing test definition

zandbelt commented 6 years ago

On 2nd thought I think I favor your 2nd line of reasoning :-)

It would not make sense for a certification suite to allow non-conformant requests like regular "OP-Response-code" to pass if there's no token_type just because it tests for something else. Every response should have it and thus should be checked an no explicit test need to exist. Please ignore this PR and @panva you my revert the commit that added the token_type test into the conformance tests.

panva commented 6 years ago

For interop, shall we do a warning when expires_in is missing too? Its RECOMMENDED