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.

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

Use challenge response when adding MFA device #47593

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Use challenge response when adding MFA device #47593

merged 1 commit into from
Oct 17, 2024

Conversation

avatus
Copy link
Contributor

@avatus avatus commented Oct 15, 2024

Part of SSO as MFA

This PR changes the requirement for the "Reauthenticate" step when adding an MFA device. Currently, we rely on the number of devices returned when we fetch the devices. However, in the future with SSOasMFA, these devices won't always be enabled.

This will allow us to check the backend if any of the available devices are "enabled" and if we've received a valid challenge for a device. if not, we forward the user through the privilege token flow instead.

This will be expanded by checking for SSO challenges as well and can now be expanded for any other type of auth.

@Joerger you can determine when/where we will need backports for this and let me know. Thank you!

@avatus avatus added the no-changelog Indicates that a PR does not require a changelog entry label Oct 15, 2024
@Joerger
Copy link
Contributor

Joerger commented Oct 15, 2024

@avatus We can backport this to v16/v15 as well, there are some edge cases unreleated to SSO MFA that this covers.

@avatus
Copy link
Contributor Author

avatus commented Oct 15, 2024

@bl-nero please take a look at this one when you get the chance 🙏

@@ -258,6 +258,21 @@ const auth = {
return api.post(cfg.api.createPrivilegeTokenPath, { secondFactorToken });
},

async getChallenge(
req: CreateAuthenticateChallengeRequest,
abortSignal?: AbortSignal
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see us using the abort signal anywhere. Consider removing it.

This PR changes the requirement for the "Reauthenticate" step when
adding an MFA device. Currently, we rely on the number of devices
returned when we fetch the devices. However, in the future with
SSOasMFA, these devices won't always be enabled.

This will allow us to check the backend if any of the available devices
are "enabled" and if we've received a valid challenge for a device. if
not, we forward the user through the privilege token flow instead.

This will be expanded by checking for SSO challenges as well and can now
be expanded for any other type of auth.
@avatus avatus enabled auto-merge October 17, 2024 14:25
@avatus avatus added this pull request to the merge queue Oct 17, 2024
Merged via the queue into master with commit 333c5a8 Oct 17, 2024
39 checks passed
@avatus avatus deleted the sso_devices branch October 17, 2024 14:49
@public-teleport-github-review-bot

@avatus See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Create PR

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

Successfully merging this pull request may close these issues.

5 participants