Skip to content

Commit

Permalink
authn: error code fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryan Koo <[email protected]>
  • Loading branch information
@rkoo19
rkoo19 committed Jan 8, 2025
1 parent b89781d commit b7e9785
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 52 deletions.
34 changes: 18 additions & 16 deletions cmd/authn/hserv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,9 @@ func (h *hserv) httpUserPut(w http.ResponseWriter, r *http.Request) {
if Conf.Verbose() {
nlog.Infof("PUT user %q", userID)
}
if err := h.mgr.updateUser(userID, updateReq); err != nil {
cmn.WriteErr(w, r, err)
if code, err := h.mgr.updateUser(userID, updateReq); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "update user", userID, err, code)
cmn.WriteErr(w, r, err, code)
}
}

Expand All @@ -235,8 +236,9 @@ func (h *hserv) userAdd(w http.ResponseWriter, r *http.Request) {
if err := cmn.ReadJSON(w, r, info); err != nil {
return
}
if err := h.mgr.addUser(info); err != nil {
cmn.WriteErrMsg(w, r, fmt.Sprintf("Failed to add user: %v", err), http.StatusInternalServerError)
if code, err := h.mgr.addUser(info); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "add user", info.ID, err, code)
cmn.WriteErr(w, r, err, code)
return
}
if Conf.Verbose() {
Expand Down Expand Up @@ -382,8 +384,9 @@ func (h *hserv) httpSrvPost(w http.ResponseWriter, r *http.Request) {
if err := cmn.ReadJSON(w, r, cluConf); err != nil {
return
}
if err := h.mgr.addCluster(cluConf); err != nil {
cmn.WriteErr(w, r, err, http.StatusInternalServerError)
if code, err := h.mgr.addCluster(cluConf); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "add cluster", cluConf.ID, err, code)
cmn.WriteErr(w, r, err, code)
}
}

Expand All @@ -400,8 +403,9 @@ func (h *hserv) httpSrvPut(w http.ResponseWriter, r *http.Request) {
return
}
cluID := apiItems[0]
if err := h.mgr.updateCluster(cluID, cluConf); err != nil {
cmn.WriteErr(w, r, err, http.StatusInternalServerError)
if code, err := h.mgr.updateCluster(cluID, cluConf); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "update cluster", cluID, err, code)
cmn.WriteErr(w, r, err, code)
}
}

Expand Down Expand Up @@ -538,8 +542,9 @@ func (h *hserv) httpRolePost(w http.ResponseWriter, r *http.Request) {
if err := cmn.ReadJSON(w, r, info); err != nil {
return
}
if err := h.mgr.addRole(info); err != nil {
cmn.WriteErrMsg(w, r, fmt.Sprintf("Failed to add role: %v", err), http.StatusInternalServerError)
if code, err := h.mgr.addRole(info); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "add role", info.Name, err, code)
cmn.WriteErr(w, r, err, code)
}
}

