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.

Sign up for GitHub

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

Jump to bottom

Add tsh touchid ls and rm commands #12505

Merged
merged 10 commits into from
May 11, 2022
Merged

Add tsh touchid ls and rm commands #12505

merged 10 commits into from
May 11, 2022

Conversation

codingllama
Copy link
Contributor

Implement touch ID credential management via tsh touchid ls and tsh touchid rm.

Departs slightly from RFD command names in order to better match the tsh mfa.

See https://github.com/gravitational/teleport/blob/master/rfd/0054-passwordless-macos.md.

#9160

@codingllama codingllama requested review from espadolini and jakule May 6, 2022 21:59
@github-actions github-actions bot added the tsh tsh - Teleport's command line tool for logging into nodes running Teleport. label May 6, 2022
@github-actions github-actions bot requested review from greedy52 and rosstimothy May 6, 2022 21:59
@codingllama
Copy link
Contributor Author

Depends on #12436 and #12475 (although the latter isn't strictly required). Target switches to master automatically once those are merged.

zmb3
zmb3 approved these changes May 6, 2022
View reviewed changes
Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

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

looks good!

lib/auth/touchid/credentials.m Show resolved Hide resolved
tool/tsh/tsh.go Outdated Show resolved Hide resolved
jakule
jakule reviewed May 8, 2022
View reviewed changes
Copy link
Contributor

@jakule jakule 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. I'll take another look tomorrow.

lib/client/mfa.go Outdated Show resolved Hide resolved
tool/tsh/tsh.go Outdated Show resolved Hide resolved
lib/auth/touchid/api_test.go Show resolved Hide resolved
lib/auth/touchid/api_test.go Show resolved Hide resolved
lib/auth/touchid/api.go Show resolved Hide resolved
lib/auth/touchid/api_darwin.go Outdated Show resolved Hide resolved
lib/auth/touchid/api_darwin.go Show resolved Hide resolved
lib/auth/touchid/api_darwin.go Show resolved Hide resolved
lib/auth/touchid/credentials.m Show resolved Hide resolved
@codingllama codingllama force-pushed the codingllama/touchid-tsh branch from e9af7b0 to a1ea67b Compare May 9, 2022 12:30
@codingllama codingllama force-pushed the codingllama/touchid-listdelete branch from 92b78d1 to ed9b3f6 Compare May 9, 2022 12:30
@codingllama
Copy link
Contributor Author

Thanks for the quick reviews, folks. PTAL.

@codingllama codingllama force-pushed the codingllama/touchid-tsh branch from a1ea67b to 12867fa Compare May 9, 2022 13:22
@codingllama codingllama force-pushed the codingllama/touchid-listdelete branch from 434d7a9 to c4b3d9f Compare May 9, 2022 13:24
Base automatically changed from codingllama/touchid-tsh to master May 9, 2022 14:46
@codingllama codingllama force-pushed the codingllama/touchid-listdelete branch 2 times, most recently from ca8156c to 143ef02 Compare May 9, 2022 15:35
@codingllama
Copy link
Contributor Author

FYI, I sneaked in an e/ update here. This pulls some files used to sign tsh locally.

@codingllama codingllama force-pushed the codingllama/touchid-listdelete branch from 1e91491 to bed5b66 Compare May 10, 2022 17:43
@codingllama
Copy link
Contributor Author

Friendly ping @jakule @espadolini ?

espadolini
espadolini approved these changes May 11, 2022
View reviewed changes
lib/auth/touchid/api.go Outdated Show resolved Hide resolved
lib/auth/touchid/api.go Show resolved Hide resolved
lib/auth/touchid/api_darwin.go Outdated Show resolved Hide resolved
lib/auth/touchid/api_darwin.go Outdated Show resolved Hide resolved
tool/tsh/touchid.go Outdated Show resolved Hide resolved
@codingllama codingllama force-pushed the codingllama/touchid-listdelete branch from bed5b66 to 3710314 Compare May 11, 2022 14:35
jakule
jakule approved these changes May 11, 2022
View reviewed changes
Copy link
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

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

LGTM, one comment.

tool/tsh/tsh.go Show resolved Hide resolved
@codingllama
Copy link
Contributor Author

For some unfathomable reason everything is green, so I'll go ahead and merge before it changes its mind. Happy to tweak command error messages in a separate PR.

@codingllama codingllama merged commit 0b34833 into master May 11, 2022
@codingllama codingllama deleted the codingllama/touchid-listdelete branch May 11, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@espadolini espadolini espadolini approved these changes

@zmb3 zmb3 zmb3 approved these changes

@jakule jakule jakule approved these changes

Assignees
No one assigned
Labels
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.
4 participants
@codingllama @espadolini @zmb3 @jakule