diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go index fb925b03fbb..371f231d3bb 100644 --- a/apiserver/admin_test.go +++ b/apiserver/admin_test.go @@ -33,6 +33,8 @@ import ( "github.com/juju/juju/core/migration" "github.com/juju/juju/core/network" "github.com/juju/juju/core/permission" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/internal/password" "github.com/juju/juju/internal/uuid" jujutesting "github.com/juju/juju/juju/testing" @@ -175,8 +177,10 @@ func (s *loginSuite) TestLoginAsDeactivatedUser(c *gc.C) { // Since these are user login tests, the nonce is empty. err = st.Login(context.Background(), u.Tag(), password, "", nil) + + // The error message should not leak that the user is disabled. c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{ - Message: fmt.Sprintf("user %q is disabled", u.Tag().Id()), + Message: "invalid entity name or password", Code: "unauthorized access", }) @@ -205,7 +209,7 @@ func (s *loginSuite) TestLoginAsDeletedUser(c *gc.C) { // Since these are user login tests, the nonce is empty. err = st.Login(context.Background(), u.Tag(), password, "", nil) c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{ - Message: fmt.Sprintf("user %q is permanently deleted", u.Tag().Id()), + Message: "invalid entity name or password", Code: "unauthorized access", }) @@ -396,14 +400,30 @@ func (s *loginSuite) infoForNewUser(c *gc.C, info *api.Info) *api.Info { // Make a copy newInfo := *info + userTag := names.NewUserTag("charlie") + password := "shhh..." + + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Charlie Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Remove the make user call when permissions are + // written to state. f, release := s.NewFactory(c, info.ModelTag.Id()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, + + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + DisplayName: "Charlie Brown", + Password: password, }) - newInfo.Tag = user.Tag() + newInfo.Tag = userTag newInfo.Password = password return &newInfo } @@ -630,19 +650,34 @@ func (s *loginSuite) TestInvalidModel(c *gc.C) { } func (s *loginSuite) TestOtherModel(c *gc.C) { + userTag := names.NewUserTag("charlie") + password := "shhh..." + + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Charlie Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - modelOwner := f.MakeUser(c, nil) + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) modelState := f.MakeModel(c, &factory.ModelParams{ - Owner: modelOwner.UserTag(), + Owner: userTag, }) defer modelState.Close() + model, err := modelState.Model() c.Assert(err, jc.ErrorIsNil) st := s.openModelAPIWithoutLogin(c, model.UUID()) - err = st.Login(context.Background(), modelOwner.UserTag(), "password", "", nil) + err = st.Login(context.Background(), userTag, password, "", nil) c.Assert(err, jc.ErrorIsNil) s.assertRemoteModel(c, st, model.ModelTag()) } @@ -780,32 +815,48 @@ func (s *loginSuite) TestOtherModelWhenNotController(c *gc.C) { assertInvalidEntityPassword(c, err) } -func (s *loginSuite) loginLocalUser(c *gc.C, info *api.Info) (*state.User, params.LoginResult) { +func (s *loginSuite) loginLocalUser(c *gc.C, info *api.Info) (names.UserTag, params.LoginResult) { + userTag := names.NewUserTag("charlie") + password := "shhh..." + + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Charlie Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Remove the make user call when permissions are + // written to state. f, release := s.NewFactory(c, info.ModelTag.Id()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ + + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), Password: password, }) + conn := s.openAPIWithoutLogin(c) var result params.LoginResult request := ¶ms.LoginRequest{ - AuthTag: user.Tag().String(), + AuthTag: userTag.String(), Credentials: password, ClientVersion: jujuversion.Current.String(), } - err := conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) + err = conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) c.Assert(err, jc.ErrorIsNil) c.Assert(result.UserInfo, gc.NotNil) - return user, result + return userTag, result } func (s *loginSuite) TestLoginResultLocalUser(c *gc.C) { info := s.ControllerModelApiInfo() - user, result := s.loginLocalUser(c, info) - c.Check(result.UserInfo.Identity, gc.Equals, user.Tag().String()) + userTag, result := s.loginLocalUser(c, info) + c.Check(result.UserInfo.Identity, gc.Equals, userTag.String()) c.Check(result.UserInfo.ControllerAccess, gc.Equals, "login") c.Check(result.UserInfo.ModelAccess, gc.Equals, "admin") } @@ -815,8 +866,8 @@ func (s *loginSuite) TestLoginResultLocalUserEveryoneCreateOnlyNonLocal(c *gc.C) setEveryoneAccess(c, s.ControllerModel(c).State(), jujutesting.AdminUser, permission.SuperuserAccess) - user, result := s.loginLocalUser(c, info) - c.Check(result.UserInfo.Identity, gc.Equals, user.Tag().String()) + userTag, result := s.loginLocalUser(c, info) + c.Check(result.UserInfo.Identity, gc.Equals, userTag.String()) c.Check(result.UserInfo.ControllerAccess, gc.Equals, "login") c.Check(result.UserInfo.ModelAccess, gc.Equals, "admin") } @@ -846,32 +897,42 @@ func (s *loginSuite) assertRemoteModel(c *gc.C, conn api.Connection, expected na } func (s *loginSuite) TestLoginUpdatesLastLoginAndConnection(c *gc.C) { - now := s.Clock.Now().UTC() + userService := s.ControllerServiceFactory(c).User() + uuid, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), + }) + c.Assert(err, jc.ErrorIsNil) f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, + + f.MakeUser(c, &factory.UserParams{ + Name: "bobbrown", + DisplayName: "Bob Brown", + Password: "password", }) + now := s.Clock.Now().UTC() + info := s.ControllerModelApiInfo() - info.Tag = user.Tag() - info.Password = password + info.Tag = names.NewUserTag("bobbrown") + info.Password = "password" + apiState, err := api.Open(info, api.DialOpts{}) c.Assert(err, jc.ErrorIsNil) defer apiState.Close() // The user now has last login updated. - err = user.Refresh() + user, err := userService.GetUser(context.Background(), uuid) c.Assert(err, jc.ErrorIsNil) - lastLogin, err := user.LastLogin() - c.Assert(err, jc.ErrorIsNil) - c.Assert(lastLogin, jc.Almost, now) + c.Check(user.LastLogin, jc.Almost, now) // The model user is also updated. model := s.ControllerModel(c) - modelUser, err := model.State().UserAccess(user.UserTag(), model.ModelTag()) + modelUser, err := model.State().UserAccess(names.NewUserTag("bobbrown"), model.ModelTag()) c.Assert(err, jc.ErrorIsNil) when, err := model.LastModelConnection(modelUser.UserTag) c.Assert(err, jc.ErrorIsNil) @@ -970,21 +1031,30 @@ func (s *loginV3Suite) TestClientLoginToController(c *gc.C) { } func (s *loginV3Suite) TestClientLoginToControllerNoAccessToControllerModel(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + uuid, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - NoModelUser: true, - Password: password, + f.MakeUser(c, &factory.UserParams{ + Name: "bobbrown", }) - s.OpenControllerAPIAs(c, user.Tag(), password) - // The user now has last login updated. - err := user.Refresh() - c.Assert(err, jc.ErrorIsNil) - lastLogin, err := user.LastLogin() + now := s.Clock.Now().UTC() + + s.OpenControllerAPIAs(c, names.NewUserTag("bobbrown"), "password") + + user, err := userService.GetUser(context.Background(), uuid) c.Assert(err, jc.ErrorIsNil) - c.Assert(lastLogin, gc.NotNil) + c.Check(user.LastLogin, gc.Not(jc.Before), now) } func (s *loginV3Suite) TestClientLoginToRootOldClient(c *gc.C) { @@ -1029,3 +1099,7 @@ func (t errorTransport) RoundTrip(req *http.Request) (*http.Response, error) { } return t.fallback.RoundTrip(req) } + +func ptr[T any](v T) *T { + return &v +} diff --git a/apiserver/apiserver.go b/apiserver/apiserver.go index 40f657c74e2..77dde3faa98 100644 --- a/apiserver/apiserver.go +++ b/apiserver/apiserver.go @@ -52,6 +52,8 @@ import ( "github.com/juju/juju/core/presence" "github.com/juju/juju/core/resources" coretrace "github.com/juju/juju/core/trace" + coreuser "github.com/juju/juju/core/user" + userservice "github.com/juju/juju/domain/user/service" controllermsg "github.com/juju/juju/internal/pubsub/controller" "github.com/juju/juju/internal/resource" "github.com/juju/juju/internal/servicefactory" @@ -65,6 +67,23 @@ var logger = loggo.GetLogger("juju.apiserver") var defaultHTTPMethods = []string{"GET", "POST", "HEAD", "PUT", "DELETE", "OPTIONS"} +// ControllerConfigService defines the methods required to get the controller +// configuration. +type ControllerConfigService interface { + ControllerConfig(context.Context) (controller.Config, error) +} + +// UserService defines the methods required to get user details. +type UserService interface { + // GetUserByName returns the user with the given name. + GetUserByName(context.Context, string) (coreuser.User, error) + // SetPasswordWithActivationKey will use the activation key from the user. To + // then apply the payload password. If the user does not exist an error that + // satisfies usererrors.NotFound will be return. If the nonce is not the + // correct length an error that satisfies errors.NotValid will be returned. + SetPasswordWithActivationKey(ctx context.Context, name string, nonce, box []byte) (userservice.Sealer, error) +} + // Server holds the server side of the API. type Server struct { tomb tomb.Tomb diff --git a/apiserver/auditconfig_test.go b/apiserver/auditconfig_test.go index 20691b9aa48..bbd95dc563e 100644 --- a/apiserver/auditconfig_test.go +++ b/apiserver/auditconfig_test.go @@ -10,6 +10,7 @@ import ( "github.com/juju/collections/set" "github.com/juju/errors" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" @@ -18,6 +19,8 @@ import ( servertesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/auditlog" "github.com/juju/juju/core/model" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/testing/factory" @@ -48,23 +51,35 @@ func (s *auditConfigSuite) TestLoginAddsAuditConversationEventually(c *gc.C) { Target: log, } + userTag := names.NewUserTag("bobbrown") + password := "shhh..." + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), }) conn := s.openAPIWithoutLogin(c) var result params.LoginResult request := ¶ms.LoginRequest{ - AuthTag: user.Tag().String(), + AuthTag: userTag.String(), Credentials: password, CLIArgs: "hey you guys", ClientVersion: jujuversion.Current.String(), } loginTime := s.Clock.Now() - err := conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) + err = conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) c.Assert(err, jc.ErrorIsNil) c.Assert(result.UserInfo, gc.NotNil) // Nothing's logged at this point because there haven't been any @@ -95,7 +110,7 @@ func (s *auditConfigSuite) TestLoginAddsAuditConversationEventually(c *gc.C) { return math.Abs(t.Sub(loginTime).Seconds()) < 1.0 }) c.Assert(convo, mc, auditlog.Conversation{ - Who: user.Tag().Id(), + Who: userTag.Id(), What: "hey you guys", ModelName: "controller", ModelUUID: s.ControllerModelUUID(), @@ -128,22 +143,34 @@ func (s *auditConfigSuite) TestAuditLoggingFailureOnInterestingRequest(c *gc.C) Target: log, } + userTag := names.NewUserTag("bobbrown") + password := "shhh..." + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), }) conn := s.openAPIWithoutLogin(c) var result params.LoginResult request := ¶ms.LoginRequest{ - AuthTag: user.Tag().String(), + AuthTag: userTag.String(), Credentials: password, CLIArgs: "hey you guys", ClientVersion: jujuversion.Current.String(), } - err := conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) + err = conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) // No error yet since logging the conversation is deferred until // something happens. c.Assert(err, jc.ErrorIsNil) @@ -166,22 +193,34 @@ func (s *auditConfigSuite) TestAuditLoggingUsesExcludeMethods(c *gc.C) { Target: log, } + userTag := names.NewUserTag("bobbrown") + password := "shhh..." + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), }) conn := s.openAPIWithoutLogin(c) var result params.LoginResult request := ¶ms.LoginRequest{ - AuthTag: user.Tag().String(), + AuthTag: userTag.String(), Credentials: password, CLIArgs: "hey you guys", ClientVersion: jujuversion.Current.String(), } - err := conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) + err = conn.APICall(context.Background(), "Admin", 3, "", "Login", request, &result) c.Assert(err, jc.ErrorIsNil) c.Assert(result.UserInfo, gc.NotNil) // Nothing's logged at this point because there haven't been any diff --git a/apiserver/authentication/agent.go b/apiserver/authentication/agent.go index fc050548416..0f25c847f04 100644 --- a/apiserver/authentication/agent.go +++ b/apiserver/authentication/agent.go @@ -7,15 +7,53 @@ import ( "context" "github.com/juju/errors" + "github.com/juju/names/v5" apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/state" ) -// EntityAuthenticator performs authentication for juju entities. -type AgentAuthenticator struct{} +// Logger is the minimal logging interface required by the authenticator. +type Logger interface { + Debugf(string, ...interface{}) +} + +// AgentAuthenticatorFactory is a factory for creating authenticators, which +// can create authenticators for a given state. +type AgentAuthenticatorFactory struct { + legacyState *state.State + logger Logger +} -var _ EntityAuthenticator = (*AgentAuthenticator)(nil) +// NewAgentAuthenticatorFactory returns a new agent authenticator factory, for +// a known state. +func NewAgentAuthenticatorFactory(legacyState *state.State, logger Logger) AgentAuthenticatorFactory { + return AgentAuthenticatorFactory{ + legacyState: legacyState, + logger: logger, + } +} + +// Authenticator returns an authenticator using the factory's state. +func (f AgentAuthenticatorFactory) Authenticator() EntityAuthenticator { + return agentAuthenticator{ + state: f.legacyState, + logger: f.logger, + } +} + +// AuthenticatorForState returns an authenticator for the given state. +func (f AgentAuthenticatorFactory) AuthenticatorForState(st *state.State) EntityAuthenticator { + return agentAuthenticator{ + state: st, + logger: f.logger, + } +} + +type agentAuthenticator struct { + state *state.State + logger Logger +} type taggedAuthenticator interface { state.Entity @@ -24,11 +62,20 @@ type taggedAuthenticator interface { // Authenticate authenticates the provided entity. // It takes an entityfinder and the tag used to find the entity that requires authentication. -func (*AgentAuthenticator) Authenticate(ctx context.Context, entityFinder EntityFinder, authParams AuthParams) (state.Entity, error) { - entity, err := entityFinder.FindEntity(authParams.AuthTag) +func (a agentAuthenticator) Authenticate(ctx context.Context, authParams AuthParams) (state.Entity, error) { + switch authParams.AuthTag.Kind() { + case names.UserTagKind: + return nil, errors.Trace(apiservererrors.ErrBadRequest) + default: + return a.fallbackAuth(ctx, authParams) + } +} + +func (a *agentAuthenticator) fallbackAuth(ctx context.Context, authParams AuthParams) (state.Entity, error) { + entity, err := a.state.FindEntity(authParams.AuthTag) if errors.Is(err, errors.NotFound) { logger.Debugf("cannot authenticate unknown entity: %v", authParams.AuthTag) - return nil, errors.Trace(apiservererrors.ErrBadCreds) + return nil, errors.Trace(apiservererrors.ErrUnauthorized) } if err != nil { return nil, errors.Trace(err) @@ -38,7 +85,7 @@ func (*AgentAuthenticator) Authenticate(ctx context.Context, entityFinder Entity return nil, errors.Trace(apiservererrors.ErrBadRequest) } if !authenticator.PasswordValid(authParams.Credentials) { - return nil, errors.Trace(apiservererrors.ErrBadCreds) + return nil, errors.Trace(apiservererrors.ErrUnauthorized) } // If this is a machine agent connecting, we need to check the diff --git a/apiserver/authentication/agent_test.go b/apiserver/authentication/agent_test.go index b6d71225096..f75881406d7 100644 --- a/apiserver/authentication/agent_test.go +++ b/apiserver/authentication/agent_test.go @@ -6,13 +6,17 @@ package authentication_test import ( "context" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" "github.com/juju/juju/apiserver/authentication" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" internalpassword "github.com/juju/juju/internal/password" "github.com/juju/juju/juju/testing" "github.com/juju/juju/state" + jujutesting "github.com/juju/juju/testing" "github.com/juju/juju/testing/factory" ) @@ -22,7 +26,7 @@ type agentAuthenticatorSuite struct { machineNonce string unitPassword string machine *state.Machine - user *state.User + user state.Entity unit *state.Unit relation *state.Relation } @@ -32,13 +36,20 @@ var _ = gc.Suite(&agentAuthenticatorSuite{}) func (s *agentAuthenticatorSuite) SetUpTest(c *gc.C) { s.ApiServerSuite.SetUpTest(c) - f, release := s.NewFactory(c, s.ControllerModelUUID()) - defer release() - s.user = f.MakeUser(c, &factory.UserParams{ + userService := s.ControllerServiceFactory(c).User() + userUUID, _, err := userService.AddUser(context.Background(), service.AddUserArg{ Name: "bobbrown", DisplayName: "Bob Brown", - Password: "password", + Password: ptr(auth.NewPassword("password")), + CreatorUUID: s.AdminUserUUID, }) + c.Assert(err, jc.ErrorIsNil) + user, err := userService.GetUser(context.Background(), userUUID) + c.Assert(err, jc.ErrorIsNil) + s.user = authentication.TaggedUser(user, names.NewUserTag("bobbrown")) + + f, release := s.NewFactory(c, s.ControllerModelUUID()) + defer release() // add machine for testing machine agent authentication st := s.ControllerModel(c).State() @@ -91,10 +102,6 @@ type testCase struct { func (s *agentAuthenticatorSuite) TestValidLogins(c *gc.C) { testCases := []testCase{{ - entity: s.user, - credentials: "password", - about: "user login", - }, { entity: s.machine, credentials: s.machinePassword, nonce: s.machineNonce, @@ -108,8 +115,11 @@ func (s *agentAuthenticatorSuite) TestValidLogins(c *gc.C) { st := s.ControllerModel(c).State() for i, t := range testCases { c.Logf("test %d: %s", i, t.about) - var authenticator authentication.AgentAuthenticator - entity, err := authenticator.Authenticate(context.Background(), st, authentication.AuthParams{ + factory := authentication.NewAgentAuthenticatorFactory( + st, + jujutesting.NewCheckLogger(c), + ) + entity, err := factory.Authenticator().Authenticate(context.Background(), authentication.AuthParams{ AuthTag: t.entity.Tag(), Credentials: t.credentials, Nonce: t.nonce, @@ -121,6 +131,11 @@ func (s *agentAuthenticatorSuite) TestValidLogins(c *gc.C) { func (s *agentAuthenticatorSuite) TestInvalidLogins(c *gc.C) { testCases := []testCase{{ + entity: s.user, + credentials: "password", + about: "user login", + errorMessage: "invalid request", + }, { entity: s.relation, credentials: "dummy-secret", about: "relation login", @@ -129,7 +144,7 @@ func (s *agentAuthenticatorSuite) TestInvalidLogins(c *gc.C) { entity: s.user, credentials: "wrongpassword", about: "user login for nonexistant user", - errorMessage: "invalid entity name or password", + errorMessage: "invalid request", }, { entity: s.machine, credentials: s.machinePassword, @@ -140,14 +155,17 @@ func (s *agentAuthenticatorSuite) TestInvalidLogins(c *gc.C) { entity: s.user, credentials: "wrong-secret", about: "user login for nonexistant user", - errorMessage: "invalid entity name or password", + errorMessage: "invalid request", }} st := s.ControllerModel(c).State() for i, t := range testCases { c.Logf("test %d: %s", i, t.about) - var authenticator authentication.AgentAuthenticator - entity, err := authenticator.Authenticate(context.Background(), st, authentication.AuthParams{ + factory := authentication.NewAgentAuthenticatorFactory( + st, + jujutesting.NewCheckLogger(c), + ) + entity, err := factory.Authenticator().Authenticate(context.Background(), authentication.AuthParams{ AuthTag: t.entity.Tag(), Credentials: t.credentials, Nonce: t.nonce, diff --git a/apiserver/authentication/entity.go b/apiserver/authentication/entity.go index 39e780042e7..cd4684256ef 100644 --- a/apiserver/authentication/entity.go +++ b/apiserver/authentication/entity.go @@ -5,6 +5,8 @@ package authentication import ( "github.com/juju/names/v5" + + coreuser "github.com/juju/juju/core/user" ) // TagWrapper is a utility struct to take a names tag and wrap it as to conform @@ -22,3 +24,27 @@ func (t *tagWrapper) Tag() names.Tag { func TagToEntity(t names.Tag) Entity { return &tagWrapper{t} } + +// TaggedUser is a user that has been tagged with a names.Tag. +type taggedUser struct { + coreuser.User + tag names.Tag +} + +// TaggedUser returns a user that has been tagged with a names.Tag. +func TaggedUser(u coreuser.User, t names.Tag) Entity { + return taggedUser{u, t} +} + +// Tag returns the tag of the user. +func (u taggedUser) Tag() names.Tag { + return u.tag +} + +type externalUser struct { + tag names.Tag +} + +func (e externalUser) Tag() names.Tag { + return e.tag +} diff --git a/apiserver/authentication/interfaces.go b/apiserver/authentication/interfaces.go index bc75e933385..6432c13eeda 100644 --- a/apiserver/authentication/interfaces.go +++ b/apiserver/authentication/interfaces.go @@ -73,7 +73,7 @@ type PermissionDelegator interface { // implement to authenticate juju entities. type EntityAuthenticator interface { // Authenticate authenticates the given entity. - Authenticate(ctx context.Context, entityFinder EntityFinder, authParams AuthParams) (state.Entity, error) + Authenticate(ctx context.Context, authParams AuthParams) (state.Entity, error) } // Authorizer is a function type for authorizing a request. @@ -89,11 +89,6 @@ type Entity interface { Tag() names.Tag } -// EntityFinder finds the entity described by the tag. -type EntityFinder interface { - FindEntity(tag names.Tag) (state.Entity, error) -} - // HTTPAuthenticator provides an interface for authenticating a raw http request // from a client. type HTTPAuthenticator interface { diff --git a/apiserver/authentication/user.go b/apiserver/authentication/user.go index 7cc0f5a1986..8b6f9584690 100644 --- a/apiserver/authentication/user.go +++ b/apiserver/authentication/user.go @@ -21,15 +21,58 @@ import ( apiservererrors "github.com/juju/juju/apiserver/errors" coremacaroon "github.com/juju/juju/core/macaroon" + coreuser "github.com/juju/juju/core/user" + usererrors "github.com/juju/juju/domain/user/errors" "github.com/juju/juju/state" ) +const ( + // ErrInvalidLoginMacaroon is returned when a macaroon is not valid for + // a local login request. + ErrInvalidLoginMacaroon = errors.ConstError("invalid login macaroon") +) + var logger = loggo.GetLogger("juju.apiserver.authentication") +// UserService is the interface that wraps the methods required to +// authenticate a user. +type UserService interface { + // GetUserByAuth returns the user with the given name and password. + GetUserByAuth(ctx context.Context, name, password string) (coreuser.User, error) + // GetUserByName returns the user with the given name. + GetUserByName(ctx context.Context, name string) (coreuser.User, error) +} + +// Bakery defines the subset of bakery.Bakery that we require for authentication. +type Bakery interface { + MacaroonMinter + MacaroonChecker +} + +// MacaroonChecker exposes the methods needed from bakery.Checker. +type MacaroonChecker interface { + Auth(ctx context.Context, mss ...macaroon.Slice) *bakery.AuthChecker +} + +// MacaroonMinter exposes the methods needed from bakery.Oven. +type MacaroonMinter interface { + NewMacaroon(ctx context.Context, version bakery.Version, caveats []checkers.Caveat, ops ...bakery.Op) (*bakery.Macaroon, error) +} + +// ExpirableStorageBakery extends Bakery +// with the ExpireStorageAfter method so that root keys are +// removed from storage at that time. +type ExpirableStorageBakery interface { + Bakery + + // ExpireStorageAfter returns a new ExpirableStorageBakery with + // a store that will expire items added to it at the specified time. + ExpireStorageAfter(time.Duration) (ExpirableStorageBakery, error) +} + // LocalUserAuthenticator performs authentication for local users. If a password type LocalUserAuthenticator struct { - AgentAuthenticator - + UserService UserService // Bakery holds the bakery that is used to mint and verify macaroons. Bakery ExpirableStorageBakery @@ -68,8 +111,10 @@ var _ EntityAuthenticator = (*LocalUserAuthenticator)(nil) // If and only if no password is supplied, then Authenticate will check for any // valid macaroons. Otherwise, password authentication will be performed. func (u *LocalUserAuthenticator) Authenticate( - ctx context.Context, entityFinder EntityFinder, authParams AuthParams, + ctx context.Context, authParams AuthParams, ) (state.Entity, error) { + // We know this is a user tag and can be nothing but. With those assumptions + // made, we don't need a full AgentAuthenticator. userTag, ok := authParams.AuthTag.(names.UserTag) if !ok { return nil, errors.Errorf("invalid request") @@ -77,152 +122,113 @@ func (u *LocalUserAuthenticator) Authenticate( if !userTag.IsLocal() { return nil, errors.Errorf("invalid request - expected local user") } - if authParams.Credentials == "" { - return u.authenticateMacaroons(ctx, entityFinder, userTag, authParams) - } - return u.AgentAuthenticator.Authenticate(ctx, entityFinder, authParams) -} - -// CreateLocalLoginMacaroon creates a macaroon that may be provided to a -// user as proof that they have logged in with a valid username and password. -// This macaroon may then be used to obtain a discharge macaroon so that -// the user can log in without presenting their password for a set amount -// of time. -func CreateLocalLoginMacaroon( - ctx context.Context, - tag names.UserTag, - minter MacaroonMinter, - clock clock.Clock, - version bakery.Version, -) (*bakery.Macaroon, error) { - // We create the macaroon with a random ID and random root key, which - // enables multiple clients to login as the same user and obtain separate - // macaroons without having them use the same root key. - return minter.NewMacaroon(ctx, version, []checkers.Caveat{ - {Condition: "is-authenticated-user " + tag.Id()}, - checkers.TimeBeforeCaveat(clock.Now().Add(LocalLoginInteractionTimeout)), - }, identchecker.LoginOp) -} - -// CheckLocalLoginCaveat parses and checks that the given caveat string is -// valid for a local login request, and returns the tag of the local user -// that the caveat asserts is logged in. checkers.ErrCaveatNotRecognized will -// be returned if the caveat is not recognised. -func CheckLocalLoginCaveat(caveat string) (names.UserTag, error) { - var tag names.UserTag - op, rest, err := checkers.ParseCaveat(caveat) - if err != nil { - return tag, errors.Annotatef(err, "cannot parse caveat %q", caveat) - } - if op != "is-authenticated-user" { - return tag, checkers.ErrCaveatNotRecognized - } - if !names.IsValidUser(rest) { - return tag, errors.NotValidf("username %q", rest) - } - tag = names.NewUserTag(rest) - if !tag.IsLocal() { - tag = names.UserTag{} - return tag, errors.NotValidf("non-local username %q", rest) + // Empty credentials, will attempt to authenticate with macaroons. + if authParams.Credentials == "" { + return u.authenticateMacaroons(ctx, userTag, authParams) } - return tag, nil -} -// CheckLocalLoginRequest checks that the given HTTP request contains at least -// one valid local login macaroon minted by the given service using -// CreateLocalLoginMacaroon. It returns an error with a -// *bakery.VerificationError cause if the macaroon verification failed. -func CheckLocalLoginRequest( - ctx context.Context, - auth MacaroonChecker, - req *http.Request, -) error { - a := auth.Auth(ctx, httpbakery.RequestMacaroons(req)...) - ai, err := a.Allow(ctx, identchecker.LoginOp) - if err != nil { - return errors.Annotatef(err, "local login request failed: %v", req.Header[httpbakery.MacaroonsHeader]) - } - logger.Tracef("authenticated conditions: %v", ai.Conditions()) - if len(ai.Conditions()) == 0 { - return &bakery.VerificationError{Reason: errors.New("no caveats available")} + // We believe we've got a password, so we'll try to authenticate with it. + // This will check the user service for the user, ensuring that the user + // isn't disabled or deleted. + user, err := u.UserService.GetUserByAuth(ctx, userTag.Name(), authParams.Credentials) + if errors.Is(err, usererrors.NotFound) || errors.Is(err, usererrors.Unauthorized) { + logger.Debugf("user %s not found", userTag.String()) + return nil, errors.Trace(apiservererrors.ErrUnauthorized) + } else if err != nil { + return nil, errors.Trace(err) + } else if user.Disabled { + return nil, errors.Trace(apiservererrors.ErrUnauthorized) } - return errors.Trace(err) -} -// DischargeCaveats returns the caveats to add to a login discharge macaroon. -func DischargeCaveats(tag names.UserTag, clock clock.Clock) []checkers.Caveat { - firstPartyCaveats := []checkers.Caveat{ - checkers.DeclaredCaveat(usernameKey, tag.Id()), - checkers.TimeBeforeCaveat(clock.Now().Add(localLoginExpiryTime)), - } - return firstPartyCaveats + // StateEntity requires the user to be returned as a state.Entity. + return TaggedUser(user, userTag), nil } -func (u *LocalUserAuthenticator) authenticateMacaroons( - ctx context.Context, entityFinder EntityFinder, tag names.UserTag, authParams AuthParams, -) (state.Entity, error) { +func (u *LocalUserAuthenticator) authenticateMacaroons(ctx context.Context, userTag names.UserTag, authParams AuthParams) (state.Entity, error) { // Check for a valid request macaroon. if logger.IsTraceEnabled() { mac, _ := json.Marshal(authParams.Macaroons) - logger.Tracef("authentication macaroons for %s: %s", tag, mac) + logger.Tracef("authentication macaroons for %s: %s", userTag, mac) } + + // Attempt to authenticate the user using the macaroons provided. a := u.Bakery.Auth(ctx, authParams.Macaroons...) - ai, err := a.Allow(ctx, identchecker.LoginOp) - if err == nil { - logger.Tracef("authenticated conditions: %v", ai.Conditions()) + macaroonAuthInfo, err := a.Allow(ctx, identchecker.LoginOp) + if err != nil { + return nil, u.handleDischargeRequiredError(ctx, userTag, authParams.BakeryVersion, err) + } else if macaroonAuthInfo != nil && len(macaroonAuthInfo.Conditions()) == 0 { + return nil, u.handleDischargeRequiredError(ctx, userTag, authParams.BakeryVersion, ErrInvalidLoginMacaroon) } - if err != nil || len(ai.Conditions()) == 0 { - logger.Debugf("local-login macaroon authentication failed: %v", err) - cause := err - if cause == nil { - cause = errors.New("invalid login macaroon") - } - // The root keys for these macaroons are stored in MongoDB. - // Expire the documents after after a set amount of time. - expiryTime := u.Clock.Now().Add(localLoginExpiryTime) - bakery, err := u.Bakery.ExpireStorageAfter(localLoginExpiryTime) - if err != nil { - return nil, errors.Trace(err) - } - m, err := bakery.NewMacaroon( - ctx, - authParams.BakeryVersion, - []checkers.Caveat{ - checkers.TimeBeforeCaveat(expiryTime), - checkers.NeedDeclaredCaveat( - checkers.Caveat{ - Location: u.LocalUserIdentityLocation, - Condition: "is-authenticated-user " + tag.Id(), - }, - usernameKey, - ), - }, identchecker.LoginOp) - - if err != nil { - return nil, errors.Annotate(err, "cannot create macaroon") - } - return nil, &apiservererrors.DischargeRequiredError{ - Cause: cause, - LegacyMacaroon: m.M(), - Macaroon: m, - } + logger.Tracef("authenticated conditions: %v", macaroonAuthInfo.Conditions()) + + // Locate the user name from the macaroon. + index := macaroonAuthInfo.OpIndexes[identchecker.LoginOp] + if index < 0 || index > len(macaroonAuthInfo.Macaroons) { + return nil, errors.Trace(apiservererrors.ErrUnauthorized) } - loginMac := ai.Macaroons[ai.OpIndexes[identchecker.LoginOp]] + loginMac := macaroonAuthInfo.Macaroons[index] declared := checkers.InferDeclared(coremacaroon.MacaroonNamespace, loginMac) username := declared[usernameKey] - if tag.Id() != username { + + // If the userTag id is not the same as the username, then the user is not + // authenticated. + if userTag.Id() != username { return nil, apiservererrors.ErrPerm } - entity, err := entityFinder.FindEntity(tag) - if errors.Is(err, errors.NotFound) { - logger.Debugf("entity %s not found", tag.String()) - return nil, errors.Trace(apiservererrors.ErrBadCreds) + + // We've got a valid macaroon, so we can return the user. + user, err := u.UserService.GetUserByName(ctx, userTag.Name()) + if errors.Is(err, usererrors.NotFound) || errors.Is(err, usererrors.Unauthorized) { + logger.Debugf("user %s not found", userTag.String()) + return nil, errors.Trace(apiservererrors.ErrUnauthorized) } else if err != nil { return nil, errors.Trace(err) + } else if user.Disabled { + return nil, errors.Trace(apiservererrors.ErrUnauthorized) + } + + // StateEntity requires the user to be returned as a state.Entity. + return TaggedUser(user, userTag), nil +} + +func (u *LocalUserAuthenticator) handleDischargeRequiredError(ctx context.Context, userTag names.UserTag, bakeryVersion bakery.Version, cause error) error { + logger.Debugf("local-login macaroon authentication failed: %v", cause) + + // The root keys for these macaroons are stored in MongoDB. + // Expire the documents after after a set amount of time. + expiryTime := u.Clock.Now().Add(localLoginExpiryTime) + bakery, err := u.Bakery.ExpireStorageAfter(localLoginExpiryTime) + if err != nil { + return errors.Trace(err) + } + + // Make a new macaroon with a caveat for login operation. + macaroon, err := bakery.NewMacaroon( + ctx, + bakeryVersion, + []checkers.Caveat{ + checkers.TimeBeforeCaveat(expiryTime), + checkers.NeedDeclaredCaveat( + checkers.Caveat{ + Location: u.LocalUserIdentityLocation, + Condition: "is-authenticated-user " + userTag.Id(), + }, + usernameKey, + ), + }, + identchecker.LoginOp, + ) + if err != nil { + return errors.Annotate(err, "cannot create macaroon") + } + + return &apiservererrors.DischargeRequiredError{ + Cause: cause, + LegacyMacaroon: macaroon.M(), + Macaroon: macaroon, } - return entity, nil } // ExternalMacaroonAuthenticator performs authentication for external users using @@ -247,7 +253,7 @@ var _ EntityAuthenticator = (*ExternalMacaroonAuthenticator)(nil) // Authenticate authenticates the provided entity. If there is no macaroon provided, it will // return a *DischargeRequiredError containing a macaroon that can be used to grant access. -func (m *ExternalMacaroonAuthenticator) Authenticate(ctx context.Context, _ EntityFinder, authParams AuthParams) (state.Entity, error) { +func (m *ExternalMacaroonAuthenticator) Authenticate(ctx context.Context, authParams AuthParams) (state.Entity, error) { authChecker := m.Bakery.Checker.Auth(authParams.Macaroons...) ai, identErr := authChecker.Allow(ctx, identchecker.LoginOp) if de, ok := errors.Cause(identErr).(*bakery.DischargeRequiredError); ok { @@ -285,15 +291,7 @@ func (m *ExternalMacaroonAuthenticator) Authenticate(ctx context.Context, _ Enti return nil, errors.Errorf("external identity provider has provided ostensibly local name %q", username) } } - return externalUser{tag}, nil -} - -type externalUser struct { - tag names.Tag -} - -func (e externalUser) Tag() names.Tag { - return e.tag + return externalUser{tag: tag}, nil } // IdentityFromContext implements IdentityClient.IdentityFromContext. @@ -319,29 +317,77 @@ func (ExternalMacaroonAuthenticator) DeclaredIdentity(ctx context.Context, decla return nil, errors.New("no identity declared") } -// Bakery defines the subset of bakery.Bakery that we require for authentication. -type Bakery interface { - MacaroonMinter - MacaroonChecker +// CreateLocalLoginMacaroon creates a macaroon that may be provided to a +// user as proof that they have logged in with a valid username and password. +// This macaroon may then be used to obtain a discharge macaroon so that +// the user can log in without presenting their password for a set amount +// of time. +func CreateLocalLoginMacaroon( + ctx context.Context, + tag names.UserTag, + minter MacaroonMinter, + clock clock.Clock, + version bakery.Version, +) (*bakery.Macaroon, error) { + // We create the macaroon with a random ID and random root key, which + // enables multiple clients to login as the same user and obtain separate + // macaroons without having them use the same root key. + return minter.NewMacaroon(ctx, version, []checkers.Caveat{ + {Condition: "is-authenticated-user " + tag.Id()}, + checkers.TimeBeforeCaveat(clock.Now().Add(LocalLoginInteractionTimeout)), + }, identchecker.LoginOp) } -// MacaroonChecker exposes the methods needed from bakery.Checker. -type MacaroonChecker interface { - Auth(ctx context.Context, mss ...macaroon.Slice) *bakery.AuthChecker +// CheckLocalLoginCaveat parses and checks that the given caveat string is +// valid for a local login request, and returns the tag of the local user +// that the caveat asserts is logged in. checkers.ErrCaveatNotRecognized will +// be returned if the caveat is not recognised. +func CheckLocalLoginCaveat(caveat string) (names.UserTag, error) { + var tag names.UserTag + op, rest, err := checkers.ParseCaveat(caveat) + if err != nil { + return tag, errors.Annotatef(err, "cannot parse caveat %q", caveat) + } + if op != "is-authenticated-user" { + return tag, checkers.ErrCaveatNotRecognized + } + if !names.IsValidUser(rest) { + return tag, errors.NotValidf("username %q", rest) + } + tag = names.NewUserTag(rest) + if !tag.IsLocal() { + tag = names.UserTag{} + return tag, errors.NotValidf("non-local username %q", rest) + } + return tag, nil } -// MacaroonMinter exposes the methods needed from bakery.Oven. -type MacaroonMinter interface { - NewMacaroon(ctx context.Context, version bakery.Version, caveats []checkers.Caveat, ops ...bakery.Op) (*bakery.Macaroon, error) +// CheckLocalLoginRequest checks that the given HTTP request contains at least +// one valid local login macaroon minted by the given service using +// CreateLocalLoginMacaroon. It returns an error with a +// *bakery.VerificationError cause if the macaroon verification failed. +func CheckLocalLoginRequest( + ctx context.Context, + auth MacaroonChecker, + req *http.Request, +) error { + a := auth.Auth(ctx, httpbakery.RequestMacaroons(req)...) + ai, err := a.Allow(ctx, identchecker.LoginOp) + if err != nil { + return errors.Annotatef(err, "local login request failed: %v", req.Header[httpbakery.MacaroonsHeader]) + } + logger.Tracef("authenticated conditions: %v", ai.Conditions()) + if len(ai.Conditions()) == 0 { + return &bakery.VerificationError{Reason: errors.New("no caveats available")} + } + return errors.Trace(err) } -// ExpirableStorageBakery extends Bakery -// with the ExpireStorageAfter method so that root keys are -// removed from storage at that time. -type ExpirableStorageBakery interface { - Bakery - - // ExpireStorageAfter returns a new ExpirableStorageBakery with - // a store that will expire items added to it at the specified time. - ExpireStorageAfter(time.Duration) (ExpirableStorageBakery, error) +// DischargeCaveats returns the caveats to add to a login discharge macaroon. +func DischargeCaveats(tag names.UserTag, clock clock.Clock) []checkers.Caveat { + firstPartyCaveats := []checkers.Caveat{ + checkers.DeclaredCaveat(usernameKey, tag.Id()), + checkers.TimeBeforeCaveat(clock.Now().Add(localLoginExpiryTime)), + } + return firstPartyCaveats } diff --git a/apiserver/authentication/user_test.go b/apiserver/authentication/user_test.go index 59bf5311a95..a1ff5bd99a0 100644 --- a/apiserver/authentication/user_test.go +++ b/apiserver/authentication/user_test.go @@ -22,6 +22,8 @@ import ( "github.com/juju/juju/apiserver/authentication" apiservererrors "github.com/juju/juju/apiserver/errors" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/internal/password" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/state" @@ -50,7 +52,7 @@ func (s *userAuthenticatorSuite) TestMachineLoginFails(c *gc.C) { // attempt machine login authenticator := &authentication.LocalUserAuthenticator{} - _, err = authenticator.Authenticate(context.Background(), nil, authentication.AuthParams{ + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ AuthTag: machine.Tag(), Credentials: machinePassword, Nonce: nonce, @@ -77,7 +79,7 @@ func (s *userAuthenticatorSuite) TestUnitLoginFails(c *gc.C) { // Attempt unit login authenticator := &authentication.LocalUserAuthenticator{} - _, err = authenticator.Authenticate(context.Background(), nil, authentication.AuthParams{ + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ AuthTag: unit.UnitTag(), Credentials: unitPassword, }) @@ -85,42 +87,218 @@ func (s *userAuthenticatorSuite) TestUnitLoginFails(c *gc.C) { } func (s *userAuthenticatorSuite) TestValidUserLogin(c *gc.C) { - f, release := s.NewFactory(c, s.ControllerModelUUID()) - defer release() + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), + }) + c.Assert(err, jc.ErrorIsNil) + + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + } + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bobbrown"), + Credentials: "password", + }) + c.Assert(err, jc.ErrorIsNil) + c.Check(entity.Tag(), gc.Equals, names.NewUserTag("bobbrown")) +} - user := f.MakeUser(c, &factory.UserParams{ +func (s *userAuthenticatorSuite) TestDisabledUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ Name: "bobbrown", DisplayName: "Bob Brown", - Password: "password", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), }) + c.Assert(err, jc.ErrorIsNil) + err = userService.DisableUserAuthentication(context.Background(), "bobbrown") + c.Assert(err, jc.ErrorIsNil) // User login - authenticator := &authentication.LocalUserAuthenticator{} - _, err := authenticator.Authenticate(context.Background(), s.ControllerModel(c).State(), authentication.AuthParams{ - AuthTag: user.Tag(), + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bobbrown"), Credentials: "password", }) + c.Assert(err, jc.ErrorIs, apiservererrors.ErrUnauthorized) +} + +func (s *userAuthenticatorSuite) TestRemovedUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), + }) + c.Assert(err, jc.ErrorIsNil) + err = userService.RemoveUser(context.Background(), "bobbrown") c.Assert(err, jc.ErrorIsNil) + + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bobbrown"), + Credentials: "password", + }) + c.Assert(err, jc.ErrorIs, apiservererrors.ErrUnauthorized) } func (s *userAuthenticatorSuite) TestUserLoginWrongPassword(c *gc.C) { - f, release := s.NewFactory(c, s.ControllerModelUUID()) - defer release() - - user := f.MakeUser(c, &factory.UserParams{ + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ Name: "bobbrown", DisplayName: "Bob Brown", - Password: "password", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("password")), }) + c.Assert(err, jc.ErrorIsNil) // User login - authenticator := &authentication.LocalUserAuthenticator{} - _, err := authenticator.Authenticate(context.Background(), s.ControllerModel(c).State(), authentication.AuthParams{ - AuthTag: user.Tag(), + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bobbrown"), Credentials: "wrongpassword", }) - c.Assert(err, gc.ErrorMatches, "invalid entity name or password") + c.Assert(err, jc.ErrorIs, apiservererrors.ErrUnauthorized) +} + +func (s *userAuthenticatorSuite) TestValidMacaroonUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bob", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + }) + c.Assert(err, jc.ErrorIsNil) + + mac, err := macaroon.New(nil, nil, "", macaroon.LatestVersion) + c.Assert(err, jc.ErrorIsNil) + err = mac.AddFirstPartyCaveat([]byte("declared username bob")) + c.Assert(err, jc.ErrorIsNil) + macaroons := []macaroon.Slice{{mac}} + bakeryService := mockBakeryService{} + + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + Bakery: &bakeryService, + Clock: testclock.NewClock(time.Time{}), + } + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bob"), + Macaroons: macaroons, + }) + c.Assert(err, jc.ErrorIsNil) + + bakeryService.CheckCallNames(c, "Auth") + call := bakeryService.Calls()[0] + c.Assert(call.Args, gc.HasLen, 1) + c.Assert(call.Args[0], jc.DeepEquals, macaroons) + c.Check(entity.Tag(), gc.Equals, names.NewUserTag("bob")) +} + +func (s *userAuthenticatorSuite) TestInvalidMacaroonUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + }) + c.Assert(err, jc.ErrorIsNil) + + mac, err := macaroon.New(nil, nil, "", macaroon.LatestVersion) + c.Assert(err, jc.ErrorIsNil) + err = mac.AddFirstPartyCaveat([]byte("declared username fred")) + c.Assert(err, jc.ErrorIsNil) + macaroons := []macaroon.Slice{{mac}} + bakeryService := mockBakeryService{} + + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + Bakery: &bakeryService, + Clock: testclock.NewClock(time.Time{}), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bob"), + Macaroons: macaroons, + }) + c.Assert(err, jc.ErrorIs, authentication.ErrInvalidLoginMacaroon) +} + +func (s *userAuthenticatorSuite) TestDisabledMacaroonUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + }) + c.Assert(err, jc.ErrorIsNil) + err = userService.DisableUserAuthentication(context.Background(), "bobbrown") + c.Assert(err, jc.ErrorIsNil) + + mac, err := macaroon.New(nil, nil, "", macaroon.LatestVersion) + c.Assert(err, jc.ErrorIsNil) + err = mac.AddFirstPartyCaveat([]byte("declared username bob")) + c.Assert(err, jc.ErrorIsNil) + macaroons := []macaroon.Slice{{mac}} + bakeryService := mockBakeryService{} + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + Bakery: &bakeryService, + Clock: testclock.NewClock(time.Time{}), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bob"), + Macaroons: macaroons, + }) + c.Assert(err, jc.ErrorIs, apiservererrors.ErrUnauthorized) +} + +func (s *userAuthenticatorSuite) TestRemovedMacaroonUserLogin(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bobbrown", + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + }) + c.Assert(err, jc.ErrorIsNil) + err = userService.RemoveUser(context.Background(), "bobbrown") + c.Assert(err, jc.ErrorIsNil) + + mac, err := macaroon.New(nil, nil, "", macaroon.LatestVersion) + c.Assert(err, jc.ErrorIsNil) + err = mac.AddFirstPartyCaveat([]byte("declared username bob")) + c.Assert(err, jc.ErrorIsNil) + macaroons := []macaroon.Slice{{mac}} + bakeryService := mockBakeryService{} + + // User login + authenticator := &authentication.LocalUserAuthenticator{ + UserService: s.ControllerServiceFactory(c).User(), + Bakery: &bakeryService, + Clock: testclock.NewClock(time.Time{}), + } + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: names.NewUserTag("bob"), + Macaroons: macaroons, + }) + c.Assert(err, jc.ErrorIs, apiservererrors.ErrUnauthorized) } func (s *userAuthenticatorSuite) TestInvalidRelationLogin(c *gc.C) { @@ -142,41 +320,13 @@ func (s *userAuthenticatorSuite) TestInvalidRelationLogin(c *gc.C) { // Attempt relation login authenticator := &authentication.LocalUserAuthenticator{} - _, err = authenticator.Authenticate(context.Background(), nil, authentication.AuthParams{ + _, err = authenticator.Authenticate(context.Background(), authentication.AuthParams{ AuthTag: relation.Tag(), Credentials: "dummy-secret", }) c.Assert(err, gc.ErrorMatches, "invalid request") } -func (s *userAuthenticatorSuite) TestValidMacaroonUserLogin(c *gc.C) { - f, release := s.NewFactory(c, s.ControllerModelUUID()) - defer release() - - user := f.MakeUser(c, &factory.UserParams{ - Name: "bob", - }) - mac, err := macaroon.New(nil, nil, "", macaroon.LatestVersion) - c.Assert(err, jc.ErrorIsNil) - err = mac.AddFirstPartyCaveat([]byte("declared username bob")) - c.Assert(err, jc.ErrorIsNil) - macaroons := []macaroon.Slice{{mac}} - service := mockBakeryService{} - - // User login - authenticator := &authentication.LocalUserAuthenticator{Bakery: &service, Clock: testclock.NewClock(time.Time{})} - _, err = authenticator.Authenticate(context.Background(), s.ControllerModel(c).State(), authentication.AuthParams{ - AuthTag: user.Tag(), - Macaroons: macaroons, - }) - c.Assert(err, jc.ErrorIsNil) - - service.CheckCallNames(c, "Auth") - call := service.Calls()[0] - c.Assert(call.Args, gc.HasLen, 1) - c.Assert(call.Args[0], jc.DeepEquals, macaroons) -} - func (s *userAuthenticatorSuite) TestCreateLocalLoginMacaroon(c *gc.C) { service := mockBakeryService{} clock := testclock.NewClock(time.Time{}) @@ -204,7 +354,6 @@ func (s *userAuthenticatorSuite) TestAuthenticateLocalLoginMacaroon(c *gc.C) { service.SetErrors(nil, &bakery.VerificationError{}) _, err := authenticator.Authenticate( context.Background(), - authentication.EntityFinder(nil), authentication.AuthParams{ AuthTag: names.NewUserTag("bobbrown"), }, @@ -284,33 +433,24 @@ var _ = gc.Suite(&macaroonAuthenticatorSuite{}) var authenticateSuccessTests = []struct { about string dischargedUsername string - finder authentication.EntityFinder expectTag string expectError string }{{ about: "user that can be found", dischargedUsername: "bobbrown@somewhere", expectTag: "user-bobbrown@somewhere", - finder: simpleEntityFinder{}, }, { about: "user with no @ domain", dischargedUsername: "bobbrown", - finder: simpleEntityFinder{ - "user-bobbrown@external": true, - }, - expectTag: "user-bobbrown@external", + expectTag: "user-bobbrown@external", }, { about: "invalid user name", dischargedUsername: "--", - finder: simpleEntityFinder{}, expectError: `"--" is an invalid user name`, }, { about: "ostensibly local name", dischargedUsername: "cheat@local", - finder: simpleEntityFinder{ - "cheat@local": true, - }, - expectError: `external identity provider has provided ostensibly local name "cheat@local"`, + expectError: `external identity provider has provided ostensibly local name "cheat@local"`, }} type alwaysIdent struct { @@ -346,7 +486,7 @@ func (s *macaroonAuthenticatorSuite) TestMacaroonAuthentication(c *gc.C) { } // Authenticate once to obtain the macaroon to be discharged. - _, err := authenticator.Authenticate(context.Background(), test.finder, authentication.AuthParams{}) + _, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{}) // Discharge the macaroon. dischargeErr := errors.Cause(err).(*apiservererrors.DischargeRequiredError) @@ -355,7 +495,7 @@ func (s *macaroonAuthenticatorSuite) TestMacaroonAuthentication(c *gc.C) { c.Assert(err, jc.ErrorIsNil) // Authenticate again with the discharged macaroon. - entity, err := authenticator.Authenticate(context.Background(), test.finder, authentication.AuthParams{ + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{ Macaroons: []macaroon.Slice{ms}, }) if test.expectError != "" { @@ -368,24 +508,6 @@ func (s *macaroonAuthenticatorSuite) TestMacaroonAuthentication(c *gc.C) { } } -type simpleEntityFinder map[string]bool - -func (f simpleEntityFinder) FindEntity(tag names.Tag) (state.Entity, error) { - if utag, ok := tag.(names.UserTag); ok { - // It's a user tag which we need to be in canonical form - // so we can look it up unambiguously. - tag = names.NewUserTag(utag.Id()) - } - if f[tag.String()] { - return &simpleEntity{tag}, nil - } - return nil, errors.NotFoundf("entity %q", tag) -} - -type simpleEntity struct { - tag names.Tag -} - -func (e *simpleEntity) Tag() names.Tag { - return e.tag +func ptr[T any](t T) *T { + return &t } diff --git a/apiserver/charms_test.go b/apiserver/charms_test.go index cd2e09dccee..9704e019aba 100644 --- a/apiserver/charms_test.go +++ b/apiserver/charms_test.go @@ -18,6 +18,7 @@ import ( "path/filepath" "github.com/juju/charm/v13" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "github.com/juju/utils/v4" gc "gopkg.in/check.v1" @@ -25,6 +26,8 @@ import ( "github.com/juju/juju/apiserver" "github.com/juju/juju/apiserver/common" apitesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -518,15 +521,30 @@ func (s *charmsSuite) TestMigrateCharmNotMigrating(c *gc.C) { } func (s *charmsSuite) TestMigrateCharmUnauthorized(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - user := f.MakeUser(c, &factory.UserParams{Password: "hunter2"}) + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) + url := s.charmsURL("series=quantal") url.Path = "/migrate/charms" resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ Method: "POST", URL: url.String(), - Tag: user.Tag().String(), + Tag: userTag.String(), Password: "hunter2", }) body := apitesting.AssertResponse(c, resp, http.StatusForbidden, "text/plain; charset=utf-8") diff --git a/apiserver/embeddedcli.go b/apiserver/embeddedcli.go index 786c4dd6288..4ae7121535a 100644 --- a/apiserver/embeddedcli.go +++ b/apiserver/embeddedcli.go @@ -23,7 +23,6 @@ import ( apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/httpcontext" "github.com/juju/juju/apiserver/websocket" - "github.com/juju/juju/controller" "github.com/juju/juju/core/database" "github.com/juju/juju/core/model" "github.com/juju/juju/internal/feature" @@ -234,12 +233,6 @@ func newLineReader(r io.Reader) *linereader.Reader { return result } -// ControllerConfigService defines the methods required to get the controller -// configuration. -type ControllerConfigService interface { - ControllerConfig(context.Context) (controller.Config, error) -} - // ExecEmbeddedCommandFunc defines a function which runs a named Juju command // with the whitelisted sub commands. type ExecEmbeddedCommandFunc func(ctx *cmd.Context, store jujuclient.ClientStore, whitelist []string, cmdPlusArgs string) int diff --git a/apiserver/errors/errors.go b/apiserver/errors/errors.go index d7da946ad00..294c626443b 100644 --- a/apiserver/errors/errors.go +++ b/apiserver/errors/errors.go @@ -24,7 +24,7 @@ var logger = loggo.GetLogger("juju.apiserver.common.errors") const ( // TODO(juju3): move to params ErrBadId = errors.ConstError("id not found") - ErrBadCreds = errors.ConstError("invalid entity name or password") + ErrUnauthorized = errors.ConstError("invalid entity name or password") ErrNoCreds = errors.ConstError("no credentials provided") ErrLoginExpired = errors.ConstError("login expired") ErrPerm = errors.ConstError("permission denied") @@ -59,7 +59,7 @@ var singletonErrorCodes = map[errors.ConstError]string{ leadership.ErrClaimDenied: params.CodeLeadershipClaimDenied, lease.ErrClaimDenied: params.CodeLeaseClaimDenied, ErrBadId: params.CodeNotFound, - ErrBadCreds: params.CodeUnauthorized, + ErrUnauthorized: params.CodeUnauthorized, ErrNoCreds: params.CodeNoCreds, ErrLoginExpired: params.CodeLoginExpired, ErrPerm: params.CodeUnauthorized, diff --git a/apiserver/errors/errors_test.go b/apiserver/errors/errors_test.go index c4b00ba4f5e..6f1b60aac93 100644 --- a/apiserver/errors/errors_test.go +++ b/apiserver/errors/errors_test.go @@ -91,7 +91,7 @@ var errorTransformTests = []struct { return errors.Is(e, apiservererrors.NoAddressSetError) }, }, { - err: apiservererrors.ErrBadCreds, + err: apiservererrors.ErrUnauthorized, code: params.CodeUnauthorized, status: http.StatusUnauthorized, helperFunc: params.IsCodeUnauthorized, diff --git a/apiserver/export_test.go b/apiserver/export_test.go index f4ae1a901e2..b3cd94e3a4b 100644 --- a/apiserver/export_test.go +++ b/apiserver/export_test.go @@ -27,6 +27,7 @@ import ( "github.com/juju/juju/internal/worker/trace" "github.com/juju/juju/rpc" "github.com/juju/juju/state" + jujutesting "github.com/juju/juju/testing" ) var ( @@ -111,8 +112,9 @@ func TestingAPIRoot(facades *facade.Registry) rpc.Root { // TestingAPIHandler gives you an APIHandler that isn't connected to // anything real. It's enough to let test some basic functionality though. -func TestingAPIHandler(c *gc.C, pool *state.StatePool, st *state.State, configGetter stateauthenticator.ControllerConfigGetter) (*apiHandler, *common.Resources) { - authenticator, err := stateauthenticator.NewAuthenticator(pool, configGetter, clock.WallClock) +func TestingAPIHandler(c *gc.C, pool *state.StatePool, st *state.State, controllerConfigService stateauthenticator.ControllerConfigService, userService stateauthenticator.UserService) (*apiHandler, *common.Resources) { + agentAuthFactory := authentication.NewAgentAuthenticatorFactory(st, jujutesting.NewCheckLogger(c)) + authenticator, err := stateauthenticator.NewAuthenticator(pool, st, controllerConfigService, userService, agentAuthFactory, clock.WallClock) c.Assert(err, jc.ErrorIsNil) offerAuthCtxt, err := newOfferAuthcontext(pool) c.Assert(err, jc.ErrorIsNil) @@ -152,10 +154,11 @@ func TestingAPIHandlerWithEntity( c *gc.C, pool *state.StatePool, st *state.State, - configGetter stateauthenticator.ControllerConfigGetter, + controllerConfigService stateauthenticator.ControllerConfigService, + userService stateauthenticator.UserService, entity state.Entity, ) (*apiHandler, *common.Resources) { - h, hr := TestingAPIHandler(c, pool, st, configGetter) + h, hr := TestingAPIHandler(c, pool, st, controllerConfigService, userService) h.authInfo.Entity = entity h.authInfo.Delegator = &stateauthenticator.PermissionDelegator{State: st} return h, hr @@ -168,11 +171,12 @@ func TestingAPIHandlerWithToken( c *gc.C, pool *state.StatePool, st *state.State, - configGetter stateauthenticator.ControllerConfigGetter, + controllerConfigService stateauthenticator.ControllerConfigService, + userService stateauthenticator.UserService, jwt jwt.Token, delegator authentication.PermissionDelegator, ) (*apiHandler, *common.Resources) { - h, hr := TestingAPIHandler(c, pool, st, configGetter) + h, hr := TestingAPIHandler(c, pool, st, controllerConfigService, userService) user, err := names.ParseUserTag(jwt.Subject()) c.Assert(err, jc.ErrorIsNil) h.authInfo.Entity = authjwt.TokenEntity{User: user} diff --git a/apiserver/facades/client/client/api_test.go b/apiserver/facades/client/client/api_test.go index 01b768c0d15..d22491fc204 100644 --- a/apiserver/facades/client/client/api_test.go +++ b/apiserver/facades/client/client/api_test.go @@ -4,6 +4,7 @@ package client_test import ( + "context" "fmt" "time" @@ -20,7 +21,9 @@ import ( "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/core/status" + "github.com/juju/juju/domain/user/service" "github.com/juju/juju/environs/config" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -390,23 +393,41 @@ var scenarioStatus = ¶ms.FullStatus{ // controller (bootstrap machine), so is // hopefully easier to remember as such. func (s *baseSuite) setUpScenario(c *gc.C) (entities []names.Tag) { + st := s.ControllerModel(c).State() + f, release := s.NewFactory(c, s.ControllerModelUUID()) + defer release() + + userService := s.ControllerServiceFactory(c).User() + add := func(e state.Entity) { entities = append(entities, e.Tag()) } - st := s.ControllerModel(c).State() - u, err := st.User(testing.AdminUser) + + // Add the admin user. + adminPassword := defaultPassword(testing.AdminUser) + err := userService.SetPassword(context.Background(), testing.AdminUser.Name(), auth.NewPassword(adminPassword)) c.Assert(err, jc.ErrorIsNil) - setDefaultPassword(c, u) - add(u) + add(taggedUser{tag: testing.AdminUser}) + err = s.ControllerModel(c).UpdateModelConfig(map[string]interface{}{ config.AgentVersionKey: "2.0.0"}, nil) c.Assert(err, jc.ErrorIsNil) - f, release := s.NewFactory(c, s.ControllerModelUUID()) - defer release() - u = f.MakeUser(c, &factory.UserParams{Name: "other"}) - setDefaultPassword(c, u) - add(u) + // Add another user. + userTag := names.NewUserTag("other") + userPassword := defaultPassword(userTag) + _, _, err = userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(userPassword)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. + f.MakeUser(c, &factory.UserParams{Name: userTag.Name()}) + add(taggedUser{tag: userTag}) m, err := st.AddMachine(state.UbuntuBase("12.10"), state.JobManageModel) c.Assert(err, jc.ErrorIsNil) @@ -546,3 +567,15 @@ func (s *baseSuite) setUpScenario(c *gc.C) (entities []names.Tag) { } return } + +type taggedUser struct { + tag names.Tag +} + +func (u taggedUser) Tag() names.Tag { + return u.tag +} + +func ptr[T any](t T) *T { + return &t +} diff --git a/apiserver/facades/client/client/client_test.go b/apiserver/facades/client/client/client_test.go index 7555e67add0..7af657bdc71 100644 --- a/apiserver/facades/client/client/client_test.go +++ b/apiserver/facades/client/client/client_test.go @@ -10,6 +10,7 @@ import ( "github.com/juju/charm/v13" "github.com/juju/errors" "github.com/juju/loggo/v2" + "github.com/juju/names/v5" jtesting "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" @@ -30,8 +31,10 @@ import ( coreos "github.com/juju/juju/core/os" "github.com/juju/juju/core/permission" "github.com/juju/juju/core/status" + "github.com/juju/juju/domain/user/service" "github.com/juju/juju/environs/config" envtools "github.com/juju/juju/environs/tools" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/internal/docker" "github.com/juju/juju/internal/docker/registry" "github.com/juju/juju/internal/docker/registry/image" @@ -180,10 +183,21 @@ func (s *clientWatchSuite) TestClientWatchAllReadPermission(c *gc.C) { err = m.SetProvisioned("i-0", "", agent.BootstrapNonce, nil) c.Assert(err, jc.ErrorIsNil) + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("fred") + _, _, err = userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Fred Flintstone", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("ro-password")), + }) + c.Assert(err, jc.ErrorIsNil) + f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() user := f.MakeUser(c, &factory.UserParams{ - Password: "ro-password", + Name: userTag.Name(), + Access: permission.ReadAccess, }) c.Assert(err, jc.ErrorIsNil) conn := s.OpenModelAPIAs(c, s.ControllerModelUUID(), user.UserTag(), "ro-password", "") diff --git a/apiserver/introspection_test.go b/apiserver/introspection_test.go index cbecf64513a..00639269628 100644 --- a/apiserver/introspection_test.go +++ b/apiserver/introspection_test.go @@ -4,22 +4,25 @@ package apiserver_test import ( + "context" "io" "net/http" "net/url" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" apitesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/permission" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/juju/testing" - "github.com/juju/juju/state" + "github.com/juju/juju/testing/factory" ) type introspectionSuite struct { testing.ApiServerSuite - bob *state.User url string } @@ -32,23 +35,53 @@ func (s *introspectionSuite) SetUpTest(c *gc.C) { })) } s.ApiServerSuite.SetUpTest(c) - bob, err := s.ControllerModel(c).State().AddUser("bob", "", "hunter2", "admin") - c.Assert(err, jc.ErrorIsNil) - s.bob = bob s.url = s.URL("/introspection/navel", url.Values{}).String() } func (s *introspectionSuite) TestAccess(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + s.testAccess(c, testing.AdminUser.String(), testing.AdminSecret) - _, err := s.ControllerModel(c).AddUser( - state.UserAccessSpec{ - User: s.bob.UserTag(), - CreatedBy: testing.AdminUser, - Access: permission.ReadAccess, - }, - ) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. + f, release := s.NewFactory(c, s.ControllerModelUUID()) + defer release() + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + Access: permission.ReadAccess, + }) + + s.testAccess(c, "user-bobbrown", "hunter2") +} + +func (s *introspectionSuite) TestAccessDenied(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) c.Assert(err, jc.ErrorIsNil) - s.testAccess(c, "user-bob", "hunter2") + + resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ + Method: "GET", + URL: s.url, + Tag: "user-bobbrown", + Password: "hunter2", + }) + defer resp.Body.Close() + c.Assert(resp.StatusCode, gc.Equals, http.StatusUnauthorized) } func (s *introspectionSuite) testAccess(c *gc.C, tag, password string) { @@ -64,14 +97,3 @@ func (s *introspectionSuite) testAccess(c *gc.C, tag, password string) { c.Assert(err, jc.ErrorIsNil) c.Assert(string(content), gc.Equals, "gazing") } - -func (s *introspectionSuite) TestAccessDenied(c *gc.C) { - resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ - Method: "GET", - URL: s.url, - Tag: "user-bob", - Password: "hunter2", - }) - defer resp.Body.Close() - c.Assert(resp.StatusCode, gc.Equals, http.StatusForbidden) -} diff --git a/apiserver/macaroon_test.go b/apiserver/macaroon_test.go index ac158c28a69..04ce9d58468 100644 --- a/apiserver/macaroon_test.go +++ b/apiserver/macaroon_test.go @@ -112,7 +112,7 @@ func (s *macaroonLoginSuite) login(c *gc.C, info *api.Info) (params.LoginResult, return result, err } -func (s *macaroonLoginSuite) TestremoteUserLoginToControllerNoAccess(c *gc.C) { +func (s *macaroonLoginSuite) TestRemoteUserLoginToControllerNoAccess(c *gc.C) { s.DischargerLogin = func() string { return remoteUser } @@ -124,7 +124,7 @@ func (s *macaroonLoginSuite) TestremoteUserLoginToControllerNoAccess(c *gc.C) { assertPermissionDenied(c, err) } -func (s *macaroonLoginSuite) TestremoteUserLoginToControllerLoginAccess(c *gc.C) { +func (s *macaroonLoginSuite) TestRemoteUserLoginToControllerLoginAccess(c *gc.C) { setEveryoneAccess(c, s.ControllerModel(c).State(), jujutesting.AdminUser, permission.LoginAccess) var remoteUserTag = names.NewUserTag(remoteUser) diff --git a/apiserver/objects_test.go b/apiserver/objects_test.go index 9605d3c3995..f2bfb053f05 100644 --- a/apiserver/objects_test.go +++ b/apiserver/objects_test.go @@ -15,11 +15,14 @@ import ( "strings" "github.com/juju/charm/v13" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "github.com/juju/utils/v4" gc "gopkg.in/check.v1" apitesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -553,9 +556,23 @@ func (s *putCharmObjectSuite) TestMigrateCharmNotMigrating(c *gc.C) { func (s *putCharmObjectSuite) TestMigrateCharmUnauthorized(c *gc.C) { s.setModelImporting(c) + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. fact, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - user := fact.MakeUser(c, &factory.UserParams{Password: "hunter2"}) + fact.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) ch := testcharms.Repo.CharmArchive(c.MkDir(), "dummy") f, err := os.Open(ch.Path) @@ -567,7 +584,7 @@ func (s *putCharmObjectSuite) TestMigrateCharmUnauthorized(c *gc.C) { resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ Method: "PUT", URL: url, - Tag: user.Tag().String(), + Tag: userTag.String(), Password: "hunter2", Body: f, ExtraHeaders: map[string]string{ diff --git a/apiserver/pubsub_test.go b/apiserver/pubsub_test.go index 2b125b6a61a..fda80175dff 100644 --- a/apiserver/pubsub_test.go +++ b/apiserver/pubsub_test.go @@ -4,6 +4,7 @@ package apiserver_test import ( + "context" "fmt" "io" "net/http" @@ -20,6 +21,8 @@ import ( "github.com/juju/juju/apiserver" "github.com/juju/juju/apiserver/websocket/websockettest" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -61,11 +64,26 @@ func (s *pubsubSuite) TestNoAuth(c *gc.C) { } func (s *pubsubSuite) TestRejectsUserLogins(c *gc.C) { + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - user := f.MakeUser(c, &factory.UserParams{Password: "sekrit"}) - header := jujuhttp.BasicAuthHeader(user.Tag().String(), "sekrit") - s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: user username-.* is not a controller") + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) + + header := jujuhttp.BasicAuthHeader(userTag.String(), "hunter2") + s.checkAuthFails(c, header, http.StatusForbidden, "authorization failed: user .* is not a controller") } func (s *pubsubSuite) TestRejectsNonServerMachineLogins(c *gc.C) { diff --git a/apiserver/ratelimit_test.go b/apiserver/ratelimit_test.go index 8e7a7b38d94..73ec40d78f8 100644 --- a/apiserver/ratelimit_test.go +++ b/apiserver/ratelimit_test.go @@ -4,14 +4,18 @@ package apiserver_test import ( + "context" "time" "github.com/juju/clock/testclock" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" "github.com/juju/juju/api" corecontroller "github.com/juju/juju/controller" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/testing/factory" ) @@ -31,37 +35,6 @@ func (s *rateLimitSuite) SetUpTest(c *gc.C) { s.ApiServerSuite.SetUpTest(c) } -func (s *rateLimitSuite) infoForNewMachine(c *gc.C, info *api.Info) *api.Info { - // Make a copy - newInfo := *info - - f, release := s.NewFactory(c, info.ModelTag.Id()) - defer release() - machine, password := f.MakeMachineReturningPassword( - c, &factory.MachineParams{Nonce: "fake_nonce"}) - - newInfo.Tag = machine.Tag() - newInfo.Password = password - newInfo.Nonce = "fake_nonce" - return &newInfo -} - -func (s *rateLimitSuite) infoForNewUser(c *gc.C, info *api.Info) *api.Info { - // Make a copy - newInfo := *info - - f, release := s.NewFactory(c, info.ModelTag.Id()) - defer release() - password := "shhh..." - user := f.MakeUser(c, &factory.UserParams{ - Password: password, - }) - - newInfo.Tag = user.Tag() - newInfo.Password = password - return &newInfo -} - func (s *rateLimitSuite) TestRateLimitAgents(c *gc.C) { c.Assert(s.Server.Report(), jc.DeepEquals, map[string]interface{}{ "agent-ratelimit-max": 1, @@ -102,13 +75,54 @@ func (s *rateLimitSuite) TestRateLimitNotApplicableToUsers(c *gc.C) { defer conn1.Close() // User connections are fine. - user := s.infoForNewUser(c, info) + user := s.infoForNewUser(c, info, "fredrikthordendal") conn2, err := api.Open(user, fastDialOpts) c.Assert(err, jc.ErrorIsNil) defer conn2.Close() - user2 := s.infoForNewUser(c, info) + user2 := s.infoForNewUser(c, info, "jenskidman") conn3, err := api.Open(user2, fastDialOpts) c.Assert(err, jc.ErrorIsNil) defer conn3.Close() } + +func (s *rateLimitSuite) infoForNewMachine(c *gc.C, info *api.Info) *api.Info { + // Make a copy + newInfo := *info + + f, release := s.NewFactory(c, info.ModelTag.Id()) + defer release() + machine, password := f.MakeMachineReturningPassword( + c, &factory.MachineParams{Nonce: "fake_nonce"}) + + newInfo.Tag = machine.Tag() + newInfo.Password = password + newInfo.Nonce = "fake_nonce" + return &newInfo +} + +func (s *rateLimitSuite) infoForNewUser(c *gc.C, info *api.Info, name string) *api.Info { + // Make a copy + newInfo := *info + + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag(name) + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. + f, release := s.NewFactory(c, s.ControllerModelUUID()) + defer release() + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) + + newInfo.Tag = userTag + newInfo.Password = "hunter2" + return &newInfo +} diff --git a/apiserver/registration.go b/apiserver/registration.go index 9882ede26ca..224bf33a747 100644 --- a/apiserver/registration.go +++ b/apiserver/registration.go @@ -13,22 +13,17 @@ import ( "github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery" "github.com/juju/errors" "github.com/juju/names/v5" - "golang.org/x/crypto/nacl/secretbox" "gopkg.in/macaroon.v2" "github.com/juju/juju/apiserver/common" coremacaroon "github.com/juju/juju/core/macaroon" + usererrors "github.com/juju/juju/domain/user/errors" "github.com/juju/juju/environs" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" "github.com/juju/juju/state/stateenvirons" ) -const ( - secretboxNonceLength = 24 - secretboxKeyLength = 32 -) - // registerUserHandler is an http.Handler for the "/register" endpoint. This is // used to complete a secure user registration process, and provide controller // login credentials. @@ -61,6 +56,7 @@ func (h *registerUserHandler) ServeHTTP(w http.ResponseWriter, req *http.Request req, st.State, serviceFactory.ControllerConfig(), + serviceFactory.User(), serviceFactory.Cloud(), serviceFactory.Credential(), ) @@ -116,80 +112,60 @@ func (h *registerUserHandler) processPost( req *http.Request, st *state.State, controllerConfigService ControllerConfigService, + userService UserService, cloudService common.CloudService, credentialService common.CredentialService, ) ( names.UserTag, *params.SecretKeyLoginResponse, error, ) { - ctx := req.Context() - - failure := func(err error) (names.UserTag, *params.SecretKeyLoginResponse, error) { - return names.UserTag{}, nil, err - } - data, err := io.ReadAll(req.Body) if err != nil { - return failure(err) + return names.UserTag{}, nil, errors.Trace(err) } var loginRequest params.SecretKeyLoginRequest if err := json.Unmarshal(data, &loginRequest); err != nil { - return failure(err) + return names.UserTag{}, nil, errors.Trace(err) } // Basic validation: ensure that the request contains a valid user tag, // nonce, and ciphertext of the expected length. userTag, err := names.ParseUserTag(loginRequest.User) if err != nil { - return failure(err) - } - if len(loginRequest.Nonce) != secretboxNonceLength { - return failure(errors.NotValidf("nonce")) + return names.UserTag{}, nil, errors.Trace(err) } - // Decrypt the ciphertext with the user's secret key (if it has one). - user, err := st.User(userTag) + // Decrypt the ciphertext with the user's activation key (if it has one). + sealer, err := userService.SetPasswordWithActivationKey(req.Context(), userTag.Name(), loginRequest.Nonce, loginRequest.PayloadCiphertext) if err != nil { - return failure(err) - } - if len(user.SecretKey()) != secretboxKeyLength { - return failure(errors.NotFoundf("secret key for user %q", user.Name())) - } - var key [secretboxKeyLength]byte - var nonce [secretboxNonceLength]byte - copy(key[:], user.SecretKey()) - copy(nonce[:], loginRequest.Nonce) - payloadBytes, ok := secretbox.Open(nil, loginRequest.PayloadCiphertext, &nonce, &key) - if !ok { - // Cannot decrypt the ciphertext, which implies that the secret - // key specified by the client is invalid. - return failure(errors.NotValidf("secret key")) - } - - // Unmarshal the request payload, which contains the new password to - // set for the user. - var requestPayload params.SecretKeyLoginRequestPayload - if err := json.Unmarshal(payloadBytes, &requestPayload); err != nil { - return failure(errors.Annotate(err, "cannot unmarshal payload")) - } - if err := user.SetPassword(requestPayload.Password); err != nil { - return failure(errors.Annotate(err, "setting new password")) + if errors.Is(err, usererrors.ActivationKeyNotValid) { + return names.UserTag{}, nil, errors.NotValidf("activation key") + } else if errors.Is(err, usererrors.ActivationKeyNotFound) { + return names.UserTag{}, nil, errors.NotFoundf("activation key") + } + return names.UserTag{}, nil, errors.Trace(err) } // Respond with the CA-cert and password, encrypted again with the - // secret key. - responsePayload, err := h.getSecretKeyLoginResponsePayload(ctx, st, controllerConfigService, cloudService, credentialService) + // activation key. + responsePayload, err := h.getSecretKeyLoginResponsePayload(req.Context(), st, controllerConfigService, cloudService, credentialService) if err != nil { - return failure(errors.Trace(err)) + return names.UserTag{}, nil, errors.Trace(err) } - payloadBytes, err = json.Marshal(responsePayload) + payloadBytes, err := json.Marshal(responsePayload) if err != nil { - return failure(errors.Trace(err)) + return names.UserTag{}, nil, errors.Trace(err) } - if _, err := rand.Read(nonce[:]); err != nil { - return failure(errors.Trace(err)) + if _, err := rand.Read(loginRequest.Nonce); err != nil { + return names.UserTag{}, nil, errors.Trace(err) + } + + // Seal the response payload with the user's activation key. + sealed, err := sealer.Seal(loginRequest.Nonce, payloadBytes) + if err != nil { + return names.UserTag{}, nil, errors.Trace(err) } response := ¶ms.SecretKeyLoginResponse{ - Nonce: nonce[:], - PayloadCiphertext: secretbox.Seal(nil, payloadBytes, &nonce, &key), + Nonce: loginRequest.Nonce, + PayloadCiphertext: sealed, } return userTag, response, nil } diff --git a/apiserver/registration_test.go b/apiserver/registration_test.go index 9d26dbdcb0d..254b6159092 100644 --- a/apiserver/registration_test.go +++ b/apiserver/registration_test.go @@ -23,17 +23,23 @@ import ( "github.com/juju/juju/apiserver" "github.com/juju/juju/apiserver/common" + "github.com/juju/juju/core/user" + usererrors "github.com/juju/juju/domain/user/errors" + "github.com/juju/juju/domain/user/service" "github.com/juju/juju/environs" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" - "github.com/juju/juju/state" "github.com/juju/juju/state/stateenvirons" coretesting "github.com/juju/juju/testing" + "github.com/juju/juju/testing/factory" ) type registrationSuite struct { jujutesting.ApiServerSuite - bob *state.User + userService *service.Service + userUUID user.UUID + activationKey []byte registrationURL string } @@ -41,9 +47,26 @@ var _ = gc.Suite(®istrationSuite{}) func (s *registrationSuite) SetUpTest(c *gc.C) { s.ApiServerSuite.SetUpTest(c) - bob, err := s.ControllerModel(c).State().AddUserWithSecretKey("bob", "", "admin") + + s.userService = s.ControllerServiceFactory(c).User() + var err error + s.userUUID, _, err = s.userService.AddUser(context.Background(), service.AddUserArg{ + Name: "bob", + CreatorUUID: s.AdminUserUUID, + }) c.Assert(err, jc.ErrorIsNil) - s.bob = bob + + s.activationKey, err = s.userService.ResetPassword(context.Background(), "bob") + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. + f, release := s.NewFactory(c, s.ControllerModelUUID()) + defer release() + f.MakeUser(c, &factory.UserParams{ + Name: "bob", + }) + s.registrationURL = s.URL("/register", url.Values{}).String() } @@ -73,14 +96,15 @@ func (s *registrationSuite) assertRegisterNoProxy(c *gc.C, hasProxy bool) { proxier.EXPECT().Type().Return("kubernetes-port-forward") } - // Ensure we cannot log in with the password yet. - const password = "hunter2" - c.Assert(s.bob.PasswordValid(password), jc.IsFalse) + password := "hunter2" + // It should be not possible to log in as bob with the password "hunter2" + // now. + _, err := s.userService.GetUserByAuth(context.Background(), "bob", password) + c.Assert(err, jc.ErrorIs, usererrors.Unauthorized) validNonce := []byte(strings.Repeat("X", 24)) - secretKey := s.bob.SecretKey() ciphertext := s.sealBox( - c, validNonce, secretKey, fmt.Sprintf(`{"password": "%s"}`, password), + c, validNonce, s.activationKey, fmt.Sprintf(`{"password": "%s"}`, password), ) client := jujuhttp.NewClient(jujuhttp.WithSkipHostnameVerification(true)) resp := httptesting.Do(c, httptesting.DoRequestParams{ @@ -99,10 +123,9 @@ func (s *registrationSuite) assertRegisterNoProxy(c *gc.C, hasProxy bool) { // It should be possible to log in as bob with the // password "hunter2" now, and there should be no // secret key any longer. - err := s.bob.Refresh() + user, err := s.userService.GetUserByAuth(context.Background(), "bob", password) c.Assert(err, jc.ErrorIsNil) - c.Assert(s.bob.PasswordValid(password), jc.IsTrue) - c.Assert(s.bob.SecretKey(), gc.IsNil) + c.Check(user.UUID, gc.Equals, s.userUUID) var response params.SecretKeyLoginResponse bodyData, err := io.ReadAll(resp.Body) @@ -110,7 +133,9 @@ func (s *registrationSuite) assertRegisterNoProxy(c *gc.C, hasProxy bool) { err = json.Unmarshal(bodyData, &response) c.Assert(err, jc.ErrorIsNil) c.Assert(response.Nonce, gc.HasLen, len(validNonce)) - plaintext := s.openBox(c, response.PayloadCiphertext, response.Nonce, secretKey) + + // Open the box to ensure that the response is as expected. + plaintext := s.openBox(c, response.PayloadCiphertext, response.Nonce, s.activationKey) var responsePayload params.SecretKeyLoginResponsePayload err = json.Unmarshal(plaintext, &responsePayload) @@ -177,38 +202,25 @@ func (s *registrationSuite) TestRegisterInvalidCiphertext(c *gc.C) { fmt.Sprintf( `{"user": "user-bob", "nonce": "%s"}`, base64.StdEncoding.EncodeToString(validNonce), - ), `secret key not valid`, params.CodeNotValid, + ), `activation key not valid`, params.CodeNotValid, http.StatusInternalServerError, ) } func (s *registrationSuite) TestRegisterNoSecretKey(c *gc.C) { - err := s.bob.SetPassword("anything") + err := s.userService.SetPassword(context.Background(), "bob", auth.NewPassword("anything")) c.Assert(err, jc.ErrorIsNil) + validNonce := []byte(strings.Repeat("X", 24)) s.testInvalidRequest(c, fmt.Sprintf( `{"user": "user-bob", "nonce": "%s"}`, base64.StdEncoding.EncodeToString(validNonce), - ), `secret key for user "bob" not found`, params.CodeNotFound, + ), `activation key not found`, params.CodeNotFound, http.StatusNotFound, ) } -func (s *registrationSuite) TestRegisterInvalidRequestPayload(c *gc.C) { - validNonce := []byte(strings.Repeat("X", 24)) - ciphertext := s.sealBox(c, validNonce, s.bob.SecretKey(), "[]") - s.testInvalidRequest(c, - fmt.Sprintf( - `{"user": "user-bob", "nonce": "%s", "cipher-text": "%s"}`, - base64.StdEncoding.EncodeToString(validNonce), - base64.StdEncoding.EncodeToString(ciphertext), - ), - `cannot unmarshal payload: json: cannot unmarshal array into Go value of type params.SecretKeyLoginRequestPayload`, "", - http.StatusInternalServerError, - ) -} - func (s *registrationSuite) testInvalidRequest(c *gc.C, requestBody, errorMessage, errorCode string, statusCode int) { client := jujuhttp.NewClient(jujuhttp.WithSkipHostnameVerification(true)) httptesting.AssertJSONCall(c, httptesting.JSONCallParams{ @@ -234,8 +246,8 @@ func (s *registrationSuite) sealBox(c *gc.C, nonce, key []byte, message string) func (s *registrationSuite) openBox(c *gc.C, ciphertext, nonce, key []byte) []byte { var nonceArray [24]byte var keyArray [32]byte - c.Assert(copy(nonceArray[:], nonce), gc.Equals, len(nonceArray)) - c.Assert(copy(keyArray[:], key), gc.Equals, len(keyArray)) + c.Assert(copy(nonceArray[:], nonce), gc.Equals, len(nonceArray), gc.Commentf("nonce: %v", nonce)) + c.Assert(copy(keyArray[:], key), gc.Equals, len(keyArray), gc.Commentf("key: %v", key)) message, ok := secretbox.Open(nil, ciphertext, &nonceArray, &keyArray) c.Assert(ok, jc.IsTrue) return message diff --git a/apiserver/server_test.go b/apiserver/server_test.go index 510d7c111cb..512687c787f 100644 --- a/apiserver/server_test.go +++ b/apiserver/server_test.go @@ -272,7 +272,7 @@ func (s *serverSuite) TestAPIHandlerHasPermissionLogin(c *gc.C) { serviceFactory := s.ControllerServiceFactory(c) st := s.ControllerModel(c).State() - handler, _ := apiserver.TestingAPIHandlerWithEntity(c, s.StatePool(), st, serviceFactory.ControllerConfig(), u) + handler, _ := apiserver.TestingAPIHandlerWithEntity(c, s.StatePool(), st, serviceFactory.ControllerConfig(), serviceFactory.User(), u) defer handler.Kill() apiserver.AssertHasPermission(c, handler, permission.LoginAccess, ctag, true) @@ -286,7 +286,7 @@ func (s *serverSuite) TestAPIHandlerHasPermissionSuperUser(c *gc.C) { serviceFactory := s.ControllerServiceFactory(c) st := s.ControllerModel(c).State() - handler, _ := apiserver.TestingAPIHandlerWithEntity(c, s.StatePool(), st, serviceFactory.ControllerConfig(), u) + handler, _ := apiserver.TestingAPIHandlerWithEntity(c, s.StatePool(), st, serviceFactory.ControllerConfig(), serviceFactory.User(), u) defer handler.Kill() ua, err := st.SetUserAccess(user, ctag, permission.SuperuserAccess) @@ -313,7 +313,7 @@ func (s *serverSuite) TestAPIHandlerHasPermissionLoginToken(c *gc.C) { delegator := &jwt.PermissionDelegator{Token: token} st := s.ControllerModel(c).State() - handler, _ := apiserver.TestingAPIHandlerWithToken(c, s.StatePool(), st, serviceFactory.ControllerConfig(), token, delegator) + handler, _ := apiserver.TestingAPIHandlerWithToken(c, s.StatePool(), st, serviceFactory.ControllerConfig(), serviceFactory.User(), token, delegator) defer handler.Kill() apiserver.AssertHasPermission(c, handler, permission.LoginAccess, coretesting.ControllerTag, true) @@ -337,7 +337,7 @@ func (s *serverSuite) TestAPIHandlerMissingPermissionLoginToken(c *gc.C) { delegator := &jwt.PermissionDelegator{token} st := s.ControllerModel(c).State() - handler, _ := apiserver.TestingAPIHandlerWithToken(c, s.StatePool(), st, serviceFactory.ControllerConfig(), token, delegator) + handler, _ := apiserver.TestingAPIHandlerWithToken(c, s.StatePool(), st, serviceFactory.ControllerConfig(), serviceFactory.User(), token, delegator) defer handler.Kill() err = handler.HasPermission(permission.AdminAccess, coretesting.ModelTag) var reqError *errors.AccessRequiredError @@ -369,7 +369,7 @@ func (s *serverSuite) TestAPIHandlerConnectedModel(c *gc.C) { serviceFactory := s.ControllerServiceFactory(c) - handler, _ := apiserver.TestingAPIHandler(c, s.StatePool(), otherState, serviceFactory.ControllerConfig()) + handler, _ := apiserver.TestingAPIHandler(c, s.StatePool(), otherState, serviceFactory.ControllerConfig(), serviceFactory.User()) defer handler.Kill() c.Check(handler.ConnectedModel(), gc.Equals, otherState.ModelUUID()) } @@ -428,7 +428,7 @@ func assertStateBecomesClosed(c *gc.C, st *state.State) { func (s *serverSuite) checkAPIHandlerTeardown(c *gc.C, st *state.State) { serviceFactory := s.ControllerServiceFactory(c) - handler, resources := apiserver.TestingAPIHandler(c, s.StatePool(), st, serviceFactory.ControllerConfig()) + handler, resources := apiserver.TestingAPIHandler(c, s.StatePool(), st, serviceFactory.ControllerConfig(), serviceFactory.User()) resource := new(fakeResource) resources.Register(resource) diff --git a/apiserver/stateauthenticator/auth.go b/apiserver/stateauthenticator/auth.go index 4e11211eac8..1d4ee0b1ca5 100644 --- a/apiserver/stateauthenticator/auth.go +++ b/apiserver/stateauthenticator/auth.go @@ -44,9 +44,9 @@ var AgentTags = []string{ // This Authenticator only works with requests that have been handled // by one of the httpcontext.*ModelHandler handlers. type Authenticator struct { - statePool *state.StatePool - controllerConfigGetter ControllerConfigGetter - authContext *authContext + statePool *state.StatePool + controllerConfigService ControllerConfigService + authContext *authContext } // PermissionDelegator implements authentication.PermissionDelegator @@ -54,30 +54,29 @@ type PermissionDelegator struct { State *state.State } -// ControllerConfigGetter is an interface that can be implemented by +// ControllerConfigService is an interface that can be implemented by // types that can return a controller config. -type ControllerConfigGetter interface { +type ControllerConfigService interface { ControllerConfig(context.Context) (controller.Config, error) } // NewAuthenticator returns a new Authenticator using the given StatePool. func NewAuthenticator( statePool *state.StatePool, - controllerConfigGetter ControllerConfigGetter, + systemState *state.State, + controllerConfigService ControllerConfigService, + userService UserService, + agentAuthFactory AgentAuthenticatorFactory, clock clock.Clock, ) (*Authenticator, error) { - systemState, err := statePool.SystemState() - if err != nil { - return nil, errors.Trace(err) - } - authContext, err := newAuthContext(systemState, controllerConfigGetter, clock) + authContext, err := newAuthContext(systemState, controllerConfigService, userService, agentAuthFactory, clock) if err != nil { return nil, errors.Trace(err) } return &Authenticator{ - statePool: statePool, - controllerConfigGetter: controllerConfigGetter, - authContext: authContext, + statePool: statePool, + controllerConfigService: controllerConfigService, + authContext: authContext, }, nil } @@ -107,13 +106,8 @@ func (a *Authenticator) CreateLocalLoginMacaroon(ctx context.Context, tag names. // AddHandlers adds the handlers to the given mux for handling local // macaroon logins. func (a *Authenticator) AddHandlers(mux *apiserverhttp.Mux) error { - systemState, err := a.statePool.SystemState() - if err != nil { - return errors.Trace(err) - } h := &localLoginHandlers{ authCtxt: a.authContext, - finder: systemState, userTokens: map[string]string{}, } h.AddHandlers(mux) @@ -164,10 +158,10 @@ func (a *Authenticator) AuthenticateLoginRequest( } defer st.Release() - authenticator := a.authContext.authenticator(serverHost) - authInfo, err := a.checkCreds(ctx, st.State, authParams, true, authenticator) + authenticator := a.authContext.authenticatorForState(serverHost, st.State) + authInfo, err := a.checkCreds(ctx, st.State, authParams, authenticator) if err == nil { - return authInfo, err + return authInfo, nil } var dischargeRequired *apiservererrors.DischargeRequiredError @@ -176,19 +170,21 @@ func (a *Authenticator) AuthenticateLoginRequest( return authentication.AuthInfo{}, errors.Trace(err) } - _, isMachineTag := authParams.AuthTag.(names.MachineTag) - _, isControllerAgentTag := authParams.AuthTag.(names.ControllerAgentTag) systemState, errS := a.statePool.SystemState() if errS != nil { return authentication.AuthInfo{}, errors.Trace(err) } + + _, isMachineTag := authParams.AuthTag.(names.MachineTag) + _, isControllerAgentTag := authParams.AuthTag.(names.ControllerAgentTag) if (isMachineTag || isControllerAgentTag) && !st.IsController() { // Controller agents are allowed to log into any model. + authenticator := a.authContext.authenticator(serverHost) var err2 error authInfo, err2 = a.checkCreds( ctx, systemState, - authParams, false, authenticator, + authParams, authenticator, ) if err2 == nil && authInfo.Controller { err = nil @@ -197,7 +193,8 @@ func (a *Authenticator) AuthenticateLoginRequest( if err != nil { return authentication.AuthInfo{}, errors.NewUnauthorized(err, "") } - authInfo.Delegator = &PermissionDelegator{systemState} + + authInfo.Delegator = &PermissionDelegator{State: systemState} return authInfo, nil } @@ -225,17 +222,9 @@ func (a *Authenticator) checkCreds( ctx context.Context, st *state.State, authParams authentication.AuthParams, - userLogin bool, authenticator authentication.EntityAuthenticator, ) (authentication.AuthInfo, error) { - var entityFinder authentication.EntityFinder = st - if userLogin { - // When looking up model users, use a custom - // entity finder that looks up both the local user (if the user - // tag is in the local domain) and the model user. - entityFinder = modelUserEntityFinder{st} - } - entity, err := authenticator.Authenticate(ctx, entityFinder, authParams) + entity, err := authenticator.Authenticate(ctx, authParams) if err != nil { return authentication.AuthInfo{}, errors.Trace(err) } @@ -244,22 +233,124 @@ func (a *Authenticator) checkCreds( Delegator: &PermissionDelegator{State: st}, Entity: entity, } - type withIsManager interface { - IsManager() bool + + switch entity.Tag().Kind() { + case names.UserTagKind: + // TODO (stickupkid): This is incorrect. We should only be updating the + // last login time if they've been authorized (not just authenticated). + // For now we'll leave it as is, but we should fix this. + userTag := entity.Tag().(names.UserTag) + + st := a.authContext.st + model, err := st.Model() + if err != nil { + return authentication.AuthInfo{}, errors.Trace(err) + } + modelAccess, err := st.UserAccess(userTag, model.ModelTag()) + if err != nil && !errors.Is(err, errors.NotFound) { + return authentication.AuthInfo{}, errors.Trace(err) + } + + // This is permission checking at the wrong level, but we can keep it + // here for now. + if err := a.checkPerms(ctx, modelAccess, userTag); err != nil { + return authentication.AuthInfo{}, errors.Trace(err) + } + + if err := a.updateUserLastLogin(ctx, modelAccess, userTag, model); err != nil { + return authentication.AuthInfo{}, errors.Trace(err) + } + + case names.MachineTagKind, names.ControllerAgentTagKind: + // Currently only machines and controller agents are managers in the + // context of a controller. + authInfo.Controller = a.isManager(entity) } - if entity, ok := entity.(withIsManager); ok { - authInfo.Controller = entity.IsManager() + + return authInfo, nil +} + +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() { + return nil } - type withLogin interface { - UpdateLastLogin() error + // No model user found, so see if the user has been granted + // access to the controller. + if permission.IsEmptyUserAccess(modelAccess) { + st := a.authContext.st + controllerAccess, err := state.ControllerAccess(st, userTag) + if err != nil && !errors.Is(err, errors.NotFound) { + return errors.Trace(err) + } + // TODO(perrito666) remove the following section about everyone group + // when groups are implemented, this accounts only for the lack of a local + // ControllerUser when logging in from an external user that has not been granted + // permissions on the controller but there are permissions for the special + // everyone group. + if permission.IsEmptyUserAccess(controllerAccess) && !userTag.IsLocal() { + everyoneTag := names.NewUserTag(common.EveryoneTagName) + controllerAccess, err = st.UserAccess(everyoneTag, st.ControllerTag()) + if err != nil && !errors.Is(err, errors.NotFound) { + return errors.Annotatef(err, "obtaining ControllerUser for everyone group") + } + } + if permission.IsEmptyUserAccess(controllerAccess) { + return errors.NotFoundf("model or controller user") + } } - if entity, ok := entity.(withLogin); ok { - if err := entity.UpdateLastLogin(); err != nil { - logger.Warningf("updating last login time for %v", authParams.AuthTag) + return nil +} + +func (a *Authenticator) updateUserLastLogin(ctx context.Context, modelAccess permission.UserAccess, userTag names.UserTag, model *state.Model) error { + updateLastLogin := func() error { + // If the user is not local, we don't update the last login time. + if !userTag.IsLocal() { + return nil + } + + // Update the last login time for the user. + if err := a.authContext.userService.UpdateLastLogin(ctx, userTag.Name()); err != nil { + return errors.Trace(err) } + return nil } - return authInfo, nil + + if !permission.IsEmptyUserAccess(modelAccess) { + if modelAccess.Object.Kind() != names.ModelTagKind { + return errors.NotValidf("%s as model user", modelAccess.Object.Kind()) + } + + if err := model.UpdateLastModelConnection(modelAccess.UserTag); err != nil { + // Attempt to update the users last login data, if the update + // fails, then just report it as a log message and return the + // original error message. + if err := updateLastLogin(); err != nil { + logger.Warningf("updating last login time for %v, %v", userTag, err) + } + return errors.Trace(err) + } + } + + if err := updateLastLogin(); err != nil { + logger.Warningf("updating last login time for %v, %v", userTag, err) + } + + return nil +} + +func (a *Authenticator) isManager(entity state.Entity) bool { + type withIsManager interface { + IsManager() bool + } + + m, ok := entity.(withIsManager) + if !ok { + return false + } + return m.IsManager() } // LoginRequest extracts basic auth login details from an http.Request. diff --git a/apiserver/stateauthenticator/authentication_mock_test.go b/apiserver/stateauthenticator/authentication_mock_test.go new file mode 100644 index 00000000000..531bfbc7f2c --- /dev/null +++ b/apiserver/stateauthenticator/authentication_mock_test.go @@ -0,0 +1,57 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/apiserver/authentication (interfaces: EntityAuthenticator) +// +// Generated by this command: +// +// mockgen -package stateauthenticator -destination authentication_mock_test.go github.com/juju/juju/apiserver/authentication EntityAuthenticator +// + +// Package stateauthenticator is a generated GoMock package. +package stateauthenticator + +import ( + context "context" + reflect "reflect" + + authentication "github.com/juju/juju/apiserver/authentication" + state "github.com/juju/juju/state" + gomock "go.uber.org/mock/gomock" +) + +// MockEntityAuthenticator is a mock of EntityAuthenticator interface. +type MockEntityAuthenticator struct { + ctrl *gomock.Controller + recorder *MockEntityAuthenticatorMockRecorder +} + +// MockEntityAuthenticatorMockRecorder is the mock recorder for MockEntityAuthenticator. +type MockEntityAuthenticatorMockRecorder struct { + mock *MockEntityAuthenticator +} + +// NewMockEntityAuthenticator creates a new mock instance. +func NewMockEntityAuthenticator(ctrl *gomock.Controller) *MockEntityAuthenticator { + mock := &MockEntityAuthenticator{ctrl: ctrl} + mock.recorder = &MockEntityAuthenticatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockEntityAuthenticator) EXPECT() *MockEntityAuthenticatorMockRecorder { + return m.recorder +} + +// Authenticate mocks base method. +func (m *MockEntityAuthenticator) Authenticate(arg0 context.Context, arg1 authentication.AuthParams) (state.Entity, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Authenticate", arg0, arg1) + ret0, _ := ret[0].(state.Entity) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Authenticate indicates an expected call of Authenticate. +func (mr *MockEntityAuthenticatorMockRecorder) Authenticate(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authenticate", reflect.TypeOf((*MockEntityAuthenticator)(nil).Authenticate), arg0, arg1) +} diff --git a/apiserver/stateauthenticator/authenticator_test.go b/apiserver/stateauthenticator/authenticator_test.go index c352a5220a0..db6ac390157 100644 --- a/apiserver/stateauthenticator/authenticator_test.go +++ b/apiserver/stateauthenticator/authenticator_test.go @@ -1,7 +1,7 @@ // Copyright 2014-2018 Canonical Ltd. All rights reserved. // Licensed under the AGPLv3, see LICENCE file for details. -package stateauthenticator_test +package stateauthenticator import ( "context" @@ -14,18 +14,20 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/apiserver/authentication" - "github.com/juju/juju/apiserver/stateauthenticator" - "github.com/juju/juju/state" + coreuser "github.com/juju/juju/core/user" statetesting "github.com/juju/juju/state/testing" - "github.com/juju/juju/testing/factory" ) // TODO update these tests (moved from apiserver) to test // via the public interface, and then get rid of export_test.go. type agentAuthenticatorSuite struct { statetesting.StateSuite - authenticator *stateauthenticator.Authenticator - controllerConfigGetter *MockControllerConfigGetter + + authenticator *Authenticator + entityAuthenticator *MockEntityAuthenticator + agentAuthenticatorFactory *MockAgentAuthenticatorFactory + controllerConfigService *MockControllerConfigService + userService *MockUserService } var _ = gc.Suite(&agentAuthenticatorSuite{}) @@ -33,6 +35,8 @@ var _ = gc.Suite(&agentAuthenticatorSuite{}) func (s *agentAuthenticatorSuite) TestAuthenticateLoginRequestHandleNotSupportedRequests(c *gc.C) { defer s.setupMocks(c).Finish() + s.agentAuthenticatorFactory.EXPECT().AuthenticatorForState(gomock.Any()).Return(s.entityAuthenticator) + _, err := s.authenticator.AuthenticateLoginRequest(context.Background(), "", "", authentication.AuthParams{Token: "token"}) c.Assert(err, jc.ErrorIs, errors.NotSupported) } @@ -40,74 +44,101 @@ func (s *agentAuthenticatorSuite) TestAuthenticateLoginRequestHandleNotSupported func (s *agentAuthenticatorSuite) TestAuthenticatorForTag(c *gc.C) { defer s.setupMocks(c).Finish() - user := s.Factory.MakeUser(c, &factory.UserParams{Password: "password"}) + user := coreuser.User{ + Name: "user", + } + tag := names.NewUserTag("user") - authenticator, err := stateauthenticator.EntityAuthenticator(context.Background(), s.authenticator, user.Tag()) + s.agentAuthenticatorFactory.EXPECT().Authenticator().Return(s.entityAuthenticator) + + authenticator, err := s.authenticatorForTag(context.Background(), s.authenticator, tag) c.Assert(err, jc.ErrorIsNil) - c.Assert(authenticator, gc.NotNil) - userFinder := userFinder{user} + c.Check(authenticator, gc.NotNil) + + s.userService.EXPECT().GetUserByAuth(context.Background(), "user", "password").Return(user, nil).AnyTimes() - entity, err := authenticator.Authenticate(context.Background(), userFinder, authentication.AuthParams{ - AuthTag: user.Tag(), + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{ + AuthTag: tag, Credentials: "password", Nonce: "nonce", }) c.Assert(err, jc.ErrorIsNil) - c.Assert(entity, gc.DeepEquals, user) + c.Check(entity.Tag(), gc.DeepEquals, tag) +} + +func (s *agentAuthenticatorSuite) TestNotSupportedTag(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.agentAuthenticatorFactory.EXPECT().Authenticator().Return(s.entityAuthenticator) + + authenticator, err := s.authenticatorForTag(context.Background(), s.authenticator, names.NewCloudTag("not-support")) + c.Assert(err, gc.ErrorMatches, "unexpected login entity tag: invalid request") + c.Check(authenticator, gc.IsNil) } func (s *agentAuthenticatorSuite) TestMachineGetsAgentAuthenticator(c *gc.C) { defer s.setupMocks(c).Finish() - authenticator, err := stateauthenticator.EntityAuthenticator(context.Background(), s.authenticator, names.NewMachineTag("0")) + tag := names.NewMachineTag("0") + + s.agentAuthenticatorFactory.EXPECT().Authenticator().Return(s.entityAuthenticator) + s.entityAuthenticator.EXPECT().Authenticate(gomock.Any(), authentication.AuthParams{}).Return(authentication.TagToEntity(tag), nil) + + authenticator, err := s.authenticatorForTag(context.Background(), s.authenticator, tag) + c.Assert(err, jc.ErrorIsNil) + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{}) c.Assert(err, jc.ErrorIsNil) - _, ok := authenticator.(*authentication.AgentAuthenticator) - c.Assert(ok, jc.IsTrue) + c.Check(entity.Tag(), gc.Equals, tag) } func (s *agentAuthenticatorSuite) TestModelGetsAgentAuthenticator(c *gc.C) { defer s.setupMocks(c).Finish() - authenticator, err := stateauthenticator.EntityAuthenticator(context.Background(), s.authenticator, names.NewModelTag("deadbeef-0bad-400d-8000-4b1d0d06f00d")) + tag := names.NewModelTag("deadbeef-0bad-400d-8000-4b1d0d06f00d") + + s.agentAuthenticatorFactory.EXPECT().Authenticator().Return(s.entityAuthenticator) + s.entityAuthenticator.EXPECT().Authenticate(gomock.Any(), authentication.AuthParams{}).Return(authentication.TagToEntity(tag), nil) + + authenticator, err := s.authenticatorForTag(context.Background(), s.authenticator, tag) c.Assert(err, jc.ErrorIsNil) - _, ok := authenticator.(*authentication.AgentAuthenticator) - c.Assert(ok, jc.IsTrue) + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(entity.Tag(), gc.Equals, tag) } func (s *agentAuthenticatorSuite) TestUnitGetsAgentAuthenticator(c *gc.C) { defer s.setupMocks(c).Finish() - authenticator, err := stateauthenticator.EntityAuthenticator(context.Background(), s.authenticator, names.NewUnitTag("wordpress/0")) - c.Assert(err, jc.ErrorIsNil) - _, ok := authenticator.(*authentication.AgentAuthenticator) - c.Assert(ok, jc.IsTrue) -} + tag := names.NewUnitTag("wordpress/0") -func (s *agentAuthenticatorSuite) TestNotSupportedTag(c *gc.C) { - defer s.setupMocks(c).Finish() + s.agentAuthenticatorFactory.EXPECT().Authenticator().Return(s.entityAuthenticator) + s.entityAuthenticator.EXPECT().Authenticate(gomock.Any(), authentication.AuthParams{}).Return(authentication.TagToEntity(tag), nil) - authenticator, err := stateauthenticator.EntityAuthenticator(context.Background(), s.authenticator, names.NewCloudTag("not-support")) - c.Assert(err, gc.ErrorMatches, "unexpected login entity tag: invalid request") - c.Assert(authenticator, gc.IsNil) + authenticator, err := s.authenticatorForTag(context.Background(), s.authenticator, tag) + c.Assert(err, jc.ErrorIsNil) + entity, err := authenticator.Authenticate(context.Background(), authentication.AuthParams{}) + c.Assert(err, jc.ErrorIsNil) + c.Check(entity.Tag(), gc.Equals, tag) } func (s *agentAuthenticatorSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.controllerConfigGetter = NewMockControllerConfigGetter(ctrl) - s.controllerConfigGetter.EXPECT().ControllerConfig(gomock.Any()).Return(s.ControllerConfig, nil).AnyTimes() + s.agentAuthenticatorFactory = NewMockAgentAuthenticatorFactory(ctrl) + s.entityAuthenticator = NewMockEntityAuthenticator(ctrl) + + s.controllerConfigService = NewMockControllerConfigService(ctrl) + s.controllerConfigService.EXPECT().ControllerConfig(gomock.Any()).Return(s.ControllerConfig, nil).AnyTimes() - authenticator, err := stateauthenticator.NewAuthenticator(s.StatePool, s.controllerConfigGetter, clock.WallClock) + s.userService = NewMockUserService(ctrl) + + authenticator, err := NewAuthenticator(s.StatePool, s.State, s.controllerConfigService, s.userService, s.agentAuthenticatorFactory, clock.WallClock) c.Assert(err, jc.ErrorIsNil) s.authenticator = authenticator return ctrl } -type userFinder struct { - user state.Entity -} - -func (u userFinder) FindEntity(tag names.Tag) (state.Entity, error) { - return u.user, nil +func (s *agentAuthenticatorSuite) authenticatorForTag(ctx context.Context, authenticator *Authenticator, tag names.Tag) (authentication.EntityAuthenticator, error) { + return authenticator.authContext.authenticator("testing.invalid:1234").authenticatorForTag(ctx, tag) } diff --git a/apiserver/stateauthenticator/context.go b/apiserver/stateauthenticator/context.go index db2c995db69..429c933b9fe 100644 --- a/apiserver/stateauthenticator/context.go +++ b/apiserver/stateauthenticator/context.go @@ -23,6 +23,7 @@ import ( "github.com/juju/juju/apiserver/bakeryutil" apiservererrors "github.com/juju/juju/apiserver/errors" coremacaroon "github.com/juju/juju/core/macaroon" + coreuser "github.com/juju/juju/core/user" "github.com/juju/juju/state" ) @@ -37,14 +38,37 @@ const ( localUserIdentityLocationPath = "/auth" ) +// UserService is the interface that wraps the methods required to +// authenticate a user. +type UserService interface { + // GetUserByAuth returns the user with the given name and password. + GetUserByAuth(ctx context.Context, name, password string) (coreuser.User, error) + // GetUserByName returns the user with the given name. + GetUserByName(ctx context.Context, name string) (coreuser.User, error) + // UpdateLastLogin updates the last login time for the user with the + // given name. + UpdateLastLogin(ctx context.Context, name string) error +} + +// AgentAuthenticatorFactory is a factory for creating authenticators, which +// can create authenticators for a given state. +type AgentAuthenticatorFactory interface { + // Authenticator returns an authenticator using the factory's state. + Authenticator() authentication.EntityAuthenticator + + // AuthenticatorForState returns an authenticator for the given state. + AuthenticatorForState(st *state.State) authentication.EntityAuthenticator +} + // authContext holds authentication context shared // between all API endpoints. type authContext struct { - st *state.State - controllerConfigGetter ControllerConfigGetter + st *state.State + controllerConfigService ControllerConfigService + userService UserService - clock clock.Clock - agentAuth authentication.AgentAuthenticator + clock clock.Clock + agentAuthFactory AgentAuthenticatorFactory // localUserBakery is the bakery.Bakery used by the controller // for authenticating local users. In time, we may want to use this for @@ -84,14 +108,18 @@ func (OpenLoginAuthorizer) AuthorizeOps(ctx context.Context, authorizedOp bakery // newAuthContext creates a new authentication context for st. func newAuthContext( st *state.State, - controllerConfigGetter ControllerConfigGetter, + controllerConfigService ControllerConfigService, + userService UserService, + agentAuthFactory AgentAuthenticatorFactory, clock clock.Clock, ) (*authContext, error) { ctxt := &authContext{ - st: st, - clock: clock, - controllerConfigGetter: controllerConfigGetter, - localUserInteractions: authentication.NewInteractions(), + st: st, + clock: clock, + controllerConfigService: controllerConfigService, + userService: userService, + localUserInteractions: authentication.NewInteractions(), + agentAuthFactory: agentAuthFactory, } // Create a bakery for discharging third-party caveats for @@ -185,17 +213,32 @@ func (ctxt *authContext) DischargeCaveats(tag names.UserTag) []checkers.Caveat { return authentication.DischargeCaveats(tag, ctxt.clock) } +// authenticatorForState returns an authenticator.Authenticator for the API +// connection associated with the specified API server host and state. +func (ctxt *authContext) authenticatorForState(serverHost string, st *state.State) authenticator { + return authenticator{ + ctxt: ctxt, + serverHost: serverHost, + agentAuthenticator: ctxt.agentAuthFactory.AuthenticatorForState(st), + } +} + // authenticator returns an authenticator.Authenticator for the API // connection associated with the specified API server host. func (ctxt *authContext) authenticator(serverHost string) authenticator { - return authenticator{ctxt: ctxt, serverHost: serverHost} + return authenticator{ + ctxt: ctxt, + serverHost: serverHost, + agentAuthenticator: ctxt.agentAuthFactory.Authenticator(), + } } // authenticator implements authenticator.Authenticator, delegating // to the appropriate authenticator based on the tag kind. type authenticator struct { - ctxt *authContext - serverHost string + ctxt *authContext + serverHost string + agentAuthenticator authentication.EntityAuthenticator } // Authenticate implements authentication.Authenticator @@ -203,14 +246,13 @@ type authenticator struct { // tag. func (a authenticator) Authenticate( ctx context.Context, - entityFinder authentication.EntityFinder, authParams authentication.AuthParams, ) (state.Entity, error) { auth, err := a.authenticatorForTag(ctx, authParams.AuthTag) if err != nil { return nil, errors.Trace(err) } - return auth.Authenticate(ctx, entityFinder, authParams) + return auth.Authenticate(ctx, authParams) } // authenticatorForTag returns the authenticator appropriate @@ -232,9 +274,13 @@ func (a authenticator) authenticatorForTag(ctx context.Context, tag names.Tag) ( } return auth, nil } - for _, agentKind := range AgentTags { - if tag.Kind() == agentKind { - return &a.ctxt.agentAuth, nil + // If the tag is not a user tag, it must be an agent tag, attempt to locate + // it. + // TODO (stickupkid): This should just be a switch. We don't need to loop + // through all the agent tags, it's pointless. + for _, kind := range AgentTags { + if tag.Kind() == kind { + return a.agentAuthenticator, nil } } return nil, errors.Annotatef(apiservererrors.ErrBadRequest, "unexpected login entity tag") @@ -249,6 +295,7 @@ func (a authenticator) localUserAuth() *authentication.LocalUserAuthenticator { Path: localUserIdentityLocationPath, } return &authentication.LocalUserAuthenticator{ + UserService: a.ctxt.userService, Bakery: a.ctxt.localUserBakery, Clock: a.ctxt.clock, LocalUserIdentityLocation: localUserIdentityLocation.String(), @@ -259,7 +306,7 @@ func (a authenticator) localUserAuth() *authentication.LocalUserAuthenticator { // logins for external users. If it fails once, it will always fail. func (ctxt *authContext) externalMacaroonAuth(ctx context.Context, identClient identchecker.IdentityClient) (authentication.EntityAuthenticator, error) { ctxt.macaroonAuthOnce.Do(func() { - ctxt._macaroonAuth, ctxt._macaroonAuthError = newExternalMacaroonAuth(ctx, ctxt.st, ctxt.controllerConfigGetter, ctxt.clock, externalLoginExpiryTime, identClient) + ctxt._macaroonAuth, ctxt._macaroonAuthError = newExternalMacaroonAuth(ctx, ctxt.st, ctxt.controllerConfigService, ctxt.clock, externalLoginExpiryTime, identClient) }) if ctxt._macaroonAuth == nil { return nil, errors.Trace(ctxt._macaroonAuthError) @@ -270,8 +317,8 @@ func (ctxt *authContext) externalMacaroonAuth(ctx context.Context, identClient i // newExternalMacaroonAuth returns an authenticator that can authenticate // macaroon-based logins for external users. This is just a helper function // for authCtxt.externalMacaroonAuth. -func newExternalMacaroonAuth(ctx context.Context, st *state.State, controllerConfigGetter ControllerConfigGetter, clock clock.Clock, expiryTime time.Duration, identClient identchecker.IdentityClient) (*authentication.ExternalMacaroonAuthenticator, error) { - controllerCfg, err := controllerConfigGetter.ControllerConfig(ctx) +func newExternalMacaroonAuth(ctx context.Context, st *state.State, controllerConfigService ControllerConfigService, clock clock.Clock, expiryTime time.Duration, identClient identchecker.IdentityClient) (*authentication.ExternalMacaroonAuthenticator, error) { + controllerCfg, err := controllerConfigService.ControllerConfig(ctx) if err != nil { return nil, errors.Annotate(err, "cannot get controller config") } diff --git a/apiserver/stateauthenticator/context_test.go b/apiserver/stateauthenticator/context_test.go index f3791cf702f..bbf6b3e2da5 100644 --- a/apiserver/stateauthenticator/context_test.go +++ b/apiserver/stateauthenticator/context_test.go @@ -1,7 +1,7 @@ // Copyright 2018 Canonical Ltd. All rights reserved. // Licensed under the AGPLv3, see LICENCE file for details. -package stateauthenticator_test +package stateauthenticator import ( "context" @@ -19,9 +19,10 @@ import ( gc "gopkg.in/check.v1" "gopkg.in/macaroon.v2" - "github.com/juju/juju/apiserver/stateauthenticator" + "github.com/juju/juju/apiserver/authentication" "github.com/juju/juju/controller" statetesting "github.com/juju/juju/state/testing" + "github.com/juju/juju/testing" ) // TODO(babbageclunk): These have been extracted pretty mechanically @@ -31,10 +32,11 @@ import ( type macaroonCommonSuite struct { statetesting.StateSuite - discharger *bakerytest.Discharger - authenticator *stateauthenticator.Authenticator - clock *testclock.Clock - controllerConfigGetter *MockControllerConfigGetter + discharger *bakerytest.Discharger + authenticator *Authenticator + clock *testclock.Clock + controllerConfigService *MockControllerConfigService + userService *MockUserService } func (s *macaroonCommonSuite) SetUpTest(c *gc.C) { @@ -52,10 +54,12 @@ func (s *macaroonCommonSuite) TearDownTest(c *gc.C) { func (s *macaroonCommonSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.controllerConfigGetter = NewMockControllerConfigGetter(ctrl) - s.controllerConfigGetter.EXPECT().ControllerConfig(gomock.Any()).Return(s.ControllerConfig, nil).AnyTimes() + s.controllerConfigService = NewMockControllerConfigService(ctrl) + s.controllerConfigService.EXPECT().ControllerConfig(gomock.Any()).Return(s.ControllerConfig, nil).AnyTimes() - authenticator, err := stateauthenticator.NewAuthenticator(s.StatePool, s.controllerConfigGetter, s.clock) + agentAuthFactory := authentication.NewAgentAuthenticatorFactory(s.State, testing.NewCheckLogger(c)) + + authenticator, err := NewAuthenticator(s.StatePool, s.State, s.controllerConfigService, s.userService, agentAuthFactory, s.clock) c.Assert(err, jc.ErrorIsNil) s.authenticator = authenticator @@ -104,7 +108,7 @@ func (s *macaroonAuthSuite) TestServerBakery(c *gc.C) { return nil, errors.New("unexpected caveat") }) - bsvc, err := stateauthenticator.ServerBakery(context.Background(), s.authenticator, &alwaysIdent{IdentityLocation: discharger.Location()}) + bsvc, err := ServerBakery(context.Background(), s.authenticator, &alwaysIdent{IdentityLocation: discharger.Location()}) c.Assert(err, gc.IsNil) cav := []checkers.Caveat{ @@ -135,7 +139,7 @@ func (s *macaroonAuthSuite) TestServerBakery(c *gc.C) { func (s *macaroonAuthSuite) TestExpiredKey(c *gc.C) { defer s.setupMocks(c).Finish() - bsvc, err := stateauthenticator.ServerBakeryExpiresImmediately(context.Background(), s.authenticator, &alwaysIdent{}) + bsvc, err := ServerBakeryExpiresImmediately(context.Background(), s.authenticator, &alwaysIdent{}) c.Assert(err, gc.IsNil) cav := []checkers.Caveat{ @@ -213,6 +217,6 @@ func (s *macaroonNoURLSuite) TestNoBakeryWhenNoIdentityURL(c *gc.C) { defer s.setupMocks(c).Finish() // By default, when there is no identity location, no bakery is created. - _, err := stateauthenticator.ServerBakery(context.Background(), s.authenticator, nil) + _, err := ServerBakery(context.Background(), s.authenticator, nil) c.Assert(err, gc.ErrorMatches, "macaroon authentication is not configured") } diff --git a/apiserver/stateauthenticator/controller_config_mock_test.go b/apiserver/stateauthenticator/controller_config_mock_test.go deleted file mode 100644 index 32fee1e99ce..00000000000 --- a/apiserver/stateauthenticator/controller_config_mock_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/apiserver/stateauthenticator (interfaces: ControllerConfigGetter) -// -// Generated by this command: -// -// mockgen -package stateauthenticator_test -destination controller_config_mock_test.go github.com/juju/juju/apiserver/stateauthenticator ControllerConfigGetter -// - -// Package stateauthenticator_test is a generated GoMock package. -package stateauthenticator_test - -import ( - context "context" - reflect "reflect" - - controller "github.com/juju/juju/controller" - gomock "go.uber.org/mock/gomock" -) - -// MockControllerConfigGetter is a mock of ControllerConfigGetter interface. -type MockControllerConfigGetter struct { - ctrl *gomock.Controller - recorder *MockControllerConfigGetterMockRecorder -} - -// MockControllerConfigGetterMockRecorder is the mock recorder for MockControllerConfigGetter. -type MockControllerConfigGetterMockRecorder struct { - mock *MockControllerConfigGetter -} - -// NewMockControllerConfigGetter creates a new mock instance. -func NewMockControllerConfigGetter(ctrl *gomock.Controller) *MockControllerConfigGetter { - mock := &MockControllerConfigGetter{ctrl: ctrl} - mock.recorder = &MockControllerConfigGetterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockControllerConfigGetter) EXPECT() *MockControllerConfigGetterMockRecorder { - return m.recorder -} - -// ControllerConfig mocks base method. -func (m *MockControllerConfigGetter) ControllerConfig(arg0 context.Context) (controller.Config, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControllerConfig", arg0) - ret0, _ := ret[0].(controller.Config) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ControllerConfig indicates an expected call of ControllerConfig. -func (mr *MockControllerConfigGetterMockRecorder) ControllerConfig(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerConfig", reflect.TypeOf((*MockControllerConfigGetter)(nil).ControllerConfig), arg0) -} diff --git a/apiserver/stateauthenticator/export_test.go b/apiserver/stateauthenticator/export_test.go index c0853fa7607..968ebe4a196 100644 --- a/apiserver/stateauthenticator/export_test.go +++ b/apiserver/stateauthenticator/export_test.go @@ -7,17 +7,10 @@ import ( "context" "github.com/go-macaroon-bakery/macaroon-bakery/v3/bakery/identchecker" - "github.com/juju/names/v5" "github.com/juju/juju/apiserver/authentication" ) -// TODO update the tests moved from apiserver to test via the public -// interface, and then get rid of these. -func EntityAuthenticator(ctx context.Context, authenticator *Authenticator, tag names.Tag) (authentication.EntityAuthenticator, error) { - return authenticator.authContext.authenticator("testing.invalid:1234").authenticatorForTag(ctx, tag) -} - func ServerBakery(ctx context.Context, a *Authenticator, identClient identchecker.IdentityClient) (*identchecker.Bakery, error) { auth, err := a.authContext.externalMacaroonAuth(ctx, identClient) if err != nil { @@ -27,7 +20,10 @@ func ServerBakery(ctx context.Context, a *Authenticator, identClient identchecke } func ServerBakeryExpiresImmediately(ctx context.Context, a *Authenticator, identClient identchecker.IdentityClient) (*identchecker.Bakery, error) { - auth, err := newExternalMacaroonAuth(ctx, a.authContext.st, a.authContext.controllerConfigGetter, a.authContext.clock, 0, identClient) + st := a.authContext.st + controllerConfigService := a.authContext.controllerConfigService + + auth, err := newExternalMacaroonAuth(ctx, st, controllerConfigService, a.authContext.clock, 0, identClient) if err != nil { return nil, err } diff --git a/apiserver/stateauthenticator/locallogin.go b/apiserver/stateauthenticator/locallogin.go index 3738d581568..f0200f750b1 100644 --- a/apiserver/stateauthenticator/locallogin.go +++ b/apiserver/stateauthenticator/locallogin.go @@ -20,12 +20,10 @@ import ( "github.com/juju/juju/apiserver/apiserverhttp" "github.com/juju/juju/apiserver/authentication" - "github.com/juju/juju/state" ) type localLoginHandlers struct { authCtxt *authContext - finder state.EntityFinder userTokens map[string]string } @@ -88,7 +86,7 @@ func (h *localLoginHandlers) formHandler(w http.ResponseWriter, req *http.Reques } authenticator := h.authCtxt.authenticator(req.Host) - if _, err := authenticator.Authenticate(ctx, h.finder, authentication.AuthParams{ + if _, err := authenticator.Authenticate(ctx, authentication.AuthParams{ AuthTag: userTag, Credentials: password, }); err != nil { diff --git a/apiserver/stateauthenticator/modeluser.go b/apiserver/stateauthenticator/modeluser.go deleted file mode 100644 index f4dc87e5949..00000000000 --- a/apiserver/stateauthenticator/modeluser.go +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright 2018 Canonical Ltd. -// Licensed under the AGPLv3, see LICENCE file for details. - -package stateauthenticator - -import ( - "time" - - "github.com/juju/errors" - "github.com/juju/names/v5" - - "github.com/juju/juju/apiserver/common" - "github.com/juju/juju/core/permission" - "github.com/juju/juju/state" - stateerrors "github.com/juju/juju/state/errors" -) - -// modelUserEntityFinder implements state.EntityFinder by returning -// an Entity value for model users, ensuring that the user exists in -// the state's current model, while also supporting external users. -type modelUserEntityFinder struct { - st *state.State -} - -// FindEntity implements state.EntityFinder. -func (f modelUserEntityFinder) FindEntity(tag names.Tag) (state.Entity, error) { - utag, ok := tag.(names.UserTag) - if !ok { - return f.st.FindEntity(tag) - } - - model, err := f.st.Model() - if err != nil { - return nil, errors.Trace(err) - } - modelAccess, err := f.st.UserAccess(utag, model.ModelTag()) - if err != nil && !errors.Is(err, errors.NotFound) { - return nil, errors.Trace(err) - } - - // No model user found, so see if the user has been granted - // access to the controller. - if permission.IsEmptyUserAccess(modelAccess) { - controllerAccess, err := state.ControllerAccess(f.st, utag) - if err != nil && !errors.Is(err, errors.NotFound) { - return nil, errors.Trace(err) - } - // TODO(perrito666) remove the following section about everyone group - // when groups are implemented, this accounts only for the lack of a local - // ControllerUser when logging in from an external user that has not been granted - // permissions on the controller but there are permissions for the special - // everyone group. - if permission.IsEmptyUserAccess(controllerAccess) && !utag.IsLocal() { - everyoneTag := names.NewUserTag(common.EveryoneTagName) - controllerAccess, err = f.st.UserAccess(everyoneTag, f.st.ControllerTag()) - if err != nil && !errors.Is(err, errors.NotFound) { - return nil, errors.Annotatef(err, "obtaining ControllerUser for everyone group") - } - } - if permission.IsEmptyUserAccess(controllerAccess) { - return nil, errors.NotFoundf("model or controller user") - } - } - - u := &modelUserEntity{ - st: f.st, - modelAccess: modelAccess, - tag: utag, - } - if utag.IsLocal() { - user, err := f.st.User(utag) - if err != nil { - return nil, errors.Trace(err) - } - u.user = user - } - return u, nil -} - -// modelUserEntity encapsulates a model user -// and, if the user is local, the local state user -// as well. This enables us to implement FindEntity -// in such a way that the authentication mechanisms -// can work without knowing these details. -type modelUserEntity struct { - st *state.State - - modelAccess permission.UserAccess - tag names.Tag - - // user is nil for external users. - user *state.User -} - -// Refresh implements state.Authenticator.Refresh. -func (u *modelUserEntity) Refresh() error { - if u.user == nil { - return nil - } - return u.user.Refresh() -} - -// SetPassword implements state.Authenticator.SetPassword -// by setting the password on the local user. -func (u *modelUserEntity) SetPassword(pass string) error { - if u.user == nil { - return errors.New("cannot set password on external user") - } - return u.user.SetPassword(pass) -} - -// PasswordValid implements state.Authenticator.PasswordValid. -func (u *modelUserEntity) PasswordValid(pass string) bool { - if u.user == nil { - return false - } - return u.user.PasswordValid(pass) -} - -// Tag implements state.Entity.Tag. -func (u *modelUserEntity) Tag() names.Tag { - return u.tag -} - -// LastLogin implements loginEntity.LastLogin. -func (u *modelUserEntity) LastLogin() (time.Time, error) { - // The last connection for the model takes precedence over - // the local user last login time. - var err error - var t time.Time - - model, err := u.st.Model() - if err != nil { - return t, errors.Trace(err) - } - - if !permission.IsEmptyUserAccess(u.modelAccess) { - t, err = model.LastModelConnection(u.modelAccess.UserTag) - } else { - err = stateerrors.NewNeverConnectedError("controller user") - } - if stateerrors.IsNeverConnectedError(err) || permission.IsEmptyUserAccess(u.modelAccess) { - if u.user != nil { - // There's a global user, so use that login time instead. - return u.user.LastLogin() - } - // Since we're implementing LastLogin, we need - // to implement LastLogin error semantics too. - err = stateerrors.NewNeverLoggedInError(err.Error()) - } - return t, errors.Trace(err) -} - -// UpdateLastLogin implements loginEntity.UpdateLastLogin. -func (u *modelUserEntity) UpdateLastLogin() error { - updateLastLogin := func() error { - // If user is nil, don't attempt to perform the update and exit early. - if u.user == nil { - return nil - } - - if err := u.user.UpdateLastLogin(); err != nil { - return errors.Trace(err) - } - return nil - } - - if !permission.IsEmptyUserAccess(u.modelAccess) { - if u.modelAccess.Object.Kind() != names.ModelTagKind { - return errors.NotValidf("%s as model user", u.modelAccess.Object.Kind()) - } - - model, err := u.st.Model() - if err != nil { - return errors.Trace(err) - } - - if err := model.UpdateLastModelConnection(u.modelAccess.UserTag); err != nil { - // Attempt to update the users last login data, if the update - // fails, then just report it as a log message and return the - // original error message. - if err := updateLastLogin(); err != nil { - logger.Warningf("Unable to update last login with %s", err.Error()) - } - return errors.Trace(err) - } - } - - return updateLastLogin() -} diff --git a/apiserver/stateauthenticator/package_test.go b/apiserver/stateauthenticator/package_test.go index c5cb62dfa54..827a17a3d7a 100644 --- a/apiserver/stateauthenticator/package_test.go +++ b/apiserver/stateauthenticator/package_test.go @@ -9,7 +9,8 @@ import ( coretesting "github.com/juju/juju/testing" ) -//go:generate go run go.uber.org/mock/mockgen -package stateauthenticator_test -destination controller_config_mock_test.go github.com/juju/juju/apiserver/stateauthenticator ControllerConfigGetter +//go:generate go run go.uber.org/mock/mockgen -package stateauthenticator -destination services_mock_test.go github.com/juju/juju/apiserver/stateauthenticator ControllerConfigService,UserService,AgentAuthenticatorFactory +//go:generate go run go.uber.org/mock/mockgen -package stateauthenticator -destination authentication_mock_test.go github.com/juju/juju/apiserver/authentication EntityAuthenticator func TestPackage(t *testing.T) { coretesting.MgoTestPackage(t) diff --git a/apiserver/stateauthenticator/services_mock_test.go b/apiserver/stateauthenticator/services_mock_test.go new file mode 100644 index 00000000000..abaea005fdf --- /dev/null +++ b/apiserver/stateauthenticator/services_mock_test.go @@ -0,0 +1,177 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/apiserver/stateauthenticator (interfaces: ControllerConfigService,UserService,AgentAuthenticatorFactory) +// +// Generated by this command: +// +// mockgen -package stateauthenticator -destination services_mock_test.go github.com/juju/juju/apiserver/stateauthenticator ControllerConfigService,UserService,AgentAuthenticatorFactory +// + +// Package stateauthenticator is a generated GoMock package. +package stateauthenticator + +import ( + context "context" + reflect "reflect" + + authentication "github.com/juju/juju/apiserver/authentication" + controller "github.com/juju/juju/controller" + user "github.com/juju/juju/core/user" + state "github.com/juju/juju/state" + gomock "go.uber.org/mock/gomock" +) + +// MockControllerConfigService is a mock of ControllerConfigService interface. +type MockControllerConfigService struct { + ctrl *gomock.Controller + recorder *MockControllerConfigServiceMockRecorder +} + +// MockControllerConfigServiceMockRecorder is the mock recorder for MockControllerConfigService. +type MockControllerConfigServiceMockRecorder struct { + mock *MockControllerConfigService +} + +// NewMockControllerConfigService creates a new mock instance. +func NewMockControllerConfigService(ctrl *gomock.Controller) *MockControllerConfigService { + mock := &MockControllerConfigService{ctrl: ctrl} + mock.recorder = &MockControllerConfigServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockControllerConfigService) EXPECT() *MockControllerConfigServiceMockRecorder { + return m.recorder +} + +// ControllerConfig mocks base method. +func (m *MockControllerConfigService) ControllerConfig(arg0 context.Context) (controller.Config, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerConfig", arg0) + ret0, _ := ret[0].(controller.Config) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ControllerConfig indicates an expected call of ControllerConfig. +func (mr *MockControllerConfigServiceMockRecorder) ControllerConfig(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerConfig", reflect.TypeOf((*MockControllerConfigService)(nil).ControllerConfig), arg0) +} + +// MockUserService is a mock of UserService interface. +type MockUserService struct { + ctrl *gomock.Controller + recorder *MockUserServiceMockRecorder +} + +// MockUserServiceMockRecorder is the mock recorder for MockUserService. +type MockUserServiceMockRecorder struct { + mock *MockUserService +} + +// NewMockUserService creates a new mock instance. +func NewMockUserService(ctrl *gomock.Controller) *MockUserService { + mock := &MockUserService{ctrl: ctrl} + mock.recorder = &MockUserServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUserService) EXPECT() *MockUserServiceMockRecorder { + return m.recorder +} + +// GetUserByAuth mocks base method. +func (m *MockUserService) GetUserByAuth(arg0 context.Context, arg1, arg2 string) (user.User, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserByAuth", arg0, arg1, arg2) + ret0, _ := ret[0].(user.User) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserByAuth indicates an expected call of GetUserByAuth. +func (mr *MockUserServiceMockRecorder) GetUserByAuth(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserByAuth", reflect.TypeOf((*MockUserService)(nil).GetUserByAuth), arg0, arg1, arg2) +} + +// GetUserByName mocks base method. +func (m *MockUserService) GetUserByName(arg0 context.Context, arg1 string) (user.User, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserByName", arg0, arg1) + ret0, _ := ret[0].(user.User) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserByName indicates an expected call of GetUserByName. +func (mr *MockUserServiceMockRecorder) GetUserByName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserByName", reflect.TypeOf((*MockUserService)(nil).GetUserByName), arg0, arg1) +} + +// UpdateLastLogin mocks base method. +func (m *MockUserService) UpdateLastLogin(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateLastLogin", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateLastLogin indicates an expected call of UpdateLastLogin. +func (mr *MockUserServiceMockRecorder) UpdateLastLogin(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLastLogin", reflect.TypeOf((*MockUserService)(nil).UpdateLastLogin), arg0, arg1) +} + +// MockAgentAuthenticatorFactory is a mock of AgentAuthenticatorFactory interface. +type MockAgentAuthenticatorFactory struct { + ctrl *gomock.Controller + recorder *MockAgentAuthenticatorFactoryMockRecorder +} + +// MockAgentAuthenticatorFactoryMockRecorder is the mock recorder for MockAgentAuthenticatorFactory. +type MockAgentAuthenticatorFactoryMockRecorder struct { + mock *MockAgentAuthenticatorFactory +} + +// NewMockAgentAuthenticatorFactory creates a new mock instance. +func NewMockAgentAuthenticatorFactory(ctrl *gomock.Controller) *MockAgentAuthenticatorFactory { + mock := &MockAgentAuthenticatorFactory{ctrl: ctrl} + mock.recorder = &MockAgentAuthenticatorFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAgentAuthenticatorFactory) EXPECT() *MockAgentAuthenticatorFactoryMockRecorder { + return m.recorder +} + +// Authenticator mocks base method. +func (m *MockAgentAuthenticatorFactory) Authenticator() authentication.EntityAuthenticator { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Authenticator") + ret0, _ := ret[0].(authentication.EntityAuthenticator) + return ret0 +} + +// Authenticator indicates an expected call of Authenticator. +func (mr *MockAgentAuthenticatorFactoryMockRecorder) Authenticator() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authenticator", reflect.TypeOf((*MockAgentAuthenticatorFactory)(nil).Authenticator)) +} + +// AuthenticatorForState mocks base method. +func (m *MockAgentAuthenticatorFactory) AuthenticatorForState(arg0 *state.State) authentication.EntityAuthenticator { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthenticatorForState", arg0) + ret0, _ := ret[0].(authentication.EntityAuthenticator) + return ret0 +} + +// AuthenticatorForState indicates an expected call of AuthenticatorForState. +func (mr *MockAgentAuthenticatorFactoryMockRecorder) AuthenticatorForState(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthenticatorForState", reflect.TypeOf((*MockAgentAuthenticatorFactory)(nil).AuthenticatorForState), arg0) +} diff --git a/apiserver/tools_integration_test.go b/apiserver/tools_integration_test.go index c42a958ee9b..afac7e25f0f 100644 --- a/apiserver/tools_integration_test.go +++ b/apiserver/tools_integration_test.go @@ -4,6 +4,7 @@ package apiserver_test import ( + "context" "encoding/json" "fmt" "io" @@ -20,6 +21,8 @@ import ( apiauthentication "github.com/juju/juju/api/authentication" apitesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/domain/user/service" + "github.com/juju/juju/internal/auth" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/testing/factory" @@ -122,10 +125,24 @@ func (s *toolsWithMacaroonsIntegrationSuite) TestCanPostWithDischargedMacaroon(c func (s *toolsWithMacaroonsIntegrationSuite) TestCanPostWithLocalLogin(c *gc.C) { // Create a new local user that we can log in as // using macaroon authentication. - const password = "hunter2" + password := "hunter2" + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword(password)), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() - user := f.MakeUser(c, &factory.UserParams{Password: password}) + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) // Install a "web-page" visitor that deals with the interaction // method that Juju controllers support for authenticating local @@ -141,9 +158,9 @@ func (s *toolsWithMacaroonsIntegrationSuite) TestCanPostWithLocalLogin(c *gc.C) ) bakeryClient.Client = client.Client() bakeryClient.AddInteractor(apiauthentication.NewInteractor( - user.UserTag().Id(), + userTag.Id(), func(username string) (string, error) { - c.Assert(username, gc.Equals, user.UserTag().Id()) + c.Assert(username, gc.Equals, userTag.Id()) prompted = true return password, nil }, @@ -156,7 +173,7 @@ func (s *toolsWithMacaroonsIntegrationSuite) TestCanPostWithLocalLogin(c *gc.C) resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ Method: "POST", URL: s.toolsURI(""), - Tag: user.UserTag().String(), + Tag: userTag.String(), Password: "", // no password forces macaroon usage Do: bakeryDo, }) diff --git a/apiserver/tools_test.go b/apiserver/tools_test.go index fad10dfe5e6..c63fa9d7b4f 100644 --- a/apiserver/tools_test.go +++ b/apiserver/tools_test.go @@ -19,18 +19,21 @@ import ( "time" "github.com/juju/errors" + "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" gc "gopkg.in/check.v1" apitesting "github.com/juju/juju/apiserver/testing" "github.com/juju/juju/core/objectstore" + "github.com/juju/juju/domain/user/service" "github.com/juju/juju/environs" "github.com/juju/juju/environs/simplestreams" "github.com/juju/juju/environs/storage" envtesting "github.com/juju/juju/environs/testing" envtools "github.com/juju/juju/environs/tools" toolstesting "github.com/juju/juju/environs/tools/testing" + "github.com/juju/juju/internal/auth" "github.com/juju/juju/internal/password" coretools "github.com/juju/juju/internal/tools" jujutesting "github.com/juju/juju/juju/testing" @@ -334,16 +337,31 @@ func (s *toolsSuite) TestMigrateToolsNotMigrating(c *gc.C) { ) } -func (s *toolsSuite) TestMigrateToolsUnauth(c *gc.C) { +func (s *toolsSuite) TestMigrateToolsForUser(c *gc.C) { // Try uploading as a non controller admin. + userService := s.ControllerServiceFactory(c).User() + userTag := names.NewUserTag("bobbrown") + _, _, err := userService.AddUser(context.Background(), service.AddUserArg{ + Name: userTag.Name(), + DisplayName: "Bob Brown", + CreatorUUID: s.AdminUserUUID, + Password: ptr(auth.NewPassword("hunter2")), + }) + c.Assert(err, jc.ErrorIsNil) + + // TODO (stickupkid): Permissions: This is only required to insert admin + // permissions into the state, remove when permissions are written to state. f, release := s.NewFactory(c, s.ControllerModelUUID()) defer release() + f.MakeUser(c, &factory.UserParams{ + Name: userTag.Name(), + }) + url := s.URL("/migrate/tools", nil).String() - user := f.MakeUser(c, &factory.UserParams{Password: "hunter2"}) resp := apitesting.SendHTTPRequest(c, apitesting.HTTPRequestParams{ Method: "POST", URL: url, - Tag: user.Tag().String(), + Tag: userTag.String(), Password: "hunter2", }) s.assertPlainErrorResponse( diff --git a/domain/credential/errors/errors.go b/domain/credential/errors/errors.go index 9df4f80b6d4..fc9bc61f81b 100644 --- a/domain/credential/errors/errors.go +++ b/domain/credential/errors/errors.go @@ -15,4 +15,7 @@ const ( // UnknownCloud describes an error that occurs when a credential for cloud // not known to the controller is updated. UnknownCloud = errors.ConstError("unknown cloud") + + // UserNotFound describes an error that occurs when a user is not found. + UserNotFound = errors.ConstError("user not found") ) diff --git a/domain/schema/testing/schema.go b/domain/schema/testing/schema.go index c834437010d..99f8cbc0290 100644 --- a/domain/schema/testing/schema.go +++ b/domain/schema/testing/schema.go @@ -18,7 +18,7 @@ type SchemaApplier struct { func (s *SchemaApplier) Apply(c *gc.C, ctx context.Context, runner database.TxnRunner) { s.Schema.Hook(func(i int) error { - c.Log("Applying schema change", i) + //c.Log("Applying schema change", i) return nil }) changeSet, err := s.Schema.Ensure(ctx, runner) diff --git a/domain/user/errors/errors.go b/domain/user/errors/errors.go index 850a666cebf..2b47087c97c 100644 --- a/domain/user/errors/errors.go +++ b/domain/user/errors/errors.go @@ -37,4 +37,12 @@ const ( // Unauthorized describes an error that occurs when the user does not have // the required permissions to perform an action. Unauthorized = errors.ConstError("user unauthorized") + + // ActivationKeyNotFound describes an error that occurs when the activation + // key is not found. + ActivationKeyNotFound = errors.ConstError("activation key not found") + + // ActivationKeyNotValid describes an error that occurs when the activation + // key is not valid. + ActivationKeyNotValid = errors.ConstError("activation key not valid") ) diff --git a/domain/user/service/service.go b/domain/user/service/service.go index 4572e12b440..c4c80ae8ed6 100644 --- a/domain/user/service/service.go +++ b/domain/user/service/service.go @@ -6,8 +6,10 @@ package service import ( "context" "crypto/rand" + "encoding/json" "github.com/juju/errors" + "golang.org/x/crypto/nacl/secretbox" "github.com/juju/juju/core/user" domainuser "github.com/juju/juju/domain/user" @@ -85,6 +87,11 @@ type State interface { // is returned that satisfies usererrors.NotFound. SetActivationKey(context.Context, string, []byte) error + // GetActivationKey will retrieve the activation key for the user. + // If no user is found for the supplied user name an error is returned that + // satisfies usererrors.NotFound. + GetActivationKey(context.Context, string) ([]byte, error) + // SetPasswordHash removes any active activation keys and sets the user // password hash and salt. If no user is found for the supplied user name an error // is returned that satisfies usererrors.NotFound. @@ -304,20 +311,8 @@ func (s *Service) SetPassword(ctx context.Context, name string, pass auth.Passwo return errors.Trace(err) } - salt, err := auth.NewSalt() - if err != nil { - return errors.Annotatef(err, "generating password salt for user %q", name) - } - - pwHash, err := auth.HashPassword(pass, salt) - if err != nil { - return errors.Annotatef(err, "hashing password for user %q", name) - } - - if err = s.st.SetPasswordHash(ctx, name, pwHash, salt); err != nil { - return errors.Annotatef(err, "setting password for user %q", name) - } - return nil + err := s.setPassword(ctx, name, pass) + return errors.Trace(err) } // ResetPassword will remove any active passwords for a user and generate a new @@ -398,3 +393,99 @@ func generateActivationKey() ([]byte, error) { } return activationKey[:], nil } + +// activationBoxNonceLength is the number of bytes in the nonce for the +// activation box. +const activationBoxNonceLength = 24 + +// Sealer is an interface that can be used to seal a byte slice. +// This will use the nonce and box for a given user to seal the payload. +type Sealer interface { + // Seal will seal the payload using the nonce and box for the user. + Seal(nonce, payload []byte) ([]byte, error) +} + +// SetPasswordWithActivationKey will use the activation key from the user. To +// then apply the payload password. If the user does not exist an error that +// satisfies usererrors.NotFound will be return. If the nonce is not the +// correct length an error that satisfies errors.NotValid will be returned. +// +// This will use the NaCl secretbox to open the box and then unmarshal the +// payload to set the new password for the user. If the payload cannot be +// unmarshalled an error will be returned. +// To prevent the leaking of the key and nonce (which can unbox the secret), +// a Sealer will be returned that can be used to seal the response payload. +func (s *Service) SetPasswordWithActivationKey(ctx context.Context, name string, nonce, box []byte) (Sealer, error) { + if len(nonce) != activationBoxNonceLength { + return nil, errors.NotValidf("nonce") + } + + // Get the activation key for the user. + key, err := s.st.GetActivationKey(ctx, name) + if err != nil { + return nil, errors.Trace(err) + } + + // Copy the nonce and the key to arrays which can be used for the secretbox. + var sbKey [activationKeyLength]byte + var sbNonce [activationBoxNonceLength]byte + copy(sbKey[:], key) + copy(sbNonce[:], nonce) + + // The box is the payload that has been sealed with the nonce and key, so + // let's open it. + boxPayloadBytes, ok := secretbox.Open(nil, box, &sbNonce, &sbKey) + if !ok { + return nil, usererrors.ActivationKeyNotValid + } + + // We expect the payload to be a JSON object with a password field. + var payload struct { + // Password is the new password to set for the user. + Password string `json:"password"` + } + if err := json.Unmarshal(boxPayloadBytes, &payload); err != nil { + return nil, errors.Annotate(err, "cannot unmarshal payload") + } + + if err := s.setPassword(ctx, name, auth.NewPassword(payload.Password)); err != nil { + return nil, errors.Annotate(err, "setting new password") + } + + return boxSealer{ + key: sbKey, + }, nil +} + +func (s *Service) setPassword(ctx context.Context, name string, pass auth.Password) error { + salt, err := auth.NewSalt() + if err != nil { + return errors.Annotatef(err, "generating password salt for user %q", name) + } + + pwHash, err := auth.HashPassword(pass, salt) + if err != nil { + return errors.Annotatef(err, "hashing password for user %q", name) + } + + if err = s.st.SetPasswordHash(ctx, name, pwHash, salt); err != nil { + return errors.Annotatef(err, "setting password for user %q", name) + } + + return nil +} + +// boxSealer is a Sealer that uses the NaCl secretbox to seal a payload. +type boxSealer struct { + key [activationKeyLength]byte +} + +func (s boxSealer) Seal(nonce, payload []byte) ([]byte, error) { + if len(nonce) != activationBoxNonceLength { + return nil, errors.NotValidf("nonce") + } + + var sbNonce [activationBoxNonceLength]byte + copy(sbNonce[:], nonce) + return secretbox.Seal(nil, payload, &sbNonce, &s.key), nil +} diff --git a/domain/user/service/service_test.go b/domain/user/service/service_test.go index fc9a9bcccd4..24ae9879d3a 100644 --- a/domain/user/service/service_test.go +++ b/domain/user/service/service_test.go @@ -7,12 +7,14 @@ import ( "context" "crypto/rand" "encoding/base64" + "encoding/json" "errors" "fmt" "testing" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" + "golang.org/x/crypto/nacl/secretbox" gc "gopkg.in/check.v1" "github.com/juju/juju/core/user" @@ -337,7 +339,88 @@ func (s *serviceSuite) TestDisableUserAuthentication(c *gc.C) { err := s.service().DisableUserAuthentication(context.Background(), "name") c.Assert(err, jc.ErrorIsNil) +} + +// TestSetPasswordWithActivationKey tests setting a password with an activation +// key. +func (s *serviceSuite) TestSetPasswordWithActivationKey(c *gc.C) { + defer s.setupMocks(c).Finish() + + password := "password123" + + // Create a key for the activation box. + key := make([]byte, activationKeyLength) + _, err := rand.Read(key) + c.Assert(err, jc.ErrorIsNil) + + s.state.EXPECT().GetActivationKey(gomock.Any(), "name").Return(key, nil) + s.state.EXPECT().SetPasswordHash(gomock.Any(), "name", gomock.Any(), gomock.Any()).Return(nil) + + // Create a nonce for the activation box. + nonce := make([]byte, activationBoxNonceLength) + _, err = rand.Read(nonce) + c.Assert(err, jc.ErrorIsNil) + + type payload struct { + Password string `json:"password"` + } + p := payload{ + Password: password, + } + payloadBytes, err := json.Marshal(p) + c.Assert(err, jc.ErrorIsNil) + + box := s.sealBox(c, key, nonce, payloadBytes) + + _, err = s.service().SetPasswordWithActivationKey(context.Background(), "name", nonce, box) + c.Assert(err, jc.ErrorIsNil) +} + +// TestSetPasswordWithActivationKeyWithInvalidKey tests setting a password +// with an invalid activation key. +func (s *serviceSuite) TestSetPasswordWithActivationKeyWithInvalidKey(c *gc.C) { + defer s.setupMocks(c).Finish() + + password := "password123" + + // Create a key for the activation box. + key := make([]byte, activationKeyLength) + _, err := rand.Read(key) + c.Assert(err, jc.ErrorIsNil) + + s.state.EXPECT().GetActivationKey(gomock.Any(), "name").Return(key, nil) + + // Create a nonce for the activation box. + nonce := make([]byte, activationBoxNonceLength) + _, err = rand.Read(nonce) + c.Assert(err, jc.ErrorIsNil) + + type payload struct { + Password string `json:"password"` + } + p := payload{ + Password: password, + } + payloadBytes, err := json.Marshal(p) + c.Assert(err, jc.ErrorIsNil) + + box := s.sealBox(c, key, nonce, payloadBytes) + + // Replace the nonce with a different nonce. + _, err = rand.Read(nonce) + c.Assert(err, jc.ErrorIsNil) + + _, err = s.service().SetPasswordWithActivationKey(context.Background(), "name", nonce, box) + c.Assert(err, jc.ErrorIs, usererrors.ActivationKeyNotValid) +} + +func (s *serviceSuite) sealBox(c *gc.C, key, nonce, payload []byte) []byte { + var sbKey [activationKeyLength]byte + var sbNonce [activationBoxNonceLength]byte + copy(sbKey[:], key) + copy(sbNonce[:], nonce) + return secretbox.Seal(nil, payload, &sbNonce, &sbKey) } // FuzzGetUser is a fuzz test for GetUser() that stresses the username input of diff --git a/domain/user/service/state_mock_test.go b/domain/user/service/state_mock_test.go index 157df4c3395..1b42f6b52f8 100644 --- a/domain/user/service/state_mock_test.go +++ b/domain/user/service/state_mock_test.go @@ -111,6 +111,21 @@ func (mr *MockStateMockRecorder) EnableUserAuthentication(arg0, arg1 any) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnableUserAuthentication", reflect.TypeOf((*MockState)(nil).EnableUserAuthentication), arg0, arg1) } +// GetActivationKey mocks base method. +func (m *MockState) GetActivationKey(arg0 context.Context, arg1 string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetActivationKey", arg0, arg1) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetActivationKey indicates an expected call of GetActivationKey. +func (mr *MockStateMockRecorder) GetActivationKey(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActivationKey", reflect.TypeOf((*MockState)(nil).GetActivationKey), arg0, arg1) +} + // GetAllUsers mocks base method. func (m *MockState) GetAllUsers(arg0 context.Context) ([]user.User, error) { m.ctrl.T.Helper() diff --git a/domain/user/state/state.go b/domain/user/state/state.go index fb618122ea4..595151e8efc 100644 --- a/domain/user/state/state.go +++ b/domain/user/state/state.go @@ -212,7 +212,7 @@ AND user.removed = false return user.UUID(""), errors.Trace(err) } - var result = sqlair.M{} + result := sqlair.M{} err = tx.Query(ctx, selectUserUUIDStmt, sqlair.M{"name": name}).Get(&result) if errors.Is(err, sql.ErrNoRows) { return user.UUID(""), fmt.Errorf( @@ -238,9 +238,9 @@ AND user.removed = false return user.UUID(result["userUUID"].(string)), nil } -// GetUserByName will retrieve the user with authentication information (last login, disabled) -// specified by name from the database. If the user does not exist an error that satisfies -// usererrors.NotFound will be returned. +// GetUserByName will retrieve the user with authentication information +// (last login, disabled) specified by name from the database. If the user does +// not exist an error that satisfies usererrors.NotFound will be returned. func (st *State) GetUserByName(ctx context.Context, name string) (user.User, error) { db, err := st.DB() if err != nil { @@ -262,7 +262,7 @@ AND removed = false` var result User err = tx.Query(ctx, selectGetUserByNameStmt, sqlair.M{"name": name}).Get(&result) - if err != nil && errors.Is(err, sql.ErrNoRows) { + if errors.Is(err, sql.ErrNoRows) { return errors.Annotatef(usererrors.NotFound, "%q", name) } else if err != nil { return errors.Annotatef(err, "getting user with name %q", name) @@ -281,36 +281,39 @@ AND removed = false` // GetUserByAuth will retrieve the user with checking authentication // information specified by UUID and password from the database. -// If the user does not exist or the user does not authenticate an -// error that satisfies usererrors.Unauthorized will be returned. +// If the user does not exist an error that satisfies usererrors.NotFound will +// be returned, otherwise unauthorized will be returned. func (st *State) GetUserByAuth(ctx context.Context, name string, password auth.Password) (user.User, error) { db, err := st.DB() if err != nil { return user.User{}, errors.Annotate(err, "getting DB access") } - var result User - err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - getUserWithAuthQuery := ` + getUserWithAuthQuery := ` SELECT ( - user.uuid, user.name, user.display_name, user.created_by_uuid, user.created_at, - user_password.password_hash, user_password.password_salt - ) AS (&User.*) -FROM user - LEFT JOIN user_password - ON user.uuid = user_password.user_uuid + user.uuid, user.name, user.display_name, user.created_by_uuid, user.created_at, + user.disabled, + user_password.password_hash, user_password.password_salt + ) AS (&User.*) +FROM v_user_auth AS user + LEFT JOIN user_password + ON user.uuid = user_password.user_uuid WHERE user.name = $M.name AND removed = false -` + ` - selectGetUserByAuthStmt, err := sqlair.Prepare(getUserWithAuthQuery, User{}, sqlair.M{}) - if err != nil { - return errors.Annotate(err, "preparing select getUserWithAuth query") - } + selectGetUserByAuthStmt, err := sqlair.Prepare(getUserWithAuthQuery, User{}, sqlair.M{}) + if err != nil { + return user.User{}, errors.Annotate(err, "preparing select getUserWithAuth query") + } + var result User + err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { err = tx.Query(ctx, selectGetUserByAuthStmt, sqlair.M{"name": name}).Get(&result) - if err != nil { - return errors.Annotatef(usererrors.Unauthorized, "%q", name) + if errors.Is(err, sql.ErrNoRows) { + return errors.Annotatef(usererrors.NotFound, "%q", name) + } else if err != nil { + return errors.Annotatef(err, "getting user with name %q", name) } return nil @@ -320,7 +323,11 @@ AND removed = false } passwordHash, err := auth.HashPassword(password, result.PasswordSalt) - if err != nil { + if errors.Is(err, errors.NotValid) { + // If the user has no salt, then they don't have a password correctly + // set. In this case, we should return an unauthorized error. + return user.User{}, errors.Annotatef(usererrors.Unauthorized, "%q", name) + } else if err != nil { return user.User{}, errors.Annotatef(err, "hashing password for user with name %q", name) } else if passwordHash != result.PasswordHash { return user.User{}, errors.Annotatef(usererrors.Unauthorized, "%q", name) @@ -422,6 +429,51 @@ func (st *State) SetActivationKey(ctx context.Context, name string, activationKe }) } +// GetActivationKey retrieves the activation key for the user with the supplied +// user name. If the user does not exist an error that satisfies +// usererrors.NotFound will be returned. +func (st *State) GetActivationKey(ctx context.Context, name string) ([]byte, error) { + db, err := st.DB() + if err != nil { + return nil, errors.Annotate(err, "getting DB access") + } + + uuidStmt, err := st.getActiveUUIDStmt() + if err != nil { + return nil, errors.Trace(err) + } + + m := make(sqlair.M, 1) + + selectKeyStmt, err := sqlair.Prepare("SELECT (*) AS (&ActivationKey.*) FROM user_activation_key WHERE user_uuid = $M.uuid", m, ActivationKey{}) + if err != nil { + return nil, errors.Annotate(err, "preparing activation get query") + } + + var key ActivationKey + err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + uuid, err := st.uuidForName(ctx, tx, uuidStmt, name) + if err != nil { + return errors.Trace(err) + } + + if err := tx.Query(ctx, selectKeyStmt, sqlair.M{"uuid": uuid}).Get(&key); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return errors.Annotatef(usererrors.ActivationKeyNotFound, "activation key for %q", name) + } + return errors.Annotatef(err, "selecting activation key for %q", name) + } + return errors.Trace(err) + }) + if err != nil { + return nil, errors.Annotatef(err, "getting activation key for %q", name) + } + if len(key.ActivationKey) == 0 { + return nil, errors.Annotatef(usererrors.ActivationKeyNotValid, "activation key for %q", name) + } + return []byte(key.ActivationKey), nil +} + // SetPasswordHash removes any active activation keys and sets the user // password hash and salt. If no user is found for the supplied user name an error // is returned that satisfies usererrors.NotFound. @@ -702,7 +754,7 @@ INSERT INTO user_password (user_uuid, password_hash, password_salt) FROM user WHERE name = $M.name AND removed = false -ON CONFLICT(user_uuid) DO NOTHING` +ON CONFLICT(user_uuid) DO UPDATE SET password_hash = excluded.password_hash, password_salt = excluded.password_salt` insertSetPasswordHashStmt, err := sqlair.Prepare(setPasswordHashQuery, sqlair.M{}) if err != nil { diff --git a/domain/user/state/state_test.go b/domain/user/state/state_test.go index 31ec2fbf36b..740e572db48 100644 --- a/domain/user/state/state_test.go +++ b/domain/user/state/state_test.go @@ -450,6 +450,60 @@ func (s *stateSuite) TestGetUserByAuth(c *gc.C) { c.Check(u.DisplayName, gc.Equals, "admin") c.Check(u.CreatorUUID, gc.Equals, adminUUID) c.Check(u.CreatedAt, gc.NotNil) + c.Check(u.Disabled, jc.IsFalse) +} + +// TestGetUserByAuthWithInvalidSalt asserts that we correctly send an +// unauthorized error if the user doesn't have a valid salt. +func (s *stateSuite) TestGetUserByAuthWithInvalidSalt(c *gc.C) { + st := NewState(s.TxnRunnerFactory()) + + // Add admin user. + adminUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + + err = st.AddUserWithPasswordHash( + context.Background(), adminUUID, + "admin", "admin", + adminUUID, "passwordHash", []byte{}, + ) + c.Assert(err, jc.ErrorIsNil) + + // Get the user. + _, err = st.GetUserByAuth(context.Background(), "admin", auth.NewPassword("passwordHash")) + c.Assert(err, jc.ErrorIs, usererrors.Unauthorized) +} + +// TestGetUserByAuthDisabled asserts that we can get a user by auth from the +// database and has the correct disabled flag. +func (s *stateSuite) TestGetUserByAuthDisabled(c *gc.C) { + st := NewState(s.TxnRunnerFactory()) + + // Add admin user with password hash. + adminUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + + salt, err := auth.NewSalt() + c.Assert(err, jc.ErrorIsNil) + + passwordHash, err := auth.HashPassword(auth.NewPassword("password"), salt) + c.Assert(err, jc.ErrorIsNil) + + err = st.AddUserWithPasswordHash(context.Background(), adminUUID, "admin", "admin", adminUUID, passwordHash, salt) + c.Assert(err, jc.ErrorIsNil) + + err = st.DisableUserAuthentication(context.Background(), "admin") + c.Assert(err, jc.ErrorIsNil) + + // Get the user. + u, err := st.GetUserByAuth(context.Background(), "admin", auth.NewPassword("password")) + c.Assert(err, jc.ErrorIsNil) + + c.Check(u.Name, gc.Equals, "admin") + c.Check(u.DisplayName, gc.Equals, "admin") + c.Check(u.CreatorUUID, gc.Equals, adminUUID) + c.Check(u.CreatedAt, gc.NotNil) + c.Check(u.Disabled, jc.IsTrue) } // TestGetUserByAuthUnauthorized asserts that we get an error when we try to @@ -479,14 +533,14 @@ func (s *stateSuite) TestGetUserByAuthUnauthorized(c *gc.C) { c.Assert(err, jc.ErrorIs, usererrors.Unauthorized) } -// TestGetUserByAutUnexcitingUser asserts that we get an error when we try to +// TestGetUserByAuthDoesNotExist asserts that we get an error when we try to // get a user by auth that does not exist. -func (s *stateSuite) TestGetUserByAuthNotExtantUnauthorized(c *gc.C) { +func (s *stateSuite) TestGetUserByAuthDoesNotExist(c *gc.C) { st := NewState(s.TxnRunnerFactory()) // Get the user. _, err := st.GetUserByAuth(context.Background(), "admin", auth.NewPassword("password")) - c.Assert(err, jc.ErrorIs, usererrors.Unauthorized) + c.Assert(err, jc.ErrorIs, usererrors.NotFound) } // TestRemoveUser asserts that we can remove a user from the database. @@ -691,6 +745,51 @@ WHERE user_uuid = ? c.Assert(err, jc.ErrorIs, sql.ErrNoRows) } +// TestSetPasswordHash asserts that we can set a password hash for a user twice. +func (s *stateSuite) TestSetPasswordHashTwice(c *gc.C) { + st := NewState(s.TxnRunnerFactory()) + + // Add admin user with activation key. + adminUUID, err := user.NewUUID() + c.Assert(err, jc.ErrorIsNil) + + newActivationKey, err := generateActivationKey() + c.Assert(err, jc.ErrorIsNil) + err = st.AddUserWithActivationKey( + context.Background(), adminUUID, + "admin", "admin", + adminUUID, newActivationKey, + ) + c.Assert(err, jc.ErrorIsNil) + + salt, err := auth.NewSalt() + c.Assert(err, jc.ErrorIsNil) + + // Set password hash. + err = st.SetPasswordHash(context.Background(), "admin", "passwordHash", salt) + c.Assert(err, jc.ErrorIsNil) + + // Set password hash again + err = st.SetPasswordHash(context.Background(), "admin", "passwordHashAgain", salt) + c.Assert(err, jc.ErrorIsNil) + + // Check that the password hash was set correctly. + db := s.DB() + + row := db.QueryRow(` +SELECT password_hash +FROM user_password +WHERE user_uuid = ? + `, adminUUID) + c.Assert(row.Err(), jc.ErrorIsNil) + + var passwordHash string + err = row.Scan(&passwordHash) + c.Assert(err, jc.ErrorIsNil) + + c.Check(passwordHash, gc.Equals, "passwordHashAgain") +} + // TestAddUserWithPasswordHash asserts that we can add a user with a password // hash. func (s *stateSuite) TestAddUserWithPasswordHash(c *gc.C) { @@ -767,20 +866,31 @@ func (s *stateSuite) TestAddUserWithActivationKey(c *gc.C) { c.Assert(err, jc.ErrorIsNil) // Check that the activation key was set correctly. - db := s.DB() + activationKey, err := st.GetActivationKey(context.Background(), "admin") + c.Assert(err, jc.ErrorIsNil) - row := db.QueryRow(` -SELECT activation_key -FROM user_activation_key -WHERE user_uuid = ? - `, adminUUID) - c.Assert(row.Err(), jc.ErrorIsNil) + c.Check(activationKey, gc.DeepEquals, adminActivationKey) +} - var activationKey string - err = row.Scan(&activationKey) +// TestGetActivationKeyNotFound asserts that if we try to get an activation key +// for a user that does not exist, we get an error. +func (s *stateSuite) TestGetActivationKeyNotFound(c *gc.C) { + st := NewState(s.TxnRunnerFactory()) + + // Add admin user. + adminUUID, err := user.NewUUID() c.Assert(err, jc.ErrorIsNil) - c.Check(activationKey, gc.Equals, string(adminActivationKey)) + err = st.AddUser( + context.Background(), adminUUID, + "admin", "admin", + adminUUID, + ) + c.Assert(err, jc.ErrorIsNil) + + // Check that the activation key was set correctly. + _, err = st.GetActivationKey(context.Background(), "admin") + c.Assert(err, jc.ErrorIs, usererrors.ActivationKeyNotFound) } // TestAddUserWithActivationKeyWhichCreatorDoesNotExist asserts that we get an diff --git a/domain/user/state/types.go b/domain/user/state/types.go index 26f5f042482..6638fa6425c 100644 --- a/domain/user/state/types.go +++ b/domain/user/state/types.go @@ -9,7 +9,8 @@ import ( "github.com/juju/juju/core/user" ) -// User represents a user in the state layer with the associated fields in the database. +// User represents a user in the state layer with the associated fields in the +// database. type User struct { // UUID is the unique identifier for the user. UUID user.UUID `db:"uuid"` @@ -58,3 +59,9 @@ func (u User) toCoreUser() user.User { Disabled: u.Disabled, } } + +// ActivationKey represents an activation key in the state layer with the +// associated fields in the database. +type ActivationKey struct { + ActivationKey string `db:"activation_key"` +} diff --git a/internal/auth/password.go b/internal/auth/password.go index 4329ce4027d..4361339f0d9 100644 --- a/internal/auth/password.go +++ b/internal/auth/password.go @@ -91,7 +91,7 @@ func NewSalt() ([]byte, error) { // NewPassword returns a Password struct wrapping the plain text password. func NewPassword(p string) Password { - return Password{[]byte(p)} + return Password{password: []byte(p)} } // String implements the stringer interface always returning an empty string and diff --git a/internal/worker/apicaller/connect.go b/internal/worker/apicaller/connect.go index 68d323b00fe..77ea05b5433 100644 --- a/internal/worker/apicaller/connect.go +++ b/internal/worker/apicaller/connect.go @@ -116,7 +116,7 @@ func connectFallback( if !didFallback { logger.Debugf("connecting with current password") tryConnect() - if params.IsCodeUnauthorized(err) || errors.Cause(err) == apiservererrors.ErrBadCreds { + if params.IsCodeUnauthorized(err) || errors.Cause(err) == apiservererrors.ErrUnauthorized { didFallback = true } diff --git a/internal/worker/apicaller/connect_test.go b/internal/worker/apicaller/connect_test.go index 09f42789e2d..e9b4d4d4091 100644 --- a/internal/worker/apicaller/connect_test.go +++ b/internal/worker/apicaller/connect_test.go @@ -204,7 +204,7 @@ func (s *ScaryConnectSuite) TestChangePasswordSuccessAfterUnauthorisedError(c *g func (s *ScaryConnectSuite) TestChangePasswordSuccessAfterBadCurrentPasswordError(c *gc.C) { // This will try to login with old password if current one fails. - stub := createPasswordCheckStub(apiservererrors.ErrBadCreds) + stub := createPasswordCheckStub(apiservererrors.ErrUnauthorized) s.assertChangePasswordSuccess(c, stub) } diff --git a/internal/worker/httpserverargs/authenticator.go b/internal/worker/httpserverargs/authenticator.go index 66ccb7b4157..989a7ad5d65 100644 --- a/internal/worker/httpserverargs/authenticator.go +++ b/internal/worker/httpserverargs/authenticator.go @@ -10,23 +10,38 @@ import ( "github.com/juju/errors" "github.com/juju/juju/apiserver/apiserverhttp" + "github.com/juju/juju/apiserver/authentication" "github.com/juju/juju/apiserver/authentication/macaroon" "github.com/juju/juju/apiserver/stateauthenticator" "github.com/juju/juju/controller" + coreuser "github.com/juju/juju/core/user" "github.com/juju/juju/state" ) -// ControllerConfigGetter is an interface that can be implemented by +// ControllerConfigService is an interface that can be implemented by // types that can return a controller config. -type ControllerConfigGetter interface { +type ControllerConfigService interface { ControllerConfig(context.Context) (controller.Config, error) } +// UserService is the interface that wraps the methods required to +// authenticate a user. +type UserService interface { + // GetUserByAuth returns the user with the given name and password. + GetUserByAuth(ctx context.Context, name, password string) (coreuser.User, error) + // GetUserByName returns the user with the given name. + GetUserByName(ctx context.Context, name string) (coreuser.User, error) + // UpdateLastLogin updates the last login time for the user with the + // given name. + UpdateLastLogin(ctx context.Context, name string) error +} + // NewStateAuthenticatorFunc is a function type satisfied by // NewStateAuthenticator. type NewStateAuthenticatorFunc func( statePool *state.StatePool, - controllerConfigGetter ControllerConfigGetter, + controllerConfigService ControllerConfigService, + userService UserService, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}, @@ -38,12 +53,18 @@ type NewStateAuthenticatorFunc func( // local macaroon logins. func NewStateAuthenticator( statePool *state.StatePool, - controllerConfigGetter ControllerConfigGetter, + controllerConfigService ControllerConfigService, + userService UserService, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}, ) (macaroon.LocalMacaroonAuthenticator, error) { - stateAuthenticator, err := stateauthenticator.NewAuthenticator(statePool, controllerConfigGetter, clock) + systemState, err := statePool.SystemState() + if err != nil { + return nil, errors.Trace(err) + } + agentAuthFactory := authentication.NewAgentAuthenticatorFactory(systemState, nil) + stateAuthenticator, err := stateauthenticator.NewAuthenticator(statePool, systemState, controllerConfigService, userService, agentAuthFactory, clock) if err != nil { return nil, errors.Trace(err) } diff --git a/internal/worker/httpserverargs/controller_config_mock_test.go b/internal/worker/httpserverargs/controller_config_mock_test.go deleted file mode 100644 index 6e1cf719c8f..00000000000 --- a/internal/worker/httpserverargs/controller_config_mock_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/internal/worker/httpserverargs (interfaces: ControllerConfigGetter) -// -// Generated by this command: -// -// mockgen -package httpserverargs -destination controller_config_mock_test.go github.com/juju/juju/internal/worker/httpserverargs ControllerConfigGetter -// - -// Package httpserverargs is a generated GoMock package. -package httpserverargs - -import ( - context "context" - reflect "reflect" - - controller "github.com/juju/juju/controller" - gomock "go.uber.org/mock/gomock" -) - -// MockControllerConfigGetter is a mock of ControllerConfigGetter interface. -type MockControllerConfigGetter struct { - ctrl *gomock.Controller - recorder *MockControllerConfigGetterMockRecorder -} - -// MockControllerConfigGetterMockRecorder is the mock recorder for MockControllerConfigGetter. -type MockControllerConfigGetterMockRecorder struct { - mock *MockControllerConfigGetter -} - -// NewMockControllerConfigGetter creates a new mock instance. -func NewMockControllerConfigGetter(ctrl *gomock.Controller) *MockControllerConfigGetter { - mock := &MockControllerConfigGetter{ctrl: ctrl} - mock.recorder = &MockControllerConfigGetterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockControllerConfigGetter) EXPECT() *MockControllerConfigGetterMockRecorder { - return m.recorder -} - -// ControllerConfig mocks base method. -func (m *MockControllerConfigGetter) ControllerConfig(arg0 context.Context) (controller.Config, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ControllerConfig", arg0) - ret0, _ := ret[0].(controller.Config) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ControllerConfig indicates an expected call of ControllerConfig. -func (mr *MockControllerConfigGetterMockRecorder) ControllerConfig(arg0 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerConfig", reflect.TypeOf((*MockControllerConfigGetter)(nil).ControllerConfig), arg0) -} diff --git a/internal/worker/httpserverargs/manifold.go b/internal/worker/httpserverargs/manifold.go index d9eeac70484..d0956bd1a80 100644 --- a/internal/worker/httpserverargs/manifold.go +++ b/internal/worker/httpserverargs/manifold.go @@ -72,7 +72,8 @@ func (config ManifoldConfig) start(context context.Context, getter dependency.Ge w, err := newWorker(workerConfig{ statePool: statePool, - controllerConfigGetter: controllerServiceFactory.ControllerConfig(), + controllerConfigService: controllerServiceFactory.ControllerConfig(), + userService: controllerServiceFactory.User(), mux: apiserverhttp.NewMux(), clock: clock, newStateAuthenticatorFn: config.NewStateAuthenticator, diff --git a/internal/worker/httpserverargs/manifold_test.go b/internal/worker/httpserverargs/manifold_test.go index b0e81f67b8a..35a6d9f3281 100644 --- a/internal/worker/httpserverargs/manifold_test.go +++ b/internal/worker/httpserverargs/manifold_test.go @@ -22,6 +22,7 @@ import ( "github.com/juju/juju/apiserver/authentication" "github.com/juju/juju/apiserver/authentication/macaroon" controllerconfigservice "github.com/juju/juju/domain/controllerconfig/service" + userservice "github.com/juju/juju/domain/user/service" "github.com/juju/juju/internal/servicefactory" "github.com/juju/juju/internal/worker/httpserverargs" "github.com/juju/juju/state" @@ -75,12 +76,13 @@ func (s *ManifoldSuite) newGetter(overlay map[string]any) dependency.Getter { func (s *ManifoldSuite) newStateAuthenticator( statePool *state.StatePool, - controllerConfigGetter httpserverargs.ControllerConfigGetter, + controllerConfig httpserverargs.ControllerConfigService, + userService httpserverargs.UserService, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}, ) (macaroon.LocalMacaroonAuthenticator, error) { - s.stub.MethodCall(s, "NewStateAuthenticator", statePool, controllerConfigGetter, mux, clock, abort) + s.stub.MethodCall(s, "NewStateAuthenticator", statePool, controllerConfig, userService, mux, clock, abort) if err := s.stub.NextErr(); err != nil { return nil, err } @@ -148,8 +150,8 @@ func (s *ManifoldSuite) TestStoppingWorkerClosesAuthenticator(c *gc.C) { w := s.startWorkerClean(c) s.stub.CheckCallNames(c, "NewStateAuthenticator") authArgs := s.stub.Calls()[0].Args - c.Assert(authArgs, gc.HasLen, 5) - abort := authArgs[4].(<-chan struct{}) + c.Assert(authArgs, gc.HasLen, 6) + abort := authArgs[5].(<-chan struct{}) // abort should still be open at this point. select { @@ -184,10 +186,13 @@ func (s *ManifoldSuite) TestValidate(c *gc.C) { f: func(cfg *httpserverargs.ManifoldConfig) { cfg.NewStateAuthenticator = nil }, expect: "nil NewStateAuthenticator not valid", }} + for i, test := range tests { c.Logf("test #%d (%s)", i, test.expect) + config := s.config test.f(&config) + manifold := httpserverargs.Manifold(config) w, err := manifold.Start(context.Background(), s.getter) workertest.CheckNilOrKill(c, w) @@ -229,3 +234,8 @@ func (s *stubServiceFactory) ControllerConfig() *controllerconfigservice.Watchab s.MethodCall(s, "ControllerConfig") return nil } + +func (s *stubServiceFactory) User() *userservice.Service { + s.MethodCall(s, "User") + return nil +} diff --git a/internal/worker/httpserverargs/package_test.go b/internal/worker/httpserverargs/package_test.go index 3ae84a1d8c4..1cdfdcde7a5 100644 --- a/internal/worker/httpserverargs/package_test.go +++ b/internal/worker/httpserverargs/package_test.go @@ -9,7 +9,7 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -package httpserverargs -destination controller_config_mock_test.go github.com/juju/juju/internal/worker/httpserverargs ControllerConfigGetter +//go:generate go run go.uber.org/mock/mockgen -package httpserverargs -destination services_mock_test.go github.com/juju/juju/internal/worker/httpserverargs ControllerConfigService,UserService func TestPackage(t *testing.T) { gc.TestingT(t) diff --git a/internal/worker/httpserverargs/services_mock_test.go b/internal/worker/httpserverargs/services_mock_test.go new file mode 100644 index 00000000000..a5b6e0e7708 --- /dev/null +++ b/internal/worker/httpserverargs/services_mock_test.go @@ -0,0 +1,124 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/internal/worker/httpserverargs (interfaces: ControllerConfigService,UserService) +// +// Generated by this command: +// +// mockgen -package httpserverargs -destination services_mock_test.go github.com/juju/juju/internal/worker/httpserverargs ControllerConfigService,UserService +// + +// Package httpserverargs is a generated GoMock package. +package httpserverargs + +import ( + context "context" + reflect "reflect" + + controller "github.com/juju/juju/controller" + user "github.com/juju/juju/core/user" + gomock "go.uber.org/mock/gomock" +) + +// MockControllerConfigService is a mock of ControllerConfigService interface. +type MockControllerConfigService struct { + ctrl *gomock.Controller + recorder *MockControllerConfigServiceMockRecorder +} + +// MockControllerConfigServiceMockRecorder is the mock recorder for MockControllerConfigService. +type MockControllerConfigServiceMockRecorder struct { + mock *MockControllerConfigService +} + +// NewMockControllerConfigService creates a new mock instance. +func NewMockControllerConfigService(ctrl *gomock.Controller) *MockControllerConfigService { + mock := &MockControllerConfigService{ctrl: ctrl} + mock.recorder = &MockControllerConfigServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockControllerConfigService) EXPECT() *MockControllerConfigServiceMockRecorder { + return m.recorder +} + +// ControllerConfig mocks base method. +func (m *MockControllerConfigService) ControllerConfig(arg0 context.Context) (controller.Config, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerConfig", arg0) + ret0, _ := ret[0].(controller.Config) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ControllerConfig indicates an expected call of ControllerConfig. +func (mr *MockControllerConfigServiceMockRecorder) ControllerConfig(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerConfig", reflect.TypeOf((*MockControllerConfigService)(nil).ControllerConfig), arg0) +} + +// MockUserService is a mock of UserService interface. +type MockUserService struct { + ctrl *gomock.Controller + recorder *MockUserServiceMockRecorder +} + +// MockUserServiceMockRecorder is the mock recorder for MockUserService. +type MockUserServiceMockRecorder struct { + mock *MockUserService +} + +// NewMockUserService creates a new mock instance. +func NewMockUserService(ctrl *gomock.Controller) *MockUserService { + mock := &MockUserService{ctrl: ctrl} + mock.recorder = &MockUserServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUserService) EXPECT() *MockUserServiceMockRecorder { + return m.recorder +} + +// GetUserByAuth mocks base method. +func (m *MockUserService) GetUserByAuth(arg0 context.Context, arg1, arg2 string) (user.User, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserByAuth", arg0, arg1, arg2) + ret0, _ := ret[0].(user.User) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserByAuth indicates an expected call of GetUserByAuth. +func (mr *MockUserServiceMockRecorder) GetUserByAuth(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserByAuth", reflect.TypeOf((*MockUserService)(nil).GetUserByAuth), arg0, arg1, arg2) +} + +// GetUserByName mocks base method. +func (m *MockUserService) GetUserByName(arg0 context.Context, arg1 string) (user.User, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserByName", arg0, arg1) + ret0, _ := ret[0].(user.User) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserByName indicates an expected call of GetUserByName. +func (mr *MockUserServiceMockRecorder) GetUserByName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserByName", reflect.TypeOf((*MockUserService)(nil).GetUserByName), arg0, arg1) +} + +// UpdateLastLogin mocks base method. +func (m *MockUserService) UpdateLastLogin(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateLastLogin", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateLastLogin indicates an expected call of UpdateLastLogin. +func (mr *MockUserServiceMockRecorder) UpdateLastLogin(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLastLogin", reflect.TypeOf((*MockUserService)(nil).UpdateLastLogin), arg0, arg1) +} diff --git a/internal/worker/httpserverargs/worker.go b/internal/worker/httpserverargs/worker.go index 3af63f7631f..932756ad5e3 100644 --- a/internal/worker/httpserverargs/worker.go +++ b/internal/worker/httpserverargs/worker.go @@ -15,12 +15,14 @@ import ( "github.com/juju/juju/apiserver/apiserverhttp" "github.com/juju/juju/apiserver/authentication/macaroon" "github.com/juju/juju/controller" + coreuser "github.com/juju/juju/core/user" "github.com/juju/juju/state" ) type workerConfig struct { statePool *state.StatePool - controllerConfigGetter ControllerConfigGetter + controllerConfigService ControllerConfigService + userService UserService mux *apiserverhttp.Mux clock clock.Clock newStateAuthenticatorFn NewStateAuthenticatorFunc @@ -30,8 +32,11 @@ func (w workerConfig) Validate() error { if w.statePool == nil { return errors.NotValidf("empty statePool") } - if w.controllerConfigGetter == nil { - return errors.NotValidf("empty controllerConfigGetter") + if w.controllerConfigService == nil { + return errors.NotValidf("empty controllerConfigService") + } + if w.userService == nil { + return errors.NotValidf("empty userService") } if w.mux == nil { return errors.NotValidf("empty mux") @@ -46,10 +51,10 @@ func (w workerConfig) Validate() error { } type argsWorker struct { - catacomb catacomb.Catacomb - cfg workerConfig - authenticator macaroon.LocalMacaroonAuthenticator - managedCtrlConfigGetter *managedCtrlConfigGetter + catacomb catacomb.Catacomb + cfg workerConfig + authenticator macaroon.LocalMacaroonAuthenticator + managedServices *managedServices } func newWorker(cfg workerConfig) (worker.Worker, error) { @@ -57,15 +62,18 @@ func newWorker(cfg workerConfig) (worker.Worker, error) { return nil, errors.Trace(err) } w := argsWorker{ - cfg: cfg, - managedCtrlConfigGetter: newManagedCtrlConfigGetter(cfg.controllerConfigGetter), + cfg: cfg, + managedServices: newManagedServices( + cfg.controllerConfigService, + cfg.userService, + ), } if err := catacomb.Invoke(catacomb.Plan{ Site: &w.catacomb, Work: w.loop, Init: []worker.Worker{ - w.managedCtrlConfigGetter, + w.managedServices, }, }); err != nil { return nil, errors.Trace(err) @@ -73,7 +81,8 @@ func newWorker(cfg workerConfig) (worker.Worker, error) { authenticator, err := w.cfg.newStateAuthenticatorFn( w.cfg.statePool, - w.managedCtrlConfigGetter, + w.managedServices, + w.managedServices, w.cfg.mux, w.cfg.clock, w.catacomb.Dying(), @@ -101,20 +110,24 @@ func (w *argsWorker) loop() error { return w.catacomb.ErrDying() } -// managedCtrlConfigGetter is a ControllerConfigGetter that wraps another -// ControllerConfigGetter and cancels the context when the tomb is dying. -// This is because the location of the controller config request is not -// cancellable, so we need the ability to cancel the controller config -// request when the tomb is dying. This should prevent any lockup when the -// controller is shutting down. -type managedCtrlConfigGetter struct { - tomb tomb.Tomb - configGetter ControllerConfigGetter +// managedServices is a ControllerConfigService and a UserService that wraps +// the underlying services and cancels the context when the tomb is dying. +// This is because the location of the request is not cancellable, so we need +// the ability to cancel the request when the tomb is dying. This should +// prevent any lockup when the controller is shutting down. +type managedServices struct { + tomb tomb.Tomb + controllerConfigService ControllerConfigService + userService UserService } -func newManagedCtrlConfigGetter(configGetter ControllerConfigGetter) *managedCtrlConfigGetter { - w := &managedCtrlConfigGetter{ - configGetter: configGetter, +func newManagedServices( + controllerConfigService ControllerConfigService, + userService UserService, +) *managedServices { + w := &managedServices{ + controllerConfigService: controllerConfigService, + userService: userService, } w.tomb.Go(func() error { <-w.tomb.Dying() @@ -123,17 +136,33 @@ func newManagedCtrlConfigGetter(configGetter ControllerConfigGetter) *managedCtr return w } -// ControllerConfig is part of the ControllerConfigGetter interface. -func (b *managedCtrlConfigGetter) ControllerConfig(ctx context.Context) (controller.Config, error) { - return b.configGetter.ControllerConfig(b.tomb.Context(ctx)) +// ControllerConfig is part of the ControllerConfigService interface. +func (b *managedServices) ControllerConfig(ctx context.Context) (controller.Config, error) { + return b.controllerConfigService.ControllerConfig(b.tomb.Context(ctx)) +} + +// GetUserByName is part of the UserService interface. +func (b *managedServices) GetUserByAuth(ctx context.Context, name, password string) (coreuser.User, error) { + return b.userService.GetUserByAuth(b.tomb.Context(ctx), name, password) +} + +// GetUserByName is part of the UserService interface. +func (b *managedServices) GetUserByName(ctx context.Context, name string) (coreuser.User, error) { + return b.userService.GetUserByName(b.tomb.Context(ctx), name) +} + +// UpdateLastLogin updates the last login time for the user with the +// given name. +func (b *managedServices) UpdateLastLogin(ctx context.Context, name string) error { + return b.userService.UpdateLastLogin(b.tomb.Context(ctx), name) } // Kill is part of the worker.Worker interface. -func (b *managedCtrlConfigGetter) Kill() { +func (b *managedServices) Kill() { b.tomb.Kill(nil) } // Wait is part of the worker.Worker interface. -func (b *managedCtrlConfigGetter) Wait() error { +func (b *managedServices) Wait() error { return b.tomb.Wait() } diff --git a/internal/worker/httpserverargs/worker_test.go b/internal/worker/httpserverargs/worker_test.go index fbefe4e6dee..87c0c6af98c 100644 --- a/internal/worker/httpserverargs/worker_test.go +++ b/internal/worker/httpserverargs/worker_test.go @@ -35,7 +35,8 @@ func (s *workerConfigSuite) SetUpTest(c *gc.C) { s.IsolationSuite.SetUpTest(c) s.config = workerConfig{ statePool: &state.StatePool{}, - controllerConfigGetter: &managedCtrlConfigGetter{}, + controllerConfigService: &managedServices{}, + userService: &managedServices{}, mux: &apiserverhttp.Mux{}, clock: clock.WallClock, newStateAuthenticatorFn: NewStateAuthenticator, @@ -67,16 +68,17 @@ func (s *workerConfigSuite) TestMissing(c *gc.C) { type workerSuite struct { testing.IsolationSuite - controllerConfigGetter *MockControllerConfigGetter + controllerConfigService *MockControllerConfigService + userService *MockUserService - stateAuthFunc func(*state.StatePool, ControllerConfigGetter, *apiserverhttp.Mux, clock.Clock, <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) + stateAuthFunc func(*state.StatePool, ControllerConfigService, UserService, *apiserverhttp.Mux, clock.Clock, <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) } var _ = gc.Suite(&workerSuite{}) func (s *workerSuite) TestWorkerStarted(c *gc.C) { started := make(chan struct{}) - s.stateAuthFunc = func(statePool *state.StatePool, controllerConfigGetter ControllerConfigGetter, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { + s.stateAuthFunc = func(*state.StatePool, ControllerConfigService, UserService, *apiserverhttp.Mux, clock.Clock, <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { defer close(started) return nil, nil } @@ -96,10 +98,10 @@ func (s *workerSuite) TestWorkerStarted(c *gc.C) { func (s *workerSuite) TestWorkerControllerConfigContext(c *gc.C) { defer s.setupMocks(c).Finish() - s.controllerConfigGetter.EXPECT().ControllerConfig(gomock.Any()).Return(controller.Config{}, nil) + s.controllerConfigService.EXPECT().ControllerConfig(gomock.Any()).Return(controller.Config{}, nil) started := make(chan struct{}) - s.stateAuthFunc = func(statePool *state.StatePool, controllerConfigGetter ControllerConfigGetter, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { + s.stateAuthFunc = func(*state.StatePool, ControllerConfigService, UserService, *apiserverhttp.Mux, clock.Clock, <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { defer close(started) return nil, nil } @@ -113,7 +115,7 @@ func (s *workerSuite) TestWorkerControllerConfigContext(c *gc.C) { c.Fatalf("timed out waiting for worker to start") } - config, err := w.(*argsWorker).managedCtrlConfigGetter.ControllerConfig(context.Background()) + config, err := w.(*argsWorker).managedServices.ControllerConfig(context.Background()) c.Assert(err, jc.ErrorIsNil) c.Assert(config, gc.NotNil) @@ -123,12 +125,12 @@ func (s *workerSuite) TestWorkerControllerConfigContext(c *gc.C) { func (s *workerSuite) TestWorkerControllerConfigContextDeadline(c *gc.C) { defer s.setupMocks(c).Finish() - s.controllerConfigGetter.EXPECT().ControllerConfig(gomock.Any()).DoAndReturn(func(ctx context.Context) (controller.Config, error) { + s.controllerConfigService.EXPECT().ControllerConfig(gomock.Any()).DoAndReturn(func(ctx context.Context) (controller.Config, error) { return nil, ctx.Err() }) started := make(chan struct{}) - s.stateAuthFunc = func(statePool *state.StatePool, controllerConfigGetter ControllerConfigGetter, mux *apiserverhttp.Mux, clock clock.Clock, abort <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { + s.stateAuthFunc = func(*state.StatePool, ControllerConfigService, UserService, *apiserverhttp.Mux, clock.Clock, <-chan struct{}) (macaroon.LocalMacaroonAuthenticator, error) { defer close(started) return nil, nil } @@ -144,7 +146,7 @@ func (s *workerSuite) TestWorkerControllerConfigContextDeadline(c *gc.C) { workertest.CleanKill(c, w) - _, err := w.(*argsWorker).managedCtrlConfigGetter.ControllerConfig(context.Background()) + _, err := w.(*argsWorker).managedServices.ControllerConfig(context.Background()) c.Assert(err, gc.Equals, context.Canceled) } @@ -155,11 +157,14 @@ func (s *workerSuite) newWorker(c *gc.C) worker.Worker { } func (s *workerSuite) newWorkerConfig(c *gc.C) workerConfig { + services := &managedServices{ + controllerConfigService: s.controllerConfigService, + userService: s.userService, + } return workerConfig{ - statePool: &state.StatePool{}, - controllerConfigGetter: &managedCtrlConfigGetter{ - configGetter: s.controllerConfigGetter, - }, + statePool: &state.StatePool{}, + controllerConfigService: services, + userService: services, mux: &apiserverhttp.Mux{}, clock: clock.WallClock, newStateAuthenticatorFn: s.stateAuthFunc, @@ -169,7 +174,8 @@ func (s *workerSuite) newWorkerConfig(c *gc.C) workerConfig { func (s *workerSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.controllerConfigGetter = NewMockControllerConfigGetter(ctrl) + s.controllerConfigService = NewMockControllerConfigService(ctrl) + s.userService = NewMockUserService(ctrl) return ctrl } diff --git a/juju/testing/apiserver.go b/juju/testing/apiserver.go index cb87208f0b9..b19bacef6fb 100644 --- a/juju/testing/apiserver.go +++ b/juju/testing/apiserver.go @@ -29,6 +29,7 @@ import ( "github.com/juju/juju/api" "github.com/juju/juju/apiserver" "github.com/juju/juju/apiserver/apiserverhttp" + "github.com/juju/juju/apiserver/authentication" "github.com/juju/juju/apiserver/authentication/macaroon" "github.com/juju/juju/apiserver/observer" "github.com/juju/juju/apiserver/observer/fakeobserver" @@ -55,6 +56,7 @@ import ( credentialstate "github.com/juju/juju/domain/credential/state" servicefactorytesting "github.com/juju/juju/domain/servicefactory/testing" userbootstrap "github.com/juju/juju/domain/user/bootstrap" + "github.com/juju/juju/internal/auth" databasetesting "github.com/juju/juju/internal/database/testing" internallease "github.com/juju/juju/internal/lease" "github.com/juju/juju/internal/mongo" @@ -151,6 +153,9 @@ type ApiServerSuite struct { WithUpgrading bool WithAuditLogConfig *auditlog.Config WithIntrospection func(func(string, http.Handler)) + + // AdminUserUUID is the root user for the controller. + AdminUserUUID coreuser.UUID } type noopRegisterer struct { @@ -307,7 +312,7 @@ func (s *ApiServerSuite) setupControllerModel(c *gc.C, controllerCfg controller. c.Assert(err, jc.ErrorIsNil) // Seed the test database with the controller cloud and credential etc. - SeedDatabase(c, s.TxnRunner(), controllerCfg) + s.AdminUserUUID = SeedDatabase(c, s.TxnRunner(), controllerCfg) } func (s *ApiServerSuite) setupApiServer(c *gc.C, controllerCfg controller.Config) { @@ -351,7 +356,13 @@ func (s *ApiServerSuite) setupApiServer(c *gc.C, controllerCfg controller.Config s.ObjectStoreGetter = cfg.ObjectStoreGetter // Set up auth handler. - authenticator, err := stateauthenticator.NewAuthenticator(cfg.StatePool, s.ControllerServiceFactory(c).ControllerConfig(), cfg.Clock) + factory := s.ControllerServiceFactory(c) + + systemState, err := cfg.StatePool.SystemState() + c.Assert(err, jc.ErrorIsNil) + agentAuthFactory := authentication.NewAgentAuthenticatorFactory(systemState, nil) + + authenticator, err := stateauthenticator.NewAuthenticator(cfg.StatePool, systemState, factory.ControllerConfig(), factory.User(), agentAuthFactory, cfg.Clock) c.Assert(err, jc.ErrorIsNil) cfg.LocalMacaroonAuthenticator = authenticator err = authenticator.AddHandlers(s.mux) @@ -600,19 +611,23 @@ func (s *ApiServerSuite) SeedCAASCloud(c *gc.C) { // SeedDatabase the database with a supplied controller config, and dummy // cloud and dummy credentials. -func SeedDatabase(c *gc.C, runner database.TxnRunner, controllerConfig controller.Config) { - SeedAdminUser(c, runner) +func SeedDatabase(c *gc.C, runner database.TxnRunner, controllerConfig controller.Config) coreuser.UUID { + adminUserUUID := SeedAdminUser(c, runner) err := controllerconfigbootstrap.InsertInitialControllerConfig(controllerConfig)(context.Background(), runner) c.Assert(err, jc.ErrorIsNil) SeedCloudCredentials(c, runner) + + return adminUserUUID } -func SeedAdminUser(c *gc.C, runner database.TxnRunner) { - _, userAdd := userbootstrap.AddUser(coreuser.AdminUserName) +func SeedAdminUser(c *gc.C, runner database.TxnRunner) coreuser.UUID { + adminUserUUID, userAdd := userbootstrap.AddUserWithPassword(coreuser.AdminUserName, auth.NewPassword(AdminSecret)) err := userAdd(context.Background(), runner) c.Assert(err, jc.ErrorIsNil) + + return adminUserUUID } func SeedCloudCredentials(c *gc.C, runner database.TxnRunner) { diff --git a/state/errors.go b/state/errors.go index 9d5e0bd3e35..e81010dcb94 100644 --- a/state/errors.go +++ b/state/errors.go @@ -15,7 +15,6 @@ var ( newParentDeviceHasChildrenError = stateerrors.NewParentDeviceHasChildrenError newErrCharmAlreadyUploaded = stateerrors.NewErrCharmAlreadyUploaded newDeletedUserError = stateerrors.NewDeletedUserError - newNeverLoggedInError = stateerrors.NewNeverLoggedInError newNeverConnectedError = stateerrors.NewNeverConnectedError newVersionInconsistentError = stateerrors.NewVersionInconsistentError diff --git a/state/user.go b/state/user.go index da7a32fc1e5..b45a460f90f 100644 --- a/state/user.go +++ b/state/user.go @@ -489,26 +489,6 @@ func (u *User) UserTag() names.UserTag { return names.NewLocalUserTag(name) } -// LastLogin returns when this User last connected through the API in UTC. -// The resulting time will be nil if the user has never logged in. In the -// normal case, the LastLogin is the last time that the user connected through -// the API server. -func (u *User) LastLogin() (time.Time, error) { - lastLogins, closer := u.st.db().GetRawCollection(userLastLoginC) - defer closer() - - var lastLogin userLastLoginDoc - err := lastLogins.FindId(u.doc.DocID).Select(bson.D{{"last-login", 1}}).One(&lastLogin) - if err != nil { - if err == mgo.ErrNotFound { - err = errors.Wrap(err, newNeverLoggedInError(u.UserTag().Name())) - } - return time.Time{}, errors.Trace(err) - } - - return lastLogin.LastLogin.UTC(), nil -} - // UpdateLastLogin sets the LastLogin time of the user to be now (to the // nearest second). func (u *User) UpdateLastLogin() (err error) { diff --git a/state/user_test.go b/state/user_test.go index 19848ad2565..bbea9bd0f82 100644 --- a/state/user_test.go +++ b/state/user_test.go @@ -61,9 +61,6 @@ func (s *UserSuite) TestAddUser(c *gc.C) { c.Assert(user.CreatedBy(), gc.Equals, creator) c.Assert(user.DateCreated().After(now) || user.DateCreated().Equal(now), jc.IsTrue) - lastLogin, err := user.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - c.Assert(lastLogin, gc.DeepEquals, time.Time{}) user, err = s.State.User(user.UserTag()) c.Assert(err, jc.ErrorIsNil) @@ -74,9 +71,6 @@ func (s *UserSuite) TestAddUser(c *gc.C) { c.Assert(user.CreatedBy(), gc.Equals, creator) c.Assert(user.DateCreated().After(now) || user.DateCreated().Equal(now), jc.IsTrue) - lastLogin, err = user.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - c.Assert(lastLogin, gc.DeepEquals, time.Time{}) } func (s *UserSuite) TestCheckUserExists(c *gc.C) { @@ -97,17 +91,6 @@ func (s *UserSuite) TestString(c *gc.C) { c.Assert(user.String(), gc.Equals, "foo") } -func (s *UserSuite) TestUpdateLastLogin(c *gc.C) { - now := testing.NonZeroTime().Round(time.Second).UTC() - user := s.Factory.MakeUser(c, nil) - err := user.UpdateLastLogin() - c.Assert(err, jc.ErrorIsNil) - lastLogin, err := user.LastLogin() - c.Assert(err, jc.ErrorIsNil) - c.Assert(lastLogin.After(now) || - lastLogin.Equal(now), jc.IsTrue) -} - func (s *UserSuite) TestSetPassword(c *gc.C) { user := s.Factory.MakeUser(c, nil) testSetPassword(c, s.State, func() (state.Authenticator, error) { diff --git a/testing/factory/factory_test.go b/testing/factory/factory_test.go index a89f2f36159..513d739f269 100644 --- a/testing/factory/factory_test.go +++ b/testing/factory/factory_test.go @@ -53,12 +53,6 @@ func (s *factorySuite) TestMakeUserNil(c *gc.C) { c.Assert(saved.CreatedBy(), gc.Equals, user.CreatedBy()) c.Assert(saved.DateCreated(), gc.Equals, user.DateCreated()) c.Assert(saved.IsDisabled(), gc.Equals, user.IsDisabled()) - - savedLastLogin, err := saved.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - lastLogin, err := user.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - c.Assert(savedLastLogin, gc.Equals, lastLogin) } func (s *factorySuite) TestMakeUserParams(c *gc.C) { @@ -87,12 +81,6 @@ func (s *factorySuite) TestMakeUserParams(c *gc.C) { c.Assert(saved.DateCreated(), gc.Equals, user.DateCreated()) c.Assert(saved.IsDisabled(), gc.Equals, user.IsDisabled()) - savedLastLogin, err := saved.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - lastLogin, err := user.LastLogin() - c.Assert(err, jc.Satisfies, state.IsNeverLoggedInError) - c.Assert(savedLastLogin, gc.Equals, lastLogin) - _, err = s.State.UserAccess(user.UserTag(), s.Model.ModelTag()) c.Assert(err, jc.ErrorIsNil) }