Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLATFORM-7675 totp lookup secrets bound #88

9 changes: 5 additions & 4 deletions driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ type RegistryDefault struct {

persister persistence.Persister

hookVerifier *hook.Verifier
hookSessionIssuer *hook.SessionIssuer
hookSessionDestroyer *hook.SessionDestroyer
hookAddressVerifier *hook.AddressVerifier
hookVerifier *hook.Verifier
hookSessionIssuer *hook.SessionIssuer
hookSessionDestroyer *hook.SessionDestroyer
hookAddressVerifier *hook.AddressVerifier
hookTotpSecretsDestroyer *hook.TotpSecretsDestroyer

credentialsHandler *credentials.Handler

Expand Down
15 changes: 15 additions & 0 deletions driver/registry_default_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ func (m *RegistryDefault) HookAddressVerifier() *hook.AddressVerifier {
return m.hookAddressVerifier
}

// fandom-start

func (m *RegistryDefault) HookTotpSecretsDestroyer() *hook.TotpSecretsDestroyer {
if m.hookTotpSecretsDestroyer == nil {
m.hookTotpSecretsDestroyer = hook.NewTotpSecretsDestroyer(m)
}
return m.hookTotpSecretsDestroyer
}

// fandom-end

func (m *RegistryDefault) WithHooks(hooks map[string]func(config.SelfServiceHook) interface{}) {
m.injectedSelfserviceHooks = hooks
}
Expand All @@ -48,6 +59,10 @@ func (m *RegistryDefault) getHooks(credentialsType string, configs []config.Self
i = append(i, hook.NewWebHook(m, h.Config))
case hook.KeyAddressVerifier:
i = append(i, m.HookAddressVerifier())
// fandom-start
case hook.KeyTotpLookupSecretsDestroyer:
i = append(i, m.HookTotpSecretsDestroyer())
// fandom-end
default:
var found bool
for name, m := range m.injectedSelfserviceHooks {
Expand Down
29 changes: 29 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@
"hook"
]
},
"selfServiceTotpSecretsDestroyerHook": {
"type": "object",
"properties": {
"hook": {
"const": "totp_destroys_lookup_secrets"
}
},
"additionalProperties": false,
"required": [
"hook"
]
},
"webHookAuthBasicAuthProperties": {
"properties": {
"type": {
Expand Down Expand Up @@ -640,6 +652,9 @@
"anyOf": [
{
"$ref": "#/definitions/selfServiceWebHook"
},
{
"$ref": "#/definitions/selfServiceTotpSecretsDestroyerHook"
}
]
},
Expand Down Expand Up @@ -749,6 +764,9 @@
"profile": {
"$ref": "#/definitions/selfServiceAfterSettingsMethod"
},
"totp": {
"$ref": "#/definitions/selfServiceAfterSettingsMethod"
},
"hooks": {
"$ref": "#/definitions/selfServiceHooks"
}
Expand Down Expand Up @@ -1348,6 +1366,17 @@
"type": "boolean",
"title": "Enables the lookup secret method",
"default": false
},
"config": {
"type": "object",
"title": "Lookup Secret Configuration",
"properties": {
"enabled_only_in_2fa": {
"type": "boolean",
"title": "Will enable lookup secret codes when 2FA is enabled for a user (i.e. TOTP)",
"default": false
}
}
}
}
},
Expand Down
4 changes: 4 additions & 0 deletions selfservice/hook/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ const (
KeySessionDestroyer = "revoke_active_sessions"
KeyWebHook = "web_hook"
KeyAddressVerifier = "require_verified_address"
// fandom-start

KeyTotpLookupSecretsDestroyer = "totp_destroys_lookup_secrets" // nolint:gosec
// fandom-end
)
38 changes: 38 additions & 0 deletions selfservice/hook/totp_lookup_secrets_destroyer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package hook

import (
"net/http"

"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/flow/settings"
)

var _ settings.PostHookPrePersistExecutor = new(TotpSecretsDestroyer)

type (
totpSecretsDestroyerDependencies interface {
identity.PrivilegedPoolProvider
}
TotpSecretsDestroyer struct {
r totpSecretsDestroyerDependencies
}
)

func NewTotpSecretsDestroyer(r totpSecretsDestroyerDependencies) *TotpSecretsDestroyer {
return &TotpSecretsDestroyer{r: r}
}

