Skip to content

Commit

Permalink
handle both old and new format for code credential config
Browse files Browse the repository at this point in the history
  • Loading branch information
wewelll committed Feb 25, 2025
1 parent 0186e87 commit dcfd798
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 12 deletions.
49 changes: 39 additions & 10 deletions selfservice/strategy/code/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,25 +154,54 @@ func (s *Strategy) CountActiveMultiFactorCredentials(ctx context.Context, cc map
return 0, nil
}

// First try to unmarshal as the new format
var conf identity.CredentialsCode
if err := json.Unmarshal(creds.Config, &conf); err != nil {
if err := json.Unmarshal(creds.Config, &conf); err == nil {
// New format with addresses array
if len(conf.Addresses) > 0 {
// Count valid addresses configured for MFA
validAddresses := 0
for _, addr := range conf.Addresses {
if addr.Address != "" {
validAddresses++
}
}
return validAddresses, nil
}
}

// If that fails or there are no addresses, try the legacy format
// Legacy format has address_type directly in the config
var legacyConf struct {
AddressType string `json:"address_type"`
}
if err := json.Unmarshal(creds.Config, &legacyConf); err != nil {
return 0, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to unmarshal credentials config: %s", err))
}

// If no addresses configured, return 0
if len(conf.Addresses) == 0 {
return 0, nil
// If address_type is set, count as 1 valid address
if legacyConf.AddressType == "email" || legacyConf.AddressType == "sms" {
return 1, nil
}

// Count valid addresses configured for MFA
validAddresses := 0
for _, addr := range conf.Addresses {
if addr.Address != "" {
validAddresses++
// Try to check for via field in addresses (for backward compatibility)
var viaConf struct {
Addresses []struct {
Address string `json:"address"`
Via string `json:"via"`
} `json:"addresses"`
}
if err := json.Unmarshal(creds.Config, &viaConf); err == nil && len(viaConf.Addresses) > 0 {
validAddresses := 0
for _, addr := range viaConf.Addresses {
if addr.Address != "" && (addr.Via == "email" || addr.Via == "sms") {
validAddresses++
}
}
return validAddresses, nil
}

return validAddresses, nil
return 0, nil
}

func NewStrategy(deps any) *Strategy {
Expand Down
24 changes: 22 additions & 2 deletions session/manager_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,11 +509,18 @@ func TestDoesSessionSatisfy(t *testing.T) {
Config: []byte(`{"use_password_migration_hook":true}`),
}

codeLegacy := identity.Credentials{
Type: identity.CredentialsTypeCodeAuth,
Identifiers: []string{testhelpers.RandomEmail()},
Config: []byte(`{"address_type":"email","used_at":{"Time":"0001-01-01T00:00:00Z","Valid":false}}`),
}

code := identity.Credentials{
Type: identity.CredentialsTypeCodeAuth,
Identifiers: []string{testhelpers.RandomEmail()},
Config: []byte(`{"addresses":[{"address":"[email protected]","channel":"email"}],"used_at":{"Time":"0001-01-01T00:00:00Z","Valid":false}}`),
Config: []byte(`{"addresses":[{"address":"[email protected]","via":"email"}],"used_at":{"Time":"0001-01-01T00:00:00Z","Valid":false}}`),
}

//codeEmpty := identity.Credentials{
// Type: identity.CredentialsTypeCodeAuth,
// Identifiers: []string{testhelpers.RandomEmail()},
Expand Down Expand Up @@ -687,7 +694,20 @@ func TestDoesSessionSatisfy(t *testing.T) {
errIs: new(session.ErrAALNotSatisfied),
},
{
desc: "with highest_available a recovery link user requires aal2 if they have 2fa code configured",
desc: "with highest_available a recovery link user requires aal2 if they have 2fa code configured - legacy code config",
matcher: config.HighestAvailableAAL,
creds: []identity.Credentials{codeLegacy},
withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeRecoveryLink]},
withContext: func(t *testing.T, ctx context.Context) context.Context {
return confighelpers.WithConfigValues(ctx, map[string]any{
"selfservice.methods.code.passwordless_enabled": false,
"selfservice.methods.code.mfa_enabled": true,
})
},
errIs: new(session.ErrAALNotSatisfied),
},
{
desc: "with highest_available a recovery link user requires aal2 if they have 2fa code configured - new code config",
matcher: config.HighestAvailableAAL,
creds: []identity.Credentials{code},
withAMR: session.AuthenticationMethods{amrs[identity.CredentialsTypeRecoveryLink]},
Expand Down

0 comments on commit dcfd798

Please sign in to comment.