etcdserver: use correct http status code for auth error

release-2.1
Xiang Li 2015-06-19 15:00:26 -07:00
parent 9f984ea6ae
commit c39aad0e92
3 changed files with 32 additions and 33 deletions

View File

@ -17,6 +17,7 @@ package auth
import (
"encoding/json"
"fmt"
"net/http"
"path"
"reflect"
"sort"
@ -102,17 +103,15 @@ type rwPermission struct {
}
type Error struct {
errmsg string
httpStatus int
errmsg string
}
func (se Error) Error() string { return se.errmsg }
func (ae Error) Error() string { return ae.errmsg }
func (ae Error) HTTPStatus() int { return ae.httpStatus }
func mergeErr(s string, v ...interface{}) Error {
return Error{fmt.Sprintf("auth: "+s, v...)}
}
func authErr(s string, v ...interface{}) Error {
return Error{fmt.Sprintf("auth: "+s, v...)}
func authErr(hs int, s string, v ...interface{}) Error {
return Error{httpStatus: hs, errmsg: fmt.Sprintf("auth: "+s, v...)}
}
func NewStore(server doer, timeout time.Duration) *Store {
@ -147,7 +146,7 @@ func (s *Store) GetUser(name string) (User, error) {
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return User{}, authErr("User %s does not exist.", name)
return User{}, authErr(http.StatusNotFound, "User %s does not exist.", name)
}
}
return User{}, err
@ -197,7 +196,7 @@ func (s *Store) CreateUser(user User) (User, error) {
func (s *Store) createUserInternal(user User) (User, error) {
if user.Password == "" {
return user, authErr("Cannot create user %s with an empty password", user.User)
return user, authErr(http.StatusBadRequest, "Cannot create user %s with an empty password", user.User)
}
hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), bcrypt.DefaultCost)
if err != nil {
@ -209,7 +208,7 @@ func (s *Store) createUserInternal(user User) (User, error) {
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeNodeExist {
return user, authErr("User %s already exists.", user.User)
return user, authErr(http.StatusConflict, "User %s already exists.", user.User)
}
}
}
@ -218,13 +217,13 @@ func (s *Store) createUserInternal(user User) (User, error) {
func (s *Store) DeleteUser(name string) error {
if s.AuthEnabled() && name == "root" {
return authErr("Cannot delete root user while auth is enabled.")
return authErr(http.StatusForbidden, "Cannot delete root user while auth is enabled.")
}
_, err := s.deleteResource("/users/" + name)
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return authErr("User %s does not exist", name)
return authErr(http.StatusNotFound, "User %s does not exist", name)
}
}
return err
@ -238,7 +237,7 @@ func (s *Store) UpdateUser(user User) (User, error) {
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return user, authErr("User %s doesn't exist.", user.User)
return user, authErr(http.StatusNotFound, "User %s doesn't exist.", user.User)
}
}
return old, err
@ -249,9 +248,9 @@ func (s *Store) UpdateUser(user User) (User, error) {
}
if reflect.DeepEqual(old, newUser) {
if user.Revoke != nil || user.Grant != nil {
return old, authErr("User not updated. Grant/Revoke lists didn't match any current roles.")
return old, authErr(http.StatusConflict, "User not updated. Grant/Revoke lists didn't match any current roles.")
}
return old, authErr("User not updated. Use Grant/Revoke/Password to update the user.")
return old, authErr(http.StatusBadRequest, "User not updated. Use Grant/Revoke/Password to update the user.")
}
_, err = s.updateResource("/users/"+user.User, newUser)
if err == nil {
@ -287,7 +286,7 @@ func (s *Store) GetRole(name string) (Role, error) {
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return Role{}, authErr("Role %s does not exist.", name)
return Role{}, authErr(http.StatusNotFound, "Role %s does not exist.", name)
}
}
return Role{}, err
@ -313,13 +312,13 @@ func (s *Store) CreateOrUpdateRole(r Role) (role Role, created bool, err error)
func (s *Store) CreateRole(role Role) error {
if role.Role == RootRoleName {
return authErr("Cannot modify role %s: is root role.", role.Role)
return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", role.Role)
}
_, err := s.createResource("/roles/"+role.Role, role)
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeNodeExist {
return authErr("Role %s already exists.", role.Role)
return authErr(http.StatusConflict, "Role %s already exists.", role.Role)
}
}
}
@ -331,13 +330,13 @@ func (s *Store) CreateRole(role Role) error {
func (s *Store) DeleteRole(name string) error {
if name == RootRoleName {
return authErr("Cannot modify role %s: is superuser role.", name)
return authErr(http.StatusForbidden, "Cannot modify role %s: is root role.", name)
}
_, err := s.deleteResource("/roles/" + name)
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return authErr("Role %s doesn't exist.", name)
return authErr(http.StatusNotFound, "Role %s doesn't exist.", name)
}
}
}
@ -352,7 +351,7 @@ func (s *Store) UpdateRole(role Role) (Role, error) {
if err != nil {
if e, ok := err.(*etcderr.Error); ok {
if e.ErrorCode == etcderr.EcodeKeyNotFound {
return role, authErr("Role %s doesn't exist.", role.Role)
return role, authErr(http.StatusNotFound, "Role %s doesn't exist.", role.Role)
}
}
return old, err
@ -363,9 +362,9 @@ func (s *Store) UpdateRole(role Role) (Role, error) {
}
if reflect.DeepEqual(old, newRole) {
if role.Revoke != nil || role.Grant != nil {
return old, authErr("Role not updated. Grant/Revoke lists didn't match any current permissions.")
return old, authErr(http.StatusConflict, "Role not updated. Grant/Revoke lists didn't match any current permissions.")
}
return old, authErr("Role not updated. Use Grant/Revoke to update the role.")
return old, authErr(http.StatusBadRequest, "Role not updated. Use Grant/Revoke to update the role.")
}
_, err = s.updateResource("/roles/"+role.Role, newRole)
if err == nil {
@ -380,11 +379,11 @@ func (s *Store) AuthEnabled() bool {
func (s *Store) EnableAuth() error {
if s.AuthEnabled() {
return authErr("already enabled")
return authErr(http.StatusConflict, "already enabled")
}
_, err := s.GetUser("root")
if err != nil {
return authErr("No root user available, please create one")
return authErr(http.StatusConflict, "No root user available, please create one")
}
_, err = s.GetRole(GuestRoleName)
if err != nil {
@ -406,7 +405,7 @@ func (s *Store) EnableAuth() error {
func (s *Store) DisableAuth() error {
if !s.AuthEnabled() {
return authErr("already disabled")
return authErr(http.StatusConflict, "already disabled")
}
err := s.disableAuth()
if err == nil {
@ -424,7 +423,7 @@ func (s *Store) DisableAuth() error {
func (u User) Merge(n User) (User, error) {
var out User
if u.User != n.User {
return out, mergeErr("Merging user data with conflicting usernames: %s %s", u.User, n.User)
return out, authErr(http.StatusConflict, "Merging user data with conflicting usernames: %s %s", u.User, n.User)
}
out.User = u.User
if n.Password != "" {
@ -466,7 +465,7 @@ func (r Role) Merge(n Role) (Role, error) {
var out Role
var err error
if r.Role != n.Role {
return out, mergeErr("Merging role with conflicting names: %s %s", r.Role, n.Role)
return out, authErr(http.StatusConflict, "Merging role with conflicting names: %s %s", r.Role, n.Role)
}
out.Role = r.Role
out.Permissions, err = r.Permissions.Grant(n.Grant)
@ -525,14 +524,14 @@ func (rw rwPermission) Grant(n rwPermission) (rwPermission, error) {
currentRead := types.NewUnsafeSet(rw.Read...)
for _, r := range n.Read {
if currentRead.Contains(r) {
return out, mergeErr("Granting duplicate read permission %s", r)
return out, authErr(http.StatusConflict, "Granting duplicate read permission %s", r)
}
currentRead.Add(r)
}
currentWrite := types.NewUnsafeSet(rw.Write...)
for _, w := range n.Write {
if currentWrite.Contains(w) {
return out, mergeErr("Granting duplicate write permission %s", w)
return out, authErr(http.StatusConflict, "Granting duplicate write permission %s", w)
}
currentWrite.Add(w)
}

View File

@ -313,7 +313,7 @@ func TestEnsure(t *testing.T) {
}
s := Store{d, time.Second, false}
err := s.ensureSecurityDirectories()
err := s.ensureAuthDirectories()
if err != nil {
t.Error("Unexpected error", err)
}

View File

@ -56,7 +56,7 @@ func writeError(w http.ResponseWriter, err error) {
case *httptypes.HTTPError:
e.WriteTo(w)
case auth.Error:
herr := httptypes.NewHTTPError(http.StatusBadRequest, e.Error())
herr := httptypes.NewHTTPError(e.HTTPStatus(), e.Error())
herr.WriteTo(w)
default:
plog.Errorf("got unexpected response error (%v)", err)