refactor: complete login flow refactoring

ory · Aug 25, 2020 · ad2b3db · ad2b3db
1 parent 88f581f
commit ad2b3db
Show file tree

Hide file tree

Showing 26 changed files with 185 additions and 94 deletions.
diff --git a/.schema/api.swagger.json b/.schema/api.swagger.json
@@ -1125,8 +1125,10 @@
         "parameters": [
           {
             "type": "string",
+            "description": "Error is the container's ID",
             "name": "error",
-            "in": "query"
+            "in": "query",
+            "required": true
           }
         ],
         "responses": {
@@ -1558,8 +1560,13 @@
     },
     "errorContainer": {
       "type": "object",
+      "required": [
+        "id",
+        "errors"
+      ],
       "properties": {
         "errors": {
+          "description": "Errors in the container",
           "type": "object"
         },
         "id": {

diff --git a/driver/registry_default.go b/driver/registry_default.go
@@ -490,7 +490,7 @@ func (m *RegistryDefault) RecoveryRequestPersister() recovery.RequestPersister {
 	return m.persister
 }
 
-func (m *RegistryDefault) LoginRequestPersister() login.RequestPersister {
+func (m *RegistryDefault) LoginFlowPersister() login.FlowPersister {
 	return m.persister
 }
 

diff --git a/driver/registry_default_login.go b/driver/registry_default_login.go
@@ -40,7 +40,7 @@ func (m *RegistryDefault) LoginHandler() *login.Handler {
 
 func (m *RegistryDefault) LoginRequestErrorHandler() *login.ErrorHandler {
 	if m.selfserviceLoginRequestErrorHandler == nil {
-		m.selfserviceLoginRequestErrorHandler = login.NewErrorHandler(m, m.c)
+		m.selfserviceLoginRequestErrorHandler = login.NewFlowErrorHandler(m, m.c)
 	}
 
 	return m.selfserviceLoginRequestErrorHandler

diff --git a/go.mod b/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d // indirect
 	github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0
 	github.com/armon/go-metrics v0.3.3 // indirect
+	github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869
 	github.com/bxcodec/faker/v3 v3.3.1
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/coreos/go-oidc v2.2.1+incompatible

diff --git a/internal/httpclient/client/common/get_self_service_error_parameters.go b/internal/httpclient/client/common/get_self_service_error_parameters.go
diff --git a/internal/httpclient/models/error_container.go b/internal/httpclient/models/error_container.go
diff --git a/internal/testhelpers/http.go b/internal/testhelpers/http.go
@@ -11,6 +11,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func NewHTTPGetRequestJSON(t *testing.T, url string) *http.Request {
+	req, err := http.NewRequest("GET", url, nil)
+	require.NoError(t, err)
+
+	// req.Header.Set("Content-Type", "application/json;charset=utf-8")
+	req.Header.Set("Accept", "application/json")
+	return req
+}
+
 func HTTPRequestJSON(t *testing.T, client *http.Client, method string, url string, in interface{}) ([]byte, *http.Response) {
 	var body bytes.Buffer
 	require.NoError(t, json.NewEncoder(&body).Encode(in))

diff --git a/internal/testhelpers/login.go b/internal/testhelpers/login.go
@@ -16,7 +16,7 @@ import (
 
 func NewLoginUIRequestEchoServer(t *testing.T, reg driver.Registry) *httptest.Server {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		e, err := reg.LoginRequestPersister().GetLoginRequest(r.Context(), x.ParseUUID(r.URL.Query().Get("flow")))
+		e, err := reg.LoginFlowPersister().GetLoginFlow(r.Context(), x.ParseUUID(r.URL.Query().Get("flow")))
 		require.NoError(t, err)
 		reg.Writer().Write(w, r, e)
 	}))

diff --git a/persistence/reference.go b/persistence/reference.go
@@ -27,7 +27,7 @@ type Persister interface {
 	continuity.Persister
 	identity.PrivilegedPool
 	registration.RequestPersister
-	login.RequestPersister
+	login.FlowPersister
 	settings.RequestPersister
 	courier.Persister
 	session.Persister

diff --git a/persistence/sql/migratest/migration_test.go b/persistence/sql/migratest/migration_test.go
@@ -129,7 +129,7 @@ func TestMigrations(t *testing.T) {
 					require.NoError(t, c.Select("id").All(&ids))
 
 					for _, id := range ids {
-						actual, err := d.Registry().LoginRequestPersister().GetLoginRequest(context.Background(), id.ID)
+						actual, err := d.Registry().LoginFlowPersister().GetLoginFlow(context.Background(), id.ID)
 						require.NoError(t, err)
 						compareWithFixture(t, actual, "login_request", id.ID.String())
 					}

diff --git a/persistence/sql/persister_login.go b/persistence/sql/persister_login.go
@@ -13,16 +13,16 @@ import (
 	"github.com/ory/kratos/selfservice/flow/login"
 )
 
-var _ login.RequestPersister = new(Persister)
+var _ login.FlowPersister = new(Persister)
 
-func (p *Persister) CreateLoginRequest(ctx context.Context, r *login.Flow) error {
+func (p *Persister) CreateLoginFlow(ctx context.Context, r *login.Flow) error {
 	return p.GetConnection(ctx).Eager().Create(r)
 }
 
-func (p *Persister) UpdateLoginRequest(ctx context.Context, r *login.Flow) error {
+func (p *Persister) UpdateLoginFlow(ctx context.Context, r *login.Flow) error {
 	return p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error {
 
-		rr, err := p.GetLoginRequest(ctx, r.ID)
+		rr, err := p.GetLoginFlow(ctx, r.ID)
 		if err != nil {
 			return err
 		}
@@ -44,7 +44,7 @@ func (p *Persister) UpdateLoginRequest(ctx context.Context, r *login.Flow) error
 	})
 }
 
-func (p *Persister) GetLoginRequest(ctx context.Context, id uuid.UUID) (*login.Flow, error) {
+func (p *Persister) GetLoginFlow(ctx context.Context, id uuid.UUID) (*login.Flow, error) {
 	conn := p.GetConnection(ctx)
 	var r login.Flow
 	if err := conn.Eager().Find(&r, id); err != nil {
@@ -58,10 +58,10 @@ func (p *Persister) GetLoginRequest(ctx context.Context, id uuid.UUID) (*login.F
 	return &r, nil
 }
 
-func (p *Persister) MarkRequestForced(ctx context.Context, id uuid.UUID) error {
+func (p *Persister) ForceLoginFlow(ctx context.Context, id uuid.UUID) error {
 	return p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error {
 
-		lr, err := p.GetLoginRequest(ctx, id)
+		lr, err := p.GetLoginFlow(ctx, id)
 		if err != nil {
 			return err
 		}
@@ -71,10 +71,10 @@ func (p *Persister) MarkRequestForced(ctx context.Context, id uuid.UUID) error {
 	})
 }
 
-func (p *Persister) UpdateLoginRequestMethod(ctx context.Context, id uuid.UUID, ct identity.CredentialsType, rm *login.FlowMethod) error {
+func (p *Persister) UpdateLoginFlowMethod(ctx context.Context, id uuid.UUID, ct identity.CredentialsType, rm *login.FlowMethod) error {
 	return p.Transaction(ctx, func(ctx context.Context, tx *pop.Connection) error {
 
-		rr, err := p.GetLoginRequest(ctx, id)
+		rr, err := p.GetLoginFlow(ctx, id)
 		if err != nil {
 			return err
 		}

diff --git a/persistence/sql/persister_test.go b/persistence/sql/persister_test.go
@@ -138,23 +138,23 @@ func TestPersister(t *testing.T) {
 				pop.SetLogger(pl(t))
 				identity.TestPool(p.(identity.PrivilegedPool))(t)
 			})
-			t.Run("contract=registration.TestRequestPersister", func(t *testing.T) {
+			t.Run("contract=registration.TestFlowPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
 				registration.TestRequestPersister(p)(t)
 			})
 			t.Run("contract=errorx.TestPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
 				errorx.TestPersister(p)(t)
 			})
-			t.Run("contract=login.TestRequestPersister", func(t *testing.T) {
+			t.Run("contract=login.TestFlowPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
-				login.TestRequestPersister(p)(t)
+				login.TestFlowPersister(p)(t)
 			})
-			t.Run("contract=settings.TestRequestPersister", func(t *testing.T) {
+			t.Run("contract=settings.TestFlowPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
 				settings.TestRequestPersister(p)(t)
 			})
-			t.Run("contract=session.TestRequestPersister", func(t *testing.T) {
+			t.Run("contract=session.TestFlowPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
 				session.TestPersister(p)(t)
 			})
@@ -166,7 +166,7 @@ func TestPersister(t *testing.T) {
 				pop.SetLogger(pl(t))
 				verification.TestPersister(p)(t)
 			})
-			t.Run("contract=recovery.TestRequestPersister", func(t *testing.T) {
+			t.Run("contract=recovery.TestFlowPersister", func(t *testing.T) {
 				pop.SetLogger(pl(t))
 				recovery.TestRequestPersister(p)(t)
 			})
@@ -224,14 +224,14 @@ func TestPersister_Transaction(t *testing.T) {
 		}
 		err := c.Transaction(func(tx *pop.Connection) error {
 			ctx := sql.WithTransaction(context.Background(), tx)
-			require.NoError(t, p.CreateLoginRequest(ctx, lr), "%+v", lr)
-			require.NoError(t, p.UpdateLoginRequestMethod(ctx, lr.ID, identity.CredentialsTypePassword, &login.FlowMethod{}))
-			require.NoError(t, getErr(p.GetLoginRequest(ctx, lr.ID)), "%+v", lr)
+			require.NoError(t, p.CreateLoginFlow(ctx, lr), "%+v", lr)
+			require.NoError(t, p.UpdateLoginFlowMethod(ctx, lr.ID, identity.CredentialsTypePassword, &login.FlowMethod{}))
+			require.NoError(t, getErr(p.GetLoginFlow(ctx, lr.ID)), "%+v", lr)
 			return errors.Errorf(errMessage)
 		})
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), errMessage)
-		_, err = p.GetLoginRequest(context.Background(), lr.ID)
+		_, err = p.GetLoginFlow(context.Background(), lr.ID)
 		require.Error(t, err)
 		assert.Equal(t, sqlcon.ErrNoRows.Error(), err.Error())
 	})

diff --git a/selfservice/errorx/error.go b/selfservice/errorx/error.go
@@ -10,10 +10,16 @@ import (
 
 // swagger:model errorContainer
 type ErrorContainer struct {
+	// ID of the error container.
+	//
+	// required: true
 	ID uuid.UUID `db:"id" json:"id"`
 
 	CSRFToken string `db:"csrf_token" json:"-"`
 
+	// Errors in the container
+	//
+	// required: true
 	Errors json.RawMessage `json:"errors" db:"errors"`
 
 	// CreatedAt is a helper struct field for gobuffalo.pop.

diff --git a/selfservice/errorx/handler.go b/selfservice/errorx/handler.go
@@ -61,7 +61,10 @@ type errorContainerResponse struct {
 // nolint:deadcode,unused
 // swagger:parameters getSelfServiceError
 type errorContainerParameters struct {
+	// Error is the container's ID
+	//
 	// in: query
+	// required: true
 	Error string `json:"error"`
 }
 

diff --git a/selfservice/flow/login/flow.go b/selfservice/flow/login/flow.go
@@ -143,7 +143,7 @@ func (f Flow) TableName() string {
 
 func (f *Flow) Valid() error {
 	if f.ExpiresAt.Before(time.Now()) {
-		return errors.WithStack(newRequestExpiredError(time.Since(f.ExpiresAt)))
+		return errors.WithStack(NewRequestExpiredError(time.Since(f.ExpiresAt)))
 	}
 	return nil
 }

diff --git a/selfservice/flow/login/handler.go b/selfservice/flow/login/handler.go
@@ -71,7 +71,7 @@ func (h *Handler) NewLoginFlow(w http.ResponseWriter, r *http.Request, flow flow
 		return nil, err
 	}
 
-	if err := h.d.LoginRequestPersister().CreateLoginRequest(r.Context(), a); err != nil {
+	if err := h.d.LoginFlowPersister().CreateLoginFlow(r.Context(), a); err != nil {
 		return nil, err
 	}
 	return a, nil
@@ -133,7 +133,7 @@ func (h *Handler) initAPIFlow(w http.ResponseWriter, r *http.Request, _ httprout
 	}
 
 	if a.Forced {
-		if err := h.d.LoginRequestPersister().MarkRequestForced(r.Context(), a.ID); err != nil {
+		if err := h.d.LoginFlowPersister().ForceLoginFlow(r.Context(), a.ID); err != nil {
 			h.d.Writer().WriteError(w, r, err)
 			return
 		}
@@ -184,7 +184,7 @@ func (h *Handler) initBrowserFlow(w http.ResponseWriter, r *http.Request, ps htt
 	}
 
 	if a.Forced {
-		if err := h.d.LoginRequestPersister().MarkRequestForced(r.Context(), a.ID); err != nil {
+		if err := h.d.LoginFlowPersister().ForceLoginFlow(r.Context(), a.ID); err != nil {
 			h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
 			return
 		}
@@ -251,7 +251,7 @@ func (h *Handler) adminFetchLoginRequest(w http.ResponseWriter, r *http.Request,
 }
 
 func (h *Handler) fetchLoginRequest(w http.ResponseWriter, r *http.Request) error {
-	ar, err := h.d.LoginRequestPersister().GetLoginRequest(r.Context(), x.ParseUUID(r.URL.Query().Get("id")))
+	ar, err := h.d.LoginFlowPersister().GetLoginFlow(r.Context(), x.ParseUUID(r.URL.Query().Get("id")))
 	if err != nil {
 		return err
 	}