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

Extra tests configuration option shouldn't be required to make logout tests appear #192

Closed selfissued closed 4 years ago

selfissued commented 4 years ago

Currently, only the Discovery OP logout tests appear unless "extra tests" is selected in the configuration. To see this yourself, compare the logout tests shown at https://new-op.certification.openid.net:60021/ (which doesn't include extra tests) and those shown at https://new-op.certification.openid.net:60019/ (which does include them).

We need to change the test visibility criteria for the OP logout tests so that they always appear - just like the four logout Discovery tests do.

Note that I do not want us to block production release for this issue. Until it's fixed, I'll handle it in the instructions.

rohe commented 4 years ago

So, as I wrote on the certification mailing list this will lead to 2 things.

1) The persons doing the testing will have to decide which tests they don't think apply to them. Just assuming that all presented tests should be executed is not enough. This is not just for the ones that want to test logout functionality but for everyone. They will also have to submit test protocols that are 'incomplete'. Where some of the tests that are listed for them (depending on their choice) are just missing.

2) The person/-s evaluating test result submissions will have to look at the submitted information and understand which test should be in there and which it's ok if they are missing.

I foresee extra work for the evaluators and the support team. If this is an interim thing it might be OK. I think adding code to allow choosing OP logout tests like you do for form_post and encryption support is not a big deal.

selfissued commented 4 years ago

Roland, I agree with you that the logout tests shouldn't always appear, as that would violate the UI and simplicity principle we've had to date, that people testing for Basic, Implicit, or Hybrid can simply run all the tests that they see and then submit the results.

A related problem I didn't call out is that the four logout discovery tests are currently always shown. People running them without supporting them will then have FAILED results in their logs, which will cause problems evaluating submissions. So that has to be fixed too.

The right way to fix this is to have a "logout" checkbox - just like there's a "form post" check box.

rohe commented 4 years ago

Actually I think there should be 3 checkboxes. One for each document (Session Management, Front-Channel and Back-Channel)

selfissued commented 4 years ago

Sounds good. So each of the checkboxes would also enable the RP-initiated logout tests, correct?

rohe commented 4 years ago

That is a decision we have to make. In my first attempt I placed the RP-initiated logout tests together with the session management tests. Because RP-initiated logout is defined in that document.

selfissued commented 4 years ago

As we decided during the logout tests design, there are four profiles: Session Management Logout, Front-Channel Logout, Back-Channel Logout, and RP-Initiated Logout. These are already described in the public logout testing instructions at https://openid.net/certification/logout-op-testing/, as is the relationship between RP-Initiated Logout and the other three.

As a practical matter, you have to have RP-Initiated Logout to test any of the other three, so I'd suggest turning on these tests when you turn on any of the three OP-initiated logout profiles.

rohe commented 4 years ago

OK

rohe commented 4 years ago

Support for this is now implemented in otest. To really make it available to testers changes may have to be made to the UI.

zandbelt commented 4 years ago

https://github.com/openid-certification/otest/commit/4dd6cd7035ea563be949ae084a2435c7a22c1e1a