Skip to content

Make tctl bots add prompt for MFA just once #37121

Merged
Joerger merged 7 commits intomasterfrom
joerger/reuse-mfa-tctl-bots-add
Jan 24, 2024
Merged

Make tctl bots add prompt for MFA just once #37121
Joerger merged 7 commits intomasterfrom
joerger/reuse-mfa-tctl-bots-add

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Jan 23, 2024

Updates tctl bots add to perform the admin action MFA ceremony upfront, allowing reuse for the initial version check call to CreateBot as well as the follow up calls to UpsertToken and CreateBot.

Before this change it worked like this:

$ tctl bots add terraform --roles=terraform
MFA is required for admin-level API request: "CreateBot"
Tap any security key
Detected security key tap
MFA is required for admin-level API request: "UpsertTokenV2"
Tap any security key
Detected security key tap
MFA is required for admin-level API request: "CreateBot"
Tap any security key
Detected security key tap

86feaa7e6177161e2e798f33e83f67202accccb2 fixes an issue shared by the tctl users add change

@Joerger Joerger requested a review from GavinFrazar January 23, 2024 21:48
@github-actions github-actions Bot added size/sm tctl tctl - Teleport admin tool labels Jan 23, 2024
@github-actions
Copy link
Copy Markdown
Contributor

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 added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 labels Jan 23, 2024
@GavinFrazar
Copy link
Copy Markdown
Contributor

seeing this with my v15.0.0-alpha.5 staging tenant cluster:

$ tctl bots add terraform --roles=terraform -d                                                                                                                   (base) 
2024-01-23T16:25:58-08:00 DEBU             Debug logging has been enabled. common/tctl.go:257
2024-01-23T16:25:58-08:00 DEBU             No config file or identity file, loading auth config via extension. common/tctl.go:303
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Reading certificates from path "/Users/gavin/.tsh/keys/gavin-leaf.cloud.gravitational.io/gavin.frazar@goteleport.com-ssh/gavin-leaf.cloud.gravitational.io-cert.pub". client/keystore.go:357
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Teleport TLS certificate valid until "2024-01-24 12:22:35 +0000 UTC". client/client_store.go:111
2024-01-23T16:25:58-08:00 DEBU             Found profile. proxy:https://gavin-leaf.cloud.gravitational.io:443 user:gavin.frazar@goteleport.com common/tctl.go:394
2024-01-23T16:25:58-08:00 INFO [CLIENT]    ALPN connection upgrade required for "gavin-leaf.cloud.gravitational.io:443": false. client/api.go:758
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Reading certificates from path "/Users/gavin/.tsh/keys/gavin-leaf.cloud.gravitational.io/gavin.frazar@goteleport.com-ssh/gavin-leaf.cloud.gravitational.io-cert.pub". client/keystore.go:357
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Teleport TLS certificate valid until "2024-01-24 12:22:35 +0000 UTC". client/client_store.go:111
2024-01-23T16:25:58-08:00 DEBU             Setting auth server to web proxy gavin-leaf.cloud.gravitational.io:443. common/tctl.go:432
2024-01-23T16:25:58-08:00 DEBU             Connecting to: [{gavin-leaf.cloud.gravitational.io:443 tcp }]. authclient/authclient.go:61
2024-01-23T16:25:58-08:00 DEBU             Attempting GET gavin-leaf.cloud.gravitational.io:443/webapi/find webclient/webclient.go:129
2024-01-23T16:25:58-08:00 DEBU             ALPN connection upgrade required for "gavin-leaf.cloud.gravitational.io:443": false. client/alpn_conn_upgrade.go:95
2024-01-23T16:25:58-08:00 DEBU [HTTP:PROX] No proxy set in environment, returning direct dialer. proxy/proxy.go:197

ERROR REPORT:
Original Error: *interceptors.RemoteError rpc error: code = Internal desc = transport: per-RPC creds failed due to error: Marshal called with nil
Stack Trace:
	github.com/gravitational/teleport/api/client/client.go:2306 github.com/gravitational/teleport/api/client.(*Client).UpsertToken
	github.com/gravitational/teleport/tool/tctl/common/bots_command.go:350 github.com/gravitational/teleport/tool/tctl/common.(*BotsCommand).AddBot
	github.com/gravitational/teleport/tool/tctl/common/bots_command.go:113 github.com/gravitational/teleport/tool/tctl/common.(*BotsCommand).TryRun
	github.com/gravitational/teleport/tool/tctl/common/tctl.go:228 github.com/gravitational/teleport/tool/tctl/common.TryRun
	github.com/gravitational/teleport/tool/tctl/common/tctl.go:102 github.com/gravitational/teleport/tool/tctl/common.Run
	github.com/gravitational/teleport/e/tool/tctl/main.go:20 main.main
	runtime/proc.go:267 runtime.main
	runtime/asm_arm64.s:1197 runtime.goexit
User Message: rpc error: code = Internal desc = transport: per-RPC creds failed due to error: Marshal called with nil

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Jan 24, 2024

seeing this with my v15.0.0-alpha.5 staging tenant cluster:

