Skip to content

Commit

Permalink
fix: resolve verification sql errors
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Aug 27, 2020
1 parent 9986d8f commit 784da53
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 52 deletions.
4 changes: 2 additions & 2 deletions internal/faker.go
Expand Up @@ -134,13 +134,13 @@ func RegisterFakes() {
}

if err := faker.AddProvider("recovery_flow_methods", func(v reflect.Value) (interface{}, error) {
var methods = make(map[string]*recovery.RequestMethod)
var methods = make(map[string]*recovery.FlowMethod)
for _, ct := range []string{recovery.StrategyRecoveryTokenName} {
var f form.HTMLForm
if err := faker.FakeData(&f); err != nil {
return nil, err
}
methods[ct] = &recovery.RequestMethod{
methods[ct] = &recovery.FlowMethod{
Method: ct,
Config: &recovery.RequestMethodConfig{RequestMethodConfigurator: &f},
}
Expand Down
2 changes: 1 addition & 1 deletion internal/testhelpers/sql.go
Expand Up @@ -31,7 +31,7 @@ func CleanSQL(t *testing.T, c *pop.Connection) {
new(settings.Flow).TableName(),
new(recoverytoken.Token).TableName(),
new(recovery.RequestMethods).TableName(),
new(recovery.Request).TableName(),
new(recovery.Flow).TableName(),
new(verification.Request).TableName(),
new(session.Session).TableName(),
new(identity.CredentialIdentifierCollection).TableName(),
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/recovery/error.go
Expand Up @@ -51,7 +51,7 @@ func NewErrorHandler(d errorHandlerDependencies, c configuration.Provider) *Erro
func (s *ErrorHandler) HandleRecoveryError(
w http.ResponseWriter,
r *http.Request,
rr *Request,
rr *Flow,
err error,
method string,
) {
Expand Down
4 changes: 2 additions & 2 deletions selfservice/flow/recovery/handler_test.go
Expand Up @@ -82,8 +82,8 @@ func TestRecoveryHandler(t *testing.T) {
assert.Equal(t, public.URL+recovery.PublicRecoveryInitPath, gjson.GetBytes(body, "error.details.redirect_to").String(), "%s", body)
}

newExpiredRequest := func() *recovery.Request {
return &recovery.Request{
newExpiredRequest := func() *recovery.Flow {
return &recovery.Flow{
ID: x.NewUUID(),
ExpiresAt: time.Now().Add(-time.Minute),
IssuedAt: time.Now().Add(-time.Minute * 2),
Expand Down
42 changes: 33 additions & 9 deletions selfservice/flow/recovery/persistence.go
Expand Up @@ -21,9 +21,9 @@ import (

type (
RequestPersister interface {
CreateRecoveryRequest(context.Context, *Request) error
GetRecoveryRequest(ctx context.Context, id uuid.UUID) (*Request, error)
UpdateRecoveryRequest(context.Context, *Request) error
CreateRecoveryRequest(context.Context, *Flow) error
GetRecoveryRequest(ctx context.Context, id uuid.UUID) (*Flow, error)
UpdateRecoveryRequest(context.Context, *Flow) error
}
RequestPersistenceProvider interface {
RecoveryRequestPersister() RequestPersister
Expand All @@ -36,7 +36,7 @@ func TestRequestPersister(p interface {
}) func(t *testing.T) {
viper.Set(configuration.ViperKeyDefaultIdentitySchemaURL, "file://./stub/identity.schema.json")

var clearids = func(r *Request) {
var clearids = func(r *Flow) {
r.ID = uuid.UUID{}
}

Expand All @@ -46,8 +46,8 @@ func TestRequestPersister(p interface {
require.Error(t, err)
})

var newRequest = func(t *testing.T) *Request {
var r Request
var newRequest = func(t *testing.T) *Flow {
var r Flow
require.NoError(t, faker.FakeData(&r))
clearids(&r)
return &r
Expand All @@ -60,7 +60,7 @@ func TestRequestPersister(p interface {
})

t.Run("case=should create with set ids", func(t *testing.T) {
var r Request
var r Flow
require.NoError(t, faker.FakeData(&r))
require.NoError(t, p.CreateRecoveryRequest(context.Background(), &r))
})
Expand All @@ -86,10 +86,10 @@ func TestRequestPersister(p interface {

t.Run("case=should create and update a recovery request", func(t *testing.T) {
expected := newRequest(t)
expected.Methods[StrategyRecoveryTokenName] = &RequestMethod{
expected.Methods[StrategyRecoveryTokenName] = &FlowMethod{
Method: StrategyRecoveryTokenName, Config: &RequestMethodConfig{RequestMethodConfigurator: &form.HTMLForm{Fields: []form.Field{{
Name: "zab", Type: "bar", Pattern: "baz"}}}}}
expected.Methods["password"] = &RequestMethod{
expected.Methods["password"] = &FlowMethod{
Method: "password", Config: &RequestMethodConfig{RequestMethodConfigurator: &form.HTMLForm{Fields: []form.Field{{
Name: "foo", Type: "bar", Pattern: "baz"}}}}}
err := p.CreateRecoveryRequest(context.Background(), expected)
Expand All @@ -115,5 +115,29 @@ func TestRequestPersister(p interface {
assert.EqualValues(t, []form.Field{{Name: "zab", Type: "bar", Pattern: "baz"}}, actual.
Methods[StrategyRecoveryTokenName].Config.RequestMethodConfigurator.(*form.HTMLForm).Fields)
})

t.Run("case=should not cause data loss when updating a request without changes", func(t *testing.T) {
t.Logf("Needs implementation")
t.FailNow()
// expected := newFlow(t)
// delete(expected.Methods, identity.CredentialsTypeOIDC)
// err := p.CreateLoginFlow(context.Background(), expected)
// require.NoError(t, err)
//
// actual, err := p.GetLoginFlow(context.Background(), expected.ID)
// require.NoError(t, err)
// assert.Len(t, actual.Methods, 1)
//
// require.NoError(t, p.UpdateLoginFlow(context.Background(), actual))
//
// actual, err = p.GetLoginFlow(context.Background(), expected.ID)
// require.NoError(t, err)
// require.Len(t, actual.Methods, 2)
// assert.EqualValues(t, identity.CredentialsTypePassword, actual.Active)
//
// js, _ := json.Marshal(actual.Methods)
// assert.Equal(t, string(identity.CredentialsTypePassword), actual.Methods[identity.CredentialsTypePassword].Config.FlowMethodConfigurator.(*form.HTMLForm).Action, "%s", js)
// assert.Equal(t, string(identity.CredentialsTypeOIDC), actual.Methods[identity.CredentialsTypeOIDC].Config.FlowMethodConfigurator.(*form.HTMLForm).Action)
})
}
}
32 changes: 16 additions & 16 deletions selfservice/flow/recovery/request.go
Expand Up @@ -18,14 +18,14 @@ import (
"github.com/ory/kratos/x"
)

// Request presents a recovery request
// A Recovery Flow
//
// This request is used when an identity wants to recover their account.
//
// We recommend reading the [Account Recovery Documentation](../self-service/flows/password-reset-account-recovery)
//
// swagger:model recoveryRequest
type Request struct {
// swagger:model recoveryFlow
type Flow struct {
// ID represents the request's unique ID. When performing the recovery flow, this
// represents the id in the recovery ui's query parameter: http://<selfservice.flows.recovery.ui_url>?request=<id>
//
Expand Down Expand Up @@ -65,7 +65,7 @@ type Request struct {
// processed, but for example the password is incorrect, this will contain error messages.
//
// required: true
Methods map[string]*RequestMethod `json:"methods" faker:"recovery_flow_methods" db:"-"`
Methods map[string]*FlowMethod `json:"methods" faker:"recovery_flow_methods" db:"-"`

// MethodsRaw is a helper struct field for gobuffalo.pop.
MethodsRaw RequestMethodsRaw `json:"-" faker:"-" has_many:"selfservice_recovery_flow_methods" fk_id:"selfservice_recovery_flow_id"`
Expand All @@ -92,13 +92,13 @@ type Request struct {
RecoveredIdentityID uuid.NullUUID `json:"-" faker:"-" db:"recovered_identity_id"`
}

func NewRequest(exp time.Duration, csrf string, r *http.Request, strategies Strategies) (*Request, error) {
req := &Request{
func NewRequest(exp time.Duration, csrf string, r *http.Request, strategies Strategies) (*Flow, error) {
req := &Flow{
ID: x.NewUUID(),
ExpiresAt: time.Now().UTC().Add(exp),
IssuedAt: time.Now().UTC(),
RequestURL: x.RequestURL(r).String(),
Methods: map[string]*RequestMethod{},
Methods: map[string]*FlowMethod{},
State: StateChooseMethod,
CSRFToken: csrf,
}
Expand All @@ -112,19 +112,19 @@ func NewRequest(exp time.Duration, csrf string, r *http.Request, strategies Stra
return req, nil
}

func (r Request) TableName() string {
func (r Flow) TableName() string {
return "selfservice_recovery_flows"
}

func (r *Request) URL(recoveryURL *url.URL) *url.URL {
func (r *Flow) URL(recoveryURL *url.URL) *url.URL {
return urlx.CopyWithQuery(recoveryURL, url.Values{"request": {r.ID.String()}})
}

func (r *Request) GetID() uuid.UUID {
func (r *Flow) GetID() uuid.UUID {
return r.ID
}

func (r *Request) Valid() error {
func (r *Flow) Valid() error {
if r.ExpiresAt.Before(time.Now().UTC()) {
return errors.WithStack(ErrRequestExpired.
WithReasonf("The recovery request expired %.2f minutes ago, please try again.",
Expand All @@ -133,20 +133,20 @@ func (r *Request) Valid() error {
return nil
}

func (r *Request) BeforeSave(_ *pop.Connection) error {
r.MethodsRaw = make([]RequestMethod, 0, len(r.Methods))
func (r *Flow) BeforeSave(_ *pop.Connection) error {
r.MethodsRaw = make([]FlowMethod, 0, len(r.Methods))
for _, m := range r.Methods {
r.MethodsRaw = append(r.MethodsRaw, *m)
}
r.Methods = nil
return nil
}

func (r *Request) AfterSave(c *pop.Connection) error {
func (r *Flow) AfterSave(c *pop.Connection) error {
return r.AfterFind(c)
}

func (r *Request) AfterFind(_ *pop.Connection) error {
func (r *Flow) AfterFind(_ *pop.Connection) error {
r.Methods = make(RequestMethods)
for key := range r.MethodsRaw {
m := r.MethodsRaw[key] // required for pointer dereference
Expand All @@ -156,7 +156,7 @@ func (r *Request) AfterFind(_ *pop.Connection) error {
return nil
}

func (r *Request) MethodToForm(id string) (form.Form, error) {
func (r *Flow) MethodToForm(id string) (form.Form, error) {
method, ok := r.Methods[id]
if !ok {
return nil, errors.WithStack(x.PseudoPanic.WithReasonf("Expected method %s to exist.", id))
Expand Down
16 changes: 8 additions & 8 deletions selfservice/flow/recovery/request_method.go
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/ory/kratos/selfservice/form"
)

// swagger:model recoveryRequestMethod
type RequestMethod struct {
// swagger:model recoveryFlowMethod
type FlowMethod struct {
// Method contains the request credentials type.
Method string `json:"method" db:"method"`

Expand All @@ -24,10 +24,10 @@ type RequestMethod struct {
ID uuid.UUID `json:"-" db:"id"`

// FlowID is a helper struct field for gobuffalo.pop.
FlowID uuid.UUID `json:"-" db:"selfservice_flow_request_id"`
FlowID uuid.UUID `json:"-" db:"selfservice_recovery_flow_id"`

// Flow is a helper struct field for gobuffalo.pop.
Flow *Request `json:"-" belongs_to:"selfservice_flow_request" fk_id:"FlowID"`
Flow *Flow `json:"-" belongs_to:"selfservice_flow_request" fk_id:"FlowID"`

// CreatedAt is a helper struct field for gobuffalo.pop.
CreatedAt time.Time `json:"-" db:"created_at"`
Expand All @@ -36,12 +36,12 @@ type RequestMethod struct {
UpdatedAt time.Time `json:"-" db:"updated_at"`
}

func (u RequestMethod) TableName() string {
return "selfservice_recovery_request_methods"
func (u FlowMethod) TableName() string {
return "selfservice_recovery_flow_methods"
}

type RequestMethodsRaw []RequestMethod // workaround for https://github.com/gobuffalo/pop/pull/478
type RequestMethods map[string]*RequestMethod
type RequestMethodsRaw []FlowMethod // workaround for https://github.com/gobuffalo/pop/pull/478
type RequestMethods map[string]*FlowMethod

func (u RequestMethods) TableName() string {
// This must be stay a value receiver, using a pointer receiver will cause issues with pop.
Expand Down
4 changes: 2 additions & 2 deletions selfservice/flow/recovery/request_test.go
Expand Up @@ -15,14 +15,14 @@ import (
)

func TestRequest(t *testing.T) {
must := func(r *recovery.Request, err error) *recovery.Request {
must := func(r *recovery.Flow, err error) *recovery.Flow {
require.NoError(t, err)
return r
}

u := &http.Request{URL: urlx.ParseOrPanic("http://foo/bar/baz"), Host: "foo"}
for k, tc := range []struct {
r *recovery.Request
r *recovery.Flow
expectErr bool
}{
{r: must(recovery.NewRequest(time.Hour, "", u, nil))},
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/recovery/strategy.go
Expand Up @@ -15,7 +15,7 @@ const (
type (
Strategy interface {
RecoveryStrategyID() string
PopulateRecoveryMethod(*http.Request, *Request) error
PopulateRecoveryMethod(*http.Request, *Flow) error
}
AdminHandler interface {
RegisterAdminRecoveryRoutes(admin *x.RouterAdmin)
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/verification/request.go
Expand Up @@ -66,7 +66,7 @@ type Request struct {
}

func (r Request) TableName() string {
return "selfservice_verification_requests"
return "selfservice_verification_flows"
}

func NewRequest(
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/recoverytoken/persister_conformity.go
Expand Up @@ -31,7 +31,7 @@ func TestPersister(p interface {
})

newRecoveryToken := func(t *testing.T, email string) *Token {
var req recovery.Request
var req recovery.Flow
require.NoError(t, faker.FakeData(&req))
require.NoError(t, p.CreateRecoveryRequest(context.Background(), &req))

Expand Down
12 changes: 6 additions & 6 deletions selfservice/strategy/recoverytoken/strategy.go
Expand Up @@ -99,7 +99,7 @@ func (s *Strategy) RegisterAdminRecoveryRoutes(admin *x.RouterAdmin) {
admin.POST(AdminPath, s.createRecoveryLink)
}

func (s *Strategy) PopulateRecoveryMethod(r *http.Request, req *recovery.Request) error {
func (s *Strategy) PopulateRecoveryMethod(r *http.Request, req *recovery.Flow) error {
f := form.NewHTMLForm(urlx.CopyWithQuery(
urlx.AppendPaths(s.c.SelfPublicURL(), PublicPath),
url.Values{"request": {req.ID.String()}},
Expand All @@ -108,7 +108,7 @@ func (s *Strategy) PopulateRecoveryMethod(r *http.Request, req *recovery.Request
f.SetCSRF(s.d.GenerateCSRFToken(r))
f.SetField(form.Field{Name: "email", Type: "email", Required: true})

req.Methods[s.RecoveryStrategyID()] = &recovery.RequestMethod{
req.Methods[s.RecoveryStrategyID()] = &recovery.FlowMethod{
Method: s.RecoveryStrategyID(),
Config: &recovery.RequestMethodConfig{RequestMethodConfigurator: &StrategyMethodConfig{HTMLForm: f}},
}
Expand Down Expand Up @@ -331,7 +331,7 @@ func (s *Strategy) handleSubmit(w http.ResponseWriter, r *http.Request, ps httpr
}
}

func (s *Strategy) issueSession(w http.ResponseWriter, r *http.Request, req *recovery.Request) {
func (s *Strategy) issueSession(w http.ResponseWriter, r *http.Request, req *recovery.Flow) {
req.State = recovery.StatePassedChallenge
if err := s.d.RecoveryRequestPersister().UpdateRecoveryRequest(r.Context(), req); err != nil {
s.handleError(w, r, req, err)
Expand Down Expand Up @@ -410,7 +410,7 @@ func (s *Strategy) retryFlowWithMessage(w http.ResponseWriter, r *http.Request,
)
}

func (s *Strategy) issueAndSendRecoveryToken(w http.ResponseWriter, r *http.Request, req *recovery.Request) {
func (s *Strategy) issueAndSendRecoveryToken(w http.ResponseWriter, r *http.Request, req *recovery.Flow) {
email := r.PostForm.Get("email")
if len(email) == 0 {
s.handleError(w, r, req, schema.NewRequiredError("#/email", "email"))
Expand Down Expand Up @@ -471,7 +471,7 @@ func (s *Strategy) sendToUnknownAddress(ctx context.Context, address string) err
})
}

func (s *Strategy) sendCodeToKnownAddress(ctx context.Context, req *recovery.Request, address *identity.RecoveryAddress) error {
func (s *Strategy) sendCodeToKnownAddress(ctx context.Context, req *recovery.Flow, address *identity.RecoveryAddress) error {
token := NewToken(address, req)
if err := s.d.RecoveryTokenPersister().CreateRecoveryToken(ctx, token); err != nil {
return err
Expand Down Expand Up @@ -502,7 +502,7 @@ func (s *Strategy) run(via identity.RecoveryAddressType, emailFunc func() error)
}
}

func (s *Strategy) handleError(w http.ResponseWriter, r *http.Request, req *recovery.Request, err error) {
func (s *Strategy) handleError(w http.ResponseWriter, r *http.Request, req *recovery.Flow, err error) {
if errors.Is(err, recovery.ErrRequestExpired) {
s.retryFlowWithMessage(w, r, text.NewErrorValidationRecoveryRecoveryTokenInvalidOrAlreadyUsed())
return
Expand Down
4 changes: 2 additions & 2 deletions selfservice/strategy/recoverytoken/token.go
Expand Up @@ -27,7 +27,7 @@ type Token struct {
RecoveryAddress *identity.RecoveryAddress `json:"recovery_address" belongs_to:"identity_recovery_addresses" fk_id:"RecoveryAddressID"`

// RecoveryAddress links this token to a recovery request.
Request *recovery.Request `json:"request" belongs_to:"identity_recovery_requests" fk_id:"FlowID"`
Request *recovery.Flow `json:"request" belongs_to:"identity_recovery_requests" fk_id:"FlowID"`

// CreatedAt is a helper struct field for gobuffalo.pop.
CreatedAt time.Time `json:"-" faker:"-" db:"created_at"`
Expand All @@ -43,7 +43,7 @@ func (Token) TableName() string {
return "identity_recovery_tokens"
}

func NewToken(ra *identity.RecoveryAddress, req *recovery.Request) *Token {
func NewToken(ra *identity.RecoveryAddress, req *recovery.Flow) *Token {
return &Token{
ID: x.NewUUID(),
Token: randx.MustString(32, randx.AlphaNum),
Expand Down

0 comments on commit 784da53

Please sign in to comment.