Merge pull request juju#16763 from anvial/JUJU-5285-inject-get-user-w…

…ith-auth-info-func-in-user-domain

juju#16763

This PR injects GetUserWithAuthInfo in state and service layers. Adds unit test for this function.

Also, it adds a unit test for GetAllUsers function in the service layer.

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

```
go test github.com/juju/juju/domain/user/state/... -gocheck.v
go test github.com/juju/juju/domain/user/service/... -gocheck.v
```

## Links

**Jira card:** [JUJU-5285](https://warthogs.atlassian.net/browse/JUJU-5285)



[JUJU-5285]: https://warthogs.atlassian.net/browse/JUJU-5285?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

core/user/user.go

@@ -10,6 +10,7 @@ import (
 	"github.com/juju/utils/v3"
 )
 
+// User represents a user in the system.
 type User struct {
 	// UUID is the unique identifier for the user.
 	UUID UUID
@@ -25,6 +26,12 @@ type User struct {
 
 	// CreatedAt is the time that the user was created at.
 	CreatedAt time.Time
+
+	// LastLogin is the last time the user logged in.
+	LastLogin time.Time
+
+	// Disabled is true if the user is disabled.
+	Disabled bool
 }
 
 // UUID is a unique identifier for a user.

domain/user/service/service.go

@@ -38,21 +38,20 @@ type State interface {
 	// usererrors.UserCreatorNotFound will be returned.
 	AddUserWithActivationKey(ctx context.Context, uuid user.UUID, usr user.User, creatorUUID user.UUID, activationKey []byte) error
 
-	// GetUser will retrieve the user specified by UUID from the database where
-	// the user is active. If the user does not exist an error that satisfies
+	// GetAllUsers will retrieve all users with authentication information
+	// (last login, disabled) from the database. If no users exist an empty slice
+	// will be returned.
+	GetAllUsers(context.Context) ([]user.User, error)
+
+	// GetUser will retrieve the user with authentication information (last login, disabled)
+	// specified by UUID from the database. If the user does not exist an error that satisfies
 	// usererrors.NotFound will be returned.
 	GetUser(context.Context, user.UUID) (user.User, error)
 
-	// GetUserByName will retrieve the user specified by name from the database
-	// where the user is active and has not been removed. If the user does not
-	// exist or is removed an error that satisfies usererrors.NotFound will be
-	// returned.
-	GetUserByName(context.Context, string) (user.User, error)
-
-	// GetAllUsers will retrieve all users from the database where the user is
-	// active and has not been removed. If no users exist an empty slice will be
-	// returned.
-	GetAllUsers(context.Context) ([]user.User, error)
+	// 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(ctx context.Context, name string) (user.User, error)
 
 	// RemoveUser marks the user as removed. This obviates the ability of a user
 	// to function, but keeps the user retaining provenance, i.e. auditing.
@@ -80,6 +79,11 @@ type State interface {
 	// If no user is found for the supplied UUID an error is returned that
 	// satisfies usererrors.NotFound.
 	DisableUserAuthentication(context.Context, user.UUID) error
+
+	// UpdateLastLogin will update the last login time for the user.
+	// If no user is found for the supplied UUID an error is returned that
+	// satisfies usererrors.NotFound.
+	UpdateLastLogin(context.Context, user.UUID) error
 }
 
 // Service provides the API for working with users.
@@ -114,6 +118,17 @@ func NewService(st State) *Service {
 	}
 }
 
+// GetAllUsers will retrieve all users with authentication information
+// (last login, disabled) from the database. If no users exist an empty slice
+// will be returned.
+func (s *Service) GetAllUsers(ctx context.Context) ([]user.User, error) {
+	usrs, err := s.st.GetAllUsers(ctx)
+	if err != nil {
+		return nil, errors.Annotate(err, "getting all users with auth info")
+	}
+	return usrs, nil
+}
+
 // GetUser will find and return the user with UUID. If there is no
 // user for the UUID then an error that satisfies usererrors.NotFound will
 // be returned.
@@ -155,15 +170,6 @@ func (s *Service) GetUserByName(
 	return usr, nil
 }
 
-// GetAllUsers will return all users that have not been removed.
-func (s *Service) GetAllUsers(ctx context.Context) ([]user.User, error) {
-	usr, err := s.st.GetAllUsers(ctx)
-	if err != nil {
-		return nil, errors.Annotate(err, "getting all users")
-	}
-	return usr, nil
-}
-
 // ValidateUsername takes a user name and validates that the user name is
 // conformant to our regex rules defined in usernameValidationRegex. If a
 // user name is not valid an error is returned that satisfies
@@ -403,6 +409,22 @@ func (s *Service) DisableUserAuthentication(ctx context.Context, uuid user.UUID)
 	return nil
 }
 
+// UpdateLastLogin will update the last login time for the user.
+//
+// The following error types are possible from this function:
+// - usererrors.UUIDNotValid: When the UUID supplied is not valid.
+// - usererrors.NotFound: If no user by the given UUID exists.
+func (s *Service) UpdateLastLogin(ctx context.Context, uuid user.UUID) error {
+	if err := uuid.Validate(); err != nil {
+		return errors.Annotatef(usererrors.UUIDNotValid, "%q", uuid)
+	}
+
+	if err := s.st.UpdateLastLogin(ctx, uuid); err != nil {
+		return errors.Annotatef(err, "updating last login for user with uuid %q", uuid)
+	}
+	return nil
+}
+
 // generateActivationKey is responsible for generating a new activation key that
 // can be used for supplying to a user.
 func generateActivationKey() ([]byte, error) {

domain/user/service/service_test.go

@@ -31,6 +31,7 @@ type stateUser struct {
 	passwordHash  string
 	passwordSalt  []byte
 	removed       bool
+	lastLogin     time.Time
 	disabled      bool
 }
 
@@ -101,6 +102,24 @@ func IsUserNameAlreadyExists(name string, m map[user.UUID]stateUser) bool {
 func (s *serviceSuite) setMockState(c *gc.C) map[user.UUID]stateUser {
 	mockState := map[user.UUID]stateUser{}
 
+	s.state.EXPECT().GetAllUsers(
+		gomock.Any(),
+	).DoAndReturn(func(
+		_ context.Context) ([]user.User, error) {
+		var users []user.User
+		for _, usr := range mockState {
+			users = append(users, user.User{
+				CreatorUUID: usr.creatorUUID,
+				CreatedAt:   usr.createdAt,
+				DisplayName: usr.displayName,
+				Name:        usr.name,
+				LastLogin:   usr.lastLogin,
+				Disabled:    usr.disabled,
+			})
+		}
+		return users, nil
+	}).AnyTimes()
+
 	s.state.EXPECT().GetUser(
 		gomock.Any(), gomock.Any(),
 	).DoAndReturn(func(
@@ -115,6 +134,8 @@ func (s *serviceSuite) setMockState(c *gc.C) map[user.UUID]stateUser {
 			CreatedAt:   stUser.createdAt,
 			DisplayName: stUser.displayName,
 			Name:        stUser.name,
+			LastLogin:   stUser.lastLogin,
+			Disabled:    stUser.disabled,
 		}, nil
 	}).AnyTimes()
 
@@ -130,6 +151,8 @@ func (s *serviceSuite) setMockState(c *gc.C) map[user.UUID]stateUser {
 					CreatedAt:   usr.createdAt,
 					DisplayName: usr.displayName,
 					Name:        usr.name,
+					LastLogin:   usr.lastLogin,
+					Disabled:    usr.disabled,
 				}, nil
 			}
 		}
@@ -294,6 +317,20 @@ func (s *serviceSuite) setMockState(c *gc.C) map[user.UUID]stateUser {
 		return nil
 	}).AnyTimes()
 
+	// Implement the contract defined by UpdateLastLogin
+	s.state.EXPECT().UpdateLastLogin(
+		gomock.Any(), gomock.Any(),
+	).DoAndReturn(func(
+		_ context.Context,
+		uuid user.UUID) error {
+		usr, exists := mockState[uuid]
+		if !exists || usr.removed {
+			return usererrors.NotFound
+		}
+		usr.lastLogin = time.Now()
+		mockState[uuid] = usr
+		return nil
+	}).AnyTimes()
 	return mockState
 }
 
