Skip to content

Commit

Permalink
refactor: add flow methods to verification
Browse files Browse the repository at this point in the history
Completely refactors the verification flow to support other methods. The original email verification flow now moved to the "link" method also used for recovery.

Additionally, several upstream bugs in gobuffalo/pop and gobuffalo/fizz have been addressed, patched, and merged which improves support for SQLite and CockroachDB migrations:

- gobuffalo/fizz#97
- gobuffalo/fizz#96

BREAKING CHANGE: This patch significantly changes how email verification works. The Verification Flow no longer uses its own system but now re-uses the API and Browser flows and flow methods established in other components such as login, recovery, registration.

Due to the many changes these patch notes does not cover how to upgrade this particular flow. We instead want to kindly ask you to check out the updated documentation for this flow at: https://www.ory.sh/kratos/docs/self-service/flows/verify-email-account-activation

This patch changes the SQL schema and thus requires running the SQL Migration command (e.g. `... migrate sql`).
Never apply SQL migrations without backing up your database prior.
  • Loading branch information
aeneasr committed Aug 31, 2020
1 parent 42abfa1 commit 00ee828
Show file tree
Hide file tree
Showing 164 changed files with 5,904 additions and 3,679 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Expand Up @@ -8,11 +8,11 @@ linters:
- structcheck
- typecheck
- unused
- dupl
- gosec
- varcheck
disable:
- ineffassign
- dupl # https://github.com/ory/kratos/issues/680
- godox
- bodyclose # too many false negatives

Expand Down
477 changes: 262 additions & 215 deletions .schema/api.swagger.json

Large diffs are not rendered by default.

36 changes: 26 additions & 10 deletions cmd/daemon/serve.go
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/link"
"github.com/ory/kratos/selfservice/strategy/oidc"
"github.com/ory/kratos/selfservice/strategy/password"
"github.com/ory/kratos/selfservice/strategy/profile"
Expand Down Expand Up @@ -124,26 +125,41 @@ func sqa(cmd *cobra.Command, d driver.Driver) *metricsx.Service {
healthx.AliveCheckPath,
healthx.ReadyCheckPath,
healthx.VersionPath,
"/auth/methods/oidc/",

password.RouteRegistration,
password.RouteLogin,
oidc.BasePath,
password.RouteSettings,

oidc.RouteBase,

login.RouteInitBrowserFlow,
login.RouteInitAPIFlow,
login.RouteGetFlow,
logout.BrowserLogoutPath,

logout.RouteBrowser,

registration.RouteInitBrowserFlow,
registration.RouteInitAPIFlow,
registration.RouteGetFlow,

session.RouteWhoami,
identity.IdentitiesPath,
profile.RouteSettings,
identity.RouteBase,

settings.RouteInitBrowserFlow,
settings.RouteInitAPIFlow,
settings.RouteGetFlow,

verification.RouteInitAPIFlow,
verification.RouteInitBrowserFlow,
verification.RouteGetFlow,

profile.RouteSettings,
verification.PublicVerificationCompletePath,
strings.ReplaceAll(strings.ReplaceAll(verification.PublicVerificationConfirmPath, ":via", "email"), ":code", ""),
strings.ReplaceAll(verification.PublicVerificationInitPath, ":via", "email"),
verification.PublicVerificationRequestPath,
errorx.ErrorsPath,

link.RouteAdminCreateRecoveryLink,
link.RouteRecovery,
link.RouteVerification,

errorx.RouteGet,
},
BuildVersion: d.Registry().BuildVersion(),
BuildHash: d.Registry().BuildHash(),
Expand Down
11 changes: 8 additions & 3 deletions driver/registry.go
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/link"

"github.com/ory/x/healthx"

