Add OTP fallback for SSHAgentMFAWebSessionLogin#57133
Merged
Conversation
In the case Teleport `second_factor` is set to `on`, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP. Server side would recieve an incomplete `AuthenticateWebUserRequest` object, which fails validation via `CheckAndSetDefaults` and as such only a failed login audit event was emitted with no additional logs. This mode of failure can be reproduced with `tsh bench -d web sessions --auth=local`. changelog: Fixed fallback for web login when second factor is set to `on` but only OTP is configured.
okraport
added
backport/branch/v16
backport/branch/v17
backport/branch/v18
c-ad
espadolini
approved these changes
Jul 24, 2025
| } | ||
|
|
||
| // sshAgentLoginWebCreateSession takes an existing client and login details and attempts to create a web session using OTP token | ||
| func sshAgentLoginWebCreateSession(ctx context.Context, clt *WebClient, login SSHLoginDirect) (*WebClient, types.WebSession, error) { |
Contributor
There was a problem hiding this comment.
If we take a WebClient there's no need to return it (potentially niled).
Suggested change
| func sshAgentLoginWebCreateSession(ctx context.Context, clt *WebClient, login SSHLoginDirect) (*WebClient, types.WebSession, error) { | |
| func sshAgentLoginWebCreateSession(ctx context.Context, clt *WebClient, login SSHLoginDirect) (types.WebSession, error) { |
Contributor
Author
There was a problem hiding this comment.
I pass it through to keep the caller code simpler, so the call site would be:
session, err := sshAgentLoginWebCreateSession(ctx, clt, SSHLoginDirect{})
return clt, session, errInstead of just:
return sshAgentLoginWebCreateSession(ctx, clt, SSHLoginDirect{})Nilling the client on error is consistent with the callers and so I have kept it the same, let me know if that changes your mind!
Contributor
There was a problem hiding this comment.
I agree with Edoard, this function signature feels off.
Contributor
Author
There was a problem hiding this comment.
Okay dokay, ask and you shall receive! Updated, I restructured sshAgentMFAWebSessionLogin to fit and I think it looks better now!
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
rosstimothy
approved these changes
Jul 24, 2025
public-teleport-github-review-bot
bot
removed request for
hugoShaka and
juliaogris
espadolini
approved these changes
Jul 24, 2025
Contributor
okraport
added a commit
that referenced
this pull request
Jul 25, 2025
* Add OTP fallback for SSHAgentMFAWebSessionLogin In the case Teleport `second_factor` is set to `on`, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP. Server side would recieve an incomplete `AuthenticateWebUserRequest` object, which fails validation via `CheckAndSetDefaults` and as such only a failed login audit event was emitted with no additional logs. This mode of failure can be reproduced with `tsh bench -d web sessions --auth=local`. changelog: Fixed fallback for web login when second factor is set to `on` but only OTP is configured. * improve errors for unsupported web login mfa challenge * Update lib/client/weblogin.go Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * Update lib/client/weblogin.go Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * unexport SSHAgentMFAWebSessionLogin and SSHAgentLoginWeb * update sshAgentLoginWebCreateSession signature --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
okraport
added a commit
that referenced
this pull request
Jul 25, 2025
* Add OTP fallback for SSHAgentMFAWebSessionLogin In the case Teleport `second_factor` is set to `on`, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP. Server side would recieve an incomplete `AuthenticateWebUserRequest` object, which fails validation via `CheckAndSetDefaults` and as such only a failed login audit event was emitted with no additional logs. This mode of failure can be reproduced with `tsh bench -d web sessions --auth=local`. changelog: Fixed fallback for web login when second factor is set to `on` but only OTP is configured. * improve errors for unsupported web login mfa challenge * Update lib/client/weblogin.go Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * Update lib/client/weblogin.go Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * unexport SSHAgentMFAWebSessionLogin and SSHAgentLoginWeb * update sshAgentLoginWebCreateSession signature --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
This was referenced Jul 25, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 25, 2025
* Add OTP fallback for SSHAgentMFAWebSessionLogin In the case Teleport `second_factor` is set to `on`, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP. Server side would recieve an incomplete `AuthenticateWebUserRequest` object, which fails validation via `CheckAndSetDefaults` and as such only a failed login audit event was emitted with no additional logs. This mode of failure can be reproduced with `tsh bench -d web sessions --auth=local`. changelog: Fixed fallback for web login when second factor is set to `on` but only OTP is configured. * improve errors for unsupported web login mfa challenge * Update lib/client/weblogin.go * Update lib/client/weblogin.go * unexport SSHAgentMFAWebSessionLogin and SSHAgentLoginWeb * update sshAgentLoginWebCreateSession signature --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 25, 2025
* Add OTP fallback for SSHAgentMFAWebSessionLogin In the case Teleport `second_factor` is set to `on`, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP. Server side would recieve an incomplete `AuthenticateWebUserRequest` object, which fails validation via `CheckAndSetDefaults` and as such only a failed login audit event was emitted with no additional logs. This mode of failure can be reproduced with `tsh bench -d web sessions --auth=local`. changelog: Fixed fallback for web login when second factor is set to `on` but only OTP is configured. * improve errors for unsupported web login mfa challenge * Update lib/client/weblogin.go * Update lib/client/weblogin.go * unexport SSHAgentMFAWebSessionLogin and SSHAgentLoginWeb * update sshAgentLoginWebCreateSession signature --------- Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
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
3 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.
In the case Teleport
second_factoris set toon, it is possible for a user to only have OTP configured. Prior to this commit this would result in a auth fail as the created challenge only supported TOTP.Server side would recieve an incomplete
AuthenticateWebUserRequestobject, which fails validation viaCheckAndSetDefaultsand as such only a failed login audit event was emitted with no additional logs.This mode of failure can be reproduced with
tsh bench -d web sessions --auth=local.changelog: Fixed fallback for web login when second factor is set to
onbut only OTP is configured.