Expand All @@ -562,11 +567,8 @@ func (h *hserv) httpRolePut(w http.ResponseWriter, r *http.Request) {
if Conf.Verbose() {
nlog.Infof("PUT role %q\n", role)
}
if err := h.mgr.updateRole(role, updateReq); err != nil {
if cos.IsErrNotFound(err) {
cmn.WriteErr(w, r, err, http.StatusNotFound)
} else {
cmn.WriteErr(w, r, err)
}
if code, err := h.mgr.updateRole(role, updateReq); err != nil {
err := cmn.NewErrFailedTo(h.mgr, "update role", role, err, code)
cmn.WriteErr(w, r, err, code)
}
}
97 changes: 64 additions & 33 deletions cmd/authn/mgr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,25 @@ func (*mgr) String() string { return svcName }

// Registers a new user. It is info from a user, so the password
// is not encrypted and a few fields are not filled(e.g, Access).
func (m *mgr) addUser(info *authn.User) error {
func (m *mgr) addUser(info *authn.User) (int, error) {
if info.ID == "" || info.Password == "" {
return errInvalidCredentials
return http.StatusBadRequest, errInvalidCredentials
}
// Validate user ID
if !cos.IsAlphaNice(info.ID) {
return fmt.Errorf("user ID %q is invalid: %s", info.ID, cos.OnlyNice)
err := fmt.Errorf("user ID %q is invalid: %s", info.ID, cos.OnlyNice)
return http.StatusBadRequest, err
}
_, err := m.db.GetString(usersCollection, info.ID)
if err == nil {
return fmt.Errorf("user %q already registered", info.ID)
err := fmt.Errorf("user %q already registered", info.ID)
return http.StatusConflict, err
}
info.Password = encryptPassword(info.Password)
return m.db.Set(usersCollection, info.ID, info)
if err := m.db.Set(usersCollection, info.ID, info); err != nil {
return http.StatusInternalServerError, err
}
return 0, nil
}

// Deletes an existing user
Expand All @@ -89,14 +94,16 @@ func (m *mgr) delUser(userID string) error {

// Updates an existing user. The function invalidates user tokens after
// successful update.
func (m *mgr) updateUser(userID string, updateReq *authn.User) error {
func (m *mgr) updateUser(userID string, updateReq *authn.User) (int, error) {
uInfo := &authn.User{}
err := m.db.Get(usersCollection, userID, uInfo)
if err != nil {
return cos.NewErrNotFound(m, "user "+userID)
err := cos.NewErrNotFound(m, "user "+userID)
return http.StatusNotFound, err
}
if userID == adminUserID && len(updateReq.Roles) != 0 {
return errors.New("cannot change administrator's role")
err := errors.New("cannot change administrator's role")
return http.StatusUnauthorized, err
}

if updateReq.Password != "" {
Expand All @@ -105,7 +112,10 @@ func (m *mgr) updateUser(userID string, updateReq *authn.User) error {
if len(updateReq.Roles) != 0 {
uInfo.Roles = updateReq.Roles
}
return m.db.Set(usersCollection, userID, uInfo)
if err := m.db.Set(usersCollection, userID, uInfo); err != nil {
return http.StatusInternalServerError, err
}
return 0, nil
}

func (m *mgr) lookupUser(userID string) (*authn.User, error) {
Expand Down Expand Up @@ -137,23 +147,29 @@ func (m *mgr) userList() (map[string]*authn.User, error) {
//

// Registers a new role
func (m *mgr) addRole(info *authn.Role) error {
func (m *mgr) addRole(info *authn.Role) (int, error) {
if info.Name == "" {
return errors.New("role name is undefined")
return http.StatusBadRequest, errors.New("role name is undefined")
}
// Validate role name
if !cos.IsAlphaNice(info.Name) {
return fmt.Errorf("role name %q is invalid: %s", info.Name, cos.OnlyNice)
err := fmt.Errorf("role name %q is invalid: %s", info.Name, cos.OnlyNice)
return http.StatusBadRequest, err
}
if info.IsAdmin {
return fmt.Errorf("only built-in roles can have %q permissions", adminUserID)
err := fmt.Errorf("only built-in roles can have %q permissions", adminUserID)
return http.StatusBadRequest, err
}

_, err := m.db.GetString(rolesCollection, info.Name)
if err == nil {
return fmt.Errorf("role %q already exists", info.Name)
err := fmt.Errorf("role %q already exists", info.Name)
return http.StatusConflict, err
}
if err := m.db.Set(rolesCollection, info.Name, info); err != nil {
return http.StatusInternalServerError, err
}
return m.db.Set(rolesCollection, info.Name, info)
return 0, nil
}

// Deletes an existing role
Expand All @@ -165,14 +181,16 @@ func (m *mgr) delRole(role string) error {
}

// Updates an existing role
func (m *mgr) updateRole(role string, updateReq *authn.Role) error {
func (m *mgr) updateRole(role string, updateReq *authn.Role) (int, error) {
if role == authn.AdminRole {
return fmt.Errorf("cannot modify built-in %q role", authn.AdminRole)
err := fmt.Errorf("cannot modify built-in %q role", authn.AdminRole)
return http.StatusForbidden, err
}
rInfo := &authn.Role{}
err := m.db.Get(rolesCollection, role, rInfo)
if err != nil {
return cos.NewErrNotFound(m, "role "+role)
err := cos.NewErrNotFound(m, "role "+role)
return http.StatusNotFound, err
}

if updateReq.Description != "" {
Expand All @@ -181,7 +199,11 @@ func (m *mgr) updateRole(role string, updateReq *authn.Role) error {
rInfo.ClusterACLs = mergeClusterACLs(rInfo.ClusterACLs, updateReq.ClusterACLs, "")
rInfo.BucketACLs = mergeBckACLs(rInfo.BucketACLs, updateReq.BucketACLs, "")

return m.db.Set(rolesCollection, role, rInfo)
if err := m.db.Set(rolesCollection, role, rInfo); err != nil {
return http.StatusInternalServerError, err
}

return 0, nil
}

func (m *mgr) lookupRole(roleID string) (*authn.Role, error) {
Expand Down Expand Up @@ -288,49 +310,55 @@ func (m *mgr) getCluster(cluID string) (*authn.CluACL, error) {
}

// Registers a new cluster
func (m *mgr) addCluster(clu *authn.CluACL) error {
func (m *mgr) addCluster(clu *authn.CluACL) (int, error) {
if clu.ID == "" {
return errors.New("cluster UUID is undefined")
err := errors.New("cluster UUID is undefined")
return http.StatusBadRequest, err
}
// Validate cluster alias
if !cos.IsAlphaNice(clu.Alias) {
return fmt.Errorf("cluster alias %q is invalid: %s", clu.Alias, cos.OnlyNice)
err := fmt.Errorf("cluster alias %q is invalid: %s", clu.Alias, cos.OnlyNice)
return http.StatusBadRequest, err
}
cid := m.cluLookup(clu.ID, clu.Alias)
if cid != "" {
return fmt.Errorf("cluster %s[%s] already registered", clu.ID, cid)
err := fmt.Errorf("cluster %s[%s] already registered", clu.ID, cid)
return http.StatusConflict, err
}

// secret handshake
if err := m.validateSecret(clu); err != nil {
return err
return http.StatusInternalServerError, err
}

if err := m.db.Set(clustersCollection, clu.ID, clu); err != nil {
return err
return http.StatusInternalServerError, err
}
m.createRolesForCluster(clu)

go m.syncTokenList(clu)
return nil
return 0, nil
}

func (m *mgr) updateCluster(cluID string, info *authn.CluACL) error {
func (m *mgr) updateCluster(cluID string, info *authn.CluACL) (int, error) {
if info.ID == "" {
return errors.New("cluster ID is undefined")
err := errors.New("cluster ID is undefined")
return http.StatusBadRequest, err
}
clu := &authn.CluACL{}
if err := m.db.Get(clustersCollection, cluID, clu); err != nil {
return err
return http.StatusInternalServerError, err
}
if info.Alias != "" {
// Validate cluster alias if user is changing it
if !cos.IsAlphaNice(info.Alias) {
return fmt.Errorf("cluster alias %q is invalid: %s", info.Alias, cos.OnlyNice)
err := fmt.Errorf("cluster alias %q is invalid: %s", info.Alias, cos.OnlyNice)
return http.StatusBadRequest, err
}
cid := m.cluLookup("", info.Alias)
if cid != "" && cid != clu.ID {
return fmt.Errorf("alias %q is used for cluster %q", info.Alias, cid)
err := fmt.Errorf("alias %q is used for cluster %q", info.Alias, cid)
return http.StatusConflict, err
}
clu.Alias = info.Alias
}
Expand All @@ -340,10 +368,13 @@ func (m *mgr) updateCluster(cluID string, info *authn.CluACL) error {

// secret handshake
if err := m.validateSecret(clu); err != nil {
return err
return http.StatusInternalServerError, err
}

return m.db.Set(clustersCollection, cluID, clu)
if err := m.db.Set(clustersCollection, cluID, clu); err != nil {
return http.StatusInternalServerError, err
}
return 0, nil
}

// Unregister an existing cluster
Expand Down
6 changes: 3 additions & 3 deletions cmd/authn/unit_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func init() {
func createUsers(mgr *mgr, t *testing.T) {
for idx := range users {
user := &authn.User{ID: users[idx], Password: passs[idx], Roles: []*authn.Role{guestRole}}
err := mgr.addUser(user)
_, err := mgr.addUser(user)
if err != nil {
t.Errorf("Failed to create user %s: %v", users[idx], err)
}
Expand Down Expand Up @@ -73,7 +73,7 @@ func deleteUsers(mgr *mgr, skipNotExist bool, t *testing.T) {

func testInvalidUser(mgr *mgr, t *testing.T) {
user := &authn.User{ID: users[0], Password: passs[1], Roles: []*authn.Role{guestRole}}
err := mgr.addUser(user)
_, err := mgr.addUser(user)
if err == nil {
t.Errorf("User with the existing name %s was created", users[0])
}
Expand All @@ -91,7 +91,7 @@ func testUserDelete(mgr *mgr, t *testing.T) {
userpass = "newpass"
)
user := &authn.User{ID: username, Password: userpass, Roles: []*authn.Role{guestRole}}
err := mgr.addUser(user)
_, err := mgr.addUser(user)
if err != nil {
t.Errorf("Failed to create user %s: %v", username, err)
}
Expand Down

0 comments on commit b7e9785

Please sign in to comment.