diff --git a/identity/handler.go b/identity/handler.go index 3ac641e09377..cf4d3ff835e6 100644 --- a/identity/handler.go +++ b/identity/handler.go @@ -910,69 +910,36 @@ func (h *Handler) patch(w http.ResponseWriter, r *http.Request, ps httprouter.Pa h.r.Writer().Write(w, r, WithCredentialsMetadataAndAdminMetadataInJSON(updatedIdentity)) } -func deletCredentialWebAuthFromIdentity(identity *Identity) (*Identity, error) { - cred, ok := identity.GetCredentials(CredentialsTypeWebAuthn) - if !ok { - // This should never happend as it's checked earlier in the code; - // But we never know... - return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf("You tried to remove a CredentialsTypeWebAuthn but this user have no CredentialsTypeWebAuthn set up.")) - } - - var cc CredentialsWebAuthnConfig - if err := json.Unmarshal(cred.Config, &cc); err != nil { - // Database has been tampered or the json schema are incompatible (migration issue); - return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode identity credentials.").WithDebug(err.Error())) - } - - updated := make([]CredentialWebAuthn, 0) - for k, cred := range cc.Credentials { - if cred.IsPasswordless { - updated = append(updated, cc.Credentials[k]) - } - } - - if len(updated) == 0 { - identity.DeleteCredentialsType(CredentialsTypeWebAuthn) - return identity, nil - } - - cc.Credentials = updated - message, err := json.Marshal(cc) - if err != nil { - return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode identity credentials.").WithDebug(err.Error())) - } - - cred.Config = message - identity.SetCredentials(CredentialsTypeWebAuthn, *cred) - return identity, nil -} - // Delete Credential Parameters // // swagger:parameters deleteIdentityCredentials -// -//nolint:deadcode,unused -//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions -type deleteIdentityCredentials struct { +type _ struct { // ID is the identity's ID. // // required: true // in: path ID string `json:"id"` - // Type is the type of credentials to be deleted. + // Type is the type of credentials to delete. // // required: true // in: path Type CredentialsType `json:"type"` + + // Identifier is the identifier of the OIDC credential to delete. + // Find the identifier by calling the `GET /admin/identities/{id}?include_credential=oidc` endpoint. + // + // required: false + // in: query + Identifier string `json:"identifier"` } // swagger:route DELETE /admin/identities/{id}/credentials/{type} identity deleteIdentityCredentials // // # Delete a credential for a specific identity // -// Delete an [identity](https://www.ory.sh/docs/kratos/concepts/identity-user-model) credential by its type -// You can only delete second factor (aal2) credentials. +// Delete an [identity](https://www.ory.sh/docs/kratos/concepts/identity-user-model) credential by its type. +// You cannot delete password or code auth credentials through this API. // // Consumes: // - application/json @@ -1006,14 +973,18 @@ func (h *Handler) deleteIdentityCredentials(w http.ResponseWriter, r *http.Reque case CredentialsTypeLookup, CredentialsTypeTOTP: identity.DeleteCredentialsType(cred.Type) case CredentialsTypeWebAuthn: - identity, err = deletCredentialWebAuthFromIdentity(identity) - if err != nil { + if err = identity.deleteCredentialWebAuthFromIdentity(); err != nil { h.r.Writer().WriteError(w, r, err) return } - case CredentialsTypeOIDC, CredentialsTypePassword, CredentialsTypeCodeAuth: - h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("You can't remove first factor credentials."))) + case CredentialsTypePassword, CredentialsTypeCodeAuth: + h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("You cannot remove first factor credentials."))) return + case CredentialsTypeOIDC: + if err := identity.deleteCredentialOIDCFromIdentity(r.URL.Query().Get("identifier")); err != nil { + h.r.Writer().WriteError(w, r, err) + return + } default: h.r.Writer().WriteError(w, r, errors.WithStack(herodot.ErrBadRequest.WithReasonf("Unknown credentials type %s.", cred.Type))) return diff --git a/identity/handler_test.go b/identity/handler_test.go index ab2ed0c0cbdc..53ca219b231b 100644 --- a/identity/handler_test.go +++ b/identity/handler_test.go @@ -32,6 +32,8 @@ import ( "github.com/ory/kratos/internal/testhelpers" "github.com/ory/kratos/schema" "github.com/ory/kratos/x" + "github.com/ory/x/ioutilx" + "github.com/ory/x/randx" "github.com/ory/x/snapshotx" "github.com/ory/x/sqlxx" "github.com/ory/x/urlx" @@ -81,8 +83,9 @@ func TestHandler(t *testing.T) { res, err := base.Client().Do(req) require.NoError(t, err) + defer res.Body.Close() - require.EqualValues(t, expectCode, res.StatusCode) + require.EqualValues(t, expectCode, res.StatusCode, "%s", ioutilx.MustReadAll(res.Body)) } send := func(t *testing.T, base *httptest.Server, method, href string, expectCode int, send interface{}) gjson.Result { @@ -1497,15 +1500,15 @@ func TestHandler(t *testing.T) { t.Run("case=should delete credential of a specific user and no longer be able to retrieve it", func(t *testing.T) { ignoreDefault := []string{"id", "schema_url", "state_changed_at", "created_at", "updated_at"} - createIdentity := func(identities map[identity.CredentialsType]string) func(t *testing.T) *identity.Identity { + type M = map[identity.CredentialsType]identity.Credentials + createIdentity := func(creds M) func(*testing.T) *identity.Identity { return func(t *testing.T) *identity.Identity { i := identity.NewIdentity("") - for ct, config := range identities { - i.SetCredentials(ct, identity.Credentials{ - Type: ct, - Config: sqlxx.JSONRawMessage(config), - }) + for k, v := range creds { + v.Type = k + creds[k] = v } + i.Credentials = creds i.Traits = identity.Traits("{}") require.NoError(t, reg.Persister().CreateIdentity(context.Background(), i)) return i @@ -1516,26 +1519,83 @@ func TestHandler(t *testing.T) { remove(t, ts, "/identities/"+x.NewUUID().String()+"/credentials/azerty", http.StatusNotFound) }) t.Run("type=remove unknown type/"+name, func(t *testing.T) { - i := createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypePassword: `{"secret":"pst"}`, + i := createIdentity(M{ + identity.CredentialsTypePassword: {Config: []byte(`{"secret":"pst"}`)}, })(t) remove(t, ts, "/identities/"+i.ID.String()+"/credentials/azerty", http.StatusNotFound) }) t.Run("type=remove password type/"+name, func(t *testing.T) { - i := createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypePassword: `{"secret":"pst"}`, + i := createIdentity(M{ + identity.CredentialsTypePassword: {Config: []byte(`{"secret":"pst"}`)}, })(t) remove(t, ts, "/identities/"+i.ID.String()+"/credentials/password", http.StatusBadRequest) }) t.Run("type=remove oidc type/"+name, func(t *testing.T) { - i := createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypeOIDC: `{"id":"pst"}`, + // force ordering among github identifiers + githubSubject := "0" + randx.MustString(7, randx.Numeric) + githubSubject2 := "1" + randx.MustString(7, randx.Numeric) + googleSubject := randx.MustString(8, randx.Numeric) + initialConfig := []byte(fmt.Sprintf(`{ + "providers": [ + { + "subject": %q, + "provider": "github" + }, + { + "subject": %q, + "provider": "github" + }, + { + "subject": %q, + "provider": "google" + } + ] + }`, githubSubject, githubSubject2, googleSubject)) + identifiers := []string{ + identity.OIDCUniqueID("github", githubSubject), + identity.OIDCUniqueID("github", githubSubject2), + identity.OIDCUniqueID("google", googleSubject), + } + i := createIdentity(M{ + identity.CredentialsTypeOIDC: { + Identifiers: identifiers, + Config: initialConfig, + }, })(t) - remove(t, ts, "/identities/"+i.ID.String()+"/credentials/oidc", http.StatusBadRequest) + res := get(t, ts, "/identities/"+i.ID.String()+"?include_credential=oidc", http.StatusOK) + assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw) + assert.Len(t, res.Get("credentials.oidc.identifiers").Array(), 3, "%s", res.Raw) + assert.EqualValues(t, res.Get("credentials.oidc.identifiers.0").String(), identifiers[0], "%s", res.Raw) + assert.EqualValues(t, res.Get("credentials.oidc.identifiers.1").String(), identifiers[1], "%s", res.Raw) + assert.EqualValues(t, res.Get("credentials.oidc.identifiers.2").String(), identifiers[2], "%s", res.Raw) + + oidConfig := gjson.Parse(res.Get("credentials.oidc.config").String()) + assert.Len(t, res.Get("credentials.oidc.identifiers").Array(), 3, "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.0.provider").String(), "github", "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.0.subject").String(), githubSubject, "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.1.provider").String(), "github", "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.1.subject").String(), githubSubject2, "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.2.provider").String(), "google", "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.2.subject").String(), googleSubject, "%s", res.Raw) + + remove(t, ts, "/identities/"+i.ID.String()+"/credentials/oidc?identifier="+identifiers[1], http.StatusNoContent) + res = get(t, ts, "/identities/"+i.ID.String()+"?include_credential=oidc", http.StatusOK) + + assert.EqualValues(t, i.ID.String(), res.Get("id").String(), "%s", res.Raw) + assert.Len(t, res.Get("credentials.oidc.identifiers").Array(), 2, "%s", res.Raw) + assert.EqualValues(t, res.Get("credentials.oidc.identifiers.0").String(), identifiers[0], "%s", res.Raw) + assert.EqualValues(t, res.Get("credentials.oidc.identifiers.1").String(), identifiers[2], "%s", res.Raw) + + oidConfig = gjson.Parse(res.Get("credentials.oidc.config").String()) + assert.Len(t, res.Get("credentials.oidc.identifiers").Array(), 2, "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.0.provider").String(), "github", "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.0.subject").String(), githubSubject, "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.1.provider").String(), "google", "%s", res.Raw) + assert.EqualValues(t, oidConfig.Get("providers.1.subject").String(), googleSubject, "%s", res.Raw) }) t.Run("type=remove webauthn passwordless type/"+name, func(t *testing.T) { expected := `{"credentials":[{"id":"THTndqZP5Mjvae1BFvJMaMfEMm7O7HE1ju+7PBaYA7Y=","added_at":"2022-12-16T14:11:55Z","public_key":"pQECAyYgASFYIMJLQhJxQRzhnKPTcPCUODOmxYDYo2obrm9bhp5lvSZ3IlggXjhZvJaPUqF9PXqZqTdWYPR7R+b2n/Wi+IxKKXsS4rU=","display_name":"test","authenticator":{"aaguid":"rc4AAjW8xgpkiwsl8fBVAw==","sign_count":0,"clone_warning":false},"is_passwordless":true,"attestation_type":"none"}],"user_handle":"Ef5JiMpMRwuzauWs/9J0gQ=="}` - i := createIdentity(map[identity.CredentialsType]string{identity.CredentialsTypeWebAuthn: expected})(t) + i := createIdentity(M{identity.CredentialsTypeWebAuthn: {Config: []byte(expected)}})(t) remove(t, ts, "/identities/"+i.ID.String()+"/credentials/webauthn", http.StatusNoContent) // Check that webauthn has not been deleted res := get(t, ts, "/identities/"+i.ID.String(), http.StatusOK) @@ -1608,7 +1668,7 @@ func TestHandler(t *testing.T) { message, err := json.Marshal(config) require.NoError(t, err) - i := createIdentity(map[identity.CredentialsType]string{identity.CredentialsTypeWebAuthn: string(message)})(t) + i := createIdentity(M{identity.CredentialsTypeWebAuthn: {Config: message}})(t) remove(t, ts, "/identities/"+i.ID.String()+"/credentials/webauthn", http.StatusNoContent) // Check that webauthn has not been deleted res := get(t, ts, "/identities/"+i.ID.String(), http.StatusOK) @@ -1618,10 +1678,10 @@ func TestHandler(t *testing.T) { require.NoError(t, err) snapshotx.SnapshotT(t, identity.WithCredentialsAndAdminMetadataInJSON(*actual), snapshotx.ExceptNestedKeys(append(ignoreDefault, "hashed_password")...), snapshotx.ExceptPaths("credentials.oidc.identifiers")) }) - for ct, ctConf := range map[identity.CredentialsType]string{ - identity.CredentialsTypeLookup: `{"recovery_codes": [{"code": "aaa"}]}`, - identity.CredentialsTypeTOTP: `{"totp_url":"otpauth://totp/test"}`, - identity.CredentialsTypeWebAuthn: `{"credentials":[{"id":"THTndqZP5Mjvae1BFvJMaMfEMm7O7HE1ju+7PBaYA7Y=","added_at":"2022-12-16T14:11:55Z","public_key":"pQECAyYgASFYIMJLQhJxQRzhnKPTcPCUODOmxYDYo2obrm9bhp5lvSZ3IlggXjhZvJaPUqF9PXqZqTdWYPR7R+b2n/Wi+IxKKXsS4rU=","display_name":"test","authenticator":{"aaguid":"rc4AAjW8xgpkiwsl8fBVAw==","sign_count":0,"clone_warning":false},"is_passwordless":false,"attestation_type":"none"}],"user_handle":"Ef5JiMpMRwuzauWs/9J0gQ=="}`, + for ct, ctConf := range map[identity.CredentialsType][]byte{ + identity.CredentialsTypeLookup: []byte(`{"recovery_codes": [{"code": "aaa"}]}`), + identity.CredentialsTypeTOTP: []byte(`{"totp_url":"otpauth://totp/test"}`), + identity.CredentialsTypeWebAuthn: []byte(`{"credentials":[{"id":"THTndqZP5Mjvae1BFvJMaMfEMm7O7HE1ju+7PBaYA7Y=","added_at":"2022-12-16T14:11:55Z","public_key":"pQECAyYgASFYIMJLQhJxQRzhnKPTcPCUODOmxYDYo2obrm9bhp5lvSZ3IlggXjhZvJaPUqF9PXqZqTdWYPR7R+b2n/Wi+IxKKXsS4rU=","display_name":"test","authenticator":{"aaguid":"rc4AAjW8xgpkiwsl8fBVAw==","sign_count":0,"clone_warning":false},"is_passwordless":false,"attestation_type":"none"}],"user_handle":"Ef5JiMpMRwuzauWs/9J0gQ=="}`), } { t.Run("type=remove "+string(ct)+"/"+name, func(t *testing.T) { for _, tc := range []struct { @@ -1632,25 +1692,25 @@ func TestHandler(t *testing.T) { { desc: "with", exist: true, - setup: createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypePassword: `{"secret":"pst"}`, - ct: ctConf, + setup: createIdentity(M{ + identity.CredentialsTypePassword: {Config: []byte(`{"secret":"pst"}`)}, + ct: {Config: ctConf}, }), }, { desc: "without", exist: false, - setup: createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypePassword: `{"secret":"pst"}`, + setup: createIdentity(M{ + identity.CredentialsTypePassword: {Config: []byte(`{"secret":"pst"}`)}, }), }, { desc: "multiple", exist: true, - setup: createIdentity(map[identity.CredentialsType]string{ - identity.CredentialsTypePassword: `{"secret":"pst"}`, - identity.CredentialsTypeOIDC: `{"id":"pst"}`, - ct: ctConf, + setup: createIdentity(M{ + identity.CredentialsTypePassword: {Config: []byte(`{"secret":"pst"}`)}, + identity.CredentialsTypeOIDC: {Config: []byte(`{"id":"pst"}`)}, + ct: {Config: ctConf}, }), }, } { diff --git a/identity/identity.go b/identity/identity.go index 55dd66155ea9..3f692b831f2b 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -507,6 +507,80 @@ func (i *Identity) WithDeclassifiedCredentials(ctx context.Context, c cipher.Pro return &ii, nil } +func (i *Identity) deleteCredentialWebAuthFromIdentity() error { + cred, ok := i.GetCredentials(CredentialsTypeWebAuthn) + if !ok { + // This should never happend as it's checked earlier in the code; + // But we never know... + return errors.WithStack(herodot.ErrNotFound.WithReasonf("You tried to remove a WebAuthn credential but this user has no such credential set up.")) + } + + var cc CredentialsWebAuthnConfig + if err := json.Unmarshal(cred.Config, &cc); err != nil { + // Database has been tampered or the json schema are incompatible (migration issue); + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode identity credentials.").WithDebug(err.Error())) + } + + updated := make([]CredentialWebAuthn, 0) + for k, cred := range cc.Credentials { + if cred.IsPasswordless { + updated = append(updated, cc.Credentials[k]) + } + } + + if len(updated) == 0 { + i.DeleteCredentialsType(CredentialsTypeWebAuthn) + return nil + } + + cc.Credentials = updated + message, err := json.Marshal(cc) + if err != nil { + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode identity credentials.").WithDebug(err.Error())) + } + + cred.Config = message + i.SetCredentials(CredentialsTypeWebAuthn, *cred) + return nil +} + +func (i *Identity) deleteCredentialOIDCFromIdentity(identifierToDelete string) error { + if identifierToDelete == "" { + return errors.WithStack(herodot.ErrBadRequest.WithReasonf("You must provide an identifier to delete this credential.")) + } + _, hasOIDC := i.GetCredentials(CredentialsTypeOIDC) + if !hasOIDC { + return errors.WithStack(herodot.ErrNotFound.WithReasonf("You tried to remove an OIDC credential but this user has no such credential set up.")) + } + var oidcConfig CredentialsOIDC + creds, err := i.ParseCredentials(CredentialsTypeOIDC, &oidcConfig) + if err != nil { + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode identity credentials.").WithDebug(err.Error())) + } + + var updatedIdentifiers []string + var updatedProviders []CredentialsOIDCProvider + var found bool + for _, cfg := range oidcConfig.Providers { + if identifierToDelete == OIDCUniqueID(cfg.Provider, cfg.Subject) { + found = true + continue + } + updatedIdentifiers = append(updatedIdentifiers, OIDCUniqueID(cfg.Provider, cfg.Subject)) + updatedProviders = append(updatedProviders, cfg) + } + if !found { + return errors.WithStack(herodot.ErrNotFound.WithReasonf("The identifier `%s` was not found among OIDC credentials.", identifierToDelete)) + } + creds.Identifiers = updatedIdentifiers + creds.Config, err = json.Marshal(&CredentialsOIDC{Providers: updatedProviders}) + if err != nil { + return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode identity credentials.").WithDebug(err.Error())) + } + i.Credentials[CredentialsTypeOIDC] = *creds + return nil +} + // Patch Identities Parameters // // swagger:parameters batchPatchIdentities diff --git a/identity/identity_test.go b/identity/identity_test.go index 726011fd00eb..d14388feacd4 100644 --- a/identity/identity_test.go +++ b/identity/identity_test.go @@ -10,21 +10,16 @@ import ( "fmt" "testing" - "github.com/ory/x/snapshotx" - - "github.com/ory/kratos/cipher" - "github.com/ory/kratos/x" - + "github.com/gofrs/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tidwall/gjson" - "github.com/gofrs/uuid" - - "github.com/ory/x/sqlxx" - + "github.com/ory/kratos/cipher" "github.com/ory/kratos/driver/config" - - "github.com/stretchr/testify/assert" + "github.com/ory/kratos/x" + "github.com/ory/x/snapshotx" + "github.com/ory/x/sqlxx" ) func TestNewIdentity(t *testing.T) { @@ -387,3 +382,58 @@ func TestWithDeclassifiedCredentials(t *testing.T) { } }) } + +func TestDeleteCredentialOIDCFromIdentity(t *testing.T) { + i := NewIdentity(config.DefaultIdentityTraitsSchemaID) + + err := i.deleteCredentialOIDCFromIdentity("") + assert.Error(t, err) + err = i.deleteCredentialOIDCFromIdentity("does-not-exist") + assert.Error(t, err) + + credentials := map[CredentialsType]Credentials{ + CredentialsTypePassword: { + Identifiers: []string{"zab", "bar"}, + Type: CredentialsTypePassword, + Config: sqlxx.JSONRawMessage("{\"some\" : \"secret\"}"), + }, + CredentialsTypeOIDC: { + Type: CredentialsTypeOIDC, + Identifiers: []string{"bar:1234", "baz:5678"}, + Config: sqlxx.JSONRawMessage(`{"providers": [{"provider": "bar", "subject": "1234"}, {"provider": "baz", "subject": "5678"}]}`), + }, + CredentialsTypeWebAuthn: { + Type: CredentialsTypeWebAuthn, + Identifiers: []string{"foo", "bar"}, + Config: sqlxx.JSONRawMessage("{\"some\" : \"secret\"}"), + }, + } + i.Credentials = credentials + + err = i.deleteCredentialOIDCFromIdentity("zab") + assert.Error(t, err) + err = i.deleteCredentialOIDCFromIdentity("foo") + assert.Error(t, err) + err = i.deleteCredentialOIDCFromIdentity("bar") + assert.Error(t, err, "matches multiple OIDC credentials") + + require.NoError(t, i.deleteCredentialOIDCFromIdentity("bar:1234")) + + assert.Len(t, i.Credentials, 3) + + assert.Contains(t, i.Credentials, CredentialsTypePassword) + assert.EqualValues(t, i.Credentials[CredentialsTypePassword].Identifiers, []string{"zab", "bar"}) + + assert.Contains(t, i.Credentials, CredentialsTypeWebAuthn) + assert.EqualValues(t, i.Credentials[CredentialsTypeWebAuthn].Identifiers, []string{"foo", "bar"}) + + assert.Contains(t, i.Credentials, CredentialsTypeOIDC) + + oidc, ok := i.GetCredentials(CredentialsTypeOIDC) + require.True(t, ok) + assert.EqualValues(t, oidc.Identifiers, []string{"baz:5678"}) + var cfg CredentialsOIDC + _, err = i.ParseCredentials(CredentialsTypeOIDC, &cfg) + require.NoError(t, err) + assert.EqualValues(t, CredentialsOIDC{Providers: []CredentialsOIDCProvider{{Provider: "baz", Subject: "5678"}}}, cfg) +}