Skip to content

Commit

Permalink
feat(identities): apply code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
thomasruiz committed May 19, 2021
1 parent 36c6fd0 commit 2336eec
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 48 deletions.
19 changes: 12 additions & 7 deletions docs/docs/concepts/identity-data-model.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.

<Mermaid
chart={`
stateDiagram-v2
[*] --> Active: create
Active --> Active: update
Active --> Disabled: disable
Disabled --> [*]: delete
Disabled --> Active: enable
Active --> Inactive: disable
Inactive --> [*]: delete
Inactive --> Active: enable
`}
/>
Expand Down
11 changes: 6 additions & 5 deletions identity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package identity_test
import (
"bytes"
"encoding/json"
"github.com/bxcodec/faker/v3"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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)
})

Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 11 additions & 8 deletions identity/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"`

Expand Down
26 changes: 16 additions & 10 deletions internal/httpclient/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: |-
Expand Down
2 changes: 1 addition & 1 deletion internal/httpclient/docs/Identity.md
Original file line number Diff line number Diff line change
Expand Up @@ -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&#39;s traits. |
**SchemaUrl** | **string** | SchemaURL is the URL of the endpoint where the identity&#39;s traits schema can be fetched from. format: url |
**State** | **string** | |
**State** | **string** | State is the identity&#39;s state. |
**StateChangedAt** | Pointer to **time.Time** | | [optional]
**Traits** | **interface{}** | Traits represent an identity&#39;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 &#x60;schema_url&#x60;. |
**UpdatedAt** | Pointer to **time.Time** | UpdatedAt is a helper struct field for gobuffalo.pop. | [optional]
Expand Down
2 changes: 1 addition & 1 deletion internal/httpclient/docs/UpdateIdentity.md
Original file line number Diff line number Diff line change
Expand Up @@ -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&#39;s traits. If set will update the Identity&#39;s SchemaID. | [optional]
**State** | **string** | |
**State** | **string** | State is the identity&#39;s state. |
**Traits** | **map[string]interface{}** | Traits represent an identity&#39;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 &#x60;schema_id&#x60;. |

## Methods
Expand Down
3 changes: 2 additions & 1 deletion internal/httpclient/model_identity.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion internal/httpclient/model_update_identity.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions session/manager_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
17 changes: 12 additions & 5 deletions spec/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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`."
},
Expand Down Expand Up @@ -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`.",
Expand Down
17 changes: 12 additions & 5 deletions spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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`.",
Expand Down

0 comments on commit 2336eec

Please sign in to comment.