AuthRequestor: Add NewAutoAuthRequestor#506
AuthRequestor: Add NewAutoAuthRequestor#506chrisccoulson wants to merge 2 commits intocanonical:masterfrom
Conversation
This adds a new implementation of AuthRequestor that selects the most appropriate implementation from either Plymouth or systemd, preferring Plymouth if it is available and currently running and then falling back to systemd-ask-password if it is available. Fixes: FR-12405
chrisccoulson
force-pushed
the
auth-requestor-add-auto
branch
from
4733f65 to
cf86524
Compare
auth_requestor_auto.go
Outdated
|
|
||
| // AutoAuthRequestorStringer is used by the auto selecting implementation | ||
| // of [AuthRequestor] to obtain translated strings. | ||
| type AutoAuthRequestorStringer interface { |
There was a problem hiding this comment.
is this different from the Plymouth one? should we just have one AuthRequestorStringer ?
There was a problem hiding this comment.
I've merged these into one now.
| if r.lastUsed == nil { | ||
| return errors.New("no user credential requested yet") | ||
| } | ||
| return r.lastUsed.NotifyUserAuthResult(ctx, result, authTypes, exhaustedAuthTypes) |
There was a problem hiding this comment.
it seems because of the ping in the plymouth implementation that this could return ErrAuthRequestorNotAvailable ? is that expected, should it be documented? what should the caller do in that case?
There was a problem hiding this comment.
I think if plymouth stops in the middle, it is a weird enough context that we should completely fail and just log that error.
There was a problem hiding this comment.
Yeah, the error just gets logged in this case. To be honest, the ping is probably a bit redundant (assuming that plymouth display-message returns a non-zero exit code in this case).
pedronis
left a comment
pedronis
left a comment
There was a problem hiding this comment.
+1 but with a note to document the new error where it can occur
| UserAuthResultInvalidFormat | ||
| ) | ||
|
|
||
| var ErrAuthRequestorNotAvailable = errors.New("the auth requestor is not available") |
There was a problem hiding this comment.
can we mention this where relevant in the doc comments for AuthRequestor interface or the Auto one?
3 participants
This adds a new implementation of
AuthRequestorthat selects the mostappropriate implementation from either Plymouth or systemd, preferring
Plymouth if it is available and currently running and then falling back
to
systemd-ask-passwordif it is available.Fixes: FR-12405