func (t *TotpSecretsDestroyer) ExecuteSettingsPrePersistHook(w http.ResponseWriter, r *http.Request, _ *settings.Flow, i *identity.Identity, settingsType string) error {
// sanity check
if settingsType != "totp" {
return nil
}

_, hasTotp := i.GetCredentials(identity.CredentialsTypeTOTP)
_, hasLookupSecrets := i.GetCredentials(identity.CredentialsTypeLookup)
if !hasTotp && hasLookupSecrets {
i.DeleteCredentialsType(identity.CredentialsTypeLookup)
}

return nil
}
11 changes: 11 additions & 0 deletions selfservice/strategy/lookup/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, requestedAAL identity.Au
return err
}

// fandom-start
enabled, err := s.isEnabledForIdentity(r.Context(), sess.IdentityID)
if err != nil {
return err
}

if !enabled {
return nil
}
// fandom-end

id, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), sess.IdentityID)
if err != nil {
return err
Expand Down
89 changes: 89 additions & 0 deletions selfservice/strategy/lookup/settings.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package lookup

import (
"bytes"
"context"
"encoding/json"
"net/http"
"time"

"github.com/pquerna/otp"
"github.com/tidwall/gjson"
"github.com/tidwall/sjson"

"github.com/ory/kratos/selfservice/strategy/totp"
"github.com/ory/x/jsonx"
"github.com/ory/x/randx"

"github.com/ory/herodot"
Expand Down Expand Up @@ -88,6 +92,15 @@ func (p *submitSelfServiceSettingsFlowWithLookupMethodBody) SetFlowID(rid uuid.U
}

func (s *Strategy) Settings(w http.ResponseWriter, r *http.Request, f *settings.Flow, ss *session.Session) (*settings.UpdateContext, error) {
// fandom-start
enabled, err := s.isEnabledForIdentity(r.Context(), ss.IdentityID)
if err != nil {
return nil, errors.WithStack(err)
}
if !enabled {
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}
// fandom-end
var p submitSelfServiceSettingsFlowWithLookupMethodBody
ctxUpdate, err := settings.PrepareUpdate(s.d, w, r, f, ss, settings.ContinuityKey(s.SettingsStrategyID()), &p)
if errors.Is(err, settings.ErrContinuePreviousAction) {
Expand Down Expand Up @@ -332,6 +345,16 @@ func (s *Strategy) identityHasLookup(ctx context.Context, id uuid.UUID) (bool, e
func (s *Strategy) PopulateSettingsMethod(r *http.Request, id *identity.Identity, f *settings.Flow) error {
f.UI.SetCSRF(s.d.GenerateCSRFToken(r))

// fandom-start
enabled, err := s.isEnabledForIdentity(r.Context(), id.ID)
if err != nil {
return err
}
if !enabled {
return nil
}
// fandom-end

hasLookup, err := s.identityHasLookup(r.Context(), id.ID)
if err != nil {
return err
Expand Down Expand Up @@ -362,3 +385,69 @@ func (s *Strategy) handleSettingsError(w http.ResponseWriter, r *http.Request, c

return err
}

// fandom-start

type StrategyConfiguration struct {
EnabledOnlyIn2FA bool `json:"enabled_only_in_2fa,omitempty"`
}

func (s *Strategy) Config(ctx context.Context) (*StrategyConfiguration, error) {
var c StrategyConfiguration

conf := s.d.Config(ctx).SelfServiceStrategy(string(s.ID())).Config
if err := jsonx.
NewStrictDecoder(bytes.NewBuffer(conf)).
Decode(&c); err != nil {
s.d.Logger().WithError(err).WithField("config", conf)
return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode Lookup Secret configuration: %s", err))
}

return &c, nil
}

func (s *Strategy) isEnabledForIdentity(ctx context.Context, id uuid.UUID) (bool, error) {
conf, err := s.Config(ctx)
if err != nil {
return true, err
}

if !conf.EnabledOnlyIn2FA {
return true, nil
}

return s.identityHasTOTP(ctx, id)
}

func (s *Strategy) CountActiveTotpCredentials(cc map[identity.CredentialsType]identity.Credentials) (count int, err error) {
for _, c := range cc {
if c.Type == identity.CredentialsTypeTOTP && len(c.Config) > 0 {
var conf totp.CredentialsConfig
if err = json.Unmarshal(c.Config, &conf); err != nil {
return 0, errors.WithStack(err)
}

_, err := otp.NewKeyFromURL(conf.TOTPURL)
if len(c.Identifiers) > 0 && len(c.Identifiers[0]) > 0 && len(conf.TOTPURL) > 0 && err == nil {
count++
}
}
}
return
}

func (s *Strategy) identityHasTOTP(ctx context.Context, id uuid.UUID) (bool, error) {
confidential, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(ctx, id)
if err != nil {
return false, err
}

count, err := s.CountActiveTotpCredentials(confidential.Credentials)
if err != nil {
return false, err
}

return count > 0, nil
}

// fandom-end
11 changes: 11 additions & 0 deletions selfservice/strategy/lookup/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ func (s *Strategy) CountActiveFirstFactorCredentials(cc map[identity.Credentials
}

func (s *Strategy) CountActiveMultiFactorCredentials(cc map[identity.CredentialsType]identity.Credentials) (count int, err error) {
hasTotp := false
for _, c := range cc {
if c.Type == identity.CredentialsTypeTOTP {
hasTotp = true
}
if c.Type == s.ID() && len(c.Config) > 0 {
var conf CredentialsConfig
if err = json.Unmarshal(c.Config, &conf); err != nil {
Expand All @@ -90,6 +94,13 @@ func (s *Strategy) CountActiveMultiFactorCredentials(cc map[identity.Credentials
}
}
}
// fandom-start
// TODO: propagate context properly
conf, err := s.Config(context.Background())
if err == nil && conf.EnabledOnlyIn2FA && !hasTotp {
count = 0
}
// fandom-end
return
}

Expand Down
21 changes: 20 additions & 1 deletion selfservice/strategy/lookup/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"testing"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/selfservice/strategy/lookup"

"github.com/stretchr/testify/assert"
Expand All @@ -14,7 +15,7 @@ import (
)

func TestCountActiveFirstFactorCredentials(t *testing.T) {
_, reg := internal.NewFastRegistryWithMocks(t)
conf, reg := internal.NewFastRegistryWithMocks(t)
strategy := lookup.NewStrategy(reg)

t.Run("first factor", func(t *testing.T) {
Expand All @@ -26,20 +27,23 @@ func TestCountActiveFirstFactorCredentials(t *testing.T) {
t.Run("multi factor", func(t *testing.T) {
for k, tc := range []struct {
in identity.CredentialsCollection
config []byte
expected int
}{
{
in: identity.CredentialsCollection{{
Type: strategy.ID(),
Config: []byte{},
}},
config: []byte(`{}`),
expected: 0,
},
{
in: identity.CredentialsCollection{{
Type: strategy.ID(),
Config: []byte(`{"recovery_codes": []}`),
}},
config: []byte(`{}`),
expected: 0,
},
{
Expand All @@ -48,26 +52,41 @@ func TestCountActiveFirstFactorCredentials(t *testing.T) {
Identifiers: []string{"foo"},
Config: []byte(`{"recovery_codes": [{}]}`),
}},
config: []byte(`{}`),
expected: 1,
},
{
in: identity.CredentialsCollection{{
Type: strategy.ID(),
Config: []byte(`{}`),
}},
config: []byte(`{}`),
expected: 0,
},
{
in: identity.CredentialsCollection{{}, {}},
config: []byte(`{}`),
expected: 0,
},
// fandom-start
{
in: identity.CredentialsCollection{{
Type: strategy.ID(),
Identifiers: []string{"foo"},
Config: []byte(`{"recovery_codes": [{}]}`),
}},
config: []byte(`{"enabled_only_in_2fa": true}`),
expected: 0,
},
// fandom-end
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
cc := map[identity.CredentialsType]identity.Credentials{}
for _, c := range tc.in {
cc[c.Type] = c
}

conf.MustSet(fmt.Sprintf("%s.%s.config", config.ViperKeySelfServiceStrategyConfig, strategy.ID()), tc.config)
actual, err := strategy.CountActiveMultiFactorCredentials(cc)
require.NoError(t, err)
assert.Equal(t, tc.expected, actual)
Expand Down