From 2336eecae1e2e305f8e299a29c7fd5b5c3993f0b Mon Sep 17 00:00:00 2001 From: Thomas Ruiz Date: Wed, 19 May 2021 12:00:00 +0200 Subject: [PATCH] feat(identities): apply code review fixes --- docs/docs/concepts/identity-data-model.mdx | 19 ++++++++------ identity/handler_test.go | 11 +++++---- identity/identity.go | 19 ++++++++------ internal/httpclient/api/openapi.yaml | 26 ++++++++++++-------- internal/httpclient/docs/Identity.md | 2 +- internal/httpclient/docs/UpdateIdentity.md | 2 +- internal/httpclient/model_identity.go | 3 ++- internal/httpclient/model_update_identity.go | 3 ++- session/manager_http_test.go | 10 +++++--- spec/openapi.json | 17 +++++++++---- spec/swagger.json | 17 +++++++++---- 11 files changed, 81 insertions(+), 48 deletions(-) diff --git a/docs/docs/concepts/identity-data-model.mdx b/docs/docs/concepts/identity-data-model.mdx index 5bb91263c7ed..fd859e942898 100644 --- a/docs/docs/concepts/identity-data-model.mdx +++ b/docs/docs/concepts/identity-data-model.mdx @@ -28,6 +28,9 @@ payload is usually in JSON format. An `identity` has the following properties: # which cannot be changed or updated at a later stage. id: '9f425a8d-7efc-4768-8f23-7647a74fdf13' +# Every identity has a state. Inactive identities will not be able to log into the system. +state: active + # This section represents all the credentials associated with this identity. # It is further explained in the "Credentials" section. credentials: @@ -73,22 +76,24 @@ Identities are - `created` - via API or self-service registration; - `updated` - via API or self-service settings, account recovery, etc.; -- `disabled` - not yet implemented, see - [#598](https://github.com/ory/kratos/issues/598); +- `disabled` - via API only; - `deleted` - via API or with a self-service flow (not yet implemented see [#596](https://github.com/ory/kratos/issues/596)). -The identity state is therefore `active` or `disabled` (not yet implemented see -[#598](https://github.com/ory/kratos/issues/598)) +The identity state is therefore `active` or `inactive`. + +An inactive identity will not be able to log into the system. It will receive a +401 with a reason indicating that the account has been disabled. More states +will be added later on for specific features. Active: create Active --> Active: update -Active --> Disabled: disable -Disabled --> [*]: delete -Disabled --> Active: enable +Active --> Inactive: disable +Inactive --> [*]: delete +Inactive --> Active: enable `} /> diff --git a/identity/handler_test.go b/identity/handler_test.go index 7f1dc9728372..e2a0bcdc8e85 100644 --- a/identity/handler_test.go +++ b/identity/handler_test.go @@ -3,6 +3,7 @@ package identity_test import ( "bytes" "encoding/json" + "github.com/bxcodec/faker/v3" "io/ioutil" "net/http" "net/http/httptest" @@ -151,19 +152,19 @@ func TestHandler(t *testing.T) { ur := identity.UpdateIdentity{ Traits: []byte(`{"bar":"baz","foo":"baz"}`), SchemaID: i.SchemaID, - State: identity.StateDisabled, + State: identity.StateInactive, } 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) - assert.EqualValues(t, identity.StateDisabled, res.Get("state").String(), "%s", res.Raw) + assert.EqualValues(t, identity.StateInactive, res.Get("state").String(), "%s", res.Raw) assert.NotEqualValues(t, i.StateChangedAt, sqlxx.NullTime(res.Get("state_changed_at").Time()), "%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) - assert.EqualValues(t, identity.StateDisabled, res.Get("state").String(), "%s", res.Raw) + assert.EqualValues(t, identity.StateInactive, res.Get("state").String(), "%s", res.Raw) assert.NotEqualValues(t, i.StateChangedAt, sqlxx.NullTime(res.Get("state_changed_at").Time()), "%s", res.Raw) }) @@ -238,9 +239,9 @@ func TestHandler(t *testing.T) { id := res.Get("id").String() res = send(t, "PUT", "/identities/"+id, http.StatusBadRequest, &identity.UpdateIdentity{ State: "invalid-state", - Traits: []byte(`{"email":"` + x.NewUUID().String() + `", "department": "ory"}`), + Traits: []byte(`{"email":"` + faker.Email() + `", "department": "ory"}`), }) - assert.Contains(t, res.Get("error.reason").String(), `Identity state is not valid.`, "%s", res.Raw) + assert.Contains(t, res.Get("error.reason").String(), `identity state is not valid`, "%s", res.Raw) }) t.Run("case=should update the schema id", func(t *testing.T) { diff --git a/identity/identity.go b/identity/identity.go index 9897d3aa4409..eac3fe72531c 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -20,22 +20,22 @@ import ( "github.com/ory/kratos/x" ) -// swagger:model identityState +// swagger:enum State type State string +const ( + StateActive State = "active" + StateInactive State = "inactive" +) + func (lt State) IsValid() error { switch lt { - case StateActive, StateDisabled: + case StateActive, StateInactive: return nil } - return errors.New("Identity state is not valid.") + return errors.New("identity state is not valid") } -const ( - StateActive State = "active" - StateDisabled State = "disabled" -) - // Identity represents an Ory Kratos identity // // An identity can be a real human, a service, an IoT device - everything that @@ -69,6 +69,9 @@ type Identity struct { // State is the identity's state. // + // enum: + // - active + // - inactive // required: true State State `json:"state" faker:"-" db:"state"` diff --git a/internal/httpclient/api/openapi.yaml b/internal/httpclient/api/openapi.yaml index a1ed1ef9def4..583f128c9406 100644 --- a/internal/httpclient/api/openapi.yaml +++ b/internal/httpclient/api/openapi.yaml @@ -2487,7 +2487,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active properties: created_at: description: CreatedAt is a helper struct field for gobuffalo.pop. @@ -2514,6 +2514,10 @@ components: format: url type: string state: + description: State is the identity's state. + enum: + - active + - inactive type: string state_changed_at: format: date-time @@ -2570,8 +2574,6 @@ components: format: date-time type: string type: object - identityState: - type: string identityTraits: description: |- Traits represent an identity's traits. The identity is able to create, modify, and delete traits @@ -2752,7 +2754,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active authenticated_at: 2000-01-23T04:56:07.000+00:00 active: true id: id @@ -3029,7 +3031,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active session: expires_at: 2000-01-23T04:56:07.000+00:00 identity: @@ -3068,7 +3070,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active authenticated_at: 2000-01-23T04:56:07.000+00:00 active: true id: id @@ -3174,7 +3176,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active authenticated_at: 2000-01-23T04:56:07.000+00:00 active: true id: id @@ -3294,7 +3296,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active active: active id: id state: state @@ -3406,7 +3408,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active flow: expires_at: 2000-01-23T04:56:07.000+00:00 ui: @@ -3492,7 +3494,7 @@ components: created_at: 2000-01-23T04:56:07.000+00:00 schema_url: schema_url id: id - state: state + state: active active: active id: id state: state @@ -3910,6 +3912,10 @@ components: will update the Identity's SchemaID. type: string state: + description: State is the identity's state. + enum: + - active + - inactive type: string traits: description: |- diff --git a/internal/httpclient/docs/Identity.md b/internal/httpclient/docs/Identity.md index 8f60e0f7b943..cea64d2769e2 100644 --- a/internal/httpclient/docs/Identity.md +++ b/internal/httpclient/docs/Identity.md @@ -9,7 +9,7 @@ Name | Type | Description | Notes **RecoveryAddresses** | Pointer to [**[]RecoveryAddress**](RecoveryAddress.md) | RecoveryAddresses contains all the addresses that can be used to recover an identity. | [optional] **SchemaId** | **string** | SchemaID is the ID of the JSON Schema to be used for validating the identity's traits. | **SchemaUrl** | **string** | SchemaURL is the URL of the endpoint where the identity's traits schema can be fetched from. format: url | -**State** | **string** | | +**State** | **string** | State is the identity's state. | **StateChangedAt** | Pointer to **time.Time** | | [optional] **Traits** | **interface{}** | 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`. | **UpdatedAt** | Pointer to **time.Time** | UpdatedAt is a helper struct field for gobuffalo.pop. | [optional] diff --git a/internal/httpclient/docs/UpdateIdentity.md b/internal/httpclient/docs/UpdateIdentity.md index 1b11968f1ed0..9dbf8d55cf67 100644 --- a/internal/httpclient/docs/UpdateIdentity.md +++ b/internal/httpclient/docs/UpdateIdentity.md @@ -5,7 +5,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- **SchemaId** | Pointer to **string** | 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. | [optional] -**State** | **string** | | +**State** | **string** | State is the identity's state. | **Traits** | **map[string]interface{}** | 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`. | ## Methods diff --git a/internal/httpclient/model_identity.go b/internal/httpclient/model_identity.go index cd91103bb61c..de9fab591c7d 100644 --- a/internal/httpclient/model_identity.go +++ b/internal/httpclient/model_identity.go @@ -26,7 +26,8 @@ type Identity struct { // SchemaID is the ID of the JSON Schema to be used for validating the identity's traits. SchemaId string `json:"schema_id"` // SchemaURL is the URL of the endpoint where the identity's traits schema can be fetched from. format: url - SchemaUrl string `json:"schema_url"` + SchemaUrl string `json:"schema_url"` + // State is the identity's state. State string `json:"state"` StateChangedAt *time.Time `json:"state_changed_at,omitempty"` // 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`. diff --git a/internal/httpclient/model_update_identity.go b/internal/httpclient/model_update_identity.go index 1e2be8385164..2bab91954ca3 100644 --- a/internal/httpclient/model_update_identity.go +++ b/internal/httpclient/model_update_identity.go @@ -19,7 +19,8 @@ import ( type UpdateIdentity 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,omitempty"` - State string `json:"state"` + // State is the identity's state. + State string `json:"state"` // 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`. Traits map[string]interface{} `json:"traits"` } diff --git a/session/manager_http_test.go b/session/manager_http_test.go index 3f5973f3a5f8..db8c4b2c831d 100644 --- a/session/manager_http_test.go +++ b/session/manager_http_test.go @@ -100,9 +100,10 @@ func TestManagerHTTP(t *testing.T) { t.Run("case=valid bearer auth as fallback", func(t *testing.T) { conf.MustSet(config.ViperKeySessionLifespan, "1m") - i := identity.Identity{Traits: []byte("{}")} + i := identity.Identity{Traits: []byte("{}"), State: identity.StateActive} require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i)) - s = session.NewActiveSession(&i, conf, time.Now()) + s, err := session.NewActiveSession(&i, conf, time.Now()) + require.NoError(t, err) require.NoError(t, reg.SessionPersister().CreateSession(context.Background(), s)) require.NotEmpty(t, s.Token) @@ -119,9 +120,10 @@ func TestManagerHTTP(t *testing.T) { t.Run("case=valid x-session-token auth even if bearer is set", func(t *testing.T) { conf.MustSet(config.ViperKeySessionLifespan, "1m") - i := identity.Identity{Traits: []byte("{}")} + i := identity.Identity{Traits: []byte("{}"), State: identity.StateActive} require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &i)) - s = session.NewActiveSession(&i, conf, time.Now()) + s, err := session.NewActiveSession(&i, conf, time.Now()) + require.NoError(t, err) require.NoError(t, reg.SessionPersister().CreateSession(context.Background(), s)) req, err := http.NewRequest("GET", pts.URL+"/session/get", nil) diff --git a/spec/openapi.json b/spec/openapi.json index 846fa3b68d49..e075834b0b18 100755 --- a/spec/openapi.json +++ b/spec/openapi.json @@ -1042,7 +1042,12 @@ "type": "string" }, "state": { - "$ref": "#/components/schemas/identityState" + "description": "State is the identity's state.", + "enum": [ + "active", + "inactive" + ], + "type": "string" }, "state_changed_at": { "$ref": "#/components/schemas/NullTime" @@ -1103,9 +1108,6 @@ }, "type": "object" }, - "identityState": { - "type": "string" - }, "identityTraits": { "description": "Traits represent an identity's traits. The identity is able to create, modify, and delete traits\nin a self-service manner. The input will always be validated against the JSON Schema defined\nin `schema_url`." }, @@ -1868,7 +1870,12 @@ "type": "string" }, "state": { - "$ref": "#/components/schemas/identityState" + "description": "State is the identity's state.", + "enum": [ + "active", + "inactive" + ], + "type": "string" }, "traits": { "description": "Traits represent an identity's traits. The identity is able to create, modify, and delete traits\nin a self-service manner. The input will always be validated against the JSON Schema defined\nin `schema_id`.", diff --git a/spec/swagger.json b/spec/swagger.json index 5d6f6d6ed346..352fdc353852 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -2644,7 +2644,12 @@ "type": "string" }, "state": { - "$ref": "#/definitions/identityState" + "description": "State is the identity's state.", + "type": "string", + "enum": [ + "active", + "inactive" + ] }, "state_changed_at": { "$ref": "#/definitions/NullTime" @@ -2696,9 +2701,6 @@ } } }, - "identityState": { - "type": "string" - }, "identityTraits": { "description": "Traits represent an identity's traits. The identity is able to create, modify, and delete traits\nin a self-service manner. The input will always be validated against the JSON Schema defined\nin `schema_url`.", "type": "object" @@ -3422,7 +3424,12 @@ "type": "string" }, "state": { - "$ref": "#/definitions/identityState" + "description": "State is the identity's state.", + "type": "string", + "enum": [ + "active", + "inactive" + ] }, "traits": { "description": "Traits represent an identity's traits. The identity is able to create, modify, and delete traits\nin a self-service manner. The input will always be validated against the JSON Schema defined\nin `schema_id`.",