Skip to content

Make MFA prompt client agnostic#34087

Merged
Joerger merged 9 commits intomasterfrom
joerger/client-agnostic-mfa-prompt
Nov 9, 2023
Merged

Make MFA prompt client agnostic#34087
Joerger merged 9 commits intomasterfrom
joerger/client-agnostic-mfa-prompt

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Oct 31, 2023 •
edited
Loading

Enables providing non-CLI based clients for MFA prompts. For example, this will allow Teleport Connect to provide it's own Prompt implementation to utilize modals in the Electron App.

@Joerger Joerger requested a review from codingllama October 31, 2023 17:48
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Oct 31, 2023 •
edited
Loading

@codingllama Pinging for early feed back if you have time, just on the overall approach. I'll clean up and open the PR once I implement the connect client side.

@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch 2 times, most recently from d84dfad to 133e13b Compare November 1, 2023 00:59
codingllama
codingllama reviewed Nov 1, 2023
View reviewed changes
Comment thread lib/client/mfa/prompt.go Outdated
Comment thread lib/client/mfa/prompt.go Outdated
Comment thread lib/client/mfa/prompt.go Outdated
Comment thread lib/client/mfa/prompt.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/prompt.go Outdated
@codingllama
Copy link
Copy Markdown
Contributor

codingllama commented Nov 1, 2023

Thanks for the early ping!

@Joerger Joerger mentioned this pull request Nov 1, 2023
Merged
@Joerger Joerger marked this pull request as ready for review November 2, 2023 01:33
@Joerger Joerger requested a review from codingllama November 2, 2023 01:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 2, 2023

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions github-actions Bot added desktop-access size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Nov 2, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 2, 2023

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch from be11796 to d8eb127 Compare November 2, 2023 01:46
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Nov 2, 2023
@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch from d8eb127 to 0ac51b7 Compare November 2, 2023 19:12
ibeckermayer
ibeckermayer reviewed Nov 2, 2023
View reviewed changes
Comment thread lib/client/mfa.go Outdated
Comment thread lib/client/mfa.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
@codingllama
Copy link
Copy Markdown
Contributor

codingllama commented Nov 2, 2023 •
edited
Loading

Sorry @Joerger, but it looks like I won't be able to make another pass before the next break. If you are in a rush feel free to CC someone else and I'll post-review it.

@Joerger Joerger requested review from ibeckermayer and removed request for codingllama November 3, 2023 00:34
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Nov 3, 2023

@r0mant or @zmb3 can we exclude flaky test? It's timing out.

@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Nov 3, 2023

/excludeflake TestModeratedSessionWithMFA

@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch from bb7d376 to f984ed9 Compare November 3, 2023 19:35
@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch from f984ed9 to 93431d3 Compare November 5, 2023 17:02
codingllama
codingllama approved these changes Nov 7, 2023
View reviewed changes
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.

Thanks for all the refactors. Mostly minor comments.

Comment thread lib/client/mfa/prompt.go Outdated
Comment thread tool/tctl/common/tctl.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/cli.go Outdated
Comment thread lib/client/mfa/prompt.go Outdated
@Joerger Joerger force-pushed the joerger/client-agnostic-mfa-prompt branch from 7fd2da4 to 67919de Compare November 7, 2023 19:11
@Joerger Joerger added this pull request to the merge queue Nov 9, 2023
Merged via the queue into master with commit 621c7b3 Nov 9, 2023
@Joerger Joerger deleted the joerger/client-agnostic-mfa-prompt branch November 9, 2023 18:32
@Joerger Joerger mentioned this pull request Nov 20, 2023
Merged
@Joerger Joerger mentioned this pull request Jan 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@probakowski probakowski probakowski approved these changes

@codingllama codingllama codingllama approved these changes

@ibeckermayer ibeckermayer Awaiting requested review from ibeckermayer

Assignees

No one assigned

Labels

desktop-access no-changelog Indicates that a PR does not require a changelog entry size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Joerger @codingllama @r0mant @probakowski @ibeckermayer