Skip to content

Commit

Permalink
graph: Fix Status code when updating the password
Browse files Browse the repository at this point in the history
Up to now the /me/changePassword endpoint return a 500 Status when
issue a password change with the old password set to the wrong password.
This changes the code to return 400 (Bad Request) with an additional
message that the old password is wrong. This does not seem to weaken the
security of /me/changePassword (i.e. for allowing easier brute-force
attacks) as the endpoint is only available to already authenticated
users (and only for changing their own passwords)

See owncloud#4480
  • Loading branch information
rhafer committed Sep 1, 2022
1 parent 3b548c9 commit e056f6b
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
8 changes: 7 additions & 1 deletion services/graph/pkg/service/v0/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) {
return
}

if authRes.Status.Code != cs3rpc.Code_CODE_OK {
switch authRes.Status.Code {
case cs3rpc.Code_CODE_OK:
break
case cs3rpc.Code_CODE_UNAUTHENTICATED, cs3rpc.Code_CODE_PERMISSION_DENIED:
errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "wrong current password")
return
default:
errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "password change failed")
return
}
Expand Down
2 changes: 1 addition & 1 deletion services/graph/pkg/service/v0/password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ var _ = Describe("Users changing their own password", func() {
Entry("fails when new password is empty", "currentpassword", "", "", http.StatusBadRequest),
Entry("fails when current and new password are equal", "password", "password", "", http.StatusBadRequest),
Entry("fails authentication with current password errors", "currentpassword", "newpassword", "error", http.StatusInternalServerError),
Entry("fails when current password is wrong", "currentpassword", "newpassword", "deny", http.StatusInternalServerError),
Entry("fails when current password is wrong", "currentpassword", "newpassword", "deny", http.StatusBadRequest),
Entry("succeeds when current password is correct", "currentpassword", "newpassword", "", http.StatusNoContent),
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ Feature: an user changes its own password
| 123456 | ?&^%0 | 204 |
| 123456 | | 400 |
| 123456 | 123456 | 400 |
| wrongPass | 123456 | 500 |
| wrongPass | 123456 | 400 |
| | validPass | 400 |

0 comments on commit e056f6b

Please sign in to comment.