-
Notifications
You must be signed in to change notification settings - Fork 120
Improve authentication logic for authenticated web view #15164
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
31553c2
Add logic to check for authentication flow
itsmeichigo f49136c
Update authentication logic for AuthenticatedWebViewController
itsmeichigo 40e15b8
Remove check for previewing product
itsmeichigo f0ec140
Use separate authentication logic for atomic sites
itsmeichigo 62a9e73
Treat atomic and self-hosted sites similarly when SSO is enabled
itsmeichigo 5ae1928
Update authentication logic to support Jetpack setup
itsmeichigo ac24c7e
Update logic to avoid early returns
itsmeichigo 99be7e3
Move observation logic to a separate method
itsmeichigo fdcbadf
Inject current site to authenticationFlow method
itsmeichigo c77e321
Add tests for authenticationFlow method
itsmeichigo 085e0fd
Handle error for authentication
itsmeichigo 6d0eebd
Move ssoRedirectCookieName to constants
itsmeichigo ed3f192
Add tests for authentication failure check
itsmeichigo 8bd3e7a
Update release notes
itsmeichigo 82af7d3
Fix endless loop for authentication failure check
itsmeichigo 070ff39
Merge branch 'trunk' into task/15147-sso-authenticated-web-view
itsmeichigo e9d83df
Use shorthand for reference link
itsmeichigo 75ec99a
Merge branch 'trunk' into task/15147-sso-authenticated-web-view
itsmeichigo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1065,13 +1065,6 @@ private extension ProductFormViewController { | |
| return | ||
| } | ||
|
|
||
| let stores = ServiceLocator.stores | ||
| guard let site = stores.sessionManager.defaultSite, | ||
| stores.shouldAuthenticateAdminPage(for: site) else { | ||
| WebviewHelper.launch(url.absoluteString, with: self) | ||
| return | ||
| } | ||
|
Comment on lines
-1068
to
-1073
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using authenticated web view controller now for all product preview - the web view now can check to see if authentication is possible. If not, it would just open the original URL. |
||
|
|
||
| let viewModel = DefaultAuthenticatedWebViewModel(title: product.name, initialURL: url) | ||
| let controller = AuthenticatedWebViewController(viewModel: viewModel) | ||
| let navigationController = UINavigationController(rootViewController: controller) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
❓ Should we rename
isAuthenticationFailureintoisAuthenticationInProgress? If I understand this correctly, we are checking whether the current url is related to authentication and return false if the url is related to log. This lets the web view proceed further with the navigation and authentication.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.
I believe
isAuthenticationFailuremakes more sense here, as the web view fails to handle the authentication (POST request) and attempts to load the login page (the HTLM page) instead.