feat: dynamic user auth and user creation #21366
Open
+1,380
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit makes a first pass draft at introducing a new authentication interface within a Juju controller to establish trust between two actors. I started at this interface so as to best represent the desire of what is required for authenticating. The interface purposelly does not talk about the interface mechanisms for which two actors get authenticated and leaves this as implementation detail for higher level components.
This commit adds a new user name builder that allows building up a username based on parts such as the name and domain component. It allows logic within the controller to handle the values seperately and not have to concern itself with how they are composed together as one string.
This commit introduces a new error const to the internal/auth package and a contract update to the Authenticator interface. This new error constant allows for an authenticator to return an error that informs the caller to stop any more authentication work flows from happening. It may seem odd to perform the design in this way as you end up programming by exception. However under this circumstance the authenticator will always be in an error state. This helps the authenticator to add context to the error and inform the caller that no further action can be taken.
This commit provides the first steps for a custom implementation of an authenticator for users that come from a trusted JAAS source. The first stages of this commit go to great lengths to make the contract on offer explicit and tested.
This commit adds to the JAAS authenticator missing support for context spans. This is required for more detail introspection of the controller and its authentication workflow.
Implements a new authenticator for validating username and passwords against the controllers local user records.
Updates the error docs for the contract around authenticating unit password hashes.
Adds the start of an authenticator for agents that are under a controller.
This commit adds a suggestion by harry to the AuthResult interface on offer to consumers. The interface now allows the the holder of a AuthResult to decorate a context with auth information for auditing purposes. This will be useful in tracing and logging when trying to understand authentication flows within a Juju controller. To support this work I beefed up some of the interface docs around expected errors and the state of an AuthResult when a error is received. In support of this several new functions have been added to for adding audit information to contexts.
Adds missing docs describing the purpose of the type and why it is needed.
This commit updates the JAAS authenticator to meet the new AuthResult interface. The new update decorates contexts with auth information about the authentciated JAAS user. On top of this it was needed to make the ensure operation for a controller user only happen one per AuthResult. This allows all funcs of the AuthResult interface to be used but not result in extra database load for no reason.
Fixes a small nit I had with the JAAS docs around the type name used for the authenticator.
This commit adds to the localuser auth result a new interface requirement to add audit information to a context. Because of this AuthResult has been changed to a struct so it can carry more information to support this need. Tests have been added to assert the contracts on offer by AuthResult.
This change updates the contract for WithAuditContext to guarantee to the caller that the original context will be returned unmodified when an error occurs. I have added a test to the JAAS package to assert this fact.
alesstimec
reviewed
Jan 5, 2026
Member
alesstimec
left a comment
alesstimec
left a comment
There was a problem hiding this comment.
I can't see a macaroon authenticator in this PR. Does that mean we are dropping support for Candid?
| // | ||
| // The following errors may be returned: | ||
| // - [coreerrors.NotValid] when the user name and domain provided are not valid. | ||
| func ParseNameWithDomain(name, domain string) (Name, error) { |
Member
There was a problem hiding this comment.
nitpick: this is not so much parsing as it is composing a new name with domain - i'd suggest naming this NewNameWithDomain
| MatchesModelPasswordHash(ctx context.Context, hash string) (bool, error) | ||
| } | ||
|
|
||
| type UnitAgentPasswordService interface { |
Member
There was a problem hiding this comment.
missing a godoc for the interface
|
|
||
| // UnitAgentAuthenticator provides an authenticator capable of authenticating | ||
| // unit agents within this controller. | ||
| type UnitAgentAuthenticator struct { |
Member
There was a problem hiding this comment.
what does this do? it doesn't have any methods..
| // Authenticator is an abstract concept of something that can authenticate a | ||
| // singular context between two actors in Juju. The expectation is that given | ||
| // two actors representing separate IPCs in the context of a Juju controller | ||
| // they should be able to us an [Authenticator] to establish trust and identity |
| // Authenticators MUST not be considered safe for concurrent use. | ||
| type Authenticator interface { | ||
| // Authenticate is used to validate and verify trust between two actors in | ||
| // Juju. The result of a successful authentication is a non nill |
| ).Add(auth.ErrStopAuthentication) | ||
| case errors.Is(err, jwt.ErrInvalidIssuedAt()): | ||
| return nil, false, errors.New( | ||
| "received JAAS token has invalid issued atand cannot be used for authentication", |
| if err != nil { | ||
| return nil, false, errors.Errorf( | ||
| "parsing user tag from authenticated JAAS token: %w", err, | ||
| ) |
Member
There was a problem hiding this comment.
should this also return with .Add(auth.ErrStopAuthentication)?
| userNameStr := userTag.Name() | ||
| userNameDomainStr := userTag.Domain() | ||
|
|
||
| return AuthResult{ |
Member
There was a problem hiding this comment.
Suggested change
| return AuthResult{ | |
| return AuthResult{ | |
| jaasIdentifier: a.verifier.SourceID(), | |
| userDomain: userTag.Domain(), | |
| userName: userTag.Name(), | |
| userService: a.userService, | |
| }, true, nil |
| ctx, span := trace.Start(ctx, trace.NameFromFunc()) | ||
| defer span.End() | ||
|
|
||
| if userDomain == "" { |
Member
There was a problem hiding this comment.
this would be a highly unusual case - a user from JAAS with an empty domain
|
|
||
| // Ensure that an external user exists in the controller to represents | ||
| // the JAAS user. | ||
| userUUID, err := userService.EnsureExternalUser( |
Member
There was a problem hiding this comment.
i'm not too fond of this pattern where we ensure a user exists as a side effect of getting it's UUID. i would much rather see we ensuer a user exists in the authenticate method (before returning the AuthResult)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the start of a small amount of work to support dynamically creating users that are authenticated from external sources.
I will this in a bit more when there is something worth merging here. At the moment this just acts as discussion piece on approach and interface for anyone interested.
Checklist
QA steps
Documentation changes
Links
Jira card: JUJU-8887