From 4dc3b38c95482d22781297b794e4dc9c67c4adc7 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sat, 24 May 2025 09:31:57 -0400 Subject: [PATCH] Allow adding/changing user with password hash via v1/users API --- server/server_admin.go | 28 ++++++++++----- server/server_admin_test.go | 71 ++++++++++++++++++++++++++++++++++--- server/types.go | 1 + user/manager.go | 3 -- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/server/server_admin.go b/server/server_admin.go index fc149e7f..eb362956 100644 --- a/server/server_admin.go +++ b/server/server_admin.go @@ -42,8 +42,8 @@ func (s *Server) handleUsersAdd(w http.ResponseWriter, r *http.Request, v *visit req, err := readJSONWithLimit[apiUserAddOrUpdateRequest](r.Body, jsonBodyBytesLimit, false) if err != nil { return err - } else if !user.AllowedUsername(req.Username) || req.Password == "" { - return errHTTPBadRequest.Wrap("username invalid, or password missing") + } else if !user.AllowedUsername(req.Username) || (req.Password == "" && req.Hash == "") { + return errHTTPBadRequest.Wrap("username invalid, or password/password_hash missing") } u, err := s.userManager.User(req.Username) if err != nil && !errors.Is(err, user.ErrUserNotFound) { @@ -60,7 +60,11 @@ func (s *Server) handleUsersAdd(w http.ResponseWriter, r *http.Request, v *visit return err } } - if err := s.userManager.AddUser(req.Username, req.Password, user.RoleUser, false); err != nil { + password, hashed := req.Password, false + if req.Hash != "" { + password, hashed = req.Hash, true + } + if err := s.userManager.AddUser(req.Username, password, user.RoleUser, hashed); err != nil { return err } if tier != nil { @@ -77,8 +81,8 @@ func (s *Server) handleUsersUpdate(w http.ResponseWriter, r *http.Request, v *vi return err } else if !user.AllowedUsername(req.Username) { return errHTTPBadRequest.Wrap("username invalid") - } else if req.Password == "" && req.Tier == "" { - return errHTTPBadRequest.Wrap("need to provide at least one of \"password\" or \"tier\"") + } else if req.Password == "" && req.Hash == "" && req.Tier == "" { + return errHTTPBadRequest.Wrap("need to provide at least one of \"password\", \"password_hash\" or \"tier\"") } u, err := s.userManager.User(req.Username) if err != nil && !errors.Is(err, user.ErrUserNotFound) { @@ -87,13 +91,21 @@ func (s *Server) handleUsersUpdate(w http.ResponseWriter, r *http.Request, v *vi if u.IsAdmin() { return errHTTPForbidden } - if req.Password != "" { - if err := s.userManager.ChangePassword(req.Username, req.Password); err != nil { + if req.Hash != "" { + if err := s.userManager.ChangePassword(req.Username, req.Hash, true); err != nil { + return err + } + } else if req.Password != "" { + if err := s.userManager.ChangePassword(req.Username, req.Password, false); err != nil { return err } } } else { - if err := s.userManager.AddUser(req.Username, req.Password, user.RoleUser); err != nil { + password, hashed := req.Password, false + if req.Hash != "" { + password, hashed = req.Hash, true + } + if err := s.userManager.AddUser(req.Username, password, user.RoleUser, hashed); err != nil { return err } } diff --git a/server/server_admin_test.go b/server/server_admin_test.go index 1fdc740f..8925702e 100644 --- a/server/server_admin_test.go +++ b/server/server_admin_test.go @@ -65,12 +65,41 @@ func TestUser_AddRemove(t *testing.T) { require.Equal(t, 400, rr.Code) } +func TestUser_AddWithPasswordHash(t *testing.T) { + s := newTestServer(t, newTestConfigWithAuthFile(t)) + defer s.closeDatabases() + + // Create admin + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + + // Create user via API + rr := request(t, s, "POST", "/v1/users", `{"username": "ben", "hash":"$2a$04$2aPIIqPXQU16OfkSUZH1XOzpu1gsPRKkrfVdFLgWQ.tqb.vtTCuVe"}`, map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // Check that user can login with password + rr = request(t, s, "POST", "/v1/account/token", "", map[string]string{ + "Authorization": util.BasicAuth("ben", "ben"), + }) + require.Equal(t, 200, rr.Code) + + // Check users + users, err := s.userManager.Users() + require.Nil(t, err) + require.Equal(t, 3, len(users)) + require.Equal(t, "phil", users[0].Name) + require.Equal(t, user.RoleAdmin, users[0].Role) + require.Equal(t, "ben", users[1].Name) + require.Equal(t, user.RoleUser, users[1].Role) +} + func TestUser_ChangeUserPassword(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() // Create admin - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) // Create user via API rr := request(t, s, "POST", "/v1/users", `{"username": "ben", "password": "ben"}`, map[string]string{ @@ -108,7 +137,7 @@ func TestUser_ChangeUserTier(t *testing.T) { defer s.closeDatabases() // Create admin, tier - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) require.Nil(t, s.userManager.AddTier(&user.Tier{ Code: "tier1", })) @@ -148,7 +177,7 @@ func TestUser_ChangeUserPasswordAndTier(t *testing.T) { defer s.closeDatabases() // Create admin, tier - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) require.Nil(t, s.userManager.AddTier(&user.Tier{ Code: "tier1", })) @@ -195,13 +224,45 @@ func TestUser_ChangeUserPasswordAndTier(t *testing.T) { require.Equal(t, "tier2", users[1].Tier.Code) } +func TestUser_ChangeUserPasswordWithHash(t *testing.T) { + s := newTestServer(t, newTestConfigWithAuthFile(t)) + defer s.closeDatabases() + + // Create admin + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + + // Create user with tier via API + rr := request(t, s, "POST", "/v1/users", `{"username": "ben", "password":"not-ben"}`, map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // Try to login with first password + rr = request(t, s, "POST", "/v1/account/token", "", map[string]string{ + "Authorization": util.BasicAuth("ben", "not-ben"), + }) + require.Equal(t, 200, rr.Code) + + // Change user password and tier via API + rr = request(t, s, "PUT", "/v1/users", `{"username": "ben", "hash":"$2a$04$2aPIIqPXQU16OfkSUZH1XOzpu1gsPRKkrfVdFLgWQ.tqb.vtTCuVe"}`, map[string]string{ + "Authorization": util.BasicAuth("phil", "phil"), + }) + require.Equal(t, 200, rr.Code) + + // Try to login with second password + rr = request(t, s, "POST", "/v1/account/token", "", map[string]string{ + "Authorization": util.BasicAuth("ben", "ben"), + }) + require.Equal(t, 200, rr.Code) +} + func TestUser_DontChangeAdminPassword(t *testing.T) { s := newTestServer(t, newTestConfigWithAuthFile(t)) defer s.closeDatabases() // Create admin - require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin)) - require.Nil(t, s.userManager.AddUser("admin", "admin", user.RoleAdmin)) + require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleAdmin, false)) + require.Nil(t, s.userManager.AddUser("admin", "admin", user.RoleAdmin, false)) // Try to change password via API rr := request(t, s, "PUT", "/v1/users", `{"username": "admin", "password": "admin-new"}`, map[string]string{ diff --git a/server/types.go b/server/types.go index fd1931f5..16049ee4 100644 --- a/server/types.go +++ b/server/types.go @@ -256,6 +256,7 @@ type apiStatsResponse struct { type apiUserAddOrUpdateRequest struct { Username string `json:"username"` Password string `json:"password"` + Hash string `json:"hash"` Tier string `json:"tier"` // Do not add 'role' here. We don't want to add admins via the API. } diff --git a/user/manager.go b/user/manager.go index d691d42f..59c8d51f 100644 --- a/user/manager.go +++ b/user/manager.go @@ -868,10 +868,8 @@ func (a *Manager) AddUser(username, password string, role Role, hashed bool) err if !AllowedUsername(username) || !AllowedRole(role) { return ErrInvalidArgument } - var hash []byte var err error = nil - if hashed { hash = []byte(password) } else { @@ -880,7 +878,6 @@ func (a *Manager) AddUser(username, password string, role Role, hashed bool) err return err } } - userID := util.RandomStringPrefix(userIDPrefix, userIDLength) syncTopic, now := util.RandomStringPrefix(syncTopicPrefix, syncTopicLength), time.Now().Unix() if _, err = a.db.Exec(insertUserQuery, userID, username, hash, role, syncTopic, now); err != nil {