diff --git a/.circleci/config.yml b/.circleci/config.yml index 724cec4bc33a..cce029d1e2c7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -57,7 +57,7 @@ jobs: paths: - "/go/pkg/mod" - run: timeout 15 sh -c 'until nc -z $0 $1; do sleep 1; done' 127.0.0.1 4444 - - run: go-acc -o coverage.txt ./... -- -failfast -timeout=20m -v + - run: go-acc -o coverage.txt ./... -- -failfast -timeout=20m - run: test -z "$CIRCLE_PR_NUMBER" && goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls" workflows: diff --git a/.gitignore b/.gitignore index 2a104148f8f5..f5fa2311aabd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ cover.out .idea/ -tmp/ \ No newline at end of file +tmp/ +.DS_Store \ No newline at end of file diff --git a/identity/handler.go b/identity/handler.go index 29a801dddb94..64d24545b408 100644 --- a/identity/handler.go +++ b/identity/handler.go @@ -3,10 +3,17 @@ package identity import ( "net/http" + "github.com/google/uuid" "github.com/julienschmidt/httprouter" + "github.com/pkg/errors" + + "github.com/ory/herodot" + "github.com/ory/x/jsonx" + "github.com/ory/x/urlx" "github.com/ory/x/pagination" + "github.com/ory/hive/schema" "github.com/ory/hive/x" ) @@ -36,9 +43,8 @@ func (h *Handler) RegisterAdminRoutes(admin *x.RouterAdmin) { admin.GET("/identities/:id", h.get) admin.DELETE("/identities/:id", h.delete) - // admin.POST("/identities", h.create) - // admin.PUT("/identities", h.upsert) - // admin.PUT("/identities/:id", h.update) + admin.POST("/identities", h.create) + admin.PUT("/identities/:id", h.update) } func (h *Handler) list(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { @@ -66,131 +72,68 @@ func (h *Handler) get(w http.ResponseWriter, r *http.Request, ps httprouter.Para h.r.Writer().Write(w, r, i.WithoutCredentials()) } -// swagger:route POST /identities admin createIdentity -// -// Create an identity -// -// Use this endpoint to programmatically create an identity. If an identity with the given RequestID exist already, -// an error (409) will be returned. -// -// Consumes: -// - application/json -// -// Produces: -// - application/json -// -// Schemes: http, https -// -// Responses: -// 201: emptyResponse -// 409: genericError -// 500: genericError -// func (h *Handler) create(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { -// i := NewIdentity() -// if err := errors.WithStack(jsonx.NewStrictDecoder(r.Body).Decode(&i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// panic("hooks are missing") -// if err := errors.WithStack(h.r.IdentityValidator().Validate(i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// _, err := h.r.IdentityPool().Create(r.Context(), &i) -// if err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// h.r.Writer().WriteCreated(w, r, -// urlx.AppendPaths( -// h.c.SelfAdminURL(), -// "identities", -// i.ID, -// ).String(), -// http.StatusCreated, -// ) -// } - -// swagger:route PUT /identities admin upsertIdentity -// -// Upsert an identity -// -// Use this endpoint to update or insert an identity. If an identity with the given RequestID exists, the identity -// in the datastore will be overwritten. If such an identity does not exist, the identity will be added to the datastore. -// -// Use this endpoint with caution as it may override an existing identity if the IDs are in conflict. -// -// Consumes: -// - application/json -// -// Produces: -// - application/json -// -// Schemes: http, https -// -// Responses: -// 200: identity -// 500: genericError -// func (h *Handler) upsert(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { -// i := NewIdentity() -// d := json.NewDecoder(r.Body) -// d.DisallowUnknownFields() -// if err := errors.WithStack(d.Decode(&i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// panic("hooks are missing") -// if err := errors.WithStack(h.r.IdentityValidator().Validate(&i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// _, err := h.r.IdentityPool().Upsert(r.Context(), &i) -// if err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// h.r.Writer().Write(w, r, &i) -// } - -// func (h *Handler) update(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { -// i := NewIdentity() -// d := json.NewDecoder(r.Body) -// d.DisallowUnknownFields() -// if err := errors.WithStack(d.Decode(&i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// if i.ID != ps.ByName("id") { -// h.r.Writer().WriteError(w, r, errors.WithStack( -// herodot.ErrBadRequest.WithReasonf( -// "Value of key subject from POST body does not match subject value from URL path: %s != %s", -// i.ID, ps.ByName("id"), -// ), -// )) -// return -// } -// -// panic("hooks are missing") -// if err := errors.WithStack(h.r.IdentityValidator().Validate(&i)); err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// _, err := h.r.IdentityPool().Update(r.Context(), &i) -// if err != nil { -// h.r.Writer().WriteError(w, r, err) -// return -// } -// -// h.r.Writer().Write(w, r, &i) -// } +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 + } + + if i.ID == "" { + i.ID = uuid.New().String() + } + + if i.TraitsSchemaURL == "" { + i.TraitsSchemaURL = h.c.DefaultIdentityTraitsSchemaURL().String() + } + + if err := h.r.IdentityValidator().Validate(&i); err != nil { + if _, ok := errors.Cause(err).(schema.ResultErrors); ok { + h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("%s", err))) + return + } + h.r.Writer().WriteError(w, r, err) + return + } + + _, err := h.r.IdentityPool().Create(r.Context(), (&i).WithoutCredentials()) + if err != nil { + h.r.Writer().WriteError(w, r, err) + return + } + + h.r.Writer().WriteCreated(w, r, + urlx.AppendPaths( + h.c.SelfAdminURL(), + "identities", + i.ID, + ).String(), + &i, + ) +} + +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 { + h.r.Writer().WriteError(w, r, err) + return + } + + i.ID = ps.ByName("id") + + if err := errors.WithStack(h.r.IdentityValidator().Validate(&i)); err != nil { + h.r.Writer().WriteError(w, r, err) + return + } + + _, err := h.r.IdentityPool().Update(r.Context(), (&i).WithoutCredentials()) + if err != nil { + h.r.Writer().WriteError(w, r, err) + return + } + + h.r.Writer().Write(w, r, &i) +} func (h *Handler) delete(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { if err := h.r.IdentityPool().Delete(r.Context(), ps.ByName("id")); err != nil { diff --git a/identity/handler_test.go b/identity/handler_test.go new file mode 100644 index 000000000000..99a9b1749450 --- /dev/null +++ b/identity/handler_test.go @@ -0,0 +1,157 @@ +package identity_test + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/ory/viper" + + "github.com/ory/hive/driver/configuration" + "github.com/ory/hive/identity" + "github.com/ory/hive/internal" + "github.com/ory/hive/x" +) + +func TestHandler(t *testing.T) { + _, reg := internal.NewMemoryRegistry(t) + router := x.NewRouterAdmin() + reg.IdentityHandler().RegisterAdminRoutes(router) + ts := httptest.NewServer(router) + defer ts.Close() + + viper.Set(configuration.ViperKeyURLsSelfAdmin, ts.URL) + viper.Set(configuration.ViperKeyDefaultIdentityTraitsSchemaURL, "file://./stub/identity.schema.json") + + var get = func(t *testing.T, href string, expectCode int) gjson.Result { + res, err := ts.Client().Get(ts.URL + href) + require.NoError(t, err) + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.NoError(t, res.Body.Close()) + + require.EqualValues(t, expectCode, res.StatusCode, "%s", body) + return gjson.ParseBytes(body) + } + + var remove = func(t *testing.T, href string, expectCode int) { + req, err := http.NewRequest("DELETE", ts.URL+href, nil) + require.NoError(t, err) + + res, err := ts.Client().Do(req) + require.NoError(t, err) + + require.EqualValues(t, expectCode, res.StatusCode) + } + + var send = func(t *testing.T, method, href string, expectCode int, send interface{}) gjson.Result { + var b bytes.Buffer + require.NoError(t, json.NewEncoder(&b).Encode(send)) + req, err := http.NewRequest(method, ts.URL+href, &b) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + res, err := ts.Client().Do(req) + require.NoError(t, err) + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + require.NoError(t, res.Body.Close()) + + require.EqualValues(t, expectCode, res.StatusCode, "%s", body) + return gjson.ParseBytes(body) + } + + t.Run("case=should return an empty list", func(t *testing.T) { + parsed := get(t, "/identities", http.StatusOK) + require.True(t, parsed.IsArray()) + assert.Len(t, parsed.Array(), 0) + }) + + t.Run("case=should return 404 on a non-existing resource", func(t *testing.T) { + _ = get(t, "/identities/does-not-exist", http.StatusNotFound) + }) + + t.Run("case=should fail to create an entity because schema url does not exist", func(t *testing.T) { + var i identity.Identity + i.TraitsSchemaURL = "file://./stub/does-not-exist.schema.json" + res := send(t, "POST", "/identities", http.StatusInternalServerError, &i) + assert.Contains(t, res.Get("error.reason").String(), "no such file or directory") + }) + + t.Run("case=should fail to create an entity because schema is not validating", func(t *testing.T) { + var i identity.Identity + i.Traits = json.RawMessage(`{"bar":123}`) + res := send(t, "POST", "/identities", http.StatusBadRequest, &i) + assert.Contains(t, res.Get("error.reason").String(), "invalid type") + }) + + t.Run("case=should create an identity without an ID", func(t *testing.T) { + var i identity.Identity + i.Traits = json.RawMessage(`{"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) + }) + + t.Run("case=should create an identity with an ID", func(t *testing.T) { + var i identity.Identity + i.ID = "exists" + i.Traits = json.RawMessage(`{"bar":"baz"}`) + res := send(t, "POST", "/identities", http.StatusCreated, &i) + assert.EqualValues(t, "exists", 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) + assert.EqualValues(t, viper.GetString(configuration.ViperKeyDefaultIdentityTraitsSchemaURL), res.Get("traits_schema_url").String(), "%s", res.Raw) + }) + + t.Run("case=should be able to get the identity", func(t *testing.T) { + res := get(t, "/identities/exists", http.StatusOK) + assert.EqualValues(t, "exists", res.Get("id").String(), "%s", res.Raw) + assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw) + assert.EqualValues(t, viper.GetString(configuration.ViperKeyDefaultIdentityTraitsSchemaURL), res.Get("traits_schema_url").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) { + var i identity.Identity + i.Traits = json.RawMessage(`{"bar":"baz","foo":"baz"}`) + res := send(t, "PUT", "/identities/exists", 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) + + res = get(t, "/identities/exists", http.StatusOK) + assert.EqualValues(t, "exists", res.Get("id").String(), "%s", res.Raw) + assert.EqualValues(t, "baz", res.Get("traits.bar").String(), "%s", res.Raw) + }) + + t.Run("case=should list all identities", func(t *testing.T) { + res := get(t, "/identities", http.StatusOK) + assert.Empty(t, res.Get("0.credentials").String(), "%s", res.Raw) + assert.EqualValues(t, "baz", res.Get("0.traits.bar").String(), "%s", res.Raw) + }) + + 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 = uuid.New().String() + i.Traits = json.RawMessage(`{"bar":"baz"}`) + res := send(t, "PUT", "/identities/"+i.ID, http.StatusNotFound, &i) + assert.Contains(t, res.Get("error.reason").String(), "does not exist") + }) + + t.Run("case=should delete a client and no longer be able to retrieve it", func(t *testing.T) { + remove(t, "/identities/exists", http.StatusNoContent) + _ = get(t, "/identities/exists", http.StatusNotFound) + }) + + t.Run("case=should return 404 for non-existing clients", func(t *testing.T) { + remove(t, "/identities/does-not-exist", http.StatusNotFound) + }) +} diff --git a/identity/identity.go b/identity/identity.go index e38f22f783ae..7ebdd8670a4e 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -23,7 +23,7 @@ type Identity struct { // PK: The primary key used for hive-internal processing. It is auto-assigned and immutable. PK uint64 `json:"-" faker:"-" form:"-"` - // RequestID: A unique identifier chosen by you. It can be a URN (e.g. "arn:aws:iam::123456789012"), + // ID is a unique identifier chosen by you. It can be a URN (e.g. "arn:aws:iam::123456789012"), // a stringified integer (e.g. "123456789012"), a uuid (e.g. "9f425a8d-7efc-4768-8f23-7647a74fdf13"). It is up to you // to pick a format you'd like. It is discouraged to use a personally identifiable value here, like the username // or the email, as this field is immutable. diff --git a/identity/pool.go b/identity/pool.go index fc3b691d22e5..e54f467b99f9 100644 --- a/identity/pool.go +++ b/identity/pool.go @@ -11,10 +11,6 @@ type Pool interface { // FindByCredentialsIdentifier returns an identity by querying for it's credential identifiers. FindByCredentialsIdentifier(ctx context.Context, ct CredentialsType, match string) (*Identity, *Credentials, error) - // Upsert inserts or updates an identity in the pool. If the identity argument does not have a primary key, a new - // row will be inserted. - Upsert(context.Context, *Identity) (*Identity, error) - Create(context.Context, *Identity) (*Identity, error) List(ctx context.Context, limit, offset int) ([]Identity, error) diff --git a/identity/pool_memory.go b/identity/pool_memory.go index 18becca261f3..e7365b0d98f0 100644 --- a/identity/pool_memory.go +++ b/identity/pool_memory.go @@ -87,17 +87,6 @@ func (p *PoolMemory) Create(_ context.Context, i *Identity) (*Identity, error) { return i, nil } -func (p *PoolMemory) Upsert(ctx context.Context, i *Identity) (*Identity, error) { - ii, err := p.Update(ctx, i) - if err != nil && err.Error() == herodot.ErrNotFound.Error() { - return p.Create(ctx, i) - } else if err != nil { - return nil, err - } - - return ii, nil -} - func (p *PoolMemory) List(_ context.Context, limit, offset int) ([]Identity, error) { p.RLock() defer p.RUnlock() diff --git a/identity/pool_test.go b/identity/pool_test.go index 4dbe4a61dd82..c715be58b6b5 100644 --- a/identity/pool_test.go +++ b/identity/pool_test.go @@ -73,36 +73,14 @@ func TestPool(t *testing.T) { g, err := p.Get(ctx, identities[0].ID) require.NoError(t, err) - - require.EqualValues(t, g, &i) - - // violates uniqueness - _, err = p.Create(ctx, &i) - require.EqualError(t, err, herodot.ErrConflict.Error()) - }) - - t.Run("case=upsert", func(t *testing.T) { - i := identities[1] - _, err := p.Upsert(ctx, &i) - require.NoError(t, err) - - g, err := p.Get(ctx, identities[1].ID) - require.NoError(t, err) - require.EqualValues(t, g, &i) - i2 := identities[1] - i2.Traits = json.RawMessage(`[]`) - _, err = p.Upsert(ctx, &i2) + g, err = p.Create(ctx, &identities[1]) require.NoError(t, err) - - g, err = p.Get(ctx, identities[1].ID) - require.NoError(t, err) - - require.EqualValues(t, g, &i2) + require.EqualValues(t, g, &identities[1]) // violates uniqueness - _, err = p.Upsert(ctx, &ii) + _, err = p.Create(ctx, &i) require.EqualError(t, err, herodot.ErrConflict.Error()) }) diff --git a/identity/registry.go b/identity/registry.go index eb85a2704304..4cee6a8045eb 100644 --- a/identity/registry.go +++ b/identity/registry.go @@ -10,4 +10,5 @@ type Registry interface { type Configuration interface { SelfAdminURL() *url.URL + DefaultIdentityTraitsSchemaURL() *url.URL } diff --git a/identity/stub/identity.schema.json b/identity/stub/identity.schema.json new file mode 100644 index 000000000000..ec435cd1a957 --- /dev/null +++ b/identity/stub/identity.schema.json @@ -0,0 +1,11 @@ +{ + "$id": "https://example.com/registration.schema.json", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Person", + "type": "object", + "properties": { + "bar": { + "type": "string" + } + } +}