Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl committed Jan 7, 2025
1 parent 92d6180 commit 4134830
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 18 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ require (
github.com/ory/jsonschema/v3 v3.0.8
github.com/ory/mail/v3 v3.0.0
github.com/ory/nosurf v1.2.7
github.com/ory/x v0.0.675
github.com/ory/x v0.0.689
github.com/peterhellberg/link v1.2.0
github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5
github.com/pkg/errors v0.9.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,8 @@ github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpi
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
github.com/ory/x v0.0.675 h1:K6GpVo99BXBFv2UiwMjySNNNqCFKGswynrt7vWQJFU8=
github.com/ory/x v0.0.675/go.mod h1:zJmnDtKje2FCP4EeFvRsKk94XXiqKCSGJMZcirAfhUs=
github.com/ory/x v0.0.689 h1:pMXmnw2aoHiq4jRX9xtGXqX+VU3USEwlUUbwNCxmiZQ=
github.com/ory/x v0.0.689/go.mod h1:UpPgjobuyIyHh1pG4LxqmfMpuNOnzf2BzwyouwBeCk4=
github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8=
github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=
github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6Wq+LM=
Expand Down
16 changes: 1 addition & 15 deletions identity/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"time"

"github.com/gofrs/uuid"
"github.com/tidwall/gjson"

"github.com/ory/x/crdbx"
"github.com/ory/x/pagination/keysetpagination"
Expand Down Expand Up @@ -917,20 +916,7 @@ func (h *Handler) patch(w http.ResponseWriter, r *http.Request, ps httprouter.Pa

patchedIdentity := WithAdminMetadataInJSON(*identity)

// We need to check for all sub-paths of /credentials, since jsonx.ApplyJSONPatch
// only checks full paths.
for _, path := range gjson.GetBytes(requestBody, "#.path").Array() {
if strings.HasPrefix(path.String(), "/credentials/") {
h.r.Writer().WriteError(w, r, errors.WithStack(
herodot.
ErrBadRequest.
WithReasonf("An error occured when applying the JSON patch").
WithErrorf("patch includes denied sub-path of /credentials: %s", path.String())))
return
}
}

if err := jsonx.ApplyJSONPatch(requestBody, &patchedIdentity, "/id", "/stateChangedAt", "/credentials"); err != nil {
if err := jsonx.ApplyJSONPatch(requestBody, &patchedIdentity, "/id", "/stateChangedAt", "/credentials", "/credentials/**"); err != nil {
h.r.Writer().WriteError(w, r, errors.WithStack(
herodot.
ErrBadRequest.
Expand Down
4 changes: 2 additions & 2 deletions identity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ func TestHandler(t *testing.T) {

res := send(t, ts, "PATCH", "/identities/"+i.ID.String(), http.StatusBadRequest, &patch)

assert.EqualValues(t, "patch includes denied sub-path of /credentials: /credentials/oidc/config/providers/0/organization", res.Get("error.message").String(), "%s", res.Raw)
assert.EqualValues(t, "patch includes denied path: /credentials/oidc/config/providers/0/organization", res.Get("error.message").String(), "%s", res.Raw)
})
}
})
Expand All @@ -1249,7 +1249,7 @@ func TestHandler(t *testing.T) {

res := send(t, ts, "PATCH", "/identities/"+i.ID.String(), http.StatusBadRequest, &patch)

assert.EqualValues(t, "patch includes denied sub-path of /credentials: /credentials/password/config/hashed_password", res.Get("error.message").String(), "%s", res.Raw)
assert.EqualValues(t, "patch includes denied path: /credentials/password/config/hashed_password", res.Get("error.message").String(), "%s", res.Raw)
})
}
})
Expand Down

0 comments on commit 4134830

Please sign in to comment.