Expand Down Expand Up @@ -111,12 +112,16 @@ type Registry interface {
registration.HandlerProvider
registration.StrategyProvider

verification.PersistenceProvider
verification.FlowPersistenceProvider
verification.ErrorHandlerProvider
verification.SenderProvider
verification.HandlerProvider
verification.StrategyProvider

recovery.RequestPersistenceProvider
link.SenderProvider
link.VerificationTokenPersistenceProvider
link.RecoveryTokenPersistenceProvider

recovery.FlowPersistenceProvider
recovery.ErrorHandlerProvider
recovery.HandlerProvider
recovery.StrategyProvider
Expand Down
25 changes: 23 additions & 2 deletions driver/registry_default.go
Expand Up @@ -109,7 +109,8 @@ type RegistryDefault struct {
selfserviceVerifyErrorHandler *verification.ErrorHandler
selfserviceVerifyManager *identity.Manager
selfserviceVerifyHandler *verification.Handler
selfserviceVerifySender *verification.Sender

selfserviceLinkSender *link.Sender

selfserviceRecoveryErrorHandler *recovery.ErrorHandler
selfserviceRecoveryHandler *recovery.Handler
Expand All @@ -122,6 +123,7 @@ type RegistryDefault struct {
registrationStrategies []registration.Strategy
profileStrategies []settings.Strategy
recoveryStrategies []recovery.Strategy
verificationStrategies []verification.Strategy

buildVersion string
buildHash string
Expand Down Expand Up @@ -156,6 +158,7 @@ func (m *RegistryDefault) RegisterPublicRoutes(router *x.RouterPublic) {

if m.c.SelfServiceFlowVerificationEnabled() {
m.VerificationHandler().RegisterPublicRoutes(router)
m.VerificationStrategies().RegisterPublicRoutes(router)
}

m.HealthHandler().SetRoutes(router.Router, false)
Expand All @@ -177,6 +180,7 @@ func (m *RegistryDefault) RegisterAdminRoutes(router *x.RouterAdmin) {

if m.c.SelfServiceFlowVerificationEnabled() {
m.VerificationHandler().RegisterAdminRoutes(router)
m.VerificationStrategies().RegisterAdminRoutes(router)
}

m.HealthHandler().SetRoutes(router.Router, true)
Expand Down Expand Up @@ -281,6 +285,19 @@ func (m *RegistryDefault) LoginStrategies() login.Strategies {
return m.loginStrategies
}

func (m *RegistryDefault) VerificationStrategies() verification.Strategies {
if len(m.verificationStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
if s, ok := strategy.(verification.Strategy); ok {
if m.c.SelfServiceStrategy(s.VerificationStrategyID()).Enabled {
m.verificationStrategies = append(m.verificationStrategies, s)
}
}
}
}
return m.verificationStrategies
}

func (m *RegistryDefault) ActiveCredentialsCounterStrategies() []identity.ActiveCredentialsCounter {
if len(m.activeCredentialsCounterStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
Expand Down Expand Up @@ -510,7 +527,11 @@ func (m *RegistryDefault) CourierPersister() courier.Persister {
return m.persister
}

func (m *RegistryDefault) RecoveryTokenPersister() link.Persister {
func (m *RegistryDefault) RecoveryTokenPersister() link.RecoveryTokenPersister {
return m.Persister()
}

func (m *RegistryDefault) VerificationTokenPersister() link.VerificationTokenPersister {
return m.Persister()
}

Expand Down
2 changes: 1 addition & 1 deletion driver/registry_default_hooks.go
Expand Up @@ -7,7 +7,7 @@ import (

func (m *RegistryDefault) HookVerifier() *hook.Verifier {
if m.hookVerifier == nil {
m.hookVerifier = hook.NewVerifier(m)
m.hookVerifier = hook.NewVerifier(m, m.c)
}
return m.hookVerifier
}
Expand Down
8 changes: 4 additions & 4 deletions driver/registry_default_test.go
Expand Up @@ -23,21 +23,21 @@ import (

func TestDriverDefault_Hooks(t *testing.T) {
t.Run("case=verification", func(t *testing.T) {
_, reg := internal.NewFastRegistryWithMocks(t)
conf, reg := internal.NewFastRegistryWithMocks(t)
viper.Set(configuration.ViperKeySelfServiceVerificationEnabled, true)

t.Run("type=registration", func(t *testing.T) {
h := reg.PostRegistrationPostPersistHooks(identity.CredentialsTypePassword)
require.Len(t, h, 1)
assert.Equal(t, []registration.PostHookPostPersistExecutor{hook.NewVerifier(reg)}, h)
assert.Equal(t, []registration.PostHookPostPersistExecutor{hook.NewVerifier(reg, conf)}, h)

viper.Set(configuration.ViperKeySelfServiceRegistrationAfter+".password.hooks",
[]map[string]interface{}{{"hook": "session"}})

h = reg.PostRegistrationPostPersistHooks(identity.CredentialsTypePassword)
require.Len(t, h, 2)
assert.Equal(t, []registration.PostHookPostPersistExecutor{
hook.NewVerifier(reg),
hook.NewVerifier(reg, conf),
hook.NewSessionIssuer(reg),
}, h)
})
Expand All @@ -57,7 +57,7 @@ func TestDriverDefault_Hooks(t *testing.T) {
t.Run("type=settings", func(t *testing.T) {
h := reg.PostSettingsPostPersistHooks("profile")
require.Len(t, h, 1)
assert.Equal(t, []settings.PostHookPostPersistExecutor{hook.NewVerifier(reg)}, h)
assert.Equal(t, []settings.PostHookPostPersistExecutor{hook.NewVerifier(reg, conf)}, h)
})
})
}
Expand Down
13 changes: 7 additions & 6 deletions driver/registry_default_verify.go
Expand Up @@ -3,13 +3,14 @@ package driver
import (
"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/strategy/link"
)

func (m *RegistryDefault) VerificationPersister() verification.Persister {
func (m *RegistryDefault) VerificationFlowPersister() verification.FlowPersister {
return m.persister
}

func (m *RegistryDefault) VerificationRequestErrorHandler() *verification.ErrorHandler {
func (m *RegistryDefault) VerificationFlowErrorHandler() *verification.ErrorHandler {
if m.selfserviceVerifyErrorHandler == nil {
m.selfserviceVerifyErrorHandler = verification.NewErrorHandler(m, m.c)
}
Expand All @@ -33,10 +34,10 @@ func (m *RegistryDefault) VerificationHandler() *verification.Handler {
return m.selfserviceVerifyHandler
}

func (m *RegistryDefault) VerificationSender() *verification.Sender {
if m.selfserviceVerifySender == nil {
m.selfserviceVerifySender = verification.NewSender(m, m.c)
func (m *RegistryDefault) LinkSender() *link.Sender {
if m.selfserviceLinkSender == nil {
m.selfserviceLinkSender = link.NewSender(m, m.c)
}

return m.selfserviceVerifySender
return m.selfserviceLinkSender
}
9 changes: 3 additions & 6 deletions go.mod
Expand Up @@ -2,8 +2,6 @@ module github.com/ory/kratos

go 1.14

replace github.com/justinas/nosurf => github.com/aeneasr/nosurf v1.1.1-0.20200827112707-0bc5e56a715e

// See https://github.com/markbates/pkger/pull/112
replace github.com/markbates/pkger => github.com/falafeljan/pkger v0.17.1-0.20200722132747-95726f5b9b9b

Expand All @@ -27,8 +25,7 @@ require (
github.com/go-playground/locales v0.12.1 // indirect
github.com/go-playground/universal-translator v0.16.0 // indirect
github.com/go-swagger/go-swagger v0.25.0
// see https://github.com/gobuffalo/fizz/pull/96
github.com/gobuffalo/fizz v1.11.1-0.20200827214106-ed1b75c3b6ff
github.com/gobuffalo/fizz v1.12.1-0.20200830211954-9d93b59ca79a
github.com/gobuffalo/httptest v1.0.2
github.com/gobuffalo/packr/v2 v2.8.0 // indirect
github.com/gobuffalo/pop/v5 v5.2.4-0.20200706214017-1dd950510b30
Expand All @@ -49,15 +46,15 @@ require (
github.com/imdario/mergo v0.3.7
github.com/jteeuwen/go-bindata v3.0.7+incompatible
github.com/julienschmidt/httprouter v1.2.0
github.com/justinas/nosurf v1.1.0
github.com/justinas/nosurf v1.1.1
github.com/leodido/go-urn v1.1.0 // indirect
github.com/markbates/pkger v0.17.0
github.com/mattn/goveralls v0.0.5
github.com/mikefarah/yq v1.15.0
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
github.com/op/go-logging v0.0.0-20160315200505-970db520ece7 // indirect
github.com/ory/analytics-go/v4 v4.0.0
github.com/ory/cli v0.0.25-0.20200827222409-9207e62c455a
github.com/ory/cli v0.0.25-0.20200830213701-48ff64a35727
github.com/ory/dockertest v3.3.5+incompatible
github.com/ory/dockertest/v3 v3.5.4
github.com/ory/go-acc v0.1.0
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Expand Up @@ -47,8 +47,6 @@ github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
github.com/aeneasr/nosurf v1.1.1-0.20200827112707-0bc5e56a715e h1:+2Td31h37bKIvggVt75JmleBO/8XouEYsL3AZ68NRTA=
github.com/aeneasr/nosurf v1.1.1-0.20200827112707-0bc5e56a715e/go.mod h1:ALpWdSbuNGy2lZWtyXdjkYv4edL23oSEgfBT1gPJ5BQ=
github.com/ajg/form v0.0.0-20160822230020-523a5da1a92f h1:zvClvFQwU++UpIUBGC8YmDlfhUrweEy1R1Fj1gu5iIM=
github.com/ajg/form v0.0.0-20160822230020-523a5da1a92f/go.mod h1:uL1WgH+h2mgNtvBq0339dVnzXdBETtL2LeUXaIv25UY=
github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw=
Expand Down Expand Up @@ -352,8 +350,8 @@ github.com/gobuffalo/events v1.4.1/go.mod h1:SjXgWKpeSuvQDvGhgMz5IXx3Czu+IbL+XPL
github.com/gobuffalo/fizz v1.0.12/go.mod h1:C0sltPxpYK8Ftvf64kbsQa2yiCZY4RZviurNxXdAKwc=
github.com/gobuffalo/fizz v1.9.8/go.mod h1:w1FEn1yKNVCc49KnADGyYGRPH7jFON3ak4Bj1yUudHo=
github.com/gobuffalo/fizz v1.10.0/go.mod h1:J2XGPO0AfJ1zKw7+2BA+6FEGAkyEsdCOLvN93WCT2WI=
github.com/gobuffalo/fizz v1.11.1-0.20200827214106-ed1b75c3b6ff h1:YKQJAc2WkWJ0isE5VIwxcSZXNaS+2ngXinfbr+hZnzY=
github.com/gobuffalo/fizz v1.11.1-0.20200827214106-ed1b75c3b6ff/go.mod h1:cXLjhE5p3iuIes6AGZ/9+dfyOkehlB2Vldj0Iw2Uu38=
github.com/gobuffalo/fizz v1.12.1-0.20200830211954-9d93b59ca79a h1:y52Nujtd0ilFyRwCQ602a+1tMD2Nd2XvTAcw7RnmvFI=
github.com/gobuffalo/fizz v1.12.1-0.20200830211954-9d93b59ca79a/go.mod h1:cXLjhE5p3iuIes6AGZ/9+dfyOkehlB2Vldj0Iw2Uu38=
github.com/gobuffalo/flect v0.0.0-20180907193754-dc14d8acaf9f/go.mod h1:rCiQgmAE4axgBNl3jZWzS5rETRYTGOsrixTRaCPzNdA=
github.com/gobuffalo/flect v0.0.0-20181002182613-4571df4b1daf/go.mod h1:rCiQgmAE4axgBNl3jZWzS5rETRYTGOsrixTRaCPzNdA=
github.com/gobuffalo/flect v0.0.0-20181007231023-ae7ed6bfe683/go.mod h1:rCiQgmAE4axgBNl3jZWzS5rETRYTGOsrixTRaCPzNdA=
Expand Down Expand Up @@ -838,6 +836,8 @@ github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfV
github.com/julienschmidt/httprouter v1.2.0 h1:TDTW5Yz1mjftljbcKqRcrYhd4XeOoI98t+9HbQbYf7g=
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
github.com/justinas/nosurf v1.1.1 h1:92Aw44hjSK4MxJeMSyDa7jwuI9GR2J/JCQiaKvXXSlk=
github.com/justinas/nosurf v1.1.1/go.mod h1:ALpWdSbuNGy2lZWtyXdjkYv4edL23oSEgfBT1gPJ5BQ=
github.com/karrick/godirwalk v1.7.5/go.mod h1:2c9FRhkDxdIbgkOnCEvnSWs71Bhugbl46shStcFDJ34=
github.com/karrick/godirwalk v1.7.7/go.mod h1:2c9FRhkDxdIbgkOnCEvnSWs71Bhugbl46shStcFDJ34=
github.com/karrick/godirwalk v1.7.8 h1:VfG72pyIxgtC7+3X9CMHI0AOl4LwyRAg98WAgsvffi8=
Expand Down Expand Up @@ -1041,8 +1041,8 @@ github.com/openzipkin/zipkin-go v0.2.2 h1:nY8Hti+WKaP0cRsSeQ026wU03QsM762XBeCXBb
github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4=
github.com/ory/analytics-go/v4 v4.0.0 h1:KQ2P00j9dbj4lDC/Albw/zn/scK67041RhqeW5ptsbw=
github.com/ory/analytics-go/v4 v4.0.0/go.mod h1:FMx9cLRD9xN+XevPvZ5FDMfignpmcqPP6FUKnJ9/MmE=
github.com/ory/cli v0.0.25-0.20200827222409-9207e62c455a h1:UZBzAFt103fILKINZoODh3j1Wmsumh6eoH4bMruFcok=
github.com/ory/cli v0.0.25-0.20200827222409-9207e62c455a/go.mod h1:9AA94/MP63p9v5fvOoaGucHaWMOejKhqYPHhUQwEHNE=
github.com/ory/cli v0.0.25-0.20200830213701-48ff64a35727 h1:HoH1xPI+mVZO8xVyAfnAcvlxWqjfXY/t+rkovxYYzXM=
github.com/ory/cli v0.0.25-0.20200830213701-48ff64a35727/go.mod h1:V/iB7aUCfoB2r/DEUSva+7GHVg7kLviukdhHZm7bOsA=
github.com/ory/dockertest v3.3.5+incompatible h1:iLLK6SQwIhcbrG783Dghaaa3WPzGc+4Emza6EbVUUGA=
github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnhNrne+V0E6LAcBILJdPs=
github.com/ory/dockertest/v3 v3.5.4 h1:rYijlJuraj8D4OgC1DpYpCV8SGXrkviT3RVrjFy7OFc=
Expand Down
3 changes: 3 additions & 0 deletions identity/address.go
@@ -0,0 +1,3 @@
package identity

const AddressTypeEmail = "email"
5 changes: 1 addition & 4 deletions identity/extension_verify.go
Expand Up @@ -31,10 +31,7 @@ func (r *SchemaExtensionVerification) Run(ctx jsonschema.ValidationContext, s sc
return ctx.Error("format", "%q is not valid %q", value, "email")
}

address, err := NewVerifiableEmailAddress(fmt.Sprintf("%s", value), r.i.ID, r.lifespan)
if err != nil {
return err
}
address := NewVerifiableEmailAddress(fmt.Sprintf("%s", value), r.i.ID, r.lifespan)

if has := r.has(r.i.VerifiableAddresses, address); has != nil {
if r.has(r.v, address) == nil {
Expand Down
8 changes: 0 additions & 8 deletions identity/extension_verify_test.go
Expand Up @@ -59,7 +59,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
Status: VerifiableAddressStatusPending,
Via: VerifiableAddressTypeEmail,
IdentityID: iid,
Code: "code",
ExpiresAt: time.Now().Add(time.Minute),
},
},
Expand Down Expand Up @@ -90,7 +89,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
Status: VerifiableAddressStatusCompleted,
Via: VerifiableAddressTypeEmail,
IdentityID: iid,
Code: "code",
ExpiresAt: time.Now().Add(time.Minute),
},
{
Expand All @@ -99,7 +97,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
Status: VerifiableAddressStatusCompleted,
Via: VerifiableAddressTypeEmail,
IdentityID: iid,
Code: "code",
ExpiresAt: time.Now().Add(time.Minute),
},
},
Expand Down Expand Up @@ -130,7 +127,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
Status: VerifiableAddressStatusCompleted,
Via: VerifiableAddressTypeEmail,
IdentityID: iid,
Code: "code",
ExpiresAt: time.Now().Add(time.Minute),
},
{
Expand All @@ -139,7 +135,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
Status: VerifiableAddressStatusCompleted,
Via: VerifiableAddressTypeEmail,
IdentityID: iid,
Code: "code",
ExpiresAt: time.Now().Add(time.Minute),
},
},
Expand Down Expand Up @@ -199,9 +194,6 @@ func TestSchemaExtensionVerification(t *testing.T) {
require.Len(t, addresses, len(tc.expect))

for _, actual := range addresses {
assert.NotEmpty(t, actual.Code)
actual.Code = ""

// Prevent time synchro issues
assert.True(t, actual.ExpiresAt.After(time.Now().Add(expiresAt-time.Second)))
actual.ExpiresAt = time.Time{}
Expand Down

0 comments on commit 00ee828

Please sign in to comment.