From 5b835b3de65103130730af54f6ce6734c48729a5 Mon Sep 17 00:00:00 2001 From: Yuva Shankar Date: Mon, 12 Dec 2016 15:53:58 -0800 Subject: [PATCH 1/3] Remove `Principal` from the authN workflow and local user management Signed-off-by: Yuva Shankar --- auth/auth.go | 4 +- auth/ldap/ldap.go | 14 ++-- auth/local/local.go | 14 ++-- auth/token.go | 36 +++------ common/types/types.go | 47 ++---------- db/constants.go | 1 - db/local.go | 162 +++++++++++++++------------------------ db/local_test.go | 49 ++++-------- proxy/handlers.go | 27 +++---- proxy/helpers.go | 119 +++++++++------------------- proxy/struct.go | 15 ---- systemtests/init_test.go | 2 +- 12 files changed, 158 insertions(+), 332 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 1ce0c2ab..6c186ea7 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -36,11 +36,11 @@ func Authenticate(username, password string) (string, error) { // generateToken generates JWT(JSON Web Token) with the given user principals // params: -// principals: user principals +// principals: user principals; []string containing LDAP groups or username based on the authentication type(LDAP/Local) // username: local or AD username of the user // return values: // `Token` string on successful creation of JWT otherwise any relevant error from the subsequent function -func generateToken(principals []*types.Principal, username string) (string, error) { +func generateToken(principals []string, username string) (string, error) { log.Debugf("Generating token for user %q", username) authZ, err := NewTokenWithClaims(principals) // create a new token with default `expiry` claim diff --git a/auth/ldap/ldap.go b/auth/ldap/ldap.go index 64fa62d0..9c619662 100644 --- a/auth/ldap/ldap.go +++ b/auth/ldap/ldap.go @@ -51,8 +51,9 @@ type Manager struct { // username: username to authenticate // password: password of the user // return values: +// []string: list of principals (LDAP group names that the user belongs) // ErrLDAPConfigurationNotFound if the config is not found or as returned by ldapManager.Authenticate -func Authenticate(username, password string) ([]*types.Principal, error) { +func Authenticate(username, password string) ([]string, error) { cfg, err := db.GetLdapConfiguration() if err != nil { return nil, err @@ -77,9 +78,9 @@ func Authenticate(username, password string) ([]*types.Principal, error) { // username: username to authenticate // password: password of the user // return values: -// []*types.Principal on successful authentication else nil +// []string containing LDAP group names of the user on successful authentication else nil // error: nil on successful authentication otherwise ErrLDAPAccessDenied, ErrUserNotFound, etc. -func (lm *Manager) Authenticate(username, password string) ([]*types.Principal, error) { +func (lm *Manager) Authenticate(username, password string) ([]string, error) { // list of attributes to be fetched from the matching records var attributes = []string{ "memberof", @@ -132,13 +133,10 @@ func (lm *Manager) Authenticate(username, password string) ([]*types.Principal, return nil, err } - // XXX: this code is not complete yet - // TODO: these groups should be associated with the principals - log.Debugf("Authorized groups:%#v", groups) - log.Info("AD authentication successful") - return nil, nil + + return groups, nil } // getUserGroups performs a nested search on the given first-level user groups to uncover all the groups that the user is part of. diff --git a/auth/local/local.go b/auth/local/local.go index 0ef7015a..1b96839c 100644 --- a/auth/local/local.go +++ b/auth/local/local.go @@ -4,7 +4,6 @@ import ( log "github.com/Sirupsen/logrus" "github.com/contiv/ccn_proxy/common" ccnerrors "github.com/contiv/ccn_proxy/common/errors" - "github.com/contiv/ccn_proxy/common/types" "github.com/contiv/ccn_proxy/db" ) @@ -13,9 +12,9 @@ import ( // username: username to authenticate // password: password of the user // return values: -// []*types.Principal on successful authentication else nil +// []string containing the `PrincipalName`(username) on successful authentication else nil // error: nil on successful authentication otherwise ErrLocalAuthenticationFailed -func Authenticate(username, password string) ([]*types.Principal, error) { +func Authenticate(username, password string) ([]string, error) { user, err := db.GetLocalUser(username) if err != nil { if err == ccnerrors.ErrKeyNotFound { @@ -25,7 +24,7 @@ func Authenticate(username, password string) ([]*types.Principal, error) { return nil, err } - if user.LocalUser.Disable { + if user.Disable { log.Debugf("Local user %q is disabled", username) return nil, ccnerrors.ErrAccessDenied } @@ -35,8 +34,7 @@ func Authenticate(username, password string) ([]*types.Principal, error) { return nil, ccnerrors.ErrAccessDenied } - // TODO:this needs to be integrated with authZ module to fetch more principals based on the user role. - userPrincipals := []*types.Principal{} - userPrincipals = append(userPrincipals, &user.Principal) - return userPrincipals, nil + // local user's JWT will only contain the `username` + default claims (exp, role, etc.. ) + // user.Username is the PrincipalName for localuser + return []string{user.Username}, nil } diff --git a/auth/token.go b/auth/token.go index b239c3bd..68a1dac1 100644 --- a/auth/token.go +++ b/auth/token.go @@ -52,14 +52,16 @@ func NewToken() *Token { // return values: // *Token: a token object encapsulating authorization claims // error: nil if successful, else as returned by AddRoleClaim -func NewTokenWithClaims(principals []*types.Principal) (*Token, error) { +func NewTokenWithClaims(principals []string) (*Token, error) { authZ := NewToken() // add a claim for each principal for _, principal := range principals { - if err := authZ.AddRoleClaim(principal); err != nil { - return nil, err - } + // NOTE: principal here is the group_name or username based on the authentication type(LDAP/Local) + authZ.AddClaim(principal, true) + + authZ.AddRoleClaim(principal) + // TODO: Add role by iterating through the list of authorization for this principal } return authZ, nil @@ -70,30 +72,16 @@ func NewTokenWithClaims(principals []*types.Principal) (*Token, error) { // the user, hence an update is only performed if principal's role claim is // higher than current value of role claim. // params: -// principal: security principal associated with a user +// principal: principal(username/group_name) associated with a user // return values: // error: nil if successful, else relevant error if claim is malformed. -func (authZ *Token) AddRoleClaim(principal *types.Principal) error { - roleKey, err := GenerateClaimKey(principal.Role) - if err != nil { - return err - } +func (authZ *Token) AddRoleClaim(principal string) error { + // TODO: Iterate over the list of authz's of the given principal - val, found := authZ.tkn.Claims.(jwt.MapClaims)[roleKey] // this type casting is required due to jwt library changes - if found { - existingRole, err := types.Role(val.(string)) - if err != nil { - return err - } - - // principal's role is less privileged than what is stored, then return; otherwise update the token with new role - if principal.Role > existingRole { // highest role - Admin(0) - return nil - } + // FIXME: this is just to let the current systemtests PASS + if principal == types.Admin.String() || principal == types.Ops.String() { + authZ.AddClaim("role", principal) } - - // if the role is not part of claims, update the claim set with `Role` - authZ.AddClaim(roleKey, principal.Role.String()) return nil } diff --git a/common/types/types.go b/common/types/types.go index 26171e77..47813e4d 100644 --- a/common/types/types.go +++ b/common/types/types.go @@ -24,28 +24,6 @@ const ( // Tenant is a type to represent the name of the tenant in CCN type Tenant string -// -// Principal represents a 'user' to 'role' association. A 'user' can have many -// 'roles', and thus can have multiple principals representing it during a -// 'session'. This set is also known as the active role set (ARS). -// -// A CCN local user is a simplified version of this association, where the -// mapping is 1:1 - i.e., a CCN local user can have only one pre-defined role. -// -// A CCN ldap group (representing a LDAP group in some active directory forest) -// also has a 1:1 mapping with a principal. However, since a 'user' can be part -// of multiple ldap groups, the ARS will be determined at the time -// authentication is carried out, and may comprise of multiple principals. -// -// Fields: -// UUID: unique identifier of the principal -// Role: Role associated with a principal -// -type Principal struct { - UUID string `json:"uuid"` - Role RoleType `json:"roletype"` -} - // String returns the string representation of `RoleType` func (role RoleType) String() string { switch role { @@ -79,28 +57,13 @@ func Role(roleStr string) (RoleType, error) { // UserName: of the user. Read only field. Must be unique. // Password: of the user. Not stored anywhere. Used only for updates. // Disable: if authorizations for this local user is disabled. -// -type LocalUser struct { - Username string `json:"username"` - Password string `json:"password"` - Disable bool `json:"disable"` -} - -// InternalLocalUser information -// -// Fields: -// UserName: inherited from LocalUser -// Password: inherited from LocalUser. Not stored anywhere. Used to update password hash. -// Disble: inherited from LocalUser. -// Principal: associated principal object. -// PrincipalID: For each local user, there should be a principal created in the system. // PasswordHash: of the password string. // -type InternalLocalUser struct { - LocalUser - Principal Principal - PrincipalID string `json:"principal_id"` - PasswordHash []byte `json:"password_hash"` +type LocalUser struct { + Username string `json:"username"` + Password string `json:"password,omitempty"` + Disable bool `json:"disable"` + PasswordHash []byte `json:"password_hash,omitempty"` } // LdapConfiguration represents the LDAP/AD configuration. diff --git a/db/constants.go b/db/constants.go index 46b91747..a7575744 100644 --- a/db/constants.go +++ b/db/constants.go @@ -11,7 +11,6 @@ import ( // various data store paths. var ( RootLocalUsers = "local_users" - RootPrincipals = "principals" RootLdapConfiguration = "ldap_configuration" ) diff --git a/db/local.go b/db/local.go index 2cb85cbe..d3f37f67 100644 --- a/db/local.go +++ b/db/local.go @@ -9,61 +9,18 @@ import ( ccnerrors "github.com/contiv/ccn_proxy/common/errors" "github.com/contiv/ccn_proxy/common/types" "github.com/contiv/ccn_proxy/state" - uuid "github.com/satori/go.uuid" ) // This file contains all local user management APIs. // NOTE: Built-in users(admin, ops) cannot be changed/updated. it needs to be consumed in the way its defined in code. -// deleteUserPrincipal helper function to delete user.principal; called from `DeleteLocalUser`. +// getLocalUser helper function that retrieves a user entry from `/ccn_proxy/local_users` using username // params: -// principal: reference of the principal object to be deleted from the data store -// stateDrv: data store driver object -// return values: -// error: custom error with error from `ClearState(...)` -func deleteUserPrincipal(principal *types.Principal, stateDrv types.StateDriver) error { - if err := stateDrv.ClearState(GetPath(RootPrincipals, principal.UUID)); err != nil { - return fmt.Errorf("Failed to clear principal %#v from store %#v", principal, err) - } - - return nil -} - -// addUserPrincipal helper function to insert user.principal; called from `AddLocalUser`. -// params: -// principal: reference of the principal object to be inserted into the data store -// stateDrv: data store driver object -// retutn values: -// error: any relevant custom errors or as returned by consecutive calls -func addUserPrincipal(principal *types.Principal, stateDrv types.StateDriver) error { - _, err := stateDrv.Read(GetPath(RootPrincipals, principal.UUID)) - - switch err { - case nil: - return fmt.Errorf("%s: %q", ccnerrors.ErrKeyExists, principal.UUID) - case ccnerrors.ErrKeyNotFound: - val, err := json.Marshal(principal) - if err != nil { - return fmt.Errorf("Failed to marshal principal %#v, %#v", principal, err) - } - - if err := stateDrv.Write(GetPath(RootPrincipals, principal.UUID), val); err != nil { - return fmt.Errorf("Failed to write local user principal to data store %#v", err) - } - - return nil - default: - return err - } -} - -// getLocalUser helper function that looks up a user entry in `users` using username -// params: -// username:string; name of the user to be fetched +// username:string; name of the user to be fetched from store // return values: // *types.InternalLocalUser: reference to the internal representation of the local user object // error: as returned by the consecutive calls or any relevant custom errors -func getLocalUser(username string, stateDrv types.StateDriver) (*types.InternalLocalUser, error) { +func getLocalUser(username string, stateDrv types.StateDriver) (*types.LocalUser, error) { rawData, err := stateDrv.Read(GetPath(RootLocalUsers, username)) if err != nil { if err == ccnerrors.ErrKeyNotFound { @@ -73,7 +30,7 @@ func getLocalUser(username string, stateDrv types.StateDriver) (*types.InternalL return nil, fmt.Errorf("Failed to read local user %q data from store: %#v", username, err) } - var localUser types.InternalLocalUser + var localUser types.LocalUser if err := json.Unmarshal(rawData, &localUser); err != nil { return nil, fmt.Errorf("Failed to unmarshal local user %q info %#v", username, err) } @@ -85,36 +42,41 @@ func getLocalUser(username string, stateDrv types.StateDriver) (*types.InternalL // return values: // []types.InternalLocalUser: slice of local users // error: as returned by consecutive func calls -func GetLocalUsers() ([]*types.InternalLocalUser, error) { +func GetLocalUsers() ([]*types.LocalUser, error) { stateDrv, err := state.GetStateDriver() if err != nil { return nil, err } + users := []*types.LocalUser{} rawData, err := stateDrv.ReadAll(GetPath(RootLocalUsers)) if err != nil { + if err == ccnerrors.ErrKeyNotFound { + return users, nil + } + return nil, fmt.Errorf("Couldn't fetch users from data store") } - users := []*types.InternalLocalUser{} for _, data := range rawData { - localUser := &types.InternalLocalUser{} + localUser := &types.LocalUser{} if err := json.Unmarshal(data, localUser); err != nil { return nil, err } + users = append(users, localUser) } return users, nil } -// GetLocalUser looks up a user entry in `users` path. +// GetLocalUser looks up a user entry in `/ccn_proxy/local_users` path. // params: // username:string; name of the user to be fetched // return values: -// *types.InternalLocalUser: reference to the internal representation of the local user object +// *types.LocalUser: reference to local user object fetched from data store // error: as returned by getLocalUser(..) -func GetLocalUser(username string) (*types.InternalLocalUser, error) { +func GetLocalUser(username string) (*types.LocalUser, error) { stateDrv, err := state.GetStateDriver() if err != nil { return nil, err @@ -123,16 +85,21 @@ func GetLocalUser(username string) (*types.InternalLocalUser, error) { return getLocalUser(username, stateDrv) } -// UpdateLocalUser updates an existing entry in /ccn_proxy/users/local/. +// UpdateLocalUser updates an existing entry in /ccn_proxy/local_users/. // params: // username: string; of the user that requires update -// user: internal representation of the user (contains both user details + principal) object to be updated in the data store +// user: local user object to be updated in the data store // return values: // error: as returned by AddLocalUser, DeleteLocalUser -func UpdateLocalUser(username string, user *types.InternalLocalUser) error { +func UpdateLocalUser(username string, user *types.LocalUser) error { err := DeleteLocalUser(username) switch err { case nil: + // if the password is getting updated; clear the existing hash + if !common.IsEmpty(user.Password) { + user.PasswordHash = []byte{} + } + return AddLocalUser(user) case ccnerrors.ErrKeyNotFound, ccnerrors.ErrIllegalOperation: return err @@ -143,7 +110,7 @@ func UpdateLocalUser(username string, user *types.InternalLocalUser) error { } } -// DeleteLocalUser removes a local user and its corresponding principal. +// DeleteLocalUser removes a local user from `/ccn_proxy/local_users` // Built-in admin and ops local users cannot be deleted. // params: // username: string; user to be removed from the system @@ -160,58 +127,63 @@ func DeleteLocalUser(username string) error { return err } - user, err := getLocalUser(username, stateDrv) + // handles `ErrKeyNotFound` + _, err = getLocalUser(username, stateDrv) if err != nil { return err } - if err := deleteUserPrincipal(&user.Principal, stateDrv); err != nil { - return err - } - - if err := stateDrv.ClearState(GetPath(RootLocalUsers, username)); err != nil { - //cleanup; there is always a principal associated with user (1-1 mapping) - addUserPrincipal(&user.Principal, stateDrv) - return fmt.Errorf("Failed to clear %q from store %#v", username, err) + if err := stateDrv.Clear(GetPath(RootLocalUsers, username)); err != nil { + return fmt.Errorf("Failed to clear %q from store: %#v", username, err) } return nil } -// AddLocalUser adds a new user entry to /ccn_proxy/users/local/. It also adds a -// corresponding principal in /ccn_proxy/users/principal/. +// AddLocalUser adds a new user entry to /ccn_proxy/local_users/. // params: -// user: internal representation of the user (contains both user details + principal) object to be added to data store +// user: *types.LocalUser object that should be added to the data store // return Values: // error: ccnerrors.ErrKeyExists if the user already exists or any relevant error from state driver -func AddLocalUser(user *types.InternalLocalUser) error { +func AddLocalUser(user *types.LocalUser) error { stateDrv, err := state.GetStateDriver() if err != nil { return err } - key := GetPath(RootLocalUsers, user.LocalUser.Username) + key := GetPath(RootLocalUsers, user.Username) _, err = stateDrv.Read(key) + switch err { case nil: return ccnerrors.ErrKeyExists case ccnerrors.ErrKeyNotFound: - if err := addUserPrincipal(&user.Principal, stateDrv); err != nil { - return err + // `AddLocalUser` request from `UpdateLocalUser` carries hash in `user.PasswordHash` + if common.IsEmpty(string(user.PasswordHash)) { + user.PasswordHash, err = common.GenPasswordHash(user.Password) + + if err != nil { + log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) + return err + } } + // raw password will never be stored in the store + user.Password = "" + val, err := json.Marshal(user) if err != nil { - return fmt.Errorf("Failed to marshal user %#v, %#v", user, err) + return fmt.Errorf("Failed to marshal user %#v: %#v", user, err) } if err := stateDrv.Write(key, val); err != nil { - // cleanup; to ensure user.principal is not left behind in the data store - deleteUserPrincipal(&user.Principal, stateDrv) - return fmt.Errorf("Failed to write local user info. to data store %#v", err) + return fmt.Errorf("Failed to write local user info. to data store: %#v", err) } + // not to let the user know about password hash + user.PasswordHash = []byte{} + return nil default: return err @@ -222,35 +194,23 @@ func AddLocalUser(user *types.InternalLocalUser) error { // Default users cannot be changed(update/delete) anytime. func AddDefaultUsers() error { for _, userR := range []types.RoleType{types.Admin, types.Ops} { - // principalID is the unique ID for each user - principalID := uuid.NewV4().String() - - localUser := types.InternalLocalUser{ - LocalUser: types.LocalUser{ - Username: userR.String(), - // default user accounts are `enabled` always; it cannot be disabled - Disable: false, - }, - Principal: types.Principal{ - UUID: principalID, - Role: userR, - }, - PrincipalID: principalID, - } - - var err error + log.Printf("Adding local user %q to the system", userR.String()) - // use the role name for the default password (e.g., "admin" user password is "admin") - localUser.PasswordHash, err = common.GenPasswordHash(userR.String()) - if err != nil { - return err + localUser := types.LocalUser{ + Username: userR.String(), + // default user accounts are `enabled` always; it cannot be disabled + Disable: false, + Password: userR.String(), } - if err := AddLocalUser(&localUser); err != nil { - return err + err := AddLocalUser(&localUser) + if err == nil || err == ccnerrors.ErrKeyExists { + continue } - log.Printf("Added local user '%s'", localUser.Username) + // TODO: add default authorization as well + + return err } return nil diff --git a/db/local_test.go b/db/local_test.go index 5cef6e5e..f4d939d8 100644 --- a/db/local_test.go +++ b/db/local_test.go @@ -11,7 +11,6 @@ import ( "github.com/contiv/ccn_proxy/common/test" "github.com/contiv/ccn_proxy/common/types" "github.com/contiv/ccn_proxy/state" - uuid "github.com/satori/go.uuid" . "gopkg.in/check.v1" ) @@ -67,20 +66,10 @@ func TestMgmtSuite(t *testing.T) { // addBuiltInUsers adds built-in users to the data store. func (s *dbSuite) addBuiltInUsers(c *C) { for _, username := range builtInUsers { - roleType, rErr := types.Role(username) - c.Assert(rErr, IsNil) - - principalID := uuid.NewV4().String() - user := &types.InternalLocalUser{ - LocalUser: types.LocalUser{ - Username: username, - Disable: false, - }, - Principal: types.Principal{ - UUID: principalID, - Role: roleType, - }, - PrincipalID: principalID, + user := &types.LocalUser{ + Username: username, + Disable: false, + Password: username, } err := AddLocalUser(user) @@ -96,12 +85,9 @@ func (s *dbSuite) TestGetLocalUser(c *C) { user, err := GetLocalUser(username) c.Assert(err, IsNil) - c.Assert(user.LocalUser.Username, Equals, username) - - roleType, rErr := types.Role(username) - c.Assert(rErr, IsNil) - - c.Assert(user.Principal.Role, Equals, roleType) + c.Assert(user.Username, Equals, username) + c.Assert(user.Password, Equals, "") + c.Assert(user.PasswordHash, DeepEquals, []byte{}) } // invalid users @@ -117,17 +103,10 @@ func (s *dbSuite) TestGetLocalUser(c *C) { func (s *dbSuite) TestAddLocalUser(c *C) { // add new users for _, username := range newUsers { - principalID := uuid.NewV4().String() - user := &types.InternalLocalUser{ - LocalUser: types.LocalUser{ - Username: username, - Disable: false, - }, - Principal: types.Principal{ - UUID: principalID, - Role: types.Ops, - }, - PrincipalID: principalID, + user := &types.LocalUser{ + Username: username, + Disable: false, + Password: username, } err := AddLocalUser(user) @@ -171,7 +150,7 @@ func (s *dbSuite) TestUpdateLocalUser(c *C) { c.Assert(err, IsNil) // change the username and update - user.LocalUser.Username = username + "_u" + user.Username = username + "_u" err = UpdateLocalUser(username, user) c.Assert(err, IsNil) @@ -195,7 +174,7 @@ func (s *dbSuite) TestUpdateLocalUser(c *C) { c.Assert(err, IsNil) // change the username and update - user.LocalUser.Username = updateTo + user.Username = updateTo err = UpdateLocalUser(username, user) c.Assert(err, IsNil) @@ -220,7 +199,7 @@ func (s *dbSuite) TestGetLocalUsers(c *C) { usernames := []string{} for _, user := range users { - usernames = append(usernames, user.LocalUser.Username) + usernames = append(usernames, user.Username) } allUsers := append(newUsers, builtInUsers...) diff --git a/proxy/handlers.go b/proxy/handlers.go index 938cc38f..078c688c 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -176,6 +176,7 @@ func adminOnly(handler func(*auth.Token, http.ResponseWriter, *http.Request)) fu // convert token from string to Token type token, err := auth.ParseToken(tokenStr) if err != nil { + log.Infof("From here???") httpStatus := http.StatusInternalServerError httpResponse := []byte(ccnerrors.ErrParsingToken.Error()) processStatusCodes(httpStatus, httpResponse, w) @@ -205,8 +206,8 @@ func adminOnly(handler func(*auth.Token, http.ResponseWriter, *http.Request)) fu // addLocalUser adds a new local user to the system. // it can return various HTTP status codes: -// 201 (user added to the system) -// 400 (user exists in the system already/invalid role) +// 201 (Created; user added to the system) +// 400 (BadRequest; user exists in the system already/invalid role) // 500 (internal server error) func addLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { body, err := ioutil.ReadAll(req.Body) @@ -215,7 +216,7 @@ func addLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { return } - userCreateReq := &localUserCreateRequest{} + userCreateReq := &types.LocalUser{} if err := json.Unmarshal(body, userCreateReq); err != nil { serverError(w, errors.New("Failed to unmarshal user info. from request body: "+err.Error())) return @@ -227,9 +228,9 @@ func addLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { // deleteLocalUser deletes the given user from the system. // it can return various HTTP status codes: -// 200 (user deleted from the system) -// 404 (username not found) -// 400 (cannot delete built-in users) +// 200 (OK; user deleted from the system) +// 404 (NotFound; username not found) +// 400 (BadRequest; cannot delete built-in users) // 500 (internal server error) func deleteLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) @@ -240,9 +241,9 @@ func deleteLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request // updateLocalUser updates the existing user with the given details. // it can return various HTTP status codes: -// 204 (update was successful) -// 400 (invalid role/cannot update built-in user) -// 404 (user not found) +// 204 (NoContent; update was successful) +// 400 (BadRequest; invalid role/cannot update built-in user) +// 404 (NotFound; user not found) // 500 (internal server error) func updateLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) @@ -253,7 +254,7 @@ func updateLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request return } - userUpdateReq := &localUserCreateRequest{} + userUpdateReq := &types.LocalUser{} if err := json.Unmarshal(body, userUpdateReq); err != nil { serverError(w, errors.New("Failed to unmarshal user info. from request body: "+err.Error())) return @@ -265,7 +266,7 @@ func updateLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request // getLocalUsers returns all the local users available in the system // it can return various HTTP status codes: -// 200 (fetch was successful) +// 200 (OK; fetch was successful) // 500 (internal server error) func getLocalUsers(token *auth.Token, w http.ResponseWriter, req *http.Request) { statusCode, resp := getLocalUsersHelper() @@ -274,8 +275,8 @@ func getLocalUsers(token *auth.Token, w http.ResponseWriter, req *http.Request) // getLocalUser returns the details for the given username // it can return various HTTP status codes: -// 200 (fetch was successful) -// 404 (bad request; user not found) +// 200 (OK; fetch was successful) +// 404 (NotFound; bad request; user not found) // 500 (internal server error) func getLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) diff --git a/proxy/helpers.go b/proxy/helpers.go index d7f61539..589fb563 100644 --- a/proxy/helpers.go +++ b/proxy/helpers.go @@ -7,7 +7,6 @@ import ( "strings" log "github.com/Sirupsen/logrus" - uuid "github.com/satori/go.uuid" "github.com/contiv/ccn_proxy/auth" "github.com/contiv/ccn_proxy/common" @@ -208,19 +207,16 @@ func addLdapConfigurationHelper(ldapConfiguration *types.LdapConfiguration) (int // return values: // int: http status code // []byte: http response message; this goes along with status code -// on successful fetch from data store, it contains `localUser` object +// on successful fetch from data store, it contains `types.LocalUser` object func getLocalUserHelper(username string) (int, []byte) { user, err := db.GetLocalUser(username) switch err { case nil: - lu := localUser{ - Username: user.LocalUser.Username, - Disable: user.LocalUser.Disable, - Role: user.Principal.Role.String(), - } + user.Password = "" + user.PasswordHash = []byte{} - jData, err := json.Marshal(lu) + jData, err := json.Marshal(user) if err != nil { return http.StatusInternalServerError, []byte(err.Error()) } @@ -245,19 +241,20 @@ func getLocalUsersHelper() (int, []byte) { return http.StatusInternalServerError, []byte(err.Error()) } - localUsers := []localUser{} + localUsers := []types.LocalUser{} for _, user := range users { - lu := localUser{ - Username: user.LocalUser.Username, - Disable: user.LocalUser.Disable, - Role: user.Principal.Role.String(), + lu := types.LocalUser{ + Username: user.Username, + Disable: user.Disable, } + localUsers = append(localUsers, lu) } jData, err := json.Marshal(localUsers) if err != nil { - return http.StatusInternalServerError, []byte(err.Error()) + log.Debug("Failed to marshal %#v: %#v", jData, err) + return http.StatusInternalServerError, []byte(fmt.Sprintf("Failed to fetch local users")) } return http.StatusOK, jData @@ -266,56 +263,37 @@ func getLocalUsersHelper() (int, []byte) { // updateLocalUserInfo helper function for updateLocalUserHelper. // params: // username: of the user to be updated -// updateReq: *localUserCreateRequest contains the fields to be updated -// actual: *types.InternalLocalUser representation of `username` fetched from the data store +// updateReq: to be updated in the data store +// actual: existing user details fetched from the data store for user `username` // return values: // int: http status code // []byte: http response message; this goes along with status code -func updateLocalUserInfo(username string, updateReq *localUserCreateRequest, actual *types.InternalLocalUser) (int, []byte) { - updatedUserObj := &types.InternalLocalUser{ - PrincipalID: actual.PrincipalID, - Principal: types.Principal{ - UUID: actual.PrincipalID, - }, - LocalUser: types.LocalUser{ - // `actual.LocalUser.Username` will be the same as `username` - Username: actual.LocalUser.Username, - }, - } - - // Update `role` - if !common.IsEmpty(updateReq.Role) { - role, err := types.Role(updateReq.Role) - if err != nil { - return http.StatusBadRequest, []byte(fmt.Sprintf("Invalid role %q", updateReq.Role)) - } - updatedUserObj.Principal.Role = role +func updateLocalUserInfo(username string, updateReq *types.LocalUser, actual *types.LocalUser) (int, []byte) { + updatedUserObj := &types.LocalUser{ + // username == actual.Username + Username: actual.Username, + Disable: actual.Disable, + PasswordHash: actual.PasswordHash, + // `Password` will be empty } // Update `disable` if actual.Disable != updateReq.Disable { - updatedUserObj.LocalUser.Disable = updateReq.Disable + updatedUserObj.Disable = updateReq.Disable } // Update `password` if !common.IsEmpty(updateReq.Password) { - var err error - updatedUserObj.PasswordHash, err = common.GenPasswordHash(updateReq.Password) - if err != nil { - return http.StatusInternalServerError, []byte(fmt.Sprintf("Failed to create password hash for user %q", username)) - } + updatedUserObj.Password = updateReq.Password } err := db.UpdateLocalUser(username, updatedUserObj) switch err { case nil: - lu := localUser{ - Username: updatedUserObj.LocalUser.Username, - Disable: updatedUserObj.LocalUser.Disable, - Role: updatedUserObj.Principal.Role.String(), - } + updatedUserObj.Password = "" + updatedUserObj.PasswordHash = []byte{} - jData, err := json.Marshal(lu) + jData, err := json.Marshal(updatedUserObj) if err != nil { return http.StatusInternalServerError, []byte(err.Error()) } @@ -338,7 +316,7 @@ func updateLocalUserInfo(username string, updateReq *localUserCreateRequest, act // return values: // int: http status code // []byte: http response message; this goes along with status code -func updateLocalUserHelper(username string, userUpdateReq *localUserCreateRequest) (int, []byte) { +func updateLocalUserHelper(username string, userUpdateReq *types.LocalUser) (int, []byte) { if common.IsEmpty(username) { return http.StatusBadRequest, []byte("Empty username") } @@ -381,58 +359,35 @@ func deleteLocalUserHelper(username string) (int, []byte) { // addLocalUserHelper helper function to add given user to the data store. // params: -// userCreateReq: localUserCreateRequest object; based on which types.InternalLocalUser is created +// userCreateReq: *types.LocalUser request object; to be added to the store // return values: // int: http status code // []byte: http response message; this goes along with status code -func addLocalUserHelper(userCreateReq *localUserCreateRequest) (int, []byte) { +func addLocalUserHelper(userCreateReq *types.LocalUser) (int, []byte) { if common.IsEmpty(userCreateReq.Username) || common.IsEmpty(userCreateReq.Password) { return http.StatusBadRequest, []byte("Username/Password is empty") } - role, err := types.Role(userCreateReq.Role) - if err != nil { - return http.StatusBadRequest, []byte(fmt.Sprintf("Invalid role %q", userCreateReq.Role)) - } - - pID := uuid.NewV4().String() - user := types.InternalLocalUser{ - LocalUser: types.LocalUser{ - Username: userCreateReq.Username, - Disable: userCreateReq.Disable, - }, - Principal: types.Principal{ - UUID: pID, - Role: role, - }, - PrincipalID: pID, - } - - user.PasswordHash, err = common.GenPasswordHash(userCreateReq.Password) - if err != nil { - return http.StatusInternalServerError, []byte(fmt.Sprintf("Failed to create password hash for user %q", userCreateReq.Username)) - } - - err = db.AddLocalUser(&user) + err := db.AddLocalUser(userCreateReq) switch err { case nil: - lu := localUser{ - Username: user.LocalUser.Username, - Disable: user.LocalUser.Disable, - Role: user.Principal.Role.String(), - } + userCreateReq.Password = "" + userCreateReq.PasswordHash = []byte{} - jData, err := json.Marshal(lu) + jData, err := json.Marshal(userCreateReq) if err != nil { - return http.StatusInternalServerError, []byte(err.Error()) + log.Debugf("Failed to marshal %#v: %#v", userCreateReq, err) + return http.StatusInternalServerError, []byte(fmt.Sprintf("Failed to add local user %q to the system", userCreateReq.Username)) } return http.StatusCreated, jData case ccnerrors.ErrKeyExists: return http.StatusBadRequest, []byte(fmt.Sprintf("User %q exists already", userCreateReq.Username)) default: - return http.StatusInternalServerError, []byte(err.Error()) + log.Debugf("Failed to add local user %#v: %#v", userCreateReq, err) + return http.StatusInternalServerError, []byte(fmt.Sprintf("Failed to add local user %q to the system", userCreateReq.Username)) } + } // isTokenValid checks if the given token string is valid(correctness, expiry, etc.) and writes diff --git a/proxy/struct.go b/proxy/struct.go index 5ba61f5b..b93fa476 100644 --- a/proxy/struct.go +++ b/proxy/struct.go @@ -13,14 +13,6 @@ type LoginResponse struct { Token string `json:"token"` } -// localUserCreateRequest represents localuser create object. -type localUserCreateRequest struct { - Username string `json:"username"` - Password string `json:"password"` - Disable bool `json:"disable"` - Role string `json:"role"` -} - // // AddTenantAuthorizationRequest message is sent for AddTenantAuthorization // operation. @@ -77,10 +69,3 @@ type ListAuthorizationsReply struct { type errorResponse struct { Error string `json:"error"` } - -// localUser is the return type of `getLocalUsers` -type localUser struct { - Username string `json:"username"` - Role string `json:"role"` - Disable bool `json:"disable"` -} diff --git a/systemtests/init_test.go b/systemtests/init_test.go index c0e57ede..a2de9781 100644 --- a/systemtests/init_test.go +++ b/systemtests/init_test.go @@ -39,7 +39,7 @@ func Test(t *testing.T) { // cleanup users and principals test.CleanupDatastore(datastore, []string{ db.GetPath(db.RootLocalUsers), - db.GetPath(db.RootPrincipals), + db.GetPath(db.RootLdapConfiguration), }) if err := db.AddDefaultUsers(); err != nil { From 6d610962dac3aa8526497d231088fd7d61b3e875 Mon Sep 17 00:00:00 2001 From: Yuva Shankar Date: Tue, 13 Dec 2016 13:56:27 -0800 Subject: [PATCH 2/3] Add support for local user `first_name` and `last_name` Signed-off-by: Yuva Shankar --- common/types/types.go | 4 ++ db/local.go | 1 + db/local_test.go | 99 +++++++++++++++++++++++++++++-------------- proxy/helpers.go | 18 +++++++- 4 files changed, 89 insertions(+), 33 deletions(-) diff --git a/common/types/types.go b/common/types/types.go index 47813e4d..0e0f4a10 100644 --- a/common/types/types.go +++ b/common/types/types.go @@ -55,6 +55,8 @@ func Role(roleStr string) (RoleType, error) { // // Fields: // UserName: of the user. Read only field. Must be unique. +// FirstName: of the user +// LastName: of the user // Password: of the user. Not stored anywhere. Used only for updates. // Disable: if authorizations for this local user is disabled. // PasswordHash: of the password string. @@ -62,6 +64,8 @@ func Role(roleStr string) (RoleType, error) { type LocalUser struct { Username string `json:"username"` Password string `json:"password,omitempty"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` Disable bool `json:"disable"` PasswordHash []byte `json:"password_hash,omitempty"` } diff --git a/db/local.go b/db/local.go index d3f37f67..c495ec9e 100644 --- a/db/local.go +++ b/db/local.go @@ -201,6 +201,7 @@ func AddDefaultUsers() error { // default user accounts are `enabled` always; it cannot be disabled Disable: false, Password: userR.String(), + // FirstName, LastName = "" for built-in users } err := AddLocalUser(&localUser) diff --git a/db/local_test.go b/db/local_test.go index f4d939d8..ac655ad9 100644 --- a/db/local_test.go +++ b/db/local_test.go @@ -20,8 +20,28 @@ type dbSuite struct{} var _ = Suite(&dbSuite{}) var ( + newUsers = []types.LocalUser{ + { + Username: "aaa", + FirstName: "Temp", + LastName: "User", + Disable: true, + }, + { + Username: "bbb", + FirstName: "Temp", + LastName: "User", + Disable: true, + }, + { + Username: "ccc", + FirstName: "Temp", + LastName: "User", + Disable: true, + }, + } + invalidUsers = []string{"xxx", "yyy", "zzz"} - newUsers = []string{"aaa", "bbb", "ccc"} builtInUsers = []string{types.Admin.String(), types.Ops.String()} datastore = "" datastorePaths = []string{ @@ -87,7 +107,6 @@ func (s *dbSuite) TestGetLocalUser(c *C) { c.Assert(user.Username, Equals, username) c.Assert(user.Password, Equals, "") - c.Assert(user.PasswordHash, DeepEquals, []byte{}) } // invalid users @@ -102,18 +121,12 @@ func (s *dbSuite) TestGetLocalUser(c *C) { // TestAddLocalUser tests `AddLocalUser(...)` func (s *dbSuite) TestAddLocalUser(c *C) { // add new users - for _, username := range newUsers { - user := &types.LocalUser{ - Username: username, - Disable: false, - Password: username, - } - - err := AddLocalUser(user) + for _, user := range newUsers { + err := AddLocalUser(&user) c.Assert(err, IsNil) // add existing usernames and check for error - err = AddLocalUser(user) + err = AddLocalUser(&user) c.Assert(err, Equals, ccnerrors.ErrKeyExists) } @@ -131,12 +144,12 @@ func (s *dbSuite) TestDeleteLocalUser(c *C) { } // delete the added new users - for _, username := range newUsers { - err := DeleteLocalUser(username) + for _, user := range newUsers { + err := DeleteLocalUser(user.Username) c.Assert(err, IsNil) // delete the same user again - err = DeleteLocalUser(username) + err = DeleteLocalUser(user.Username) c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) } } @@ -145,56 +158,75 @@ func (s *dbSuite) TestDeleteLocalUser(c *C) { func (s *dbSuite) TestUpdateLocalUser(c *C) { s.TestAddLocalUser(c) - for _, username := range newUsers { - user, err := GetLocalUser(username) + for _, user := range newUsers { + uUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) // change the username and update - user.Username = username + "_u" - err = UpdateLocalUser(username, user) + newObj := &types.LocalUser{ + Username: user.Username + "_u", + PasswordHash: uUser.PasswordHash, + Disable: uUser.Disable, + FirstName: uUser.FirstName, + LastName: uUser.LastName, + } + + err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) + newObj.PasswordHash = uUser.PasswordHash // search the data store for old username - oldUser, err := GetLocalUser(username) + oldUser, err := GetLocalUser(user.Username) c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) c.Assert(oldUser, IsNil) // search the data store for new username - newUser, err := GetLocalUser(username + "_u") + newUser, err := GetLocalUser(user.Username + "_u") c.Assert(err, IsNil) - c.Assert(newUser, DeepEquals, user) + c.Assert(newUser, DeepEquals, newObj) } // revert the changes - for _, username := range newUsers { - updateTo := username - username = username + "_u" + for _, user := range newUsers { + updateTo := user.Username + user.Username = user.Username + "_u" - user, err := GetLocalUser(username) + uUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) // change the username and update - user.Username = updateTo - err = UpdateLocalUser(username, user) + newObj := &types.LocalUser{ + Username: updateTo, + PasswordHash: uUser.PasswordHash, + Disable: uUser.Disable, + FirstName: uUser.FirstName, + LastName: uUser.LastName, + } + err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) + newObj.PasswordHash = uUser.PasswordHash // search the data store for old username - oldUser, err := GetLocalUser(username) + oldUser, err := GetLocalUser(user.Username) c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) c.Assert(oldUser, IsNil) newUser, err := GetLocalUser(updateTo) c.Assert(err, IsNil) - c.Assert(newUser, DeepEquals, user) + c.Assert(newUser, DeepEquals, newObj) } } // TestGetLocalUsers tests `GetLocalUsers(...)` func (s *dbSuite) TestGetLocalUsers(c *C) { + users, err := GetLocalUsers() + c.Assert(err, IsNil) + c.Assert(users, DeepEquals, []*types.LocalUser{}) + s.TestAddLocalUser(c) s.addBuiltInUsers(c) - users, err := GetLocalUsers() + users, err = GetLocalUsers() c.Assert(err, IsNil) usernames := []string{} @@ -202,7 +234,12 @@ func (s *dbSuite) TestGetLocalUsers(c *C) { usernames = append(usernames, user.Username) } - allUsers := append(newUsers, builtInUsers...) + newUserNames := []string{} + for _, user := range newUsers { + newUserNames = append(newUserNames, user.Username) + } + + allUsers := append(newUserNames, builtInUsers...) sort.Strings(usernames) sort.Strings(allUsers) diff --git a/proxy/helpers.go b/proxy/helpers.go index 589fb563..38dbdf3c 100644 --- a/proxy/helpers.go +++ b/proxy/helpers.go @@ -244,8 +244,10 @@ func getLocalUsersHelper() (int, []byte) { localUsers := []types.LocalUser{} for _, user := range users { lu := types.LocalUser{ - Username: user.Username, - Disable: user.Disable, + Username: user.Username, + FirstName: user.FirstName, + LastName: user.LastName, + Disable: user.Disable, } localUsers = append(localUsers, lu) @@ -272,11 +274,23 @@ func updateLocalUserInfo(username string, updateReq *types.LocalUser, actual *ty updatedUserObj := &types.LocalUser{ // username == actual.Username Username: actual.Username, + FirstName: actual.FirstName, + LastName: actual.LastName, Disable: actual.Disable, PasswordHash: actual.PasswordHash, // `Password` will be empty } + // Update `first_name` + if !common.IsEmpty(updateReq.FirstName) { + updatedUserObj.FirstName = updateReq.FirstName + } + + // Update `last_name` + if !common.IsEmpty(updateReq.LastName) { + updatedUserObj.LastName = updateReq.LastName + } + // Update `disable` if actual.Disable != updateReq.Disable { updatedUserObj.Disable = updateReq.Disable From ed5d699583a734da979efbd4033d9ad742d4d254 Mon Sep 17 00:00:00 2001 From: Yuva Shankar Date: Tue, 13 Dec 2016 15:41:07 -0800 Subject: [PATCH 3/3] Allow updates(first_name, last_name, disable, password) to built-in users Signed-off-by: Yuva Shankar --- db/local.go | 100 ++++++++++------- db/local_test.go | 80 +++++++++---- proxy/handlers.go | 2 - systemtests/basic_test.go | 1 + systemtests/init_test.go | 53 +++++++-- systemtests/local_user_test.go | 199 +++++++++++++++++++++++++++++++++ 6 files changed, 359 insertions(+), 76 deletions(-) create mode 100644 systemtests/local_user_test.go diff --git a/db/local.go b/db/local.go index c495ec9e..c7bb829f 100644 --- a/db/local.go +++ b/db/local.go @@ -14,30 +14,6 @@ import ( // This file contains all local user management APIs. // NOTE: Built-in users(admin, ops) cannot be changed/updated. it needs to be consumed in the way its defined in code. -// getLocalUser helper function that retrieves a user entry from `/ccn_proxy/local_users` using username -// params: -// username:string; name of the user to be fetched from store -// return values: -// *types.InternalLocalUser: reference to the internal representation of the local user object -// error: as returned by the consecutive calls or any relevant custom errors -func getLocalUser(username string, stateDrv types.StateDriver) (*types.LocalUser, error) { - rawData, err := stateDrv.Read(GetPath(RootLocalUsers, username)) - if err != nil { - if err == ccnerrors.ErrKeyNotFound { - return nil, err - } - - return nil, fmt.Errorf("Failed to read local user %q data from store: %#v", username, err) - } - - var localUser types.LocalUser - if err := json.Unmarshal(rawData, &localUser); err != nil { - return nil, fmt.Errorf("Failed to unmarshal local user %q info %#v", username, err) - } - - return &localUser, nil -} - // GetLocalUsers returns all defined local users. // return values: // []types.InternalLocalUser: slice of local users @@ -82,7 +58,21 @@ func GetLocalUser(username string) (*types.LocalUser, error) { return nil, err } - return getLocalUser(username, stateDrv) + rawData, err := stateDrv.Read(GetPath(RootLocalUsers, username)) + if err != nil { + if err == ccnerrors.ErrKeyNotFound { + return nil, err + } + + return nil, fmt.Errorf("Failed to read local user %q data from store: %#v", username, err) + } + + var localUser types.LocalUser + if err := json.Unmarshal(rawData, &localUser); err != nil { + return nil, fmt.Errorf("Failed to unmarshal local user %q info %#v", username, err) + } + + return &localUser, nil } // UpdateLocalUser updates an existing entry in /ccn_proxy/local_users/. @@ -90,24 +80,52 @@ func GetLocalUser(username string) (*types.LocalUser, error) { // username: string; of the user that requires update // user: local user object to be updated in the data store // return values: -// error: as returned by AddLocalUser, DeleteLocalUser +// error: as returned by GetStateDriver, any consecutive function call or relevant custom error func UpdateLocalUser(username string, user *types.LocalUser) error { - err := DeleteLocalUser(username) + stateDrv, err := state.GetStateDriver() + if err != nil { + return err + } + + key := GetPath(RootLocalUsers, username) + + _, err = stateDrv.Read(key) + switch err { case nil: - // if the password is getting updated; clear the existing hash + // generate password hash only if the password is not empty, otherwise use the existing hash if !common.IsEmpty(user.Password) { - user.PasswordHash = []byte{} + user.PasswordHash, err = common.GenPasswordHash(user.Password) + + if err != nil { + log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) + return err + } + } + + // raw password will never be stored in the store + user.Password = "" + + val, err := json.Marshal(user) + if err != nil { + return fmt.Errorf("Failed to marshal user %#v: %#v", user, err) } - return AddLocalUser(user) - case ccnerrors.ErrKeyNotFound, ccnerrors.ErrIllegalOperation: + if err := stateDrv.Write(key, val); err != nil { + return fmt.Errorf("Failed to write local user info. to data store: %#v", err) + } + + // not to let the user know about password hash + user.PasswordHash = []byte{} + + return nil + case ccnerrors.ErrKeyNotFound: return err default: - // this should never be leaked to the user - log.Debugf("Failed to delete user %q as part of update process %#v", username, err) + log.Debugf("Failed to update user %q: %#v", username, err) return fmt.Errorf("Couldn't update user information: %q", username) } + } // DeleteLocalUser removes a local user from `/ccn_proxy/local_users` @@ -127,9 +145,10 @@ func DeleteLocalUser(username string) error { return err } + key := GetPath(RootLocalUsers, username) + // handles `ErrKeyNotFound` - _, err = getLocalUser(username, stateDrv) - if err != nil { + if _, err := stateDrv.Read(key); err != nil { return err } @@ -159,14 +178,11 @@ func AddLocalUser(user *types.LocalUser) error { case nil: return ccnerrors.ErrKeyExists case ccnerrors.ErrKeyNotFound: - // `AddLocalUser` request from `UpdateLocalUser` carries hash in `user.PasswordHash` - if common.IsEmpty(string(user.PasswordHash)) { - user.PasswordHash, err = common.GenPasswordHash(user.Password) + user.PasswordHash, err = common.GenPasswordHash(user.Password) - if err != nil { - log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) - return err - } + if err != nil { + log.Debugf("Failed to create password hash for user %q: %#v", user.Username, err) + return err } // raw password will never be stored in the store diff --git a/db/local_test.go b/db/local_test.go index ac655ad9..0ee40dd1 100644 --- a/db/local_test.go +++ b/db/local_test.go @@ -46,7 +46,6 @@ var ( datastore = "" datastorePaths = []string{ GetPath(RootLocalUsers), - GetPath(RootPrincipals), GetPath(RootLdapConfiguration), } ) @@ -164,54 +163,41 @@ func (s *dbSuite) TestUpdateLocalUser(c *C) { // change the username and update newObj := &types.LocalUser{ - Username: user.Username + "_u", + Username: user.Username, PasswordHash: uUser.PasswordHash, Disable: uUser.Disable, - FirstName: uUser.FirstName, - LastName: uUser.LastName, + FirstName: uUser.FirstName + "_u", + LastName: uUser.LastName + "_u", } err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) newObj.PasswordHash = uUser.PasswordHash - // search the data store for old username - oldUser, err := GetLocalUser(user.Username) - c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) - c.Assert(oldUser, IsNil) - // search the data store for new username - newUser, err := GetLocalUser(user.Username + "_u") + newUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) c.Assert(newUser, DeepEquals, newObj) } // revert the changes for _, user := range newUsers { - updateTo := user.Username - user.Username = user.Username + "_u" - uUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) // change the username and update newObj := &types.LocalUser{ - Username: updateTo, + Username: uUser.Username, // same as user.Username PasswordHash: uUser.PasswordHash, Disable: uUser.Disable, - FirstName: uUser.FirstName, - LastName: uUser.LastName, + FirstName: user.FirstName, + LastName: user.LastName, } err = UpdateLocalUser(user.Username, newObj) c.Assert(err, IsNil) newObj.PasswordHash = uUser.PasswordHash - // search the data store for old username - oldUser, err := GetLocalUser(user.Username) - c.Assert(err, Equals, ccnerrors.ErrKeyNotFound) - c.Assert(oldUser, IsNil) - - newUser, err := GetLocalUser(updateTo) + newUser, err := GetLocalUser(user.Username) c.Assert(err, IsNil) c.Assert(newUser, DeepEquals, newObj) } @@ -246,3 +232,53 @@ func (s *dbSuite) TestGetLocalUsers(c *C) { c.Assert(usernames, DeepEquals, allUsers) } + +func (s *dbSuite) TestUpdateBuiltInUsers(c *C) { + s.addBuiltInUsers(c) + + // update all the details except `password` + for _, username := range builtInUsers { + user, err := GetLocalUser(username) + c.Assert(err, IsNil) + + uUser := &types.LocalUser{ + Username: user.Username, + Disable: true, + FirstName: "BuiltIn", + LastName: "User", + PasswordHash: user.PasswordHash, + } + + err = UpdateLocalUser(username, uUser) + c.Assert(err, IsNil) + uUser.PasswordHash = user.PasswordHash + + obtainedUser, err := GetLocalUser(username) + c.Assert(err, IsNil) + c.Assert(obtainedUser, DeepEquals, uUser) + } + + // update password and check hash + for _, username := range builtInUsers { + user, err := GetLocalUser(username) + c.Assert(err, IsNil) + + uUser := &types.LocalUser{ + Username: user.Username, + Password: user.Username + "_U", + } + + err = UpdateLocalUser(username, uUser) + c.Assert(err, IsNil) + + obtainedUser, err := GetLocalUser(username) + c.Assert(err, IsNil) + c.Assert(string(obtainedUser.PasswordHash), Not(Equals), string(user.PasswordHash)) + + // all other attributes got default value + c.Assert(obtainedUser.LastName, Equals, "") + c.Assert(obtainedUser.FirstName, Equals, "") + c.Assert(obtainedUser.Disable, Equals, false) + } + +} diff --git a/proxy/handlers.go b/proxy/handlers.go index 078c688c..85b45bd6 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -176,7 +176,6 @@ func adminOnly(handler func(*auth.Token, http.ResponseWriter, *http.Request)) fu // convert token from string to Token type token, err := auth.ParseToken(tokenStr) if err != nil { - log.Infof("From here???") httpStatus := http.StatusInternalServerError httpResponse := []byte(ccnerrors.ErrParsingToken.Error()) processStatusCodes(httpStatus, httpResponse, w) @@ -242,7 +241,6 @@ func deleteLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request // updateLocalUser updates the existing user with the given details. // it can return various HTTP status codes: // 204 (NoContent; update was successful) -// 400 (BadRequest; invalid role/cannot update built-in user) // 404 (NotFound; user not found) // 500 (internal server error) func updateLocalUser(token *auth.Token, w http.ResponseWriter, req *http.Request) { diff --git a/systemtests/basic_test.go b/systemtests/basic_test.go index 81cd470d..c9587df4 100644 --- a/systemtests/basic_test.go +++ b/systemtests/basic_test.go @@ -47,6 +47,7 @@ func (s *systemtestSuite) TestRequestProxying(c *C) { token := adminToken(c) resp, body := proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) c.Assert(data, Equals, string(body)) // check that the Content-Type header was set properly. diff --git a/systemtests/init_test.go b/systemtests/init_test.go index a2de9781..b575f1ee 100644 --- a/systemtests/init_test.go +++ b/systemtests/init_test.go @@ -130,7 +130,7 @@ func login(username, password string) (string, *http.Response, error) { return "", nil, err } - resp, data, err := insecurePostJSONBody("", proxy.LoginPath, loginBytes) + resp, data, err := insecureJSONBody("", proxy.LoginPath, "POST", loginBytes) if err != nil { return "", resp, err } @@ -200,7 +200,6 @@ func proxyGet(c *C, token, path string) (*http.Response, []byte) { resp, err := insecureClient().Do(req) c.Assert(err, IsNil) - c.Assert(resp.StatusCode, Equals, 200) defer resp.Body.Close() @@ -210,26 +209,60 @@ func proxyGet(c *C, token, path string) (*http.Response, []byte) { return resp, data } +// proxyDelete is a convenience function which sends an insecure HTTPS DELETE +// request to the proxy. +func proxyDelete(c *C, token, path string) (*http.Response, []byte) { + url := "https://" + proxyHost + path + + log.Info("GET to ", url) + + req, err := http.NewRequest("DELETE", url, nil) + c.Assert(err, IsNil) + + if len(token) > 0 { + log.Println("Setting X-Auth-token to:", token) + req.Header.Set("X-Auth-Token", token) + } + + resp, err := insecureClient().Do(req) + c.Assert(err, IsNil) + + defer resp.Body.Close() + + data, err := ioutil.ReadAll(resp.Body) + c.Assert(err, IsNil) + + return resp, data +} + +// proxyPatch is a convenience function which sends an insecure HTTPS PATCH +// request with the specified body to the proxy. +func proxyPatch(c *C, token, path string, body []byte) (*http.Response, []byte) { + resp, body, err := insecureJSONBody(token, path, "PATCH", body) + c.Assert(err, IsNil) + + return resp, body +} + // proxyPost is a convenience function which sends an insecure HTTPS POST // request with the specified body to the proxy. func proxyPost(c *C, token, path string, body []byte) (*http.Response, []byte) { - resp, body, err := insecurePostJSONBody(token, path, body) + resp, body, err := insecureJSONBody(token, path, "POST", body) c.Assert(err, IsNil) - c.Assert(resp.StatusCode/100, Equals, 2) // 2xx is okay return resp, body } -// insecurePostBody sends an insecure HTTPS POST request with the specified +// insecureJSONBody sends an insecure HTTPS POST request with the specified // JSON payload as the body. -func insecurePostJSONBody(token, path string, body []byte) (*http.Response, []byte, error) { +func insecureJSONBody(token, path, requestType string, body []byte) (*http.Response, []byte, error) { url := "https://" + proxyHost + path - log.Info("POST to ", url) + log.Info(requestType, " to ", url) - req, err := http.NewRequest("POST", url, bytes.NewBuffer(body)) + req, err := http.NewRequest(requestType, url, bytes.NewBuffer(body)) if err != nil { - log.Debugf("POST request creation failed: %s", err) + log.Debugf("%v request creation failed: %s", requestType, err) return nil, nil, err } @@ -242,7 +275,7 @@ func insecurePostJSONBody(token, path string, body []byte) (*http.Response, []by resp, err := insecureClient().Do(req) if err != nil { - log.Debugf("POST request failed: %s", err) + log.Debugf("%v request failed: %s", requestType, err) return nil, nil, err } diff --git a/systemtests/local_user_test.go b/systemtests/local_user_test.go new file mode 100644 index 00000000..f1be2605 --- /dev/null +++ b/systemtests/local_user_test.go @@ -0,0 +1,199 @@ +package systemtests + +import ( + "github.com/contiv/ccn_proxy/common/types" + "github.com/contiv/ccn_proxy/proxy" + . "gopkg.in/check.v1" +) + +var ( + builtInUsers = []string{types.Admin.String(), types.Ops.String()} + newUsers = []string{"xxx", "yyy", "zzz"} +) + +// TestLocalUserEndpoints tests ccn_proxy's local user endpoints +func (s *systemtestSuite) TestLocalUserEndpoints(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range newUsers { + endpoint := "/api/v1/ccn_proxy/local_users" + resp, body := proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(len(body), Not(Equals), 0) + + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + // get `username` + endpoint = "/api/v1/ccn_proxy/local_users/" + username + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(string(body), DeepEquals, respBody) + + // try login using `username` + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // delete `username` + resp, body = proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 204) + c.Assert(len(body), Equals, 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 404) + c.Assert(len(body), Equals, 0) + } + + }) +} + +// TestLocalUserUpdateEndpoint tests ccn_proxy's local user update endpoint +func (s *systemtestSuite) TestLocalUserUpdateEndpoint(c *C) { + s.userUpdate(c) + s.builtInUserUpdate(c) +} + +// userUpdateEndpoint tests update on local user +func (s *systemtestSuite) userUpdate(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range newUsers { + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + // try login using `username` + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update `testuser` details + data = `{"first_name":"Temp", "last_name": "User"}` + respBody = `{"username":"` + username + `","first_name":"Temp","last_name":"User","disable":false}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using `username` after update + testuserToken = loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update `username`'s password + data = `{"password":"test"}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using old password + testuserToken, resp, err := login(username, username) + c.Assert(err, IsNil) + c.Assert(resp.StatusCode, Equals, 401) + c.Assert(len(testuserToken), Equals, 0) + + // try login again using new password + testuserToken = loginAs(c, username, "test") + c.Assert(len(testuserToken), Not(Equals), 0) + } + }) +} + +// builtInUserUpdate tests built-in user update functionality +func (s *systemtestSuite) builtInUserUpdate(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + for _, username := range builtInUsers { + // update user details + data := `{"first_name":"Built-in", "last_name": "User", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"Built-in","last_name":"User","disable":false}` + s.updateLocalUser(c, username, data, respBody, token) + + // login + testuserToken := loginAs(c, username, username) + c.Assert(len(testuserToken), Not(Equals), 0) + + // update password + data = `{"password":"test"}` + s.updateLocalUser(c, username, data, respBody, token) + + // try login again using old password + testuserToken, resp, err := login(username, username) + c.Assert(err, IsNil) + c.Assert(resp.StatusCode, Equals, 401) + c.Assert(len(testuserToken), Equals, 0) + + // try login again using new password + testuserToken = loginAs(c, username, "test") + c.Assert(len(testuserToken), Not(Equals), 0) + + // revert password so that it wont block other tests + data = `{"password":"` + username + `"}` + s.updateLocalUser(c, username, data, respBody, token) + } + }) +} + +// TestLocalUserDeleteEndpoint tests ccn_proxy's local user delete endpoint +func (s *systemtestSuite) TestLocalUserDeleteEndpoint(c *C) { + + runTest(func(p *proxy.Server, ms *MockServer) { + token := adminToken(c) + + // add and delete new users + for _, username := range newUsers { + // add new local_user to the system + data := `{"username":"` + username + `","password":"` + username + `", "disable":false}` + respBody := `{"username":"` + username + `","first_name":"","last_name":"","disable":false}` + s.addLocalUser(c, data, respBody, token) + + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + // delete `username` + resp, body := proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 204) + c.Assert(len(body), Equals, 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 404) + c.Assert(len(body), Equals, 0) + } + + // delete built-in users + for _, username := range builtInUsers { + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + // delete `username` + resp, body := proxyDelete(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 400) + c.Assert(len(body), Not(Equals), 0) + + // get `username` + resp, body = proxyGet(c, token, endpoint) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(len(body), Not(Equals), 0) + } + }) +} + +// addLocalUser helper function for the tests +func (s *systemtestSuite) addLocalUser(c *C, data, expectedRespBody, token string) { + endpoint := "/api/v1/ccn_proxy/local_users" + + resp, body := proxyPost(c, token, endpoint, []byte(data)) + c.Assert(resp.StatusCode, Equals, 201) + c.Assert(string(body), DeepEquals, expectedRespBody) +} + +// updateLocalUser helper function for the tests +func (s *systemtestSuite) updateLocalUser(c *C, username, data, expectedRespBody, token string) { + endpoint := "/api/v1/ccn_proxy/local_users/" + username + + resp, body := proxyPatch(c, token, endpoint, []byte(data)) + c.Assert(resp.StatusCode, Equals, 200) + c.Assert(string(body), DeepEquals, expectedRespBody) +}