always require MFA when joining a session if at least one role requires session MFA#39602
Merged
capnspacehook merged 9 commits intomasterfrom Mar 23, 2024
Merged
Conversation
…when joining sessions
capnspacehook
added
mfa
capnspacehook
requested review from
rosstimothy
and removed request for
espadolini and
lxea
rosstimothy
approved these changes
Mar 20, 2024
codingllama
reviewed
Mar 20, 2024
Contributor
codingllama
left a comment
codingllama
left a comment
There was a problem hiding this comment.
Looks good, just one small question.
codingllama
approved these changes
Mar 20, 2024
Contributor
Author
|
Manual tests didn't show any breakage so I'll go ahead and merge this. |
Contributor
Author
|
@r0mant can I get a bypass for the flaky test detector, it's timing out |
Collaborator
|
/excludeflake * |
|
@capnspacehook See the table below for backport results.
|
capnspacehook
added a commit
that referenced
this pull request
Mar 25, 2024
…es session MFA (#39602) * extend role 'join_sessions' so MFA can be required at the role level when joining sessions * add test cases * update operator CRDs * remove change to 'join_sessions', instead require MFA if any roles require it * add comment * address feedback * add additional test cases * add authhandler test * fix role from test case not getting used in test
This was referenced Mar 25, 2024
Merged
Merged
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Mar 26, 2024
…es session MFA (#39602) (#39814) * extend role 'join_sessions' so MFA can be required at the role level when joining sessions * add test cases * update operator CRDs * remove change to 'join_sessions', instead require MFA if any roles require it * add comment * address feedback * add additional test cases * add authhandler test * fix role from test case not getting used in test
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
The OS login used when joining sessions is
-teleport-internal-joinwhich caused RBAC not to match the role where therequire_session_mfarole option is set. Instead require MFA when the OS login is-teleport-internal-joinand at least one role has therequire_session_mfarole option set.Previously reviewed here: https://github.com/gravitational/teleport-private/pull/1420
Fixes https://github.com/gravitational/teleport-private/issues/619.
Manual cases tested to ensure no regressions:
All tests were preformed with Auth
require_session_mfa=true, as a user with a role that hasrequire_session_mfa=trueset, and with both Auth and the user's role requiring per-session MFA.changelog: fix MFA checks not being prompted when joining a session