From 7ceb1bca546f1c42d670faf3b5c424986a922637 Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Thu, 24 Feb 2022 17:24:21 +0100 Subject: [PATCH] feat: password, social sign, verified email in import This patch introduces the ability to import passwords (cleartext, PKBDF2, Argon2, BCrypt) and Social Sign In connections when creating identities! Closes #605 --- ...o_import_users-with_argon2id_password.json | 17 ++++ ...to_import_users-with_bcrypt2_password.json | 17 ++++ ...eartext_password_and_oidc_credentials.json | 42 +++++++++ ..._to_import_users-with_pkbdf2_password.json | 17 ++++ ..._import_users-without_any_credentials.json | 16 ++++ identity/handler.go | 34 ++++++-- identity/handler_import.go | 86 +++++++++++++++++++ identity/handler_test.go | 77 ++++++++++++++++- selfservice/strategy/password/registration.go | 6 ++ selfservice/strategy/password/settings.go | 4 +- 10 files changed, 300 insertions(+), 16 deletions(-) create mode 100644 identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_argon2id_password.json create mode 100644 identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_bcrypt2_password.json create mode 100644 identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_cleartext_password_and_oidc_credentials.json create mode 100644 identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_pkbdf2_password.json create mode 100644 identity/.snapshots/TestHandler-case=should_be_able_to_import_users-without_any_credentials.json create mode 100644 identity/handler_import.go diff --git a/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_argon2id_password.json b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_argon2id_password.json new file mode 100644 index 000000000000..cf7eaa05345f --- /dev/null +++ b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_argon2id_password.json @@ -0,0 +1,17 @@ +{ + "credentials": { + "password": { + "type": "password", + "identifiers": [ + "import-5@ory.sh" + ], + "config": { + } + } + }, + "schema_id": "default", + "state": "active", + "traits": { + "email": "import-5@ory.sh" + } +} diff --git a/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_bcrypt2_password.json b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_bcrypt2_password.json new file mode 100644 index 000000000000..32bd0337b665 --- /dev/null +++ b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_bcrypt2_password.json @@ -0,0 +1,17 @@ +{ + "credentials": { + "password": { + "type": "password", + "identifiers": [ + "import-4@ory.sh" + ], + "config": { + } + } + }, + "schema_id": "default", + "state": "active", + "traits": { + "email": "import-4@ory.sh" + } +} diff --git a/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_cleartext_password_and_oidc_credentials.json b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_cleartext_password_and_oidc_credentials.json new file mode 100644 index 000000000000..b13a4c7f06bd --- /dev/null +++ b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_cleartext_password_and_oidc_credentials.json @@ -0,0 +1,42 @@ +{ + "credentials": { + "oidc": { + "type": "oidc", + "identifiers": [ + "google:import-2", + "github:import-2" + ], + "config": { + "providers": [ + { + "subject": "import-2", + "provider": "google", + "initial_id_token": "", + "initial_access_token": "", + "initial_refresh_token": "" + }, + { + "subject": "import-2", + "provider": "github", + "initial_id_token": "", + "initial_access_token": "", + "initial_refresh_token": "" + } + ] + } + }, + "password": { + "type": "password", + "identifiers": [ + "import-2@ory.sh" + ], + "config": { + } + } + }, + "schema_id": "default", + "state": "active", + "traits": { + "email": "import-2@ory.sh" + } +} diff --git a/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_pkbdf2_password.json b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_pkbdf2_password.json new file mode 100644 index 000000000000..9de95347dde9 --- /dev/null +++ b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-with_pkbdf2_password.json @@ -0,0 +1,17 @@ +{ + "credentials": { + "password": { + "type": "password", + "identifiers": [ + "import-3@ory.sh" + ], + "config": { + } + } + }, + "schema_id": "default", + "state": "active", + "traits": { + "email": "import-3@ory.sh" + } +} diff --git a/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-without_any_credentials.json b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-without_any_credentials.json new file mode 100644 index 000000000000..a8cf8009ba7e --- /dev/null +++ b/identity/.snapshots/TestHandler-case=should_be_able_to_import_users-without_any_credentials.json @@ -0,0 +1,16 @@ +{ + "credentials": { + "password": { + "type": "password", + "identifiers": [ + "import-1@ory.sh" + ], + "config": {} + } + }, + "schema_id": "default", + "state": "active", + "traits": { + "email": "import-1@ory.sh" + } +} diff --git a/identity/handler.go b/identity/handler.go index bd9d4f3dd1e2..d5e71a0f6ebc 100644 --- a/identity/handler.go +++ b/identity/handler.go @@ -3,6 +3,7 @@ package identity import ( "context" "encoding/json" + "github.com/ory/kratos/hash" "net/http" "time" @@ -34,6 +35,7 @@ type ( config.Provider x.CSRFProvider cipher.Provider + hash.HashProvider } HandlerProvider interface { IdentityHandler() *Handler @@ -236,7 +238,7 @@ type AdminIdentityImportCredentials struct { OIDC *AdminIdentityImportCredentialsOIDC `json:"oidc,omitempty"` } -// swagger:model AdminCreateIdentityImportCredentialsPassword +// swagger:model adminCreateIdentityImportCredentialsPassword type AdminIdentityImportCredentialsPassword struct { // The hashed password in [PHC format]( https://www.ory.sh/docs/kratos/concepts/credentials/username-email-password#hashed-password-format) HashedPassword string `json:"hashed_password"` @@ -245,12 +247,22 @@ type AdminIdentityImportCredentialsPassword struct { Password string `json:"password"` } -// swagger:model AdminCreateIdentityImportCredentialsOIDC +// swagger:model adminCreateIdentityImportCredentialsOidc type AdminIdentityImportCredentialsOIDC struct { + // A list of OpenID Connect Providers + Providers []AdminCreateIdentityImportCredentialsOidcProvider `json:"providers"` +} + +// swagger:model adminCreateIdentityImportCredentialsOidcProvider +type AdminCreateIdentityImportCredentialsOidcProvider struct { // The subject (`sub`) of the OpenID Connect connection. Usually the `sub` field of the ID Token. + // + // required: true Subject string `json:"subject"` // The OpenID Connect provider to link the subject to. Usually something like `google` or `github`. + // + // required: true Provider string `json:"provider"` } @@ -297,16 +309,18 @@ func (h *Handler) create(w http.ResponseWriter, r *http.Request, _ httprouter.Pa } i := &Identity{ - SchemaID: cr.SchemaID, - Traits: []byte(cr.Traits), - State: state, - StateChangedAt: &stateChangedAt, - //Credentials: cr.Credentials, + SchemaID: cr.SchemaID, + Traits: []byte(cr.Traits), + State: state, + StateChangedAt: &stateChangedAt, VerifiableAddresses: cr.VerifiableAddresses, RecoveryAddresses: cr.RecoveryAddresses, } - //i.Traits = identity.Traits(p.Traits) - //i.SetCredentials(s.ID(), identity.Credentials{Type: s.ID(), Identifiers: []string{}, Config: co}) + + if err := h.importCredentials(r.Context(), i, cr.Credentials); err != nil { + h.r.Writer().WriteError(w, r, err) + return + } if err := h.r.IdentityManager().Create(r.Context(), i); err != nil { h.r.Writer().WriteError(w, r, err) @@ -339,6 +353,8 @@ type adminUpdateIdentity struct { type AdminUpdateIdentityBody 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. + // + // required: true SchemaID string `json:"schema_id"` // Traits represent an identity's traits. The identity is able to create, modify, and delete traits diff --git a/identity/handler_import.go b/identity/handler_import.go new file mode 100644 index 000000000000..2aed1f5c382c --- /dev/null +++ b/identity/handler_import.go @@ -0,0 +1,86 @@ +package identity + +import ( + "context" + "encoding/json" + "github.com/ory/herodot" + "github.com/ory/kratos/hash" + "github.com/ory/kratos/x" + "github.com/pkg/errors" +) + +func (h *Handler) importCredentials(ctx context.Context, i *Identity, creds *AdminIdentityImportCredentials) error { + if creds == nil { + return nil + } + + if creds.Password != nil { + if err := h.importPasswordCredentials(ctx, i, creds.Password); err != nil { + return err + } + } + + if creds.OIDC != nil { + if err := h.importOIDCCredentials(ctx, i, creds.OIDC); err != nil { + return err + } + } + + return nil +} + +func (h *Handler) importPasswordCredentials(ctx context.Context, i *Identity, creds *AdminIdentityImportCredentialsPassword) (err error) { + // In here we deliberately ignore any password policies as the point here is to import passwords, even if they + // are not matching the policy, as the user needs to able to sign in with their old password. + hashed := []byte(creds.HashedPassword) + if len(creds.Password) > 0 { + // Importing a clear text password + hashed, err = h.r.Hasher().Generate(ctx, []byte(creds.Password)) + if err != nil { + return err + } + + creds.HashedPassword = string(hashed) + } + + if !(hash.IsArgon2idHash(hashed) || hash.IsBcryptHash(hashed) || hash.IsPbkdf2Hash(hashed)) { + return errors.WithStack(herodot.ErrBadRequest.WithReasonf("The imported password does not match any known hash format. For more information see https://www.ory.sh/dr/2")) + } + + return i.SetCredentialsWithConfig(CredentialsTypePassword, Credentials{}, CredentialsPassword{HashedPassword: string(hashed)}) +} + +func (h *Handler) importOIDCCredentials(_ context.Context, i *Identity, creds *AdminIdentityImportCredentialsOIDC) error { + var target CredentialsOIDC + c, ok := i.GetCredentials(CredentialsTypeOIDC) + if !ok { + var providers []CredentialsOIDCProvider + var ids []string + for _, p := range creds.Providers { + ids = append(ids, OIDCUniqueID(p.Provider, p.Subject)) + providers = append(providers, CredentialsOIDCProvider{ + Subject: p.Subject, + Provider: p.Provider, + }) + } + + return i.SetCredentialsWithConfig( + CredentialsTypeOIDC, + Credentials{Identifiers: ids}, + CredentialsOIDC{Providers: providers}, + ) + } + + if err := json.Unmarshal(c.Config, &target); err != nil { + return errors.WithStack(x.PseudoPanic.WithWrap(err)) + } + + for _, p := range creds.Providers { + c.Identifiers = append(c.Identifiers, OIDCUniqueID(p.Provider, p.Subject)) + target.Providers = append(target.Providers, CredentialsOIDCProvider{ + Subject: p.Subject, + Provider: p.Provider, + }) + } + return i.SetCredentialsWithConfig(CredentialsTypeOIDC, *c, &target) +} diff --git a/identity/handler_test.go b/identity/handler_test.go index 03d5cc7ef0c8..bd54fc1a0834 100644 --- a/identity/handler_test.go +++ b/identity/handler_test.go @@ -5,6 +5,9 @@ import ( "context" "encoding/json" "fmt" + "github.com/gofrs/uuid" + "github.com/ory/kratos/hash" + "github.com/ory/x/snapshotx" "io/ioutil" "net/http" "net/http/httptest" @@ -93,17 +96,14 @@ func TestHandler(t *testing.T) { }) t.Run("case=should return 404 on a non-existing resource", func(t *testing.T) { - for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} { t.Run("endpoint="+name, func(t *testing.T) { _ = get(t, ts, "/identities/does-not-exist", http.StatusNotFound) - }) } }) t.Run("case=should fail to create an identity because schema id does not exist", func(t *testing.T) { - for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} { t.Run("endpoint="+name, func(t *testing.T) { var i identity.AdminCreateIdentityBody @@ -122,7 +122,6 @@ func TestHandler(t *testing.T) { i.Traits = []byte(`{"bar":123}`) res := send(t, ts, "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") - }) } }) @@ -150,6 +149,76 @@ func TestHandler(t *testing.T) { } }) + t.Run("case=should be able to import users", func(t *testing.T) { + ignoreDefault := []string{"id", "schema_url", "state_changed_at", "created_at", "updated_at"} + t.Run("without any credentials", func(t *testing.T) { + res := send(t, adminTS, "POST", "/identities", http.StatusCreated, identity.AdminCreateIdentityBody{Traits: []byte(`{"email": "import-1@ory.sh"}`)}) + actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String())) + require.NoError(t, err) + + snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsInJSON(*actual), ignoreDefault) + }) + + t.Run("with cleartext password and oidc credentials", func(t *testing.T) { + res := send(t, adminTS, "POST", "/identities", http.StatusCreated, identity.AdminCreateIdentityBody{Traits: []byte(`{"email": "import-2@ory.sh"}`), + Credentials: &identity.AdminIdentityImportCredentials{ + Password: &identity.AdminIdentityImportCredentialsPassword{ + Password: "123456", + }, + OIDC: &identity.AdminIdentityImportCredentialsOIDC{ + Providers: []identity.AdminCreateIdentityImportCredentialsOidcProvider{ + {Subject: "import-2", Provider: "google"}, + {Subject: "import-2", Provider: "github"}, + }, + }, + }, + }) + + actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String())) + require.NoError(t, err) + + snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsInJSON(*actual), append(ignoreDefault, "hashed_password")) + + require.NoError(t, hash.Compare(ctx, []byte("123456"), []byte(gjson.GetBytes(actual.Credentials[identity.CredentialsTypePassword].Config, "hashed_password").String()))) + }) + + t.Run("with pkbdf2 password", func(t *testing.T) { + res := send(t, adminTS, "POST", "/identities", http.StatusCreated, identity.AdminCreateIdentityBody{Traits: []byte(`{"email": "import-3@ory.sh"}`), + Credentials: &identity.AdminIdentityImportCredentials{Password: &identity.AdminIdentityImportCredentialsPassword{ + HashedPassword: "$pbkdf2-sha256$i=1000,l=128$e8/arsEf4cvQihdNgqj0Nw$5xQQKNTyeTHx2Ld5/JDE7A"}}}) + actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String())) + require.NoError(t, err) + + snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsInJSON(*actual), append(ignoreDefault, "hashed_password")) + + require.NoError(t, hash.Compare(ctx, []byte("123456"), []byte(gjson.GetBytes(actual.Credentials[identity.CredentialsTypePassword].Config, "hashed_password").String()))) + }) + + t.Run("with bcrypt2 password", func(t *testing.T) { + res := send(t, adminTS, "POST", "/identities", http.StatusCreated, identity.AdminCreateIdentityBody{Traits: []byte(`{"email": "import-4@ory.sh"}`), + Credentials: &identity.AdminIdentityImportCredentials{Password: &identity.AdminIdentityImportCredentialsPassword{ + HashedPassword: "$2a$10$ZsCsoVQ3xfBG/K2z2XpBf.tm90GZmtOqtqWcB5.pYd5Eq8y7RlDyq"}}}) + actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String())) + require.NoError(t, err) + + snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsInJSON(*actual), append(ignoreDefault, "hashed_password")) + + require.NoError(t, hash.Compare(ctx, []byte("123456"), []byte(gjson.GetBytes(actual.Credentials[identity.CredentialsTypePassword].Config, "hashed_password").String()))) + }) + + t.Run("with argon2id password", func(t *testing.T) { + res := send(t, adminTS, "POST", "/identities", http.StatusCreated, identity.AdminCreateIdentityBody{Traits: []byte(`{"email": "import-5@ory.sh"}`), + Credentials: &identity.AdminIdentityImportCredentials{Password: &identity.AdminIdentityImportCredentialsPassword{ + HashedPassword: "$argon2id$v=19$m=16,t=2,p=1$bVI1aE1SaTV6SGQ3bzdXdw$fnjCcZYmEPOUOjYXsT92Cg"}}}) + actual, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, uuid.FromStringOrNil(res.Get("id").String())) + require.NoError(t, err) + + snapshotx.SnapshotTExceptMatchingKeys(t, identity.WithCredentialsInJSON(*actual), append(ignoreDefault, "hashed_password")) + + require.NoError(t, hash.Compare(ctx, []byte("123456"), []byte(gjson.GetBytes(actual.Credentials[identity.CredentialsTypePassword].Config, "hashed_password").String()))) + }) + }) + t.Run("case=unable to set ID itself", func(t *testing.T) { for name, ts := range map[string]*httptest.Server{"public": publicTS, "admin": adminTS} { t.Run("endpoint="+name, func(t *testing.T) { diff --git a/selfservice/strategy/password/registration.go b/selfservice/strategy/password/registration.go index f449f9831c21..bc4e90ed67d4 100644 --- a/selfservice/strategy/password/registration.go +++ b/selfservice/strategy/password/registration.go @@ -139,6 +139,12 @@ func (s *Strategy) validateCredentials(ctx context.Context, i *identity.Identity return schema.NewMissingIdentifierError() } + // Sometimes, no identifier is set, but we still want to validate the password! + ids := c.Identifiers + if len(ids) == 0 { + ids = []string{""} + } + for _, id := range c.Identifiers { if err := s.d.PasswordValidator().Validate(ctx, id, pw); err != nil { if _, ok := errorsx.Cause(err).(*herodot.DefaultError); ok { diff --git a/selfservice/strategy/password/settings.go b/selfservice/strategy/password/settings.go index 08b5271e6ba5..ee0c1f3e41dd 100644 --- a/selfservice/strategy/password/settings.go +++ b/selfservice/strategy/password/settings.go @@ -129,9 +129,7 @@ func (s *Strategy) continueSettingsFlow( c, ok := i.GetCredentials(s.ID()) if !ok { - c = &identity.Credentials{Type: s.ID(), - // We need to insert a random identifier now... - Identifiers: []string{x.NewUUID().String()}} + c = &identity.Credentials{Type: s.ID()} } if err := i.SetCredentialsWithConfig(s.ID(), *c, &identity.CredentialsPassword{HashedPassword: string(hpw)}); err != nil {