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

sso doesn't report error if permission request is dismissed #39

Open
David-Development opened this issue Dec 29, 2018 · 6 comments
Open

Comments

@David-Development
Copy link
Member

If the user clicks outside the permission request dialog, the sso library doesn't return a proper error message.

@tobiasKaminsky
Copy link
Member

Is this still valid?

@stefan-niedermann
Copy link
Member

It hasn't been implemented yet. To be honest i didn't miss it, but if @David-Development still needs this, it can be implemented either by

@tobiasKaminsky
Copy link
Member

As this prevents sso from finishing, it is a good idea to throw AccountImportCancelledException 👍

@stefan-niedermann
Copy link
Member

Fine with it, just want to note that this was a breaking change because 3rd party clients which do not yet catch this exception at this place will break in case a user dismisses the permission

→ Requires a new major release

@David-Development
Copy link
Member Author

David-Development commented Jul 10, 2020

I fully agree with Stefan. New major release and the Exceptions sounds great to me as well! 👍

@stefan-niedermann @tobiasKaminsky fyi: Probably this issue is also related to: #202

If I press anywhere outside the message, the message disappears and a moving circle is shown for ever.

@stefan-niedermann
Copy link
Member

As far as i can see, this has to be fixed in the Files app.

In the SSO lib AccountImporter#onActivityResult the result code RESULT_OK is given, but it should be RESULT_CANCELED. Then the wanted AccountImportCancelledException would be thrown.

I also wouldn't consider this a breaking change anymore because the method already throws AccountImportCancelledException since quite a long time.

@tobiasKaminsky Should i transfer this issue in the Files app repository or open a separate one?

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

No branches or pull requests

3 participants