@@ -1063,6 +1100,62 @@ func (s *serviceSuite) TestGetUserByNameNotFound(c *gc.C) {
 	c.Assert(len(mockState), gc.Equals, 0)
 }
 
+// TestGetAllUsers tests the happy path for GetAllUsers.
+func (s *serviceSuite) TestGetAllUsers(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+	mockState := s.setMockState(c)
+	uuid1, err := user.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	lastLogin := time.Now().Add(-time.Minute * 2)
+	mockState[uuid1] = stateUser{
+		name:        "Jürgen.test",
+		createdAt:   time.Now().Add(-time.Minute * 5),
+		displayName: "Old mate 👍",
+		lastLogin:   lastLogin,
+	}
+	uuid2, err := user.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	mockState[uuid2] = stateUser{
+		name:        "杨-test",
+		createdAt:   time.Now().Add(-time.Minute * 5),
+		displayName: "test1",
+		lastLogin:   lastLogin,
+	}
+
+	users, err := s.service().GetAllUsers(context.Background())
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(len(users), gc.Equals, 2)
+	c.Assert(users[0].Name, gc.Equals, "Jürgen.test")
+	c.Assert(users[0].DisplayName, gc.Equals, "Old mate 👍")
+	c.Assert(users[0].LastLogin, gc.Equals, lastLogin)
+	c.Assert(users[1].Name, gc.Equals, "杨-test")
+	c.Assert(users[1].DisplayName, gc.Equals, "test1")
+	c.Assert(users[1].LastLogin, gc.Equals, lastLogin)
+}
+
+// TestGetUserWithAuthInfo tests the happy path for GetUser.
+func (s *serviceSuite) TestGetUserWithAuthInfo(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+	mockState := s.setMockState(c)
+	uuid, err := user.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	lastLogin := time.Now().Add(-time.Minute * 2)
+	mockState[uuid] = stateUser{
+		name:        "Jürgen.test",
+		createdAt:   time.Now().Add(-time.Minute * 5),
+		displayName: "Old mate 👍",
+		lastLogin:   lastLogin,
+		disabled:    true,
+	}
+
+	user, err := s.service().GetUser(context.Background(), uuid)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(user.Name, gc.Equals, "Jürgen.test")
+	c.Assert(user.DisplayName, gc.Equals, "Old mate 👍")
+	c.Assert(user.LastLogin, gc.Equals, lastLogin)
+	c.Assert(user.Disabled, gc.Equals, true)
+}
+
 // TestGetUserByNameInvalidUsername is here to assert that when we ask for a user with
 // a username that is invalid we get a UsernameNotValid error. We also check
 // here that the service doesn't let invalid usernames flow through to the state
