diff --git a/api/system/spec.json b/api/system/spec.json index 764e70a81..34837f122 100644 --- a/api/system/spec.json +++ b/api/system/spec.json @@ -880,6 +880,31 @@ } ] } + }, + { + "name": "setPassword", + "method": "POST", + "title": "Set's or changes user's password", + "path": "/{userID}/password", + "parameters": { + "path": [ + { + "type": "uint64", + "name": "userID", + "required": true, + "title": "User ID" + } + ], + "post": [ + { + "name": "password", + "type": "string", + "required": true, + "sensitive": true, + "title": "New password" + } + ] + } } ] }, diff --git a/api/system/spec/user.json b/api/system/spec/user.json index c9dee7109..1cd05bff8 100644 --- a/api/system/spec/user.json +++ b/api/system/spec/user.json @@ -219,6 +219,31 @@ } ] } + }, + { + "Name": "setPassword", + "Method": "POST", + "Title": "Set's or changes user's password", + "Path": "/{userID}/password", + "Parameters": { + "path": [ + { + "name": "userID", + "required": true, + "title": "User ID", + "type": "uint64" + } + ], + "post": [ + { + "name": "password", + "required": true, + "sensitive": true, + "title": "New password", + "type": "string" + } + ] + } } ] } \ No newline at end of file diff --git a/docs/system/README.md b/docs/system/README.md index 9f8795cd3..b9f697230 100644 --- a/docs/system/README.md +++ b/docs/system/README.md @@ -734,6 +734,7 @@ An organisation may have many roles. Roles may have many channels available. Acc | `DELETE` | `/users/{userID}` | Remove user | | `POST` | `/users/{userID}/suspend` | Suspend user | | `POST` | `/users/{userID}/unsuspend` | Unsuspend user | +| `POST` | `/users/{userID}/password` | Set's or changes user's password | ## Search users (Directory) @@ -848,4 +849,19 @@ An organisation may have many roles. Roles may have many channels available. Acc | --------- | ---- | ------ | ----------- | ------- | --------- | | userID | uint64 | PATH | User ID | N/A | YES | +## Set's or changes user's password + +#### Method + +| URI | Protocol | Method | Authentication | +| --- | -------- | ------ | -------------- | +| `/users/{userID}/password` | HTTP/S | POST | Client ID, Session ID | + +#### Request parameters + +| Parameter | Type | Method | Description | Default | Required? | +| --------- | ---- | ------ | ----------- | ------- | --------- | +| userID | uint64 | PATH | User ID | N/A | YES | +| password | string | POST | New password | N/A | YES | + --- \ No newline at end of file diff --git a/system/internal/repository/user_test.go b/system/internal/repository/user_test.go index e85ae103a..e4d69b8bd 100644 --- a/system/internal/repository/user_test.go +++ b/system/internal/repository/user_test.go @@ -58,7 +58,7 @@ func TestUser(t *testing.T) { } { - users, err := userRepo.Find(&types.UserFilter{Query: "John User Doe"}) + users, _, err := userRepo.Find(types.UserFilter{Query: "John User Doe"}) test.Assert(t, err == nil, "Owner.Find error: %+v", err) test.Assert(t, len(users) == 1, "Owner.Find: expected 1 user, got %d", len(users)) } diff --git a/system/internal/service/auth.go b/system/internal/service/auth.go index 31eb35e1c..b222dee42 100644 --- a/system/internal/service/auth.go +++ b/system/internal/service/auth.go @@ -56,6 +56,9 @@ type ( SendPasswordResetToken(email string) (err error) LoadRoleMemberships(*types.User) error + + checkPasswordStrength(string) error + changePassword(uint64, string) error } ) @@ -331,13 +334,7 @@ func (svc auth) InternalSignUp(input *types.User, password string) (u *types.Use _ = svc.autoPromote(u) if len(password) > 0 { - var hash []byte - hash, err = svc.hashPassword(password) - if err != nil { - return nil, err - } - - err = svc.changePassword(u.ID, hash) + err = svc.changePassword(u.ID, password) if err != nil { return nil, err } @@ -475,18 +472,12 @@ func (svc auth) SetPassword(userID uint64, newPassword string) (err error) { return errors.New("internal authentication disabled") } - if len(newPassword) == 0 { - return errors.New("new password missing") - } - - var hash []byte - hash, err = svc.hashPassword(newPassword) - if err != nil { - return err + if err = svc.checkPasswordStrength(newPassword); err != nil { + return } return svc.db.Transaction(func() error { - if err != svc.changePassword(userID, hash) { + if err != svc.changePassword(userID, newPassword) { return err } @@ -507,14 +498,8 @@ func (svc auth) ChangePassword(userID uint64, oldPassword, newPassword string) ( return errors.New("old password missing") } - if len(newPassword) == 0 { - return errors.New("new password missing") - } - - var hash []byte - hash, err = svc.hashPassword(newPassword) - if err != nil { - return err + if err = svc.checkPasswordStrength(newPassword); err != nil { + return } return svc.db.Transaction(func() error { @@ -532,7 +517,7 @@ func (svc auth) ChangePassword(userID uint64, oldPassword, newPassword string) ( return errors.Wrap(err, "could not change password") } - if err != svc.changePassword(userID, hash) { + if err != svc.changePassword(userID, newPassword) { return err } @@ -542,6 +527,7 @@ func (svc auth) ChangePassword(userID uint64, oldPassword, newPassword string) ( } func (svc auth) hashPassword(password string) (hash []byte, err error) { + // @todo refactor and/or merge with user.hashPasssword() hash, err = bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { return nil, errors.Wrap(err, "could not hash password") @@ -550,12 +536,27 @@ func (svc auth) hashPassword(password string) (hash []byte, err error) { return } +func (svc auth) checkPasswordStrength(password string) error { + if len(password) <= 4 { + return errors.New("password too short") + } + + // @todo proper strength checking + + return nil +} + // ChangePassword (soft) deletes old password entry and creates a new one // // Expects hashed password as an input -func (svc auth) changePassword(userID uint64, hash []byte) (err error) { +func (svc auth) changePassword(userID uint64, password string) (err error) { + var hash []byte + if hash, err = svc.hashPassword(password); err != nil { + return err + } + if err = svc.credentials.DeleteByKind(userID, credentialsTypePassword); err != nil { - return errors.Wrap(err, "could not remove old passwords") + return errors.Wrap(err, "could not delete old credentials") } _, err = svc.credentials.Create(&types.Credentials{ diff --git a/system/internal/service/user.go b/system/internal/service/user.go index 82444ef03..b3be09019 100644 --- a/system/internal/service/user.go +++ b/system/internal/service/user.go @@ -4,10 +4,13 @@ import ( "context" "io" + "github.com/pkg/errors" "github.com/titpetric/factory" "go.uber.org/zap" + "go.uber.org/zap/zapcore" internalAuth "github.com/cortezaproject/corteza-server/internal/auth" + "github.com/cortezaproject/corteza-server/pkg/logger" "github.com/cortezaproject/corteza-server/system/internal/repository" "github.com/cortezaproject/corteza-server/system/types" ) @@ -23,8 +26,18 @@ type ( ctx context.Context logger *zap.Logger - ac userAccessController - user repository.UserRepository + settings authSettings + + auth userAuth + + ac userAccessController + user repository.UserRepository + credentials repository.CredentialsRepository + } + + userAuth interface { + checkPasswordStrength(string) error + changePassword(uint64, string) error } userAccessController interface { @@ -55,27 +68,35 @@ type ( Suspend(id uint64) error Unsuspend(id uint64) error - // ValidateCredentials(username, password string) (*types.User, error) + SetPassword(userID uint64, password string) error } ) func User(ctx context.Context) UserService { return (&user{ logger: DefaultLogger.Named("user"), - ac: DefaultAccessControl, }).With(ctx) } +// log() returns zap's logger with requestID from current context and fields. +func (svc user) log(fields ...zapcore.Field) *zap.Logger { + return logger.AddRequestID(svc.ctx, svc.logger).With(fields...) +} + func (svc user) With(ctx context.Context) UserService { db := repository.DB(ctx) return &user{ ctx: ctx, db: db, - ac: svc.ac, logger: svc.logger, - user: repository.User(ctx, db), + ac: DefaultAccessControl, + settings: DefaultAuthSettings, + auth: DefaultAuth, + + user: repository.User(ctx, db), + credentials: repository.Credentials(ctx, db), } } @@ -217,3 +238,37 @@ func (svc user) Unsuspend(ID uint64) (err error) { return svc.user.UnsuspendByID(ID) }) } + +// SetPassword sets new password for a user +// +// Expecting setter to have permissions to update modify users and internal authentication enabled +func (svc user) SetPassword(userID uint64, newPassword string) (err error) { + log := svc.log(zap.Uint64("userID", userID)) + + if !svc.settings.internalEnabled { + return errors.New("internal authentication disabled") + } + + var u *types.User + if u, err = svc.user.FindByID(userID); err != nil { + return + } + + if !svc.ac.CanUpdateUser(svc.ctx, u) { + return ErrNoPermissions.withStack() + } + + if err = svc.auth.checkPasswordStrength(newPassword); err != nil { + return + } + + return svc.db.Transaction(func() error { + if err := svc.auth.changePassword(userID, newPassword); err != nil { + return err + } + + log.Info("password changed") + + return nil + }) +} diff --git a/system/rest/handlers/user.go b/system/rest/handlers/user.go index ff8b189a5..68ac67745 100644 --- a/system/rest/handlers/user.go +++ b/system/rest/handlers/user.go @@ -36,17 +36,19 @@ type UserAPI interface { Delete(context.Context, *request.UserDelete) (interface{}, error) Suspend(context.Context, *request.UserSuspend) (interface{}, error) Unsuspend(context.Context, *request.UserUnsuspend) (interface{}, error) + SetPassword(context.Context, *request.UserSetPassword) (interface{}, error) } // HTTP API interface type User struct { - List func(http.ResponseWriter, *http.Request) - Create func(http.ResponseWriter, *http.Request) - Update func(http.ResponseWriter, *http.Request) - Read func(http.ResponseWriter, *http.Request) - Delete func(http.ResponseWriter, *http.Request) - Suspend func(http.ResponseWriter, *http.Request) - Unsuspend func(http.ResponseWriter, *http.Request) + List func(http.ResponseWriter, *http.Request) + Create func(http.ResponseWriter, *http.Request) + Update func(http.ResponseWriter, *http.Request) + Read func(http.ResponseWriter, *http.Request) + Delete func(http.ResponseWriter, *http.Request) + Suspend func(http.ResponseWriter, *http.Request) + Unsuspend func(http.ResponseWriter, *http.Request) + SetPassword func(http.ResponseWriter, *http.Request) } func NewUser(h UserAPI) *User { @@ -191,6 +193,26 @@ func NewUser(h UserAPI) *User { resputil.JSON(w, value) } }, + SetPassword: func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + params := request.NewUserSetPassword() + if err := params.Fill(r); err != nil { + logger.LogParamError("User.SetPassword", r, err) + resputil.JSON(w, err) + return + } + + value, err := h.SetPassword(r.Context(), params) + if err != nil { + logger.LogControllerError("User.SetPassword", r, err, params.Auditable()) + resputil.JSON(w, err) + return + } + logger.LogControllerCall("User.SetPassword", r, params.Auditable()) + if !serveHTTP(value, w, r) { + resputil.JSON(w, value) + } + }, } } @@ -204,5 +226,6 @@ func (h User) MountRoutes(r chi.Router, middlewares ...func(http.Handler) http.H r.Delete("/users/{userID}", h.Delete) r.Post("/users/{userID}/suspend", h.Suspend) r.Post("/users/{userID}/unsuspend", h.Unsuspend) + r.Post("/users/{userID}/password", h.SetPassword) }) } diff --git a/system/rest/request/user.go b/system/rest/request/user.go index 62a5af887..014d1668d 100644 --- a/system/rest/request/user.go +++ b/system/rest/request/user.go @@ -467,3 +467,59 @@ func (r *UserUnsuspend) Fill(req *http.Request) (err error) { } var _ RequestFiller = NewUserUnsuspend() + +// User setPassword request parameters +type UserSetPassword struct { + UserID uint64 `json:",string"` + Password string +} + +func NewUserSetPassword() *UserSetPassword { + return &UserSetPassword{} +} + +func (r UserSetPassword) Auditable() map[string]interface{} { + var out = map[string]interface{}{} + + out["userID"] = r.UserID + out["password"] = "*masked*sensitive*data*" + + return out +} + +func (r *UserSetPassword) Fill(req *http.Request) (err error) { + if strings.ToLower(req.Header.Get("content-type")) == "application/json" { + err = json.NewDecoder(req.Body).Decode(r) + + switch { + case err == io.EOF: + err = nil + case err != nil: + return errors.Wrap(err, "error parsing http request body") + } + } + + if err = req.ParseForm(); err != nil { + return err + } + + get := map[string]string{} + post := map[string]string{} + urlQuery := req.URL.Query() + for name, param := range urlQuery { + get[name] = string(param[0]) + } + postVars := req.Form + for name, param := range postVars { + post[name] = string(param[0]) + } + + r.UserID = parseUInt64(chi.URLParam(req, "userID")) + if val, ok := post["password"]; ok { + r.Password = val + } + + return err +} + +var _ RequestFiller = NewUserSetPassword() diff --git a/system/rest/user.go b/system/rest/user.go index a974cade7..6ff0f99dd 100644 --- a/system/rest/user.go +++ b/system/rest/user.go @@ -85,6 +85,10 @@ func (ctrl User) Unsuspend(ctx context.Context, r *request.UserUnsuspend) (inter return resputil.OK(), ctrl.user.With(ctx).Unsuspend(r.UserID) } +func (ctrl User) SetPassword(ctx context.Context, r *request.UserSetPassword) (interface{}, error) { + return resputil.OK(), ctrl.user.With(ctx).SetPassword(r.UserID, r.Password) +} + func (ctrl User) makeFilterPayload(ctx context.Context, uu types.UserSet, f types.UserFilter, err error) (*userSetPayload, error) { if err != nil { return nil, err