Skip to content

Commit

Permalink
fix: stricter JSON patch checking for PATCH identities
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl committed Jan 6, 2025
1 parent 23f3232 commit 26e0e13
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 0 deletions.
12 changes: 12 additions & 0 deletions identity/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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 @@ -916,6 +917,17 @@ func (h *Handler) patch(w http.ResponseWriter, r *http.Request, ps httprouter.Pa

patchedIdentity := WithAdminMetadataInJSON(*identity)

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 {
h.r.Writer().WriteError(w, r, errors.WithStack(
herodot.
Expand Down
51 changes: 51 additions & 0 deletions identity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,57 @@ func TestHandler(t *testing.T) {
}
})

t.Run("case=PATCH should fail if credential orgs are updated", func(t *testing.T) {
uuid := x.NewUUID().String()
email := uuid + "@ory.sh"
i := &identity.Identity{Traits: identity.Traits(`{"email":"` + email + `"}`)}
i.SetCredentials(identity.CredentialsTypeOIDC, identity.Credentials{
Type: identity.CredentialsTypeOIDC,
Identifiers: []string{email},
Config: sqlxx.JSONRawMessage(`{"providers": [{"provider": "some-provider"}]}`),
})
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))

for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
t.Run("endpoint="+name, func(t *testing.T) {
patch := []patch{
{"op": "replace", "path": "/credentials/oidc/config/providers/0/organization", "value": "foo"},
}

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)
})
}
})

t.Run("case=PATCH should fail to update credential password", func(t *testing.T) {
uuid := x.NewUUID().String()
email := uuid + "@ory.sh"
password := "ljanf123akf"
p, err := reg.Hasher(ctx).Generate(context.Background(), []byte(password))
require.NoError(t, err)
i := &identity.Identity{Traits: identity.Traits(`{"email":"` + email + `"}`)}
i.SetCredentials(identity.CredentialsTypePassword, identity.Credentials{
Type: identity.CredentialsTypePassword,
Identifiers: []string{email},
Config: sqlxx.JSONRawMessage(`{"hashed_password":"` + string(p) + `"}`),
})
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))

for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} {
t.Run("endpoint="+name, func(t *testing.T) {
patch := []patch{
{"op": "replace", "path": "/credentials/password/config/hashed_password", "value": "foo"},
}

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)
})
}
})

t.Run("case=PATCH should not invalidate credentials ory/cloud#148", func(t *testing.T) {
// see https://github.com/ory/cloud/issues/148

Expand Down

0 comments on commit 26e0e13

Please sign in to comment.