@@ -1075,6 +1168,29 @@ func (s *serviceSuite) TestGetUserByNameInvalidUsername(c *gc.C) {
 	}
 }
 
+// TestGetUserWithAuthInfoByName tests the happy path for GetUserByName.
+func (s *serviceSuite) TestGetUserWithAuthInfoByName(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+	mockState := s.setMockState(c)
+	uuid, err := user.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	lastLogin := time.Now().Add(-time.Minute * 2)
+	mockState[uuid] = stateUser{
+		name:        "Jürgen.test",
+		createdAt:   time.Now().Add(-time.Minute * 5),
+		displayName: "Old mate 👍",
+		lastLogin:   lastLogin,
+		disabled:    true,
+	}
+
+	user, err := s.service().GetUserByName(context.Background(), "Jürgen.test")
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(user.Name, gc.Equals, "Jürgen.test")
+	c.Assert(user.DisplayName, gc.Equals, "Old mate 👍")
+	c.Assert(user.LastLogin, gc.Equals, lastLogin)
+	c.Assert(user.Disabled, gc.Equals, true)
+}
+
 // TestEnableUserAuthentication tests the happy path for EnableUserAuthentication.
 func (s *serviceSuite) TestEnableUserAuthentication(c *gc.C) {
 	defer s.setupMocks(c).Finish()
@@ -1182,3 +1298,23 @@ func (s *serviceSuite) TestUsernameValidation(c *gc.C) {
 		}
 	}
 }
+
+// TestUpdateLastLogin tests the happy path for UpdateLastLogin.
+func (s *serviceSuite) TestUpdateLastLogin(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+	mockState := s.setMockState(c)
+	now := time.Now()
+	uuid, err := user.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+
+	mockState[uuid] = stateUser{
+		name:      "username",
+		lastLogin: now,
+	}
+
+	err = s.service().UpdateLastLogin(context.Background(), uuid)
+	c.Assert(err, jc.ErrorIsNil)
+
+	userState := mockState[uuid]
+	c.Assert(userState.lastLogin, gc.NotNil)
+}