From 37c80f4f69ca01843fb9df378a1d8a1c7b30dfa7 Mon Sep 17 00:00:00 2001 From: Rajshekar Chavakula Date: Wed, 14 Feb 2024 21:42:26 +0530 Subject: [PATCH 1/4] condition change to support password reset Signed-off-by: Rajshekar Chavakula --- backend/apid/middlewares/authorization_attributes.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/apid/middlewares/authorization_attributes.go b/backend/apid/middlewares/authorization_attributes.go index cffa691c63..386750cc73 100644 --- a/backend/apid/middlewares/authorization_attributes.go +++ b/backend/apid/middlewares/authorization_attributes.go @@ -93,9 +93,8 @@ func (a AuthorizationAttributes) Then(next http.Handler) http.Handler { attrs.Resource = types.LocalSelfUserResource } - // Change the resource to LocalSelfUserResource if a user tries to change - // its own password - if attrs.Verb == "update" && vars["subresource"] == "password" { + switch vars["subresource"] { + case "password", "": attrs.Resource = types.LocalSelfUserResource } } From d63e7afaf572e3d8409f106e06260fa1ef739cfc Mon Sep 17 00:00:00 2001 From: Rajshekar Chavakula Date: Thu, 29 Feb 2024 14:19:27 +0530 Subject: [PATCH 2/4] creation of new router for password change from webui Signed-off-by: Rajshekar Chavakula --- .../middlewares/authorization_attributes.go | 7 +++- backend/apid/routers/users.go | 32 +++++++++++++++++++ backend/apid/routers/users_test.go | 15 +++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/backend/apid/middlewares/authorization_attributes.go b/backend/apid/middlewares/authorization_attributes.go index 386750cc73..c55350d53a 100644 --- a/backend/apid/middlewares/authorization_attributes.go +++ b/backend/apid/middlewares/authorization_attributes.go @@ -93,8 +93,13 @@ func (a AuthorizationAttributes) Then(next http.Handler) http.Handler { attrs.Resource = types.LocalSelfUserResource } + // check if request comes from webui + if strings.Contains(r.URL.Path, "/change_password") { + attrs.Resource = types.LocalSelfUserResource + } + switch vars["subresource"] { - case "password", "": + case "password": attrs.Resource = types.LocalSelfUserResource } } diff --git a/backend/apid/routers/users.go b/backend/apid/routers/users.go index 2b81d4e913..69714eaff9 100644 --- a/backend/apid/routers/users.go +++ b/backend/apid/routers/users.go @@ -59,6 +59,9 @@ func (r *UsersRouter) Mount(parent *mux.Router) { // Password change & reset routes.Path("{id}/{subresource:password}", r.updatePassword).Methods(http.MethodPut) routes.Path("{id}/{subresource:reset_password}", r.resetPassword).Methods(http.MethodPut) + + // password update from web ui + routes.Path("{id}/{subresource:change_password}", r.changePasswordFromWeb).Methods(http.MethodPut) } func (r *UsersRouter) get(req *http.Request) (interface{}, error) { @@ -153,6 +156,35 @@ func (r *UsersRouter) updatePassword(req *http.Request) (interface{}, error) { return nil, err } +// changePasswordFromWeb updates user password when requests are sent from web UI +func (r *UsersRouter) changePasswordFromWeb(req *http.Request) (interface{}, error) { + params := map[string]string{} + if err := UnmarshalBody(req, ¶ms); err != nil { + return nil, err + } + + vars := mux.Vars(req) + username, err := url.PathUnescape(vars["id"]) + if err != nil { + return nil, err + } + newPassword := params["newPassword"] + oldPassword := params["password"] + + user, err := r.controller.AuthenticateUser(req.Context(), username, oldPassword) + if err != nil { + return nil, err + } + + // set new password for updating into store + user.Password = newPassword + + // Remove any old password hash + user.PasswordHash = "" + err = r.controller.CreateOrReplace(req.Context(), user) + return nil, err +} + // resetPassword updates a user password without any kind of verification func (r *UsersRouter) resetPassword(req *http.Request) (interface{}, error) { params := map[string]string{} diff --git a/backend/apid/routers/users_test.go b/backend/apid/routers/users_test.go index 7d8b0cfe59..30c754fa48 100644 --- a/backend/apid/routers/users_test.go +++ b/backend/apid/routers/users_test.go @@ -235,6 +235,21 @@ func TestUsersRouter(t *testing.T) { }, wantStatusCode: http.StatusCreated, }, + { + name: "update password from web ui", + method: http.MethodPut, + path: path.Join(fixture.URIPath(), "change_password"), + body: []byte(`{"username":"foo","password":"admin123","newPassword":"admin123"}`), + controllerFunc: func(c *mockUserController) { + c.On("AuthenticateUser", mock.Anything, mock.Anything, mock.Anything). + Return(&corev2.User{Username: "foo", Password: "admin123", PasswordHash: "admin123_hash"}, nil). + Once() + c.On("CreateOrReplace", mock.Anything, mock.Anything). + Return(nil). + Once() + }, + wantStatusCode: http.StatusCreated, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 0ad2ebe8f3c6bda100d686b3ce973bf5302b1424 Mon Sep 17 00:00:00 2001 From: Rajshekar Chavakula Date: Fri, 1 Mar 2024 17:21:24 +0530 Subject: [PATCH 3/4] variable name change to maintain consistency Signed-off-by: Rajshekar Chavakula --- backend/apid/routers/users.go | 2 +- backend/apid/routers/users_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/apid/routers/users.go b/backend/apid/routers/users.go index 69714eaff9..9a200a1206 100644 --- a/backend/apid/routers/users.go +++ b/backend/apid/routers/users.go @@ -168,7 +168,7 @@ func (r *UsersRouter) changePasswordFromWeb(req *http.Request) (interface{}, err if err != nil { return nil, err } - newPassword := params["newPassword"] + newPassword := params["password_new"] oldPassword := params["password"] user, err := r.controller.AuthenticateUser(req.Context(), username, oldPassword) diff --git a/backend/apid/routers/users_test.go b/backend/apid/routers/users_test.go index 30c754fa48..039e3cf9f3 100644 --- a/backend/apid/routers/users_test.go +++ b/backend/apid/routers/users_test.go @@ -239,7 +239,7 @@ func TestUsersRouter(t *testing.T) { name: "update password from web ui", method: http.MethodPut, path: path.Join(fixture.URIPath(), "change_password"), - body: []byte(`{"username":"foo","password":"admin123","newPassword":"admin123"}`), + body: []byte(`{"username":"foo","password":"admin123","password_new":"admin123"}`), controllerFunc: func(c *mockUserController) { c.On("AuthenticateUser", mock.Anything, mock.Anything, mock.Anything). Return(&corev2.User{Username: "foo", Password: "admin123", PasswordHash: "admin123_hash"}, nil). From a3e6388c7c9791c54dd5625b68abddce0748be43 Mon Sep 17 00:00:00 2001 From: Rajshekar Chavakula Date: Fri, 15 Mar 2024 11:39:04 +0530 Subject: [PATCH 4/4] review comment addressal related to url fragment check Signed-off-by: Rajshekar Chavakula --- backend/apid/middlewares/authorization_attributes.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/backend/apid/middlewares/authorization_attributes.go b/backend/apid/middlewares/authorization_attributes.go index c55350d53a..ca748cd313 100644 --- a/backend/apid/middlewares/authorization_attributes.go +++ b/backend/apid/middlewares/authorization_attributes.go @@ -93,13 +93,8 @@ func (a AuthorizationAttributes) Then(next http.Handler) http.Handler { attrs.Resource = types.LocalSelfUserResource } - // check if request comes from webui - if strings.Contains(r.URL.Path, "/change_password") { - attrs.Resource = types.LocalSelfUserResource - } - switch vars["subresource"] { - case "password": + case "password", "change_password": attrs.Resource = types.LocalSelfUserResource } }