Use domain when checking login credentials #17292
Open
Conversation
This file contains 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
manadart
force-pushed
the
dqlite-check-creds
branch
from
39f5a7e
to
25ebd84
Compare
|
/build |
manadart
force-pushed
the
dqlite-check-creds
branch
4 times, most recently
from
4833e3e
to
92ffea6
Compare
manadart
force-pushed
the
dqlite-check-creds
branch
from
92ffea6
to
04aa010
Compare
manadart
commented
May 27, 2024
| @@ -271,37 +276,43 @@ func (a *Authenticator) checkCreds( | |||
| } | |||
|
|
|||
| func (a *Authenticator) checkPerms(ctx context.Context, modelAccess permission.UserAccess, userTag names.UserTag) error { | |||
| // If the user tag is not local, we don't need to check for the model user | |||
| // permissions. This is generally the case for macaroon-based logins. | |||
| if !userTag.IsLocal() { | |||
There was a problem hiding this comment.
This was added to the top of this block in a prior refactoring, but it is incorrect. We check this situation further into the method.
apiserver/admin_test.go
Outdated
| when, err := model.LastModelConnection(modelUser.UserTag) | ||
| c.Assert(err, jc.ErrorIsNil) | ||
| c.Assert(when, jc.Almost, now) | ||
| /* |
There was a problem hiding this comment.
We'll need to restore this as part of @Aflynn50's work.
manadart
force-pushed
the
dqlite-check-creds
branch
4 times, most recently
from
1a0de1e
to
3b30e99
Compare
is part of login authentication.
ReadUserAccessForTarget to UserService interface in apiserver auth.
We rely instead on the name validation from the names package as the legacy state did. We use IsValidUser instead of IsValidUserName, because we allow external users of the form "user@domain".
accommodate the fact that this is where the authentication logic now looks. Authenticator logic uses tag.Id instead of tag.Name for users, so that the domain is preserved for locating users by name.
indicating JWT usage.
new Dqlite-backed access domain. At this stage we a hijacking the permission check for controller access, because we don't currently grant access based on controller UUID.
access, check for the specific NotFound variants emitted by the access domain. We need to do this in order to limit the information we return for failed auth actions.
incorrectly from admin.checkUserPermissions in a prior patch. That method now uses the Dqlite-backed access service for authorisation checks.
instead of the legacy state.
manadart
force-pushed
the
dqlite-check-creds
branch
from
265b2d7
to
be5b59a
Compare
manadart
force-pushed
the
dqlite-check-creds
branch
from
be5b59a
to
2ea6715
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 patch constitutes some progress towards using the permissions domain for login authorisation.
We use the
IDmethod instead of theNamemethod on user tags in order to preserve the@domainsection for external users. Such users will reside in the user table, where under Mongo they had an entry only in the model or controller users collections.In several places we have migrated permission checks to the new access service backing.
There are a couple of to-do comments regarding:
everyone@externalby direct service access.The commented block for testing the last user login will be reinstated in a patch to come.
To keep this patch manageable, these changes should follow:
QA steps
Bootstrap and login to a controller.
Documentation changes
None.
Links
Jira card: JUJU-5913