multiple: refactor arguments given to NewUser into a struct instead

olivercalder · Aug 26, 2022 · aa5b512 · aa5b512
1 parent 3b0f3e6
commit aa5b512
Show file tree

Hide file tree

Showing 13 changed files with 293 additions and 52 deletions.
diff --git a/daemon/api_base_test.go b/daemon/api_base_test.go
@@ -346,7 +346,12 @@ func (s *apiBaseSuite) asUserAuth(c *check.C, req *http.Request) {
 	if s.authUser == nil {
 		st := s.d.Overlord().State()
 		st.Lock()
-		u, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+		u, err := auth.NewUser(st, auth.NewUserData{
+			Username:   "username",
+			Email:      "email@test.com",
+			Macaroon:   "macaroon",
+			Discharges: []string{"discharge"},
+		})
 		st.Unlock()
 		c.Assert(err, check.IsNil)
 		s.authUser = u

diff --git a/daemon/api_find_test.go b/daemon/api_find_test.go
@@ -363,7 +363,12 @@ func (s *findSuite) TestFindOneWithAuth(c *check.C) {
 
 	state := d.Overlord().State()
 	state.Lock()
-	user, err := auth.NewUser(state, "username", "email@test.com", "macaroon", []string{"discharge"})
+	user, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	state.Unlock()
 	c.Check(err, check.IsNil)
 

diff --git a/daemon/api_general_test.go b/daemon/api_general_test.go
@@ -317,7 +317,12 @@ func (s *generalSuite) TestSysInfoIsManaged(c *check.C) {
 
 	st := d.Overlord().State()
 	st.Lock()
-	_, err := auth.NewUser(st, "someuser", "mymail@test.com", "macaroon", []string{"discharge"})
+	_, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "someuser",
+		Email:      "mymail@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Assert(err, check.IsNil)
 

diff --git a/daemon/api_snaps_test.go b/daemon/api_snaps_test.go
@@ -316,7 +316,12 @@ func (s *snapsSuite) TestSnapsInfoStoreWithAuth(c *check.C) {
 
 	state := d.Overlord().State()
 	state.Lock()
-	user, err := auth.NewUser(state, "username", "email@test.com", "macaroon", []string{"discharge"})
+	user, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	state.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -1433,7 +1438,12 @@ func (s *snapsSuite) TestPostSnapSetsUser(c *check.C) {
 
 	state := d.Overlord().State()
 	state.Lock()
-	user, err := auth.NewUser(state, "username", "email@test.com", "macaroon", []string{"discharge"})
+	user, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	state.Unlock()
 	c.Check(err, check.IsNil)
 

diff --git a/daemon/api_test.go b/daemon/api_test.go
@@ -98,7 +98,12 @@ func (s *apiSuite) TestUserFromRequestHeaderCorrectMissingUser(c *check.C) {
 
 func (s *apiSuite) TestUserFromRequestHeaderValidUser(c *check.C) {
 	s.st.Lock()
-	expectedUser, err := auth.NewUser(s.st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	expectedUser, err := auth.NewUser(s.st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	s.st.Unlock()
 	c.Check(err, check.IsNil)
 

diff --git a/daemon/api_users.go b/daemon/api_users.go
@@ -170,7 +170,12 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
 		user.Email = loginData.Email
 		err = auth.UpdateUser(st, user)
 	} else {
-		user, err = auth.NewUser(st, loginData.Username, loginData.Email, macaroon, []string{discharge})
+		user, err = auth.NewUser(st, auth.NewUserData{
+			Username:   loginData.Username,
+			Email:      loginData.Email,
+			Macaroon:   macaroon,
+			Discharges: []string{discharge},
+		})
 	}
 	st.Unlock()
 	if err != nil {
@@ -569,7 +574,12 @@ func setupLocalUser(st *state.State, username, email string) error {
 
 	// setup new user, local-only
 	st.Lock()
-	authUser, err := auth.NewUser(st, username, email, "", nil)
+	authUser, err := auth.NewUser(st, auth.NewUserData{
+		Username:   username,
+		Email:      email,
+		Macaroon:   "",
+		Discharges: nil,
+	})
 	st.Unlock()
 	if err != nil {
 		return fmt.Errorf("cannot persist authentication details: %v", err)

diff --git a/daemon/api_users_test.go b/daemon/api_users_test.go
@@ -210,7 +210,12 @@ func (s *userSuite) TestLoginUserNoEmailWithExistentLocalUser(c *check.C) {
 
 	// setup local-only user
 	state.Lock()
-	localUser, err := auth.NewUser(state, "username", "email@test.com", "", nil)
+	localUser, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "",
+		Discharges: nil,
+	})
 	state.Unlock()
 	c.Assert(err, check.IsNil)
 
@@ -253,7 +258,12 @@ func (s *userSuite) TestLoginUserWithExistentLocalUser(c *check.C) {
 
 	// setup local-only user
 	state.Lock()
-	localUser, err := auth.NewUser(state, "username", "email@test.com", "", nil)
+	localUser, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "",
+		Discharges: nil,
+	})
 	state.Unlock()
 	c.Assert(err, check.IsNil)
 
@@ -296,7 +306,12 @@ func (s *userSuite) TestLoginUserNewEmailWithExistentLocalUser(c *check.C) {
 
 	// setup local-only user
 	state.Lock()
-	localUser, err := auth.NewUser(state, "username", "email@test.com", "", nil)
+	localUser, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "",
+		Discharges: nil,
+	})
 	state.Unlock()
 	c.Assert(err, check.IsNil)
 
@@ -339,7 +354,12 @@ func (s *userSuite) TestLogoutUser(c *check.C) {
 	s.expectLoginAccess()
 
 	state.Lock()
-	user, err := auth.NewUser(state, "username", "email@test.com", "macaroon", []string{"discharge"})
+	user, err := auth.NewUser(state, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	state.Unlock()
 	c.Assert(err, check.IsNil)
 
@@ -617,7 +637,12 @@ func (s *userSuite) TestPostUserActionRemoveNoUsername(c *check.C) {
 func (s *userSuite) TestPostUserActionRemoveDelUserErr(c *check.C) {
 	st := s.d.Overlord().State()
 	st.Lock()
-	_, err := auth.NewUser(st, "some-user", "email@test.com", "macaroon", []string{"discharge"})
+	_, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "some-user",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -680,7 +705,12 @@ func (s *userSuite) TestPostUserActionRemoveNoUserInState(c *check.C) {
 func (s *userSuite) TestPostUserActionRemove(c *check.C) {
 	st := s.d.Overlord().State()
 	st.Lock()
-	user, err := auth.NewUser(st, "some-user", "email@test.com", "macaroon", []string{"discharge"})
+	user, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "some-user",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -1058,7 +1088,12 @@ func (s *userSuite) TestPostCreateUserFromAssertionAllKnownButOwnedErrors(c *che
 
 	st := s.d.Overlord().State()
 	st.Lock()
-	_, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	_, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -1076,7 +1111,12 @@ func (s *userSuite) TestPostCreateUserAutomaticManagedDoesNotActOrError(c *check
 
 	st := s.d.Overlord().State()
 	st.Lock()
-	_, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	_, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -1150,7 +1190,12 @@ func (s *userSuite) TestPostCreateUserFromAssertionAllKnownButOwned(c *check.C)
 
 	st := s.d.Overlord().State()
 	st.Lock()
-	_, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	_, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Check(err, check.IsNil)
 
@@ -1244,7 +1289,12 @@ func (s *userSuite) TestUsersEmpty(c *check.C) {
 func (s *userSuite) TestUsersHasUser(c *check.C) {
 	st := s.d.Overlord().State()
 	st.Lock()
-	u, err := auth.NewUser(st, "someuser", "mymail@test.com", "macaroon", []string{"discharge"})
+	u, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "someuser",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Assert(err, check.IsNil)
 

diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go
@@ -122,7 +122,12 @@ func (s *daemonSuite) TestCommandMethodDispatch(c *check.C) {
 	d := newTestDaemon(c)
 	st := d.Overlord().State()
 	st.Lock()
-	authUser, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	authUser, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Assert(err, check.IsNil)
 
@@ -440,7 +445,12 @@ func (s *daemonSuite) TestWriteAccessWithUser(c *check.C) {
 	d := newTestDaemon(c)
 	st := d.Overlord().State()
 	st.Lock()
-	authUser, err := auth.NewUser(st, "username", "email@test.com", "macaroon", []string{"discharge"})
+	authUser, err := auth.NewUser(st, auth.NewUserData{
+		Username:   "username",
+		Email:      "email@test.com",
+		Macaroon:   "macaroon",
+		Discharges: []string{"discharge"},
+	})
 	st.Unlock()
 	c.Assert(err, check.IsNil)
 

diff --git a/overlord/auth/auth.go b/overlord/auth/auth.go
@@ -134,8 +134,19 @@ func newUserMacaroon(macaroonKey []byte, userID int) (string, error) {
 
 // TODO: possibly move users' related functions to a userstate package
 
+type NewUserData struct {
+	// Username is the name of the user on the system
+	Username string
+	// Email is the email associated with the user
+	Email string
+	// Macaroon is the store associated macaroon for the user
+	Macaroon string
+	// Discharges
+	Discharges []string
+}
+
 // NewUser tracks a new authenticated user and saves its details in the state
-func NewUser(st *state.State, username, email, macaroon string, discharges []string) (*UserState, error) {
+func NewUser(st *state.State, userData NewUserData) (*UserState, error) {
 	var authStateData AuthState
 
 	err := st.Get("auth", &authStateData)
@@ -159,15 +170,15 @@ func NewUser(st *state.State, username, email, macaroon string, discharges []str
 		return nil, err
 	}
 
-	sort.Strings(discharges)
+	sort.Strings(userData.Discharges)
 	authenticatedUser := UserState{
 		ID:              authStateData.LastID,
-		Username:        username,
-		Email:           email,
+		Username:        userData.Username,
+		Email:           userData.Email,
 		Macaroon:        localMacaroon,
 		Discharges:      nil,
-		StoreMacaroon:   macaroon,
-		StoreDischarges: discharges,
+		StoreMacaroon:   userData.Macaroon,
+		StoreDischarges: userData.Discharges,
 	}
 	authStateData.Users = append(authStateData.Users, authenticatedUser)