Skip to content

Commit

Permalink
fix: resolve identity admin api issues
Browse files Browse the repository at this point in the history 
This patch resolves several issues that occurred when creating or updating identities using the Admin API. Now, all hooks are running properly and updating privileged properties no longer causes errors.

Closes #435
See #500
  • Loading branch information
@aeneasr
aeneasr committed Jul 17, 2020
1 parent 7c7d59e commit df0a29d
Show file tree
Hide file tree
Showing 8 changed files with 306 additions and 79 deletions.
95 changes: 70 additions & 25 deletions identity/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
package identity

import (
"encoding/json"
"net/http"

"github.com/ory/herodot"

"github.com/gofrs/uuid"
"github.com/julienschmidt/httprouter"
"github.com/pkg/errors"

Expand All @@ -23,6 +21,7 @@ const IdentitiesPath = "/identities"
type (
handlerDependencies interface {
PoolProvider
PrivilegedPoolProvider
ManagementProvider
x.WriterProvider
}
Expand Down Expand Up @@ -137,6 +136,28 @@ func (h *Handler) get(w http.ResponseWriter, r *http.Request, ps httprouter.Para
h.r.Writer().Write(w, r, i)
}

// swagger:parameters createIdentity
type createIdentityRequest struct {
// in: body
Body CreateIdentityRequestPayload
}

type CreateIdentityRequestPayload struct {
// SchemaID is the ID of the JSON Schema to be used for validating the identity's traits.
//
// required: true
// in: body
SchemaID string `json:"schema_id"`

// Traits represent an identity's traits. The identity is able to create, modify, and delete traits
// in a self-service manner. The input will always be validated against the JSON Schema defined
// in `schema_url`.
//
// required: true
// in: body
Traits json.RawMessage `json:"traits"`
}

// swagger:route POST /identities admin createIdentity
//
// Create an identity
Expand All @@ -158,25 +179,15 @@ func (h *Handler) get(w http.ResponseWriter, r *http.Request, ps httprouter.Para
// 201: identityResponse
// 400: genericError
// 500: genericError
func (h *Handler) create(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
var i Identity
if err := errors.WithStack(jsonx.NewStrictDecoder(r.Body).Decode(&i)); err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

// Make sure the SchemaURL is only set by kratos
if i.SchemaURL != "" {
h.r.Writer().WriteError(w, r, herodot.ErrBadRequest.WithReason("Use the schema_id to set a traits schema."))
func (h *Handler) create(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
var cr CreateIdentityRequestPayload
if err := jsonx.NewStrictDecoder(r.Body).Decode(&cr); err != nil {
h.r.Writer().WriteErrorCode(w, r, http.StatusBadRequest, errors.WithStack(err))
return
}
// We do not allow setting credentials using this method
i.Credentials = nil
// We do not allow setting the ID using this method
i.ID = uuid.Nil

err := h.r.IdentityManager().Create(r.Context(), &i)
if err != nil {
i := &Identity{SchemaID: cr.SchemaID, Traits: []byte(cr.Traits)}
if err := h.r.IdentityManager().Create(r.Context(), i); err != nil {
h.r.Writer().WriteError(w, r, err)
return
}
Expand All @@ -187,10 +198,29 @@ func (h *Handler) create(w http.ResponseWriter, r *http.Request, ps httprouter.P
"identities",
i.ID.String(),
).String(),
&i,
i,
)
}

// swagger:parameters updateIdentity
type updateIdentityRequest struct {
// in: body
Body UpdateIdentityRequestPayload
}

type UpdateIdentityRequestPayload struct {
// SchemaID is the ID of the JSON Schema to be used for validating the identity's traits. If set
// will update the Identity's SchemaID.
SchemaID string `json:"schema_id"`

// Traits represent an identity's traits. The identity is able to create, modify, and delete traits
// in a self-service manner. The input will always be validated against the JSON Schema defined
// in `schema_id`.
//
// required: true
Traits json.RawMessage `json:"traits"`
}

// swagger:route PUT /identities/{id} admin updateIdentity
//
// Update an identity
Expand All @@ -216,19 +246,34 @@ func (h *Handler) create(w http.ResponseWriter, r *http.Request, ps httprouter.P
// 404: genericError
// 500: genericError
func (h *Handler) update(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
var i Identity
if err := errors.WithStack(jsonx.NewStrictDecoder(r.Body).Decode(&i)); err != nil {
var ur UpdateIdentityRequestPayload
if err := errors.WithStack(jsonx.NewStrictDecoder(r.Body).Decode(&ur)); err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

i.ID = x.ParseUUID(ps.ByName("id"))
if err := h.r.IdentityManager().Update(r.Context(), &i); err != nil {
id := x.ParseUUID(ps.ByName("id"))
identity, err := h.r.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), id)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

h.r.Writer().Write(w, r, i)
if ur.SchemaID != "" {
identity.SchemaID = ur.SchemaID
}

identity.Traits = []byte(ur.Traits)
if err := h.r.IdentityManager().Update(
r.Context(),
identity,
ManagerAllowWriteProtectedTraits,
); err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

h.r.Writer().Write(w, r, identity)
}

// swagger:route DELETE /identities/{id} admin deleteIdentity
Expand Down
174 changes: 123 additions & 51 deletions identity/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/ory/x/urlx"

"github.com/ory/kratos/internal/testhelpers"
"github.com/ory/kratos/schema"

"github.com/stretchr/testify/assert"
Expand All @@ -32,15 +33,14 @@ func TestHandler(t *testing.T) {
defer ts.Close()

mockServerURL := urlx.ParseOrPanic("http://example.com")
defaultSchemaInternalURL := urlx.ParseOrPanic("file://./stub/identity.schema.json")
defaultSchema := schema.Schema{
ID: "default",
URL: defaultSchemaInternalURL,
}
defaultSchemaExternalURL := defaultSchema.SchemaURL(mockServerURL).String()
defaultSchemaExternalURL := (&schema.Schema{ID: "default"}).SchemaURL(mockServerURL).String()

viper.Set(configuration.ViperKeyAdminBaseURL, ts.URL)
viper.Set(configuration.ViperKeyDefaultIdentitySchemaURL, defaultSchemaInternalURL.String())
testhelpers.SetDefaultIdentitySchema("file://./stub/identity.schema.json")
testhelpers.SetIdentitySchemas(map[string]string{
"customer": "file://./stub/handler/customer.schema.json",
"employee": "file://./stub/handler/employee.schema.json",
})
viper.Set(configuration.ViperKeyPublicBaseURL, mockServerURL.String())

var get = func(t *testing.T, href string, expectCode int) gjson.Result {
Expand Down Expand Up @@ -91,67 +91,147 @@ func TestHandler(t *testing.T) {
})

t.Run("case=should fail to create an identity because schema id does not exist", func(t *testing.T) {
var i identity.Identity
var i identity.CreateIdentityRequestPayload
i.SchemaID = "does-not-exist"
res := send(t, "POST", "/identities", http.StatusBadRequest, &i)
assert.Contains(t, res.Get("error.reason").String(), "does-not-exist", "%s", res)
})

t.Run("case=should fail to create an entity because schema is not validating", func(t *testing.T) {
var i identity.Identity
i.Traits = identity.Traits(`{"bar":123}`)
var i identity.CreateIdentityRequestPayload
i.Traits = []byte(`{"bar":123}`)
res := send(t, "POST", "/identities", http.StatusBadRequest, &i)
assert.Contains(t, res.Get("error.reason").String(), "I[#/traits/bar] S[#/properties/traits/properties/bar/type] expected string, but got number")
})

t.Run("case=should fail to create an entity with schema_url set", func(t *testing.T) {
var i identity.Identity
i.SchemaURL = "http://example.com"
res := send(t, "POST", "/identities", http.StatusBadRequest, &i)
assert.Contains(t, res.Get("error.reason").String(), "set a traits schema")
res := send(t, "POST", "/identities", http.StatusBadRequest, json.RawMessage(`{"schema_url":"12345","traits":{}}`))
assert.Contains(t, res.Get("error.message").String(), "schema_url")
})

t.Run("case=should create an identity without an ID", func(t *testing.T) {
var i identity.Identity
i.Traits = identity.Traits(`{"bar":"baz"}`)
var i identity.CreateIdentityRequestPayload
i.Traits = []byte(`{"bar":"baz"}`)
res := send(t, "POST", "/identities", http.StatusCreated, &i)
assert.NotEmpty(t, res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.Empty(t, res.Get("credentials").String(), "%s", res.Raw)
})

var i identity.Identity
t.Run("case=should create an identity with an ID which is ignored", func(t *testing.T) {
i.ID = x.NewUUID()
i.Traits = identity.Traits(`{"bar":"baz"}`)
res := send(t, "POST", "/identities", http.StatusCreated, &i)
assert.NotEqual(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
t.Run("case=unable to set ID itself", func(t *testing.T) {
res := send(t, "POST", "/identities", http.StatusBadRequest, json.RawMessage(`{"id":"12345","traits":{}}`))
assert.Contains(t, res.Raw, "id")
})

i.ID = x.ParseUUID(res.Get("id").String())
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.Empty(t, res.Get("credentials").String(), "%s", res.Raw)
assert.EqualValues(t, defaultSchemaExternalURL, res.Get("schema_url").String(), "%s", res.Raw)
assert.EqualValues(t, configuration.DefaultIdentityTraitsSchemaID, res.Get("schema_id").String(), "%s", res.Raw)
t.Run("suite=create and update", func(t *testing.T) {
var i identity.Identity
t.Run("case=should create an identity with an ID which is ignored", func(t *testing.T) {
res := send(t, "POST", "/identities", http.StatusCreated, json.RawMessage(`{"traits": {"bar":"baz"}}`))

i.Traits = []byte(res.Get("traits").Raw)
i.ID = x.ParseUUID(res.Get("id").String())
assert.NotEmpty(t, res.Get("id").String())

assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.Empty(t, res.Get("credentials").String(), "%s", res.Raw)
assert.EqualValues(t, defaultSchemaExternalURL, res.Get("schema_url").String(), "%s", res.Raw)
assert.EqualValues(t, configuration.DefaultIdentityTraitsSchemaID, res.Get("schema_id").String(), "%s", res.Raw)
})

t.Run("case=should be able to get the identity", func(t *testing.T) {
res := get(t, "/identities/"+i.ID.String(), http.StatusOK)
assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.EqualValues(t, defaultSchemaExternalURL, res.Get("schema_url").String(), "%s", res.Raw)
assert.EqualValues(t, configuration.DefaultIdentityTraitsSchemaID, res.Get("schema_id").String(), "%s", res.Raw)
assert.Empty(t, res.Get("credentials").String(), "%s", res.Raw)
})

t.Run("case=should update an identity and persist the changes", func(t *testing.T) {
ur := identity.UpdateIdentityRequestPayload{Traits: []byte(`{"bar":"baz","foo":"baz"}`), SchemaID: i.SchemaID}
res := send(t, "PUT", "/identities/"+i.ID.String(), http.StatusOK, &ur)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.foo").String(), "%s", res.Raw)

res = get(t, "/identities/"+i.ID.String(), http.StatusOK)
assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
})

t.Run("case=should delete a client and no longer be able to retrieve it", func(t *testing.T) {
remove(t, "/identities/"+i.ID.String(), http.StatusNoContent)
_ = get(t, "/identities/"+i.ID.String(), http.StatusNotFound)
})
})

t.Run("case=should be able to get the identity", func(t *testing.T) {
res := get(t, "/identities/"+i.ID.String(), http.StatusOK)
assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.EqualValues(t, defaultSchemaExternalURL, res.Get("schema_url").String(), "%s", res.Raw)
assert.EqualValues(t, configuration.DefaultIdentityTraitsSchemaID, res.Get("schema_id").String(), "%s", res.Raw)
assert.Empty(t, res.Get("credentials").String(), "%s", res.Raw)
t.Run("case=should not be able to create an identity with an invalid schema", func(t *testing.T) {
var cr identity.CreateIdentityRequestPayload
cr.SchemaID = "unknown"
cr.Traits = []byte(`{"email":"` + x.NewUUID().String() + `@ory.sh"}`)
res := send(t, "POST", "/identities", http.StatusBadRequest, &cr)
assert.Contains(t, res.Raw, "unknown")
})

t.Run("case=should update an identity and persist the changes", func(t *testing.T) {
i.Traits = identity.Traits(`{"bar":"baz","foo":"baz"}`)
res := send(t, "PUT", "/identities/"+i.ID.String(), http.StatusOK, &i)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.foo").String(), "%s", res.Raw)
t.Run("case=should create an identity with a different schema", func(t *testing.T) {
var cr identity.CreateIdentityRequestPayload
cr.SchemaID = "employee"
cr.Traits = []byte(`{"email":"` + x.NewUUID().String() + `@ory.sh"}`)

res = get(t, "/identities/"+i.ID.String(), http.StatusOK)
assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw)
assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw)
res := send(t, "POST", "/identities", http.StatusCreated, &cr)
assert.JSONEq(t, string(cr.Traits), res.Get("traits").Raw, "%s", res.Raw)
assert.EqualValues(t, "employee", res.Get("schema_id").String(), "%s", res.Raw)
assert.EqualValues(t, mockServerURL.String()+"/schemas/employee", res.Get("schema_url").String(), "%s", res.Raw)
})

t.Run("case=should create and sync metadata and update privileged traits", func(t *testing.T) {
var cr identity.CreateIdentityRequestPayload
cr.SchemaID = "employee"
originalEmail := x.NewUUID().String() + "@ory.sh"
cr.Traits = []byte(`{"email":"` + originalEmail + `"}`)
res := send(t, "POST", "/identities", http.StatusCreated, &cr)
assert.EqualValues(t, originalEmail, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
assert.EqualValues(t, originalEmail, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)

id := res.Get("id").String()
updatedEmail := x.NewUUID().String() + "@ory.sh"
res = send(t, "PUT", "/identities/"+id, http.StatusOK, &identity.UpdateIdentityRequestPayload{
Traits: []byte(`{"email":"` + updatedEmail + `", "department": "ory"}`),
})

assert.EqualValues(t, "employee", res.Get("schema_id").String(), "%s", res.Raw)
assert.EqualValues(t, mockServerURL.String()+"/schemas/employee", res.Get("schema_url").String(), "%s", res.Raw)
assert.EqualValues(t, updatedEmail, res.Get("traits.email").String(), "%s", res.Raw)
assert.EqualValues(t, "ory", res.Get("traits.department").String(), "%s", res.Raw)
assert.EqualValues(t, updatedEmail, res.Get("recovery_addresses.0.value").String(), "%s", res.Raw)
assert.EqualValues(t, updatedEmail, res.Get("verifiable_addresses.0.value").String(), "%s", res.Raw)
})

t.Run("case=should update the schema id and fail because traits are invalid", func(t *testing.T) {
var cr identity.CreateIdentityRequestPayload
cr.SchemaID = "employee"
cr.Traits = []byte(`{"email":"` + x.NewUUID().String() + `@ory.sh", "department": "ory"}`)
res := send(t, "POST", "/identities", http.StatusCreated, &cr)

id := res.Get("id").String()
res = send(t, "PUT", "/identities/"+id, http.StatusBadRequest, &identity.UpdateIdentityRequestPayload{
SchemaID: "customer",
Traits: cr.Traits,
})
assert.Contains(t, res.Get("error.reason").String(), `additionalProperties "department" not allowed`, "%s", res.Raw)
})

t.Run("case=should update the schema id", func(t *testing.T) {
var cr identity.CreateIdentityRequestPayload
cr.SchemaID = "employee"
cr.Traits = []byte(`{"email":"` + x.NewUUID().String() + `@ory.sh", "department": "ory"}`)
res := send(t, "POST", "/identities", http.StatusCreated, &cr)

id := res.Get("id").String()
res = send(t, "PUT", "/identities/"+id, http.StatusOK, &identity.UpdateIdentityRequestPayload{
SchemaID: "customer",
Traits: []byte(`{"email":"` + x.NewUUID().String() + `@ory.sh", "address": "ory street"}`),
})
assert.EqualValues(t, "ory street", res.Get("traits.address").String(), "%s", res.Raw)
})

t.Run("case=should list all identities", func(t *testing.T) {
Expand All @@ -161,18 +241,10 @@ func TestHandler(t *testing.T) {
})

t.Run("case=should not be able to update an identity that does not exist yet", func(t *testing.T) {
var i identity.Identity
i.ID = x.NewUUID()
i.Traits = identity.Traits(`{"bar":"baz"}`)
res := send(t, "PUT", "/identities/"+i.ID.String(), http.StatusNotFound, &i)
res := send(t, "PUT", "/identities/not-found", http.StatusNotFound, json.RawMessage(`{"traits": {"bar":"baz"}}`))
assert.Contains(t, res.Get("error.message").String(), "Unable to locate the resource", "%s", res.Raw)
})

t.Run("case=should delete a client and no longer be able to retrieve it", func(t *testing.T) {
remove(t, "/identities/"+i.ID.String(), http.StatusNoContent)
_ = get(t, "/identities/"+i.ID.String(), http.StatusNotFound)
})

t.Run("case=should return 404 for non-existing identities", func(t *testing.T) {
remove(t, "/identities/"+x.NewUUID().String(), http.StatusNotFound)
})
Expand Down
Loading

0 comments on commit df0a29d

Please sign in to comment.