Repository navigation
Conversation
e6f26ff to
9903564
Compare
da5f963 to
8615527
Compare
|
@tdiesler Thanks for the PR. I see the potential issues with the PR:
In general, this stuff (client attribute and complex logic) is done just because of the conformance testsuite. As in production, the client developers don't care much whether the error page is shown or whether there is redirect. If the client developer use incorrect parameter, he would be able to quickly figure it (regardless of whether there is error-page or redirect) and fix his client to use correct parameters (redirect_uri, response_type, response_mode or others). TBH I would like to have more knowledge of what exactly is the problem, which we are trying to solve? AFAIK it is the fact that conformance testsuite is showing error page instead of redirect to client, which is harder to automate and test is not shown as "Green" in the results. Am I correct? For the perspective of the certification conformance, the tests do not always need to be green. The state "review" is completely fine for the error tests to pass the certification. For example, Keycloak is certified FAPI provider. From this page with the certified providers [1] , you can click on the "View" link on the first item of Keycloak, which is this link [2]. It shows the successful certification results. As you can see, not all the tests were "Green", but there were some tests in "Review" . And having those tests in "Review" is completely fine for the certification. The test just needs to upload the image with the screenshot of the error page. Some example is this one [3] . I am thinking if it is possible to automate the conformance testsuite with the tests in the "Review" state? Basically, the automated test can perform the screenshot, upload it, and continue to the next test. Would it be possible? [1] https://openid.net/certification/certified-fapi1-adv-openid-providers-profiles/ |
…request-object-not-used closes keycloak#48047 Signed-off-by: Thomas Diesler <tdiesler@proton.me>
closes keycloak#48542 Signed-off-by: Thomas Diesler <tdiesler@proton.me>
|
Ok, lets try a different approach. I propose to leave the authorization handling unchanged and first collect all conditions were it would not work in headless environments, one of which being the automated conformance tests. The first such error condition is described here. PR #49788 now contains the changes from this PR to make When all error conditions are collected in that way, we likely have a better view of what is actually needed to improve headless authorization. Let's skip these changes for now and perhaps revisit later 8000 . |
closes #48542
While processing an authorization request there are potential errors coming from
most of these errors map to an unchecked ErrorPageException, which then displays an html error page.
When the request passes the above, errors tend to be reported back to the caller via redirect_uri already.
This PR proposes to first catch these ErrorPageExceptions and then (when configured to do so) make a "best effort" to use the redirect_uri path - otherwise, the error is returned to the caller as json object.
Without the
authorization.preferErrorOnRedirectattribute on the client, this PR changes nothing. As a result, no test cases needed to be touched.Automated conformance tests are headless - they prefer errors to be delivered on the redirect_uri when possible.
depends on