Support a browser-based login flow for LDAP and Active Directory providers #1163
Conversation
This file contains 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
To keep this backwards compatible, this PR changes how the cli deals with ambiguous flows. Previously, if there was more than one flow advertised, the cli would require users to set the flag --upstream-identity-provider-flow. Now it chooses the first one in the list. Signed-off-by: Margo Crawford <margaretc@vmware.com>
Also change state param to include IDP type
Also fix some test failures on the callback handler, register the new login handler in manager.go and add a (half baked) integration test Signed-off-by: Margo Crawford <margaretc@vmware.com>
The other handlers for GET and POST requests are not yet implemented in this commit. The shared handler code in login_handler.go takes care of things checking the method, checking the CSRF cookie, decoding the state param, and adding security headers on behalf of both the GET and POST handlers. Some code has been extracted from callback_handler.go to be shared.
also add more unit tests Signed-off-by: Margo Crawford <margaretc@vmware.com>
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Also add autocomplete attribute and title element Signed-off-by: Margo Crawford <margaretc@vmware.com>
Also extract some helpers from auth_handler.go so they can be shared with the new handler.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Also use more specific test assertions where security headers are expected. And run the unit tests for the login package in parallel.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Also: - Add CSS to login page - Refactor login page HTML and CSS into a new package - New custom CSP headers for the login page, because the requirements are different from the form_post page
Also do some refactoring to share more common test setup code in supervisor_login_test.go.
Also fix some comments that didn't fit onto one line in the yaml examples, be consistent about putting a blank line above `---` yaml separators, and some other small doc improvements.
Codecov Report
@@ Coverage Diff @@
## main #1163 +/- ##
==========================================
- Coverage 79.73% 79.30% -0.43%
==========================================
Files 136 141 +5
Lines 10055 10240 +185
==========================================
+ Hits 8017 8121 +104
- Misses 1767 1847 +80
- Partials 271 272 +1
Continue to review full report at Codecov.
|
enj
reviewed
May 13, 2022
site/content/docs/howto/configure-supervisor-with-workspace_one_access.md
Outdated
Show resolved
Hide resolved
enj
reviewed
May 23, 2022
enj
suggested changes
May 23, 2022
enj
approved these changes
May 24, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implements the LDAP/AD web UI proposal from #1116.
Note that this PR changes how the
pinniped get kubeconfigcli deals with ambiguous flows. Previously, if there was more than one flow advertised for an IDP, the cli would require users to use the flag--upstream-identity-provider-flow. Now it chooses a default flow based on the IDP type (by choosing the first flow type in the Supervisor's discovery response).Here's a screenshot of the new LDAP/AD login page, which is hosted by Supervisor:
When using the Pinniped CLI, a successful login takes the user to the regular form_post success page, just like the previously supported browser authcode flow for an OIDCIdentityProvider:
Release note: