Skip to content

OpenID Connect RP Initiated Logout#693

Merged
kadikraman merged 5 commits into
FormidableLabs:mainfrom
gispada:main
Jan 31, 2022
Merged

OpenID Connect RP Initiated Logout#693
kadikraman merged 5 commits into
FormidableLabs:mainfrom
gispada:main

Conversation

@gispada

@gispada gispada commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

Fixes #621 #68

Description

This pull request adds the OIDC RP initiated logout (link to the specification). Now it should be possible to properly logout a user, avoiding the automatic login after a simple token revocation.

The feature has already been developed on the native AppAuth libraries, for Android and iOS. I added the missing invocations of the appropriate methods.

Note: on iOS the system alert before logging out still appears. I guess it's something related to ASWebAuthenticationSession .

Steps to verify

  • Authenticate using OpenID Connect (scopes: ['openid']) and get an idToken;
  • Logout passing the idToken and a redirect url;
  • Authenticate again, the login form should require to insert username and password again, as the session was properly terminated.

@BlagojeVukovicVicert

BlagojeVukovicVicert commented Dec 13, 2021

Copy link
Copy Markdown

This could be potentially solve my issue. I am currious how do you reproduce second step "Logout passing the idToken and a redirect url", do you provide redirect url from the setup on server side, or is it something else? Thanks in advance!

Also logout method is throwing me and undefined exception: [TypeError: undefined is not a function (near '...(0, _reactNativeAppAuth.logout)...')] while revoke method is working with similar config

@gispada

gispada commented Dec 13, 2021

Copy link
Copy Markdown
Contributor Author

This could be potentially solve my issue. I am currious how do you reproduce second step "Logout passing the idToken and a redirect url", do you provide redirect url from the setup on server side, or is it something else? Thanks in advance!

The post logout redirect url must be provided as a parameter in the logout method. It might also be necessary to add it in the server configuration, among the allowed redirect URIs.

const result = await logout(config, {
  idToken: '<ID_TOKEN>',
  postLogoutRedirectUrl: '<POST_LOGOUT_URL>' // <---
});

Also logout method is throwing me and undefined exception: [TypeError: undefined is not a function (near '...(0, _reactNativeAppAuth.logout)...')] while revoke method is working with similar config

Could you provide the full error and more details on the device?

@BlagojeVukovicVicert

Copy link
Copy Markdown

I tried it again this morning, and I guess I made some mistakes in my setup. It works! But the only thing is that this native Webview still appears, on both platforms for me, but I guess that's something outside of our power. So far so good, than you very much for this solution!

@gispada

gispada commented Dec 15, 2021

Copy link
Copy Markdown
Contributor Author

I'm not sure if it is possible to avoid opening SFSafariViewController/ASWebAuthenticationSession/Custom Tabs when logging out.
My understanding is that the logout should be made with the same user agent used for login, because cookies are shared and the authorization server can remove or edit them after the session was ended.

@activebiz

activebiz commented Dec 21, 2021

Copy link
Copy Markdown

This PR has quite an important functionality of the lib which is missing from the main code. It would be worth getting this merge into main?

I am sorry but the authorize(..) does not work as expected when logging out.

@kadikraman

@gispada

gispada commented Dec 21, 2021

Copy link
Copy Markdown
Contributor Author

I am sorry but the authorize(..) does not work as expected when logging out.

Can you be more specific? What issues did you encounter?

@tonygomez

Copy link
Copy Markdown

@activebiz @kadikraman - Can you please elaborate on what is blocking this PR from being merged? We also need this functionality. Thank you!

fyi - @gispada

@kadikraman kadikraman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for tackling this feature @gispada - looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

authorize() caches credentials

6 participants