$ tctl bots add terraform --roles=terraform -d                                                                                                                   (base) 
2024-01-23T16:25:58-08:00 DEBU             Debug logging has been enabled. common/tctl.go:257
2024-01-23T16:25:58-08:00 DEBU             No config file or identity file, loading auth config via extension. common/tctl.go:303
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Reading certificates from path "/Users/gavin/.tsh/keys/gavin-leaf.cloud.gravitational.io/gavin.frazar@goteleport.com-ssh/gavin-leaf.cloud.gravitational.io-cert.pub". client/keystore.go:357
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Teleport TLS certificate valid until "2024-01-24 12:22:35 +0000 UTC". client/client_store.go:111
2024-01-23T16:25:58-08:00 DEBU             Found profile. proxy:https://gavin-leaf.cloud.gravitational.io:443 user:gavin.frazar@goteleport.com common/tctl.go:394
2024-01-23T16:25:58-08:00 INFO [CLIENT]    ALPN connection upgrade required for "gavin-leaf.cloud.gravitational.io:443": false. client/api.go:758
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Reading certificates from path "/Users/gavin/.tsh/keys/gavin-leaf.cloud.gravitational.io/gavin.frazar@goteleport.com-ssh/gavin-leaf.cloud.gravitational.io-cert.pub". client/keystore.go:357
2024-01-23T16:25:58-08:00 DEBU [KEYSTORE]  Teleport TLS certificate valid until "2024-01-24 12:22:35 +0000 UTC". client/client_store.go:111
2024-01-23T16:25:58-08:00 DEBU             Setting auth server to web proxy gavin-leaf.cloud.gravitational.io:443. common/tctl.go:432
2024-01-23T16:25:58-08:00 DEBU             Connecting to: [{gavin-leaf.cloud.gravitational.io:443 tcp }]. authclient/authclient.go:61
2024-01-23T16:25:58-08:00 DEBU             Attempting GET gavin-leaf.cloud.gravitational.io:443/webapi/find webclient/webclient.go:129
2024-01-23T16:25:58-08:00 DEBU             ALPN connection upgrade required for "gavin-leaf.cloud.gravitational.io:443": false. client/alpn_conn_upgrade.go:95
2024-01-23T16:25:58-08:00 DEBU [HTTP:PROX] No proxy set in environment, returning direct dialer. proxy/proxy.go:197

ERROR REPORT:
Original Error: *interceptors.RemoteError rpc error: code = Internal desc = transport: per-RPC creds failed due to error: Marshal called with nil
Stack Trace:
	github.com/gravitational/teleport/api/client/client.go:2306 github.com/gravitational/teleport/api/client.(*Client).UpsertToken
	github.com/gravitational/teleport/tool/tctl/common/bots_command.go:350 github.com/gravitational/teleport/tool/tctl/common.(*BotsCommand).AddBot
	github.com/gravitational/teleport/tool/tctl/common/bots_command.go:113 github.com/gravitational/teleport/tool/tctl/common.(*BotsCommand).TryRun
	github.com/gravitational/teleport/tool/tctl/common/tctl.go:228 github.com/gravitational/teleport/tool/tctl/common.TryRun
	github.com/gravitational/teleport/tool/tctl/common/tctl.go:102 github.com/gravitational/teleport/tool/tctl/common.Run
	github.com/gravitational/teleport/e/tool/tctl/main.go:20 main.main
	runtime/proc.go:267 runtime.main
	runtime/asm_arm64.s:1197 runtime.goexit
User Message: rpc error: code = Internal desc = transport: per-RPC creds failed due to error: Marshal called with nil

Lisa and I ran into a similar issue when testing #37071. The issue is that #37065 has not yet made it to cloud staging, and in that PR is a fix for IsMFARequired checks returning the opposite value expected (poor merge conflict management on my part).

57bc9c65a0293bf0c40ec5ee0c97c38c0374cc0d should fix this nil marshal issue, but instead a admin-level API request requires MFA verification error will be returned until #37065 is merged. Testing this PR locally works fine.

@Joerger Joerger force-pushed the joerger/reuse-mfa-tctl-bots-add branch from 0085aa7 to 538a29b Compare January 24, 2024 20:08
@Joerger Joerger enabled auto-merge January 24, 2024 20:08
@Joerger Joerger added this pull request to the merge queue Jan 24, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 24, 2024
@Joerger Joerger added this pull request to the merge queue Jan 24, 2024
Merged via the queue into master with commit 9b733e8 Jan 24, 2024
@Joerger Joerger deleted the joerger/reuse-mfa-tctl-bots-add branch January 24, 2024 22:04
@public-teleport-github-review-bot
Copy link
Copy Markdown

@Joerger See the table below for backport results.

Branch Result
branch/v15 Failed

Joerger added a commit that referenced this pull request Jan 25, 2024
* Reuse MFA for tctl bots add.

* Fix MFA required check for admin role.

* Remove broken auth preference check; Fix IsMFARequiredCheck for built in Admin role.

* Check for nil MFA response in MFA retry logic.

* Refactor MFA ceremony to return a custom error when MFA is not required.

* Fix TestAdminActionMFA unit tests.

* Fix unit test.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 25, 2024
* Reuse MFA for tctl bots add.

* Fix MFA required check for admin role.

* Remove broken auth preference check; Fix IsMFARequiredCheck for built in Admin role.

* Check for nil MFA response in MFA retry logic.

* Refactor MFA ceremony to return a custom error when MFA is not required.

* Fix TestAdminActionMFA unit tests.

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

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm tctl tctl - Teleport admin tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants