Move hardware key change pin stderr message to cliPrompt#54207
Move hardware key change pin stderr message to cliPrompt#54207
cliPrompt#54207Conversation
github-actions
Bot
added
size/sm
tsh
| return pin, nil | ||
| } | ||
|
|
||
| pin, err := prompt.AskPIN(ctx, requirement, keyInfo) |
There was a problem hiding this comment.
Do we want to put a timeout on this context so we don't end up with a prompt that sits unanswered forever and holds the mutex preventing other operations from completing?
There was a problem hiding this comment.
I'm not sure if that's necessary - an unanswered PIN prompt would result in a paused program, waiting for the signature requiring the PIN to complete. Though it wouldn't hurt to add a 1 minute timeout here.
Joerger
added
the
no-changelog
Joerger
changed the base branch from
master
to
joerger/key-specific-pin-caching
| // If an invalid PIN or PUK is provided, the user will be re-prompted until a | ||
| // valid value is provided. | ||
| func (c *cliPrompt) ChangePIN(ctx context.Context, _ ContextualKeyInfo) (*PINAndPUK, error) { | ||
| fmt.Fprintf(os.Stderr, "The default PIN %q is not supported.\n", DefaultPIN) |
There was a problem hiding this comment.
We're already showing the same message on line 116. Maybe it’s enough to display it only when the user actually enters the default PIN?
There was a problem hiding this comment.
It sort of works as a warning against picking the default PIN, doesn't it?
There was a problem hiding this comment.
Yeah, I think the following flow would be bad UX:
Enter your YubiKey PIV PIN [blank to use default]:
// 123456
Please set a new 6-8 character PIN.
// 123456
The default PIN 123456 is not supported.
Joerger
force-pushed
the
joerger/tsh-default-pin-message
branch
from
d7546ea to
f34564b
Compare
public-teleport-github-review-bot
Bot
removed the request for review
from flyinghermit
Joerger
force-pushed
the
joerger/tsh-default-pin-message
branch
from
f34564b to
74fdfb7
Compare
4 participants
This stderr message should only be output for the
CLIPrompt.Note that
ChangePINis only called when the user provides the default pin or"", so the output now looks like:Alternatively we could pass the context of whether
""or the default pin was passed by the user to determine whether or not to display the extra line, but I think it doesn't hurt to always output it.resolves #54144 (comment)