-
Notifications
You must be signed in to change notification settings - Fork 231
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
Added WebAuth Auth0 Session Clear #115
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small detail in the comment the rest looks good. I have only one doubt, what is the user interaction with the logout?
Auth0/SafariWebAuth.swift
Outdated
@@ -194,3 +201,19 @@ private func generateDefaultState() -> String? { | |||
guard result == 0 else { return nil } | |||
return data.a0_encodeBase64URLSafe() | |||
} | |||
|
|||
private class SilentSFSafariViewController: SFSafariViewController, SFSafariViewControllerDelegate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this so a diff file
@hzalaz I would say it's as good as transparent, you can see it faintly at the top. |
Auth0/WebAuth.swift
Outdated
- parameter federated: Bool to enable signout of the IdP | ||
- parameter callback: callback called with the result of the logout | ||
*/ | ||
func clearSession(useFederated federated: Bool, callback: @escaping (Bool) -> Void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just federated
and probably make it an optional parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally was going to default to false
(not a fan of optional booleans) then thought the user should understand what method they are calling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good making it mandatory then
Auth0/WebAuth.swift
Outdated
@@ -200,4 +200,20 @@ public protocol WebAuth: Trackable, Loggable { | |||
- parameter callback: callback called with the result of the WebAuth flow | |||
*/ | |||
func start(_ callback: @escaping (Result<Credentials>) -> Void) | |||
|
|||
/** | |||
Clear the SSO Cookie in Auth0 and optionally signout from IdP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removes Auth0 session and optionally allows to remove the Identity Provider session.
Auth0/WebAuth.swift
Outdated
|
||
/** | ||
Clear the SSO Cookie in Auth0 and optionally signout from IdP. | ||
More info: https://auth0.com/docs/logout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use note
or seeAlso
like in the rest of the docs of the public API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Annoyingly, seeAlso
appears to be broken in Xcode, few bug reports out there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, they changed the parser yet again
self.modalPresentationStyle = .overCurrentContext | ||
} | ||
|
||
func safariViewController(_ controller: SFSafariViewController, didCompleteInitialLoad didLoadSuccessfully: Bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if it fails? should we handle that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand, the controller is dismissed and the success status didLoadSuccessfully
is returned in the callback.
Are you saying we should wrap it into an Error type for coding style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the url is invalid? if there is no internet connection? what will happened then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the URL called in the SFSafariViewController responds with anything other than HTTP 200 OK the callback will return false. (Have also manually tested this).
Auth0.podspec
Outdated
@@ -10,6 +10,7 @@ web_auth_files = [ | |||
'Auth0/WebAuthError.swift', | |||
'Auth0/SafariWebAuth.swift', | |||
'Auth0/SafariSession.swift', | |||
'SilentSafariViewController.swift', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why its in the root and not with the rest ?
fb1615c
to
930c8e3
Compare
This call is silent/hidden with no redirect as it's not a web app so the only requirement, there is a callback so the user can do something if they want to from the result.