Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EndSessionRequest #31

Closed
smiLLe opened this issue Jul 8, 2019 · 29 comments
Closed

EndSessionRequest #31

smiLLe opened this issue Jul 8, 2019 · 29 comments

Comments

@smiLLe
Copy link

smiLLe commented Jul 8, 2019

Hey,

i just saw that AppAuth for iOS finally implemented a way to end the session.
see: openid/AppAuth-iOS#407

Unfortunately there is no implementation for Android, yet.
Do you plan to add this to your library? Even if it is just for iOS at the moment.

Greetings

@MaikuB
Copy link
Owner

MaikuB commented Jul 8, 2019

I didn't add this in because there wasn't an implementation on the Android side and I was aware of the end session implementation on iOS. The PR you linked is actually for a fix so it's actually been in there for a bit longer. So to answer your question, no plans to do this at the moment

@smiLLe
Copy link
Author

smiLLe commented Jul 8, 2019

Thank you for your fast answer :)
It would be great to have this feature, at least for iOS. For Android it would throw a not implemented or assert during development.

However, i am wondering. How are you doing a logout? Do you add a prompt=login and force the user to log in every time?

Maybe i am just missing an alternative approach to EndSessionRequest.

@MaikuB
Copy link
Owner

MaikuB commented Jul 8, 2019

Admittedly I haven't gotten to that point yet. I worked on this plugin as it looked like there wasn't much available for doing authentication with other identity providers (e.g. Azure B2C, my day job involves doing .NET development) and made it available for others to use. You're probably making more use of this than me :)

I don't think you're missing anything around the end session request. From what I recall, the proposal that included details around ending a session was still in draft as such I think some identity providers may provider different ways to end a session. As such I would think for devs that look to implement signing out that they write their own Android and iOS code to open a browser that would trigger the sign out and redirect back to the app. Perhaps it'll even work to use the launcher plugin to do so but that's just a random, untested thought...

@smiLLe
Copy link
Author

smiLLe commented Jul 9, 2019

Thank you for you answer :)
I am also working in a .NET Env everyday and we are using the IdS3, so the end session request would work.

I think i will close this issue for now and try to implement a signout on my own. I will come back here and update as soon as i have something "working" to share with you :)

Thanks and keep up the great work!

@smiLLe smiLLe closed this as completed Jul 9, 2019
@StefanJansson
Copy link

One workaround is to perform an authorize with 'connect/endsession' instead of '/connect/authorize' as the authorization endpoint... Not beutiful but it works...

@smiLLe
Copy link
Author

smiLLe commented Jul 24, 2019

@StefanJansson can you share some more details on how you do it in code? would be great :)

@StefanJansson
Copy link

Sure, here it comes.

var result = await _appAuth.authorize(
  AuthorizationTokenRequest([client_id], [redirect_uri],
    clientSecret: [client_secret],
    issuer: [issuer],
    discoveryUrl: "[issuer]/.well-known/openid-configuration",
    scopes: [scopes],
    additionalParameters: {
                          "id_token_hint": [idToken],
                          "post_logout_redirect_uri": [redirect_uri]
                        },
                        serviceConfiguration: AuthorizationServiceConfiguration(
                            "[issuer]/connect/endsession", "[issuer]/connect/token"),
                      ),
                    );

@draganjovanovic1
Copy link

@StefanJansson, I tried to use your end session hack and it works well on Android.

However, there is an issue with iOS app. I get logout screen and then on redirect to post_logout_redirect_uri app crashes:

🔥  To hot reload changes while running, press "r". To hot restart (and rebuild state), press "R".
An Observatory debugger and profiler on iPhone Xʀ is available at: http://127.0.0.1:55984/q14SAo8jYa4=/
For a more detailed help message, press "h". To detach, press "d"; to quit, press "q".
*** First throw call stack:
(
	0   CoreFoundation                      0x00000001078a88db __exceptionPreprocess + 331
	1   libobjc.A.dylib                     0x0000000106e4bac5 objc_exception_throw + 48
	2   CoreFoundation                      0x00000001077f6fac _CFThrowFormattedException + 194
	3   CoreFoundation                      0x0000000107917bb6 -[__NSDictionaryM setObject:forKey:] + 1046
	4   flutter_appauth                     0x0000000106833bc5 __127-[FlutterAppauthPlugin performAuthorization:clientId:clientSecret:scopes:redirectUrl:additionalParameters:result:exchangeCode:]_block_invoke.237 + 309
	5   AppAuth                             0x00000001046ac32d -[OIDAuthorizationSession didFinishWithResponse:error:] + 189
	6   AppAuth                             0x00000001046ac0ef __62-[OIDAuthorizationSession resumeExtern<…>
Lost connection to device.

I use the same redirect uri for login and logout and I registered it in ios/Runner/Info.plist:

    <key>CFBundleURLTypes</key>
    <array>
        <dict>
            <key>CFBundleTypeRole</key>
            <string>Editor</string>
            <key>CFBundleURLSchemes</key>
            <array>
                <string>my.callback.uri</string>
            </array>
        </dict>
    </array>

Thoughts? Thanks in advance.

@StefanJansson
Copy link

Hmm... Shit was my first thought. I've verified the error and don't have a workaround for it unfortunately. Maybe author of this package could implement end session for iOS which is available in openid/appauth-iOS and maybe use my workaround for Android until they release the end session functionality in opened/appauth-android?

What do you say @MaikuB, is it possible?

@smiLLe
Copy link
Author

smiLLe commented Aug 30, 2019

@StefanJansson It would be greate to implement endsession for iOS. It should be easy to implement and we could assert() for Android until they provide a native implementation.
But i don't believe @MaikuB is open for this feature.

@MaikuB
Copy link
Owner

MaikuB commented Sep 11, 2019

Happy to look at a PR for this if someone intends to take a stab

@MaikuB MaikuB mentioned this issue Oct 3, 2019
@AurelianTimu
Copy link

I have the end session functionality working nicely on iOS. I will submit a PR later this week.

@maxhelskens
Copy link

Any updates on the end session functionality?

@AurelianTimu
Copy link

AurelianTimu commented Nov 7, 2019

The reason why I did not submitted a PR yet due to how ASWebAuthenticationSession currently works in iOS.

AppAuth-IOS is using the following classes for authentication sessions :

For iOS 12 & 13 -> the new ASWebAuthenticationSession.
Previously (before WWDC) it was using SFAuthenticationSession but it was deprecated by apple and the replacement is now ASWebAuthenticationSession.

For iOS 11 -> SFAuthenticationSession
For iOS 9 & 10 -> SFSafariViewController

ASWebAuthenticationSession and SFAuthenticationSession benefit from SSO while in SFSafariViewController there is no SSO from iOS 11+ as it no longer shares cookies.

The issue with SSO (for ASWebAuthenticationSession and SFAuthenticationSession) is that a user permission prompt is displayed when the authentication session is accessed. This is great and works as expected when you first want to log in, it correctly asks for permission.
However, if the authentication session is opened again (for end session purposes), the prompt is displayed again. With the current Apple API, there is no way you can hide the prompt display. The API does simply not allow it.

67417564-156f6700-f597-11e9-85c1-4e3958c83fb3

If you want to have the benefits of SSO, you have to accept that annoying permission prompt when you click the Sign Out button in your application.
The user will see a Application Wants to use identityprovider.com to sign in when he presses on the Sign Out button and its just ruining the UX experience..

There are a few ways to get rid of the permission prompt, but you lose the SSO benefits.

I can see that there are a lot of developers complaining about this issue but I haven't seen yet a response from Apple team. It looks like the future of OAuth on iOS is not too bright.. With this API limitation it looks like they will slowly eject social identity provider logins on iOS and they will enforce their own system : Sign in with Apple

@MaikuB What do you think? Does it makes sense to submit a PR for end session functionality, with a boolean switch to select between SSO (ASWebAuthenticationSession) and non SSO (SFSafariViewController) behaviour?

@MaikuB
Copy link
Owner

MaikuB commented Nov 8, 2019

@AurelianTimu: Haven't looked at the AppAuth iOS SDK lately but does it even allow for specifying whether or to use ASWebAuthenticationSession or SFSafariViewController? Do you have a fork that I could check out that has an updated example?

@AurelianTimu
Copy link

(I accidently submitted a PR - please ignore that..)

If you create an implementation of OIDExternalUserAgentSession you can use either ASWebAuthenticationSession or SFSafariViewController.

Take a look at this branch.
Look for the comments i left in FlutterAppauthPlugin.m

Let me know please if you have any questions/comments.

@MaikuB
Copy link
Owner

MaikuB commented Nov 9, 2019

Thanks will see if I can take a look and play around with it soon. Based on what you've said so far, sounds like a good idea and the flag could be named forceSafariVC like the url_launcher plugin (see https://pub.dev/documentation/url_launcher/latest/url_launcher/launch.html)

@MaikuB
Copy link
Owner

MaikuB commented Nov 15, 2019

@AurelianTimu tried this out and I reckon it would be best to go with what I mentioned above on having a forceSafariVC flag. Unless I'm missing something, having a flag for ASWebAuthenticationSession doesn't seem to make much sense as it's only available on iOS 12+.

Given the library is a wrapper around the native SDKs, I'd see the name of the parameters be aligned with what the SDK uses as much as possible (e.g. postLogoutRedirectUrl instead of redirectUrl). Speaking of redirect, when I tried your branch with the example app that points to the demo IdentityServer instance, I noticed that after signing out that it didn't redirect back to the app. Not sure if there's a bug here that needs look into further

@MaikuB
Copy link
Owner

MaikuB commented Nov 15, 2019

FYI I'm going to release a new version soon to tidy up some code and bump Android dependencies so your branch will need to be rebased.

@MaikuB
Copy link
Owner

MaikuB commented Nov 15, 2019

Something I forgot to add is I wonder though if Apple will reject apps if they don't use the appropriate API for authentication. Another thing is my understanding of the SFSafariViewController is that data is no longer shared between Safari and apps that use it from iOS 11 onwards. As such, I wonder if there's much benefit in adding this behaviour as users end up losing the benefits of single sign-on. So thinking on this more perhaps a PR to add end session support would suffice for now and support for the flag could be revisited if needed

@AurelianTimu
Copy link

The redirect does not work with the IdentityServer demo instance because of this line :
https://github.com/IdentityServer/IdentityServer4.Demo/blob/75d23dca5e0ebdd452d3f5bef13b6c6b8d30051d/src/IdentityServer4Demo/Quickstart/Account/AccountOptions.cs#L16

If AutomaticRedirectAfterSignOut = true then it will automatically redirect.

Something I forgot to add is I wonder though if Apple will reject apps if they don't use the appropriate API for authentication

I was thinking about the same thing. I found some discussions online where someone complained that their app got rejected due to something very similar.
See here an example :
https://forums.expo.io/t/app-rejected-from-ios-app-store-due-to-authsession/7571
Not sure what APIs he was using though..

Noted the naming convention you suggested and the forceSafariVC flag.

I`ll think about this more in the upcoming days

@MaikuB
Copy link
Owner

MaikuB commented Nov 15, 2019

Sure and thanks for the info

@codenamics
Copy link

hello any update on that?

@alereisan
Copy link

@MaikuB any update about forceSafariVC flag?

@MaikuB
Copy link
Owner

MaikuB commented May 18, 2020

@alereisan not for me to comment on as the branch/fork mentioned earlier is not something I worked on to begin with and the link to the fork is there for you to look at

@IonVillarreal
Copy link

@StefanJansson, I tried to use your end session hack and it works well on Android.

However, there is an issue with iOS app. I get logout screen and then on redirect to post_logout_redirect_uri app crashes:

🔥  To hot reload changes while running, press "r". To hot restart (and rebuild state), press "R".
An Observatory debugger and profiler on iPhone Xʀ is available at: http://127.0.0.1:55984/q14SAo8jYa4=/
For a more detailed help message, press "h". To detach, press "d"; to quit, press "q".
*** First throw call stack:
(
	0   CoreFoundation                      0x00000001078a88db __exceptionPreprocess + 331
	1   libobjc.A.dylib                     0x0000000106e4bac5 objc_exception_throw + 48
	2   CoreFoundation                      0x00000001077f6fac _CFThrowFormattedException + 194
	3   CoreFoundation                      0x0000000107917bb6 -[__NSDictionaryM setObject:forKey:] + 1046
	4   flutter_appauth                     0x0000000106833bc5 __127-[FlutterAppauthPlugin performAuthorization:clientId:clientSecret:scopes:redirectUrl:additionalParameters:result:exchangeCode:]_block_invoke.237 + 309
	5   AppAuth                             0x00000001046ac32d -[OIDAuthorizationSession didFinishWithResponse:error:] + 189
	6   AppAuth                             0x00000001046ac0ef __62-[OIDAuthorizationSession resumeExtern<…>
Lost connection to device.

I use the same redirect uri for login and logout and I registered it in ios/Runner/Info.plist:

    <key>CFBundleURLTypes</key>
    <array>
        <dict>
            <key>CFBundleTypeRole</key>
            <string>Editor</string>
            <key>CFBundleURLSchemes</key>
            <array>
                <string>my.callback.uri</string>
            </array>
        </dict>
    </array>

Thoughts? Thanks in advance.

@draganjovanovic1 could you solve your problem? I have the same error
image

@draganjovanovic1
Copy link

@IonVillarreal, no I haven't. The project where I tried using this lib has been abandoned so I stopped looking for a solution.

@danielweil
Copy link

@MaikuB Any updates on the IOS fix for this?

@MaikuB
Copy link
Owner

MaikuB commented Jan 15, 2021

@danielweil not sure exactly which issue you referring to here given the discussions that have gone in this thread. If this is about end session support then no. There's already an existing issue on this that I marked as needing help and I had mentioned problems getting this working myself (#48 (comment)). PRs are welcome

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

No branches or pull requests

10 participants