Skip to content

DB multi-session MFA Part 2: MFA reuse for GenerateUserCerts#54069

Merged
greedy52 merged 8 commits intomasterfrom
STeve/51679_multi_session_mfa
May 5, 2025
Merged

DB multi-session MFA Part 2: MFA reuse for GenerateUserCerts#54069
greedy52 merged 8 commits intomasterfrom
STeve/51679_multi_session_mfa

Conversation

@greedy52
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 commented Apr 16, 2025

related:

Changelog: New tsh db exec that executes commands on multiple target databases. When per-session MFA is required, only a single MFA tap is needed every 5 minutes.

The change may seem big but most files are only touched because of refactoring.

Sample role:

kind: role
metadata:
  name: access-mysql
spec:
  allow:
    db_labels:
      env: dev 
    db_users: [teleport-admin]
    db_names: ["*"]
  options:
    require_session_mfa: true
version: v7

Sample tsh db exec run:

$ cat sleep.sql 
DO SLEEP(120);
SELECT @@hostname AS Hostname;

$ tsh db exec "source sleep.sql" --db-user teleport-admin --dbs mysql1,mysql2,mysql3,mysql4
Fetching databases by name ...

Executing command for "mysql1".
MFA is required to access Database "mysql1"
Tap any security key
Detected security key tap
Hostname
64f979fb76c9

Executing command for "mysql2".
Hostname
6bc83bfda8d1

Executing command for "mysql3".
Hostname
64f979fb76c9

Executing command for "mysql4".
Your MFA validation has timed out.
MFA is required to access Database "mysql4"
Tap any security key
Detected security key tap
Hostname
6bc83bfda8d1

Summary: 4 of 4 succeeded.

@greedy52 greedy52 changed the title DB multi-session MFA Part 1: MFA reuse for GenerateUserCerts DB multi-session MFA Part 2: MFA reuse for GenerateUserCerts Apr 16, 2025

failedPrompt := fakePrompt{err: errors.New("prompt failed intentionally")}

defaultGenerateUserCerts := func(ctx context.Context, req proto.UserCertsRequest) (*proto.Certs, error) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved from below

@greedy52 greedy52 requested a review from Joerger April 16, 2025 19:15
@greedy52 greedy52 marked this pull request as ready for review April 16, 2025 19:15
@greedy52 greedy52 requested a review from GavinFrazar April 16, 2025 19:15
@github-actions github-actions Bot requested a review from atburke April 16, 2025 19:15
@github-actions github-actions Bot requested a review from probakowski April 16, 2025 19:15
@github-actions github-actions Bot added the tsh tsh - Teleport's command line tool for logging into nodes running Teleport. label Apr 16, 2025
@greedy52 greedy52 force-pushed the STeve/51679_base_tsh_db_exec branch 2 times, most recently from 1398bc7 to 64e6c76 Compare April 17, 2025 18:42
@GavinFrazar GavinFrazar added database-access Database access related issues and PRs and removed desktop-access labels Apr 18, 2025
Base automatically changed from STeve/51679_base_tsh_db_exec to master April 24, 2025 20:48
@greedy52 greedy52 force-pushed the STeve/51679_multi_session_mfa branch from 41ebbbe to 287bb05 Compare April 24, 2025 21:06
@greedy52
Copy link
Copy Markdown
Contributor Author

@Joerger do you mind take a look at this implementation for reuse? thanks 🙏

Comment thread lib/client/cluster_client.go Outdated
Comment thread lib/client/cluster_client.go Outdated
Comment thread lib/services/mfa.go Outdated
@greedy52 greedy52 force-pushed the STeve/51679_multi_session_mfa branch from 637ed2d to be97395 Compare April 25, 2025 16:04
@greedy52 greedy52 requested a review from Joerger April 25, 2025 16:26
@greedy52 greedy52 requested review from Tener and removed request for atburke and probakowski April 29, 2025 14:34
Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

first pass

Comment thread lib/auth/grpcserver_test.go
Comment thread lib/client/api.go
Comment thread lib/client/cluster_client.go Outdated
Comment thread lib/client/cluster_client.go Outdated
Comment thread lib/client/cluster_client.go
@greedy52
Copy link
Copy Markdown
Contributor Author

greedy52 commented May 1, 2025

@Joerger @Tener PTAL 🙏

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from GavinFrazar May 5, 2025 09:28
@greedy52 greedy52 added this pull request to the merge queue May 5, 2025
Merged via the queue into master with commit 3e86548 May 5, 2025
42 checks passed
@greedy52 greedy52 deleted the STeve/51679_multi_session_mfa branch May 5, 2025 16:06
greedy52 added a commit that referenced this pull request Jun 13, 2025
* enable multi-session mfa

* add ut

* add lib/client ut

* fix ut and proofread

* result.MFAResponse --> result.ReusableMFAResponse

* use errors.Is

* address comments
github-merge-queue Bot pushed a commit that referenced this pull request Jun 25, 2025
* DB multi-session MFA Part 1: implement basic "tsh db exec" (#53296)

* Implement basic "tsh db exec"

* adding ut

* minor refactor, fix race, rename iter func

* add help

* always use service name

* overwrite max connections with env var

* single get databases call

* remove prefix output

* fix some flags

* iterutils

* ensure each database

* add summery

* refactor, tests

* revert auto rename change by editor

* revert migrate

* remove unused var

* review comments

* renaming --max-connections to --parallel

* make exec return result instead of error

* hint TELEPORT_PARALLEL_JOBS

* fix golint

* address PR comments

* DB multi-session MFA Part 2: MFA reuse for GenerateUserCerts (#54069)

* enable multi-session mfa

* add ut

* add lib/client ut

* fix ut and proofread

* result.MFAResponse --> result.ReusableMFAResponse

* use errors.Is

* address comments

* fix logger

* fix test

* remove new tests from master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 database-access Database access related issues and PRs size/md tsh tsh - Teleport's command line tool for logging into nodes running Teleport.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants