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.

Sign up for GitHub

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

Jump to bottom

feat: SSO MFA - SSO MFA challenge creation and verification #47684

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

feat: SSO MFA - SSO MFA challenge creation and verification #47684

wants to merge 8 commits into from
+2,737 −1,139

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Oct 17, 2024 •
edited
Loading

Part of the implementation of SSO MFA

Depends on https://github.com/gravitational/teleport.e/pull/5176

Joerger
Joerger commented Oct 17, 2024
View reviewed changes
lib/auth/grpcserver.go
Comment on lines +2166 to +2169
// TODO(Joerger): Extend SSO MFA support for moderated sessions.
var ssoClientRedirectURL string
Copy link
Contributor Author

@Joerger Joerger Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be implemented in a follow up PR when I can get to it, but it's not a priority over per-session MFA and other MFA features.

@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Oct 17, 2024
@Joerger Joerger mentioned this pull request Oct 17, 2024
Open
Base automatically changed from joerger/sso-mfa-session-data to master October 21, 2024 20:08
@Joerger Joerger mentioned this pull request Oct 21, 2024
Open
@marcoandredinis
Copy link
Contributor

Is this testable? Or just adds code that will be used in later PRs?
If testable, would you mind giving the required steps to test this one?

@Joerger
Copy link
Contributor Author

Joerger commented Oct 22, 2024 •
edited
Loading

Is this testable? Or just adds code that will be used in later PRs? If testable, would you mind giving the required steps to test this one?

It's not quite functional, the next PR makes use of the challenges/responses. It's based off this PR so you could just test that.

edit: that PR only works with tsh and somewhat Teleport Connect

marcoandredinis
marcoandredinis reviewed Oct 23, 2024
View reviewed changes
e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this one? It seems to be trying to update to a non-master commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcoandredinis marcoandredinis marcoandredinis left review comments

@nklaassen nklaassen nklaassen approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
no-changelog Indicates that a PR does not require a changelog entry size/lg
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Joerger @marcoandredinis @nklaassen