fix: add redirect_after_logout_uri for ODIC that do not have an end_session_endpoint by luoluoyuyu · Pull Request #10653 · apache/apisix

luoluoyuyu · 2023-12-15T00:29:07Z

Description

Problem Analysis

When the openid service does not support end_session_endpoint, the ODIC logout fails to redirect to post_logout_redirect_uri. this issue is caused by the OpenID standard. end_session_endpoint does not necessarily need to be implemented.

Solution

Use the redirect_after_logout_uri defined in lua-resty-openidc for redirection when the OpenID service does not support end_session_endpoint.

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

…ssion_endpoint

Sn0rt · 2023-12-15T03:37:48Z

new test for cover
user doc

luoluoyuyu · 2023-12-15T04:00:27Z

user doc

It's not necessary to add documentation, this PR is just to support services like Google that don't provide the end_session_endpoint configuration, and OIDC can also correctly using the post_logout_redirect_uri configuration.

luoluoyuyu · 2023-12-15T04:16:06Z

new test for cover

Auth0 now supports end_session_endpoint, now only found google openid service does not support end_session_endpoint, if you want to add test cases for google need to google account password, I think add test cases is not appropriate.
Here is the OpenID service that supports end_session_endpoint that I have organized:
Okta: https://developer.okta.com/docs/reference/api/oidc/#response-properties-10
Azure: https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc#single-sign-out
Auth0: https://auth0.com/docs/authenticate/login/logout/log-users-out-of-auth0#endpoint-discovery
Curity: https://curity.io/resources/learn/openid-connect-logout/#rp-initiated-logout
Connect2id: https://connect2id.com/products/nimbus-oauth-openid-connect-sdk#specs
Gluu: https://gluu.org/docs/gluu-server/4.3/api-guide/openid-connect-api/#openid-connect-end-session-api
IdentityServer: https://docs.duendesoftware.com/identityserver/v6/ui/logout/session_cleanup/#prompting-the-user-to-logout

moonming

can not merge without test cases

moonming · 2023-12-22T09:37:14Z

+
+
+
+=== TEST 35: Check whether outh0 can redirect normally using post_logout_redirect_uri configuration


…isix into ODIC_redirect_after_logout

shreemaan-abhishek

great job

kayx23 · 2023-12-27T01:22:30Z

@luoluoyuyu a question for the below:

What if someone configures both post_logout_redirect_uri and redirect_after_logout_uri?

https://github.com/zmartzone/lua-resty-openidc/blob/master/tests/spec/logout_spec.lua#L264-L299

luoluoyuyu · 2023-12-28T04:32:55Z

@luoluoyuyu a question for the below:

What if someone configures both post_logout_redirect_uri and redirect_after_logout_uri?

https://github.com/zmartzone/lua-resty-openidc/blob/master/tests/spec/logout_spec.lua#L264-L299

Hi @kayx23 'redirect_after_logout_uri' is not defined in the schema. If 'redirect_after_logout_uri' is configured, an error will occur. apisix/apisix/plugins/openid-connect.lua at master · apache/apisix (github.com)

kayx23 · 2024-01-02T22:43:40Z

If 'redirect_after_logout_uri' is configured, an error will occur.

Not really true as previously shown.

However I will remove redirect_after_logout_uri from the new doc given this change.

luoluoyuyu added 3 commits December 15, 2023 08:22

fix: add post_logout_redirect_uri for ODIC that do not have an end_se…

44ffff5

…ssion_endpoint

fix restart apisix check schema failure

fa7da23

add log

0f71dc2

luoluoyuyu added 2 commits December 19, 2023 12:30

update doc

719be44

fix document format

2a08fd7

juststillthinking reviewed Dec 21, 2023

View reviewed changes

Comment thread apisix/plugins/openid-connect.lua Outdated

juststillthinking reviewed Dec 21, 2023

View reviewed changes

Comment thread apisix/plugins/openid-connect.lua Outdated

juststillthinking reviewed Dec 21, 2023

View reviewed changes

Comment thread apisix/plugins/openid-connect.lua Outdated

return the correct response code

16d2b57

juststillthinking previously approved these changes Dec 21, 2023

View reviewed changes

juststillthinking added the approved label Dec 21, 2023

juststillthinking requested a review from moonming December 21, 2023 08:39

moonming requested changes Dec 21, 2023

View reviewed changes

add test case

c53da0c

luoluoyuyu dismissed juststillthinking’s stale review via c53da0c December 21, 2023 11:10

luoluoyuyu added 4 commits December 21, 2023 21:26

fix code format

19fa0d7

fix comment format

838c1ba

add google test case

5f1e5af

Merge branch 'master' into ODIC_redirect_after_logout

c985bfb

juststillthinking previously approved these changes Dec 22, 2023

View reviewed changes

moonming requested changes Dec 22, 2023

View reviewed changes

luoluoyuyu added 2 commits December 22, 2023 18:07

fix comment format

dd41c60

Merge branch 'ODIC_redirect_after_logout' of github.com:luoluoyuyu/ap…

9ded05a

…isix into ODIC_redirect_after_logout

luoluoyuyu dismissed juststillthinking’s stale review via 9ded05a December 22, 2023 10:12

moonming approved these changes Dec 22, 2023

View reviewed changes

shreemaan-abhishek approved these changes Dec 22, 2023

View reviewed changes

kayx23 approved these changes Dec 23, 2023

View reviewed changes

juststillthinking merged commit 129844c into apache:master Dec 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add redirect_after_logout_uri for ODIC that do not have an end_session_endpoint#10653

fix: add redirect_after_logout_uri for ODIC that do not have an end_session_endpoint#10653
juststillthinking merged 13 commits into
apache:masterfrom
luoluoyuyu:ODIC_redirect_after_logout

luoluoyuyu commented Dec 15, 2023 •

edited

Loading

Uh oh!

Sn0rt commented Dec 15, 2023

Uh oh!

luoluoyuyu commented Dec 15, 2023 •

edited

Loading

Uh oh!

luoluoyuyu commented Dec 15, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

moonming left a comment

Uh oh!

moonming Dec 22, 2023

Uh oh!

shreemaan-abhishek left a comment

Uh oh!

kayx23 commented Dec 27, 2023

Uh oh!

luoluoyuyu commented Dec 28, 2023 •

edited

Loading

Uh oh!

kayx23 commented Jan 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants




		=== TEST 35: Check whether outh0 can redirect normally using post_logout_redirect_uri configuration

Uh oh!

Conversation

luoluoyuyu commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem Analysis

Solution

Checklist

Uh oh!

Sn0rt commented Dec 15, 2023

Uh oh!

luoluoyuyu commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luoluoyuyu commented Dec 15, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

moonming left a comment

Choose a reason for hiding this comment

Uh oh!

moonming Dec 22, 2023

Choose a reason for hiding this comment

Uh oh!

shreemaan-abhishek left a comment

Choose a reason for hiding this comment

Uh oh!

kayx23 commented Dec 27, 2023

Uh oh!

luoluoyuyu commented Dec 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kayx23 commented Jan 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

luoluoyuyu commented Dec 15, 2023 •

edited

Loading

luoluoyuyu commented Dec 15, 2023 •

edited

Loading

luoluoyuyu commented Dec 28, 2023 •

edited

Loading