Fix Per-session MFA for leaf apps #51394
Open
+535
−105
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
f4ac8f6 to
fa93b48
Compare
fa93b48 to
add9cc9
Compare
9ca5eae to
e2d0ae8
Compare
ravicious
approved these changes
Jan 24, 2025
lib/web/mfa.go
Outdated
Comment on lines
202
to
213
| // If the MFA requirement check is being performed for a leaf host, we must check directly | ||
| // with the leaf cluster before the authentication challenge request through root. | ||
| if req.IsMFARequiredRequest.ClusterID != "" && req.IsMFARequiredRequest.ClusterID != c.cfg.RootClusterName { |
There was a problem hiding this comment.
Can we add some test coverage for the fix?
2e285ab to
5c887d6
Compare
…etupTrustedCluster.
eae69cf to
de47e12
Compare
98892fa to
d18d4ff
Compare
flyinghermit
approved these changes
Jan 31, 2025
| // If this is an mfa required check for a leaf host, we need to check the requirement through | ||
| // the leaf cluster, rather than through root in the authenticate challenge request below | ||
| // | ||
| // TODO(Joerger): Currently, we only leafs hosts that we check mfa requirements for directly |
There was a problem hiding this comment.
nit: this commentry reads like its missing some words
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.
MFA for app access in the WebUI mistakenly used the user's root client to check if mfa is required when connecting to a leaf app:
Fixes #51350
Changelog: Fix connecting to Apps in a leaf cluster when Per-session MFA is enabled.