Skip to content

Commit

Permalink
feat: implement account recovery skeleton
Browse files Browse the repository at this point in the history
This patch implements the account recovery request skeleton with endpoints such as "Init Account Recovery", a new config value `urls�.recovery_ui` and so on.

Additionally, some refactoring was made to DRY code and make naming consistent.

See #37

BREAKING CHANGEs: The field `identity.addresses` has moved to `identity.verifiable_addresses`. A new field has been added
`identity.recovery_addresses`. Configuration key `selfservice.verify` was renamed to `selfservice.verification`. Configuration key `selfservice.verification.link_lifespan`
has been merged with  `selfservice.verification.request_lifespan`.
  • Loading branch information
aeneasr committed Jun 3, 2020
1 parent 8d15307 commit b1e9183
Show file tree
Hide file tree
Showing 79 changed files with 2,951 additions and 252 deletions.
33 changes: 33 additions & 0 deletions docs/docs/self-service/flows/password-reset-account-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,36 @@ recovered. Common use cases include:

The forgot password flow is a work in progress and will be implemented in a
future release of ORY Kratos.

## Questions

> One option is to allow the user to self-construct their own questions. The problem with this though is that you end up with either painfully obvious questions:
>
> - **What colour is the sky?**
> - **How do you spell “password”?**
>
> Questions which can put people in an uncomfortable position when a human uses the secret question for verification (such as in a call centre):
>
> **Who did I sleep with at the Christmas party?**
>
> When it comes to secret questions, people need to be saved from themselves! In other words, the site itself should
define the secret question, or rather define a series of secret questions from which the user can choose.
And not just choose one either; ideally, the user should define two or more secret questions at the time of account
registration which can then be used as a second channel of identity verification. Having multiple questions adds
a higher degree of confidence to the verification process plus gives you opportunity to add randomness (not always
show the same question) plus provides a bit of redundancy should someone legitimate forget an answer.
>
> So what makes a good secret question? There are a few different factors:
>
> * It should be concise – the question is to the point and unambiguous
> * The answer is specific – you don’t want a question which could be answered in different ways by the same person
> * The possible answers must be diverse – a question about someone’s favourite colour would result in a small subset of possible answers
> * Answer discovery should be hard – if you can readily find the answer for anyone (think high-profile people) then it’s no good
> * The answer must be constant over time – asking for someone’s favourite movie may result in a different answer a year from now
>
> [Source](https://www.troyhunt.com/everything-you-ever-wanted-to-know/)
Here are some good examples:

- What was the first concert you ever went to and where? (e.g. "Pink Floyd at Gotham City Stadium")
- ...
4 changes: 3 additions & 1 deletion driver/configuration/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ type Provider interface {
VerificationURL() *url.URL
ErrorURL() *url.URL
MultiFactorURL() *url.URL
RecoveryURL() *url.URL

SessionLifespan() time.Duration
SelfServiceSettingsRequestLifespan() time.Duration
SelfServiceVerificationRequestLifespan() time.Duration
SelfServiceLoginRequestLifespan() time.Duration
SelfServiceRegistrationRequestLifespan() time.Duration
SelfServiceRecoveryRequestLifespan() time.Duration

SelfServiceStrategy(strategy string) *SelfServiceStrategy
SelfServiceLoginBeforeHooks() []SelfServiceHook
Expand All @@ -82,7 +84,7 @@ type Provider interface {
SelfServiceSettingsAfterHooks(strategy string) []SelfServiceHook
SelfServiceSettingsReturnTo(strategy string, defaultReturnTo *url.URL) *url.URL
SelfServiceLogoutRedirectURL() *url.URL
SelfServiceVerificationLinkLifespan() time.Duration

SelfServicePrivilegedSessionMaxAge() time.Duration
SelfServiceVerificationReturnTo() *url.URL

Expand Down
22 changes: 13 additions & 9 deletions driver/configuration/provider_viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const (
ViperKeyURLsLogin = "urls.login_ui"
ViperKeyURLsError = "urls.error_ui"
ViperKeyURLsVerification = "urls.verify_ui"
ViperKeyURLsRecovery = "urls.recovery_ui"
ViperKeyURLsSettings = "urls.settings_ui"
ViperKeyURLsMFA = "urls.mfa_ui"
ViperKeyURLsRegistration = "urls.registration_ui"
Expand All @@ -73,9 +74,10 @@ const (
ViperKeySelfServiceSettingsRequestLifespan = "selfservice.settings.request_lifespan"
ViperKeySelfServicePrivilegedAuthenticationAfter = "selfservice.settings.privileged_session_max_age"

ViperKeySelfServiceLifespanLink = "selfservice.verify.link_lifespan"
ViperKeySelfServiceLifespanVerificationRequest = "selfservice.verify.request_lifespan"
ViperKeySelfServiceVerifyReturnTo = "selfservice.verify.return_to"
ViperKeySelfServiceLifespanRecoveryRequest = "selfservice.recovery.request_lifespan"

ViperKeySelfServiceLifespanVerificationRequest = "selfservice.verification.request_lifespan"
ViperKeySelfServiceVerifyReturnTo = "selfservice.verification.return_to"

ViperKeyDefaultIdentityTraitsSchemaURL = "identity.traits.default_schema_url"
ViperKeyIdentityTraitsSchemas = "identity.traits.schemas"
Expand Down Expand Up @@ -299,6 +301,10 @@ func (p *ViperProvider) RegisterURL() *url.URL {
return mustParseURLFromViper(p.l, ViperKeyURLsRegistration)
}

func (p *ViperProvider) RecoveryURL() *url.URL {
return mustParseURLFromViper(p.l, ViperKeyURLsRecovery)
}

func (p *ViperProvider) SessionLifespan() time.Duration {
return viperx.GetDuration(p.l, ViperKeyLifespanSession, time.Hour)
}
Expand Down Expand Up @@ -375,20 +381,18 @@ func (p *ViperProvider) VerificationURL() *url.URL {
return mustParseURLFromViper(p.l, ViperKeyURLsVerification)
}

// SelfServiceVerificationRequestLifespan defines the lifespan of a verification request (the ui interaction). This
// does not specify the lifespan of a verification code!
func (p *ViperProvider) SelfServiceVerificationRequestLifespan() time.Duration {
return viperx.GetDuration(p.l, ViperKeySelfServiceLifespanVerificationRequest, time.Hour)
}

func (p *ViperProvider) SelfServiceVerificationLinkLifespan() time.Duration {
return viperx.GetDuration(p.l, ViperKeySelfServiceLifespanLink, time.Hour*24)
}

func (p *ViperProvider) SelfServiceVerificationReturnTo() *url.URL {
return mustParseURLFromViper(p.l, ViperKeySelfServiceVerifyReturnTo)
}

func (p *ViperProvider) SelfServiceRecoveryRequestLifespan() time.Duration {
return viperx.GetDuration(p.l, ViperKeySelfServiceLifespanRecoveryRequest, time.Hour)
}

func (p *ViperProvider) SelfServicePrivilegedSessionMaxAge() time.Duration {
return viperx.GetDuration(p.l, ViperKeySelfServicePrivilegedAuthenticationAfter, time.Hour)
}
Expand Down
7 changes: 7 additions & 0 deletions driver/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/ory/kratos/continuity"
"github.com/ory/kratos/courier"
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verify"

Expand Down Expand Up @@ -113,6 +114,12 @@ type Registry interface {
verify.SenderProvider
verify.HandlerProvider

recovery.RequestPersistenceProvider
recovery.ErrorHandlerProvider
recovery.StrategyProvider
recovery.HandlerProvider
recovery.StrategyProvider

x.CSRFTokenGeneratorProvider
}

Expand Down
37 changes: 12 additions & 25 deletions driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/ory/kratos/continuity"
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verify"
"github.com/ory/kratos/selfservice/hook"
Expand Down Expand Up @@ -106,13 +107,18 @@ type RegistryDefault struct {
selfserviceVerifyHandler *verify.Handler
selfserviceVerifySender *verify.Sender

selfserviceRecoveryErrorHandler *recovery.ErrorHandler
selfserviceRecoveryHandler *recovery.Handler
selfserviceRecoverySender *recovery.Sender

selfserviceLogoutHandler *logout.Handler

selfserviceStrategies []interface{}
loginStrategies []login.Strategy
activeCredentialsCounterStrategies []identity.ActiveCredentialsCounter
registrationStrategies []registration.Strategy
profileStrategies []settings.Strategy
recoveryStrategies []recovery.Strategy

buildVersion string
buildHash string
Expand All @@ -133,6 +139,7 @@ func (m *RegistryDefault) RegisterPublicRoutes(router *x.RouterPublic) {
m.SelfServiceErrorHandler().RegisterPublicRoutes(router)
m.SchemaHandler().RegisterPublicRoutes(router)
m.VerificationHandler().RegisterPublicRoutes(router)
m.RecoveryHandler().RegisterPublicRoutes(router)
m.HealthHandler().SetRoutes(router.Router, false)
}

Expand All @@ -145,6 +152,7 @@ func (m *RegistryDefault) RegisterAdminRoutes(router *x.RouterAdmin) {
m.IdentityHandler().RegisterAdminRoutes(router)
m.SessionHandler().RegisterAdminRoutes(router)
m.SelfServiceErrorHandler().RegisterAdminRoutes(router)
m.RecoveryHandler().RegisterAdminRoutes(router)
m.HealthHandler().SetRoutes(router.Router, true)
}

Expand Down Expand Up @@ -181,20 +189,6 @@ func (m *RegistryDefault) WithLogger(l logrus.FieldLogger) Registry {
return m
}

func (m *RegistryDefault) SettingsHandler() *settings.Handler {
if m.selfserviceSettingsHandler == nil {
m.selfserviceSettingsHandler = settings.NewHandler(m, m.c)
}
return m.selfserviceSettingsHandler
}

func (m *RegistryDefault) SettingsRequestErrorHandler() *settings.ErrorHandler {
if m.selfserviceSettingsErrorHandler == nil {
m.selfserviceSettingsErrorHandler = settings.NewErrorHandler(m, m.c)
}
return m.selfserviceSettingsErrorHandler
}

func (m *RegistryDefault) LogoutHandler() *logout.Handler {
if m.selfserviceLogoutHandler == nil {
m.selfserviceLogoutHandler = logout.NewHandler(m, m.c)
Expand Down Expand Up @@ -235,17 +229,6 @@ func (m *RegistryDefault) selfServiceStrategies() []interface{} {
return m.selfserviceStrategies
}

func (m *RegistryDefault) SettingsStrategies() settings.Strategies {
if len(m.profileStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
if s, ok := strategy.(settings.Strategy); ok {
m.profileStrategies = append(m.profileStrategies, s)
}
}
}
return m.profileStrategies
}

func (m *RegistryDefault) RegistrationStrategies() registration.Strategies {
if len(m.registrationStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
Expand Down Expand Up @@ -472,6 +455,10 @@ func (m *RegistryDefault) RegistrationRequestPersister() registration.RequestPer
return m.persister
}

func (m *RegistryDefault) RecoveryRequestPersister() recovery.RequestPersister {
return m.persister
}

func (m *RegistryDefault) LoginRequestPersister() login.RequestPersister {
return m.persister
}
Expand Down
40 changes: 40 additions & 0 deletions driver/registry_default_recovery.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package driver

import (
"github.com/ory/kratos/selfservice/flow/recovery"
)

func (m *RegistryDefault) RecoveryRequestErrorHandler() *recovery.ErrorHandler {
if m.selfserviceRecoveryErrorHandler == nil {
m.selfserviceRecoveryErrorHandler = recovery.NewErrorHandler(m, m.c)
}

return m.selfserviceRecoveryErrorHandler
}

func (m *RegistryDefault) RecoveryHandler() *recovery.Handler {
if m.selfserviceRecoveryHandler == nil {
m.selfserviceRecoveryHandler = recovery.NewHandler(m, m.c)
}

return m.selfserviceRecoveryHandler
}

func (m *RegistryDefault) RecoverySender() *recovery.Sender {
if m.selfserviceRecoverySender == nil {
m.selfserviceRecoverySender = recovery.NewSender(m, m.c)
}

return m.selfserviceRecoverySender
}

func (m *RegistryDefault) RecoveryStrategies() recovery.Strategies {
if len(m.recoveryStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
if s, ok := strategy.(recovery.Strategy); ok {
m.recoveryStrategies = append(m.recoveryStrategies, s)
}
}
}
return m.recoveryStrategies
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,28 @@ func (m *RegistryDefault) SettingsHookExecutor() *settings.HookExecutor {
}
return m.selfserviceSettingsExecutor
}

func (m *RegistryDefault) SettingsHandler() *settings.Handler {
if m.selfserviceSettingsHandler == nil {
m.selfserviceSettingsHandler = settings.NewHandler(m, m.c)
}
return m.selfserviceSettingsHandler
}

func (m *RegistryDefault) SettingsRequestErrorHandler() *settings.ErrorHandler {
if m.selfserviceSettingsErrorHandler == nil {
m.selfserviceSettingsErrorHandler = settings.NewErrorHandler(m, m.c)
}
return m.selfserviceSettingsErrorHandler
}

func (m *RegistryDefault) SettingsStrategies() settings.Strategies {
if len(m.profileStrategies) == 0 {
for _, strategy := range m.selfServiceStrategies() {
if s, ok := strategy.(settings.Strategy); ok {
m.profileStrategies = append(m.profileStrategies, s)
}
}
}
return m.profileStrategies
}
6 changes: 5 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module github.com/ory/kratos

go 1.14

replace github.com/ory/x => ../x

require (
github.com/Masterminds/sprig/v3 v3.0.0
github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751 // indirect
Expand Down Expand Up @@ -64,12 +66,13 @@ require (
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
github.com/pkg/errors v0.9.1
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e
github.com/sirupsen/logrus v1.5.0
github.com/sirupsen/logrus v1.6.0
github.com/spf13/cobra v0.0.7
github.com/sqs/goreturns v0.0.0-20181028201513-538ac6014518
github.com/stretchr/testify v1.5.1
github.com/tidwall/gjson v1.3.5
github.com/tidwall/sjson v1.0.4
github.com/uber/jaeger-lib v2.2.0+incompatible // indirect
github.com/urfave/negroni v1.0.0
go.mongodb.org/mongo-driver v1.3.3 // indirect
golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37
Expand All @@ -79,6 +82,7 @@ require (
golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9 // indirect
golang.org/x/tools v0.0.0-20200515010526-7d3b6ebf133d
gopkg.in/go-playground/validator.v9 v9.28.0
gopkg.in/gorp.v1 v1.7.2 // indirect
gopkg.in/ini.v1 v1.56.0 // indirect
gopkg.in/yaml.v2 v2.3.0 // indirect
)
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ github.com/konsorten/go-windows-terminal-sequences v0.0.0-20180402223658-b729f26
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/konsorten/go-windows-terminal-sequences v1.0.2 h1:DB17ag19krx9CFsz4o3enTrPXyIXCl+2iCXH/aMAp9s=
github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
Expand Down Expand Up @@ -1033,6 +1034,8 @@ github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/sirupsen/logrus v1.5.0 h1:1N5EYkVAPEywqZRJd7cwnRtCb6xJx7NH3T3WUTF980Q=
github.com/sirupsen/logrus v1.5.0/go.mod h1:+F7Ogzej0PZc/94MaYx/nvG9jOFMD2osvC3s+Squfpo=
github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I=
github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
github.com/smartystreets/goconvey v0.0.0-20180222194500-ef6db91d284a/go.mod h1:XDJAKZRPZ1CvBcN2aX5YOUTYGHki24fSF0Iv48Ibg0s=
Expand Down
4 changes: 2 additions & 2 deletions identity/extension_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (r *SchemaExtensionVerify) Run(ctx jsonschema.ValidationContext, s schema.E
return err
}

if has := r.has(r.i.Addresses, address); has != nil {
if has := r.has(r.i.VerifiableAddresses, address); has != nil {
if r.has(r.v, address) == nil {
r.v = append(r.v, *has)
}
Expand Down Expand Up @@ -65,6 +65,6 @@ func (r *SchemaExtensionVerify) has(haystack []VerifiableAddress, needle *Verifi
}

func (r *SchemaExtensionVerify) Finish() error {
r.i.Addresses = r.v
r.i.VerifiableAddresses = r.v
return nil
}
4 changes: 2 additions & 2 deletions identity/extension_verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func TestSchemaExtensionVerify(t *testing.T) {
},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
id := &Identity{ID: iid, Addresses: tc.existing}
id := &Identity{ID: iid, VerifiableAddresses: tc.existing}
c := jsonschema.NewCompiler()
runner, err := schema.NewExtensionRunner(schema.ExtensionRunnerIdentityMetaSchema)
require.NoError(t, err)
Expand All @@ -195,7 +195,7 @@ func TestSchemaExtensionVerify(t *testing.T) {

require.NoError(t, e.Finish())

addresses := id.Addresses
addresses := id.VerifiableAddresses
require.Len(t, addresses, len(tc.expect))

for _, actual := range addresses {
Expand Down
Loading

0 comments on commit b1e9183

Please sign in to comment.