Skip to content

Require device trust for initial device registration endpoints#38451

Merged
Joerger merged 5 commits intomasterfrom
joerger/require-device-trust-for-add-mfa
Feb 27, 2024
Merged

Require device trust for initial device registration endpoints#38451
Joerger merged 5 commits intomasterfrom
joerger/require-device-trust-for-add-mfa

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Feb 20, 2024

Changelog: When device trust is required and MFA is optional, users will need to add their first MFA device from a trusted device.

Note: This this will make it impossible for a user to add their first MFA device from the WebUI, as device trust is not yet supported in the WebUI. They can instead add it with tsh. It may also be worth adding device management to Teleport Connect. A fix for device trust in the WebUI may be developed soon - https://github.com/gravitational/teleport.e/issues/3236.

@Joerger Joerger force-pushed the joerger/require-device-trust-for-add-mfa branch from 41d0871 to 0d87538 Compare February 20, 2024 21:34
Copy link
Copy Markdown
Contributor

@avatus avatus left a comment

Choose a reason for hiding this comment

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

Code looks fine but I'm not sure we want to disable first-time registration on web until another option is in place. Although, that isn't up to me I suppose! I don't feel comfortable approving as my context here is pretty light.

Copy link
Copy Markdown
Contributor

@codingllama codingllama left a comment

Choose a reason for hiding this comment

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

Looks good. No product commentary on my part either.

Code looks fine but I'm not sure we want to disable first-time registration on web until another option is in place

@avatus note that, as this is written, enforcement is based on the global device trust mode, so that does alleviate the "breakage" somehow.

Comment thread lib/auth/auth_with_roles.go Outdated
Comment thread lib/auth/auth_with_roles.go
Comment thread lib/auth/auth_with_roles.go Outdated
Comment thread lib/auth/auth_with_roles.go
Comment thread lib/auth/grpcserver_test.go Outdated
Comment thread lib/auth/grpcserver_test.go Outdated
Comment thread lib/auth/grpcserver_test.go
Comment thread lib/auth/grpcserver_test.go Outdated
Comment thread lib/auth/grpcserver_test.go Outdated
@Joerger Joerger requested a review from codingllama February 23, 2024 22:28
@Joerger Joerger force-pushed the joerger/require-device-trust-for-add-mfa branch from 9807a15 to c61a22d Compare February 23, 2024 22:32
@Joerger Joerger force-pushed the joerger/require-device-trust-for-add-mfa branch from 52a2fac to 1846fcd Compare February 23, 2024 22:55
Comment thread lib/auth/auth_with_roles.go
Comment thread lib/auth/auth.go
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from fheinecke February 26, 2024 17:45
@Joerger Joerger added this pull request to the merge queue Feb 27, 2024
Merged via the queue into master with commit c82dce6 Feb 27, 2024
@Joerger Joerger deleted the joerger/require-device-trust-for-add-mfa branch February 27, 2024 01:52
@public-teleport-github-review-bot
Copy link
Copy Markdown

@Joerger See the table below for backport results.

Branch Result
branch/v15 Create PR

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants