feat: refactor credentials fetching (#3183)

This change revamps the way we fetch identity credentials. We no longer need most of the helper fields for gobuffalo/pop inside the `Identity` and `Credentials` structures, and we collect all the credentials in one joined query rather than using pop's `EagerPreload` functionality.

go.mod

@@ -42,7 +42,7 @@ require (
 	github.com/go-swagger/go-swagger v0.30.3
 	github.com/gobuffalo/fizz v1.14.4
 	github.com/gobuffalo/httptest v1.5.2
-	github.com/gobuffalo/pop/v6 v6.1.2-0.20230124165254-ec9229dbf7d7
+	github.com/gobuffalo/pop/v6 v6.1.2-0.20230318123913-c85387acc9a0
 	github.com/gofrs/uuid v4.3.1+incompatible
 	github.com/golang-jwt/jwt/v4 v4.1.0
 	github.com/golang/gddo v0.0.0-20190904175337-72a348e765d2

go.sum

@@ -501,8 +501,8 @@ github.com/gobuffalo/packr/v2 v2.2.0/go.mod h1:CaAwI0GPIAv+5wKLtv8Afwl+Cm78K/I/V
 github.com/gobuffalo/plush/v4 v4.1.16/go.mod h1:6t7swVsarJ8qSLw1qyAH/KbrcSTwdun2ASEQkOznakg=
 github.com/gobuffalo/plush/v4 v4.1.18 h1:bnPjdMTEUQHqj9TNX2Ck3mxEXYZa+0nrFMNM07kpX9g=
 github.com/gobuffalo/plush/v4 v4.1.18/go.mod h1:xi2tJIhFI4UdzIL8sxZtzGYOd2xbBpcFbLZlIPGGZhU=
-github.com/gobuffalo/pop/v6 v6.1.2-0.20230124165254-ec9229dbf7d7 h1:lwf/5cRw46IrLrhZnCg8J9NKgskkwMPuVvEOc2Wy72I=
-github.com/gobuffalo/pop/v6 v6.1.2-0.20230124165254-ec9229dbf7d7/go.mod h1:1n7jAmI1i7fxuXPZjZb0VBPQDbksRtCoFnrDV5IsvaI=
+github.com/gobuffalo/pop/v6 v6.1.2-0.20230318123913-c85387acc9a0 h1:+LF3Enal3HZ+rFmaLZfBRNHKqtnoA0d8jk0Iio8InZM=
+github.com/gobuffalo/pop/v6 v6.1.2-0.20230318123913-c85387acc9a0/go.mod h1:1n7jAmI1i7fxuXPZjZb0VBPQDbksRtCoFnrDV5IsvaI=
 github.com/gobuffalo/syncx v0.0.0-20190224160051-33c29581e754/go.mod h1:HhnNqWY95UYwwW3uSASeV7vtgYkT2t16hJgV3AEPUpw=
 github.com/gobuffalo/tags/v3 v3.1.4 h1:X/ydLLPhgXV4h04Hp2xlbI2oc5MDaa7eub6zw8oHjsM=
 github.com/gobuffalo/tags/v3 v3.1.4/go.mod h1:ArRNo3ErlHO8BtdA0REaZxijuWnWzF6PUXngmMXd2I0=

identity/credentials.go

@@ -8,12 +8,9 @@ import (
 	"reflect"
 	"time"
 
-	"github.com/gobuffalo/pop/v6"
-
-	"github.com/ory/kratos/ui/node"
-
 	"github.com/gofrs/uuid"
 
+	"github.com/ory/kratos/ui/node"
 	"github.com/ory/x/sqlxx"
 )
 
@@ -87,7 +84,8 @@ type Credentials struct {
 	ID uuid.UUID `json:"-" db:"id"`
 
 	// Type discriminates between different types of credentials.
-	Type CredentialsType `json:"type" db:"-"`
+	Type                     CredentialsType `json:"type" db:"-"`
+	IdentityCredentialTypeID uuid.UUID       `json:"-" db:"identity_credential_type_id"`
 
 	// Identifiers represents a list of unique identifiers this credential type matches.
 	Identifiers []string `json:"identifiers" db:"-"`
@@ -107,26 +105,6 @@ type Credentials struct {
 	// UpdatedAt is a helper struct field for gobuffalo.pop.
 	UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
 	NID       uuid.UUID `json:"-"  faker:"-" db:"nid"`
-
-	IdentityCredentialTypeID uuid.UUID                      `json:"-" db:"identity_credential_type_id"`
-	IdentityCredentialType   CredentialsTypeTable           `json:"-" faker:"-" belongs_to:"identity_credential_types"`
-	CredentialIdentifiers    CredentialIdentifierCollection `json:"-" faker:"-" has_many:"identity_credential_identifiers" fk_id:"identity_credential_id" order_by:"id asc"`
-}
-
-func (c *Credentials) AfterEagerFind(tx *pop.Connection) error {
-	return c.setCredentials()
-}
-
-func (c *Credentials) setCredentials() error {
-	c.Type = c.IdentityCredentialType.Name
-	c.Identifiers = make([]string, 0, len(c.CredentialIdentifiers))
-	for _, id := range c.CredentialIdentifiers {
-		if c.NID != id.NID {
-			continue
-		}
-		c.Identifiers = append(c.Identifiers, id.Identifier)
-	}
-	return nil
 }
 
 func (c Credentials) TableName(ctx context.Context) string {
@@ -155,12 +133,6 @@ type (
 		Name CredentialsType `json:"-" db:"name"`
 	}
 
-	// swagger:ignore
-	CredentialsCollection []Credentials
-
-	// swagger:ignore
-	CredentialIdentifierCollection []CredentialIdentifier
-
 	// swagger:ignore
 	ActiveCredentialsCounter interface {
 		ID() CredentialsType
@@ -178,14 +150,6 @@ func (c CredentialsTypeTable) TableName(ctx context.Context) string {
 	return "identity_credential_types"
 }
 
-func (c CredentialsCollection) TableName(ctx context.Context) string {
-	return "identity_credentials"
-}
-
-func (c CredentialIdentifierCollection) TableName(ctx context.Context) string {
-	return "identity_credential_identifiers"
-}
-
 func (c CredentialIdentifier) TableName(ctx context.Context) string {
 	return "identity_credential_identifiers"
 }

identity/expandables.go

@@ -9,11 +9,9 @@ type Expandable = sqlxx.Expandable
 type Expandables = sqlxx.Expandables
 
 const (
-	ExpandFieldVerifiableAddresses   Expandable = "VerifiableAddresses"
-	ExpandFieldRecoveryAddresses     Expandable = "RecoveryAddresses"
-	ExpandFieldCredentials           Expandable = "InternalCredentials"
-	ExpandFieldCredentialType        Expandable = "InternalCredentials.IdentityCredentialType"
-	ExpandFieldCredentialIdentifiers Expandable = "InternalCredentials.CredentialIdentifiers"
+	ExpandFieldVerifiableAddresses Expandable = "VerifiableAddresses"
+	ExpandFieldRecoveryAddresses   Expandable = "RecoveryAddresses"
+	ExpandFieldCredentials         Expandable = "Credentials"
 )
 
 // ExpandNothing expands nothing
@@ -31,15 +29,11 @@ var ExpandDefault = Expandables{
 // ExpandCredentials expands the identity's credentials.
 var ExpandCredentials = Expandables{
 	ExpandFieldCredentials,
-	ExpandFieldCredentialType,
-	ExpandFieldCredentialIdentifiers,
 }
 
 // ExpandEverything expands all the fields of an identity.
 var ExpandEverything = Expandables{
 	ExpandFieldVerifiableAddresses,
 	ExpandFieldRecoveryAddresses,
 	ExpandFieldCredentials,
-	ExpandFieldCredentialType,
-	ExpandFieldCredentialIdentifiers,
 }

identity/identity.go

@@ -13,8 +13,6 @@ import (
 
 	"github.com/samber/lo"
 
-	"github.com/gobuffalo/pop/v6"
-
 	"github.com/tidwall/sjson"
 
 	"github.com/tidwall/gjson"
@@ -121,9 +119,6 @@ type Identity struct {
 	// Store metadata about the user which is only accessible through admin APIs such as `GET /admin/identities/<id>`.
 	MetadataAdmin sqlxx.NullJSONRawMessage `json:"metadata_admin,omitempty" faker:"-" db:"metadata_admin"`
 
-	// InternalCredentials is an internal representation of the credentials.
-	InternalCredentials CredentialsCollection `json:"-" faker:"-" has_many:"identity_credentials" fk_id:"identity_id" order_by:"id asc"`
-
 	// CreatedAt is a helper struct field for gobuffalo.pop.
 	CreatedAt time.Time `json:"created_at" db:"created_at"`
 
@@ -132,36 +127,6 @@ type Identity struct {
 	NID       uuid.UUID `json:"-"  faker:"-" db:"nid"`
 }
 
-func (i *Identity) AfterEagerFind(tx *pop.Connection) error {
-	if err := i.setCredentials(tx); err != nil {
-		return err
-	}
-
-	if err := i.Validate(); err != nil {
-		return err
-	}
-
-	return UpgradeCredentials(i)
-}
-
-func (i *Identity) setCredentials(tx *pop.Connection) error {
-	creds := i.InternalCredentials
-	i.Credentials = make(map[CredentialsType]Credentials, len(creds))
-	for k := range creds {
-		cred := &creds[k]
-		if cred.NID != i.NID {
-			continue
-		}
-		if err := cred.AfterEagerFind(tx); err != nil {
-			return err
-
-		}
-		i.Credentials[cred.Type] = *cred
-	}
-
-	return nil
-}
-
 // 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`.

identity/test/pool.go

@@ -123,7 +123,6 @@ func TestPool(ctx context.Context, conf *config.Config, p persistence.Persister,
 					assert.Empty(t, actual.RecoveryAddresses)
 					assert.Empty(t, actual.VerifiableAddresses)
 					assert.Empty(t, actual.Credentials)
-					assert.Empty(t, actual.InternalCredentials)
 				})
 			})
 
@@ -142,7 +141,6 @@ func TestPool(ctx context.Context, conf *config.Config, p persistence.Persister,
 			t.Run("expand=recovery address", func(t *testing.T) {
 				runner(t, sqlxx.Expandables{identity.ExpandFieldRecoveryAddresses}, func(t *testing.T, actual *identity.Identity) {
 					assert.Empty(t, actual.Credentials)
-					assert.Empty(t, actual.InternalCredentials)
 					assert.Empty(t, actual.VerifiableAddresses)
 
 					require.Len(t, actual.RecoveryAddresses, 1)
@@ -153,7 +151,6 @@ func TestPool(ctx context.Context, conf *config.Config, p persistence.Persister,
 			t.Run("expand=verification address", func(t *testing.T) {
 				runner(t, sqlxx.Expandables{identity.ExpandFieldVerifiableAddresses}, func(t *testing.T, actual *identity.Identity) {
 					assert.Empty(t, actual.Credentials)
-					assert.Empty(t, actual.InternalCredentials)
 					assert.Empty(t, actual.RecoveryAddresses)
 
 					require.Len(t, actual.VerifiableAddresses, 1)
@@ -165,7 +162,6 @@ func TestPool(ctx context.Context, conf *config.Config, p persistence.Persister,
 				runner(t, identity.ExpandDefault, func(t *testing.T, actual *identity.Identity) {
 
 					assert.Empty(t, actual.Credentials)
-					assert.Empty(t, actual.InternalCredentials)
 
 					require.Len(t, actual.RecoveryAddresses, 1)
 					assertx.EqualAsJSONExcept(t, expected.RecoveryAddresses, actual.RecoveryAddresses, []string{"0.updated_at", "0.created_at"})