[flutter_appauth] add cancellation method for as web authentication session#645
Conversation
|
Thanks for the PR. Can you add some details to your original post on what this PR solves. Currently I'm taking a guess from the API docs for the new method. I would be keen to see if you're able to share info on how to reproduce the problem and/or a recording. Is the example app impacted by the problem this that PR solves? If so, can the example app be updated to show how it makes use of the addition as well? |
|
We would be keen to see this PR merged as our application needs it. Our use case is that we need to close the |
|
@vkammerer any chance you show a video and/or demo code using what this PR provides? I didn't get what you mean by your explanation. My understanding is this is about closing the browser and that's normally handled as part of the redirect so wondering if I'm missing/misunderstanding something |
|
@MaikuB our use case is that we want to close the
|
|
|
MaikuB
left a comment
There was a problem hiding this comment.
So sorry about confusion @kodejack. The original post still kept the PR message template that is meant to be replaced that I missed you had linked to the issue number underneath it to describe about the problem in more depth. Functionality-wise PR is fine but would you be able to look into the following
- rename the method. The use of the word "session" refers to
ASWebAuthenticationSessionbut this ends up causing a clash with the notion of a session that refers to a session after a user has logged in. I would recommend something likedismissExternalUserAgentas it's close to the name used within the AppAuth iOS SDK and the notion of an external user agent already exists in this plugin. The documentation would end needing adjustment as a result - The API docs for the method within the AppAuth iOS SDK takes a boolean parameter to indicate if the dismissal will be animated or note. Would you be able to expose a similar boolean parameter?
| break; | ||
| case CANCEL_PENDING_SESSION_METHOD: | ||
| // No-op on Android; only iOS/macOS use ASWebAuthenticationSession. | ||
| result.success(new HashMap<>()); |
There was a problem hiding this comment.
Was there a reason to pass an empty HashMap<>()? I believe passing null would suffice
| } else if ([CANCEL_PENDING_SESSION_METHOD isEqualToString:call.method]) { | ||
| [authorization cancelPendingSessionWithCompletion:^{ | ||
| self.currentAuthorizationFlow = nil; | ||
| result(@{}); |
There was a problem hiding this comment.
Similar to my comment on Android side, from what I've seen and tried, calling result(nil) would have sufficed
As this repository hosts two packages, please ensure the PR title starts with the name of the package that it relates to using square brackets (e.g. [flutter_appauth])
#643