Skip to content

Comments

AuthRequestor: Add NewAutoAuthRequestor#506

Open
chrisccoulson wants to merge 1 commit intocanonical:masterfrom
chrisccoulson:auth-requestor-add-auto
Open

AuthRequestor: Add NewAutoAuthRequestor#506
chrisccoulson wants to merge 1 commit intocanonical:masterfrom
chrisccoulson:auth-requestor-add-auto

Conversation

@chrisccoulson
Copy link
Collaborator

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

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 chrisccoulson force-pushed the auth-requestor-add-auto branch from 4733f65 to cf86524 Compare February 10, 2026 11:56
@chrisccoulson chrisccoulson marked this pull request as ready for review February 10, 2026 15:05
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

coupe of questions


// AutoAuthRequestorStringer is used by the auto selecting implementation
// of [AuthRequestor] to obtain translated strings.
type AutoAuthRequestorStringer interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this different from the Plymouth one? should we just have one AuthRequestorStringer ?

if r.lastUsed == nil {
return errors.New("no user credential requested yet")
}
return r.lastUsed.NotifyUserAuthResult(ctx, result, authTypes, exhaustedAuthTypes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think if plymouth stops in the middle, it is a weird enough context that we should completely fail and just log that error.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants