Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history
* fix(security): code credential does not respect `highest_available` setting

This patch fixes a security vulnerability which prevents the `code` method to properly report it's credentials count to the `highest_available` mechanism.

For more details on this issue please refer to the [security advisory](GHSA-wc43-73w7-x2f5).

* fix: normalize code credentials and deprecate via parameter

Before this, code credentials for passwordless and mfa login were incorrectly stored and normalized. This could cause issues where the system would not detect the user's phone number, and where SMS/email MFA would not properly work with the `highest_available` setting.

Breaking changes: Please note that the `via` parameter is deprecated when performing SMS 2FA. It will be removed in a future version. If the parameter is not included in the request, the user will see all their phone/email addresses from which to perform the flow.

Before upgrading, ensure that your identity schema has the appropriate code configuration when using the code method for passwordless or 2fa login.

If you are using the code method for 2FA login already, or you are using it for 1FA login but have not yet configured the code identifier, set `selfservice.methods.code.config.missing_credential_fallback_enabled` to `true` to prevent users from being locked out.
  • Loading branch information
aeneasr authored Aug 28, 2024
1 parent 7945104 commit 123e807
Show file tree
Hide file tree
Showing 165 changed files with 4,854 additions and 1,442 deletions.
2 changes: 1 addition & 1 deletion cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ func init() {
"NewErrorValidationLoginLinkedCredentialsDoNotMatch": text.NewErrorValidationLoginLinkedCredentialsDoNotMatch(),
"NewErrorValidationAddressUnknown": text.NewErrorValidationAddressUnknown(),
"NewInfoSelfServiceLoginCodeMFA": text.NewInfoSelfServiceLoginCodeMFA(),
"NewInfoSelfServiceLoginCodeMFAHint": text.NewInfoSelfServiceLoginCodeMFAHint("{maskedIdentifier}"),
"NewInfoLoginPassword": text.NewInfoLoginPassword(),
"NewErrorValidationAccountNotFound": text.NewErrorValidationAccountNotFound(),
"NewInfoSelfServiceLoginAAL2CodeAddress": text.NewInfoSelfServiceLoginAAL2CodeAddress("{channel}", "{address}"),
}
}

Expand Down
5 changes: 5 additions & 0 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ const (
ViperKeyLinkLifespan = "selfservice.methods.link.config.lifespan"
ViperKeyLinkBaseURL = "selfservice.methods.link.config.base_url"
ViperKeyCodeLifespan = "selfservice.methods.code.config.lifespan"
ViperKeyCodeConfigMissingCredentialFallbackEnabled = "selfservice.methods.code.config.missing_credential_fallback_enabled"
ViperKeyPasswordHaveIBeenPwnedHost = "selfservice.methods.password.config.haveibeenpwned_host"
ViperKeyPasswordHaveIBeenPwnedEnabled = "selfservice.methods.password.config.haveibeenpwned_enabled"
ViperKeyPasswordMaxBreaches = "selfservice.methods.password.config.max_breaches"
Expand Down Expand Up @@ -1330,6 +1331,10 @@ func (p *Config) SelfServiceCodeMethodLifespan(ctx context.Context) time.Duratio
return p.GetProvider(ctx).DurationF(ViperKeyCodeLifespan, time.Hour)
}

func (p *Config) SelfServiceCodeMethodMissingCredentialFallbackEnabled(ctx context.Context) bool {
return p.GetProvider(ctx).Bool(ViperKeyCodeConfigMissingCredentialFallbackEnabled)
}

func (p *Config) DatabaseCleanupSleepTables(ctx context.Context) time.Duration {
return p.GetProvider(ctx).Duration(ViperKeyDatabaseCleanupSleepTables)
}
Expand Down
2 changes: 2 additions & 0 deletions driver/config/testhelpers/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ func (t *TestConfigProvider) Config(ctx context.Context, config *configx.Provide
if !ok {
return config
}

opts := make([]configx.OptionModifier, 0, len(values))
opts = append(opts, configx.WithValues(config.All()))
for _, v := range values {
opts = append(opts, configx.WithValues(v))
}
Expand Down
4 changes: 4 additions & 0 deletions driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,10 @@ func (m *RegistryDefault) LoginCodePersister() code.LoginCodePersister {
return m.Persister()
}

func (m *RegistryDefault) TransactionalPersisterProvider() x.TransactionalPersister {
return m.Persister()
}

func (m *RegistryDefault) Persister() persistence.Persister {
return m.persister
}
Expand Down
7 changes: 7 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,13 @@
"pattern": "^([0-9]+(ns|us|ms|s|m|h))+$",
"default": "1h",
"examples": ["1h", "1m", "1s"]
},
"missing_credential_fallback_enabled": {
"type": "boolean",
"title": "Enable Code OTP as a Fallback",
"description": "Enabling this allows users to sign in with the code method, even if their identity schema or their credentials are not set up to use the code method. If enabled, a verified address (such as an email) will be used to send the code to the user. Use with caution and only if actually needed.",

"default": false
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ require (
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/moby/docker-image-spec v1.3.1 // indirect
github.com/moby/term v0.5.0 // indirect
github.com/nyaruka/phonenumbers v1.3.6 // indirect
github.com/nyaruka/phonenumbers v1.3.6
github.com/ogier/pflag v0.0.1 // indirect
github.com/oklog/ulid v1.3.1 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "password",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "password",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
18 changes: 18 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=10.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "sms",
"address": "+4917667111638"
},
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
18 changes: 18 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=11.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "sms",
"address": "+4917667111638"
},
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "webauthn",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "password",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "webauthn",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "webauthn",
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
14 changes: 14 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=6.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
14 changes: 14 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=7.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
14 changes: 14 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=8.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
18 changes: 18 additions & 0 deletions identity/.snapshots/TestSchemaExtensionCredentials-case=9.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "code",
"config": {
"addresses": [
{
"channel": "sms",
"address": "+4917667111638"
},
{
"channel": "email",
"address": "foo@ory.sh"
}
]
},
"version": 0,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "4d64fa08-20fc-450d-bebd-ebd7c7b6e249",
"credentials": {
"code": {
"type": "code",
"identifiers": [
"hi@example.org"
],
"config": {
"addresses": [
{
"address": "hi@example.org",
"channel": "email"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
},
"schema_id": "",
"schema_url": "",
"state": "",
"traits": null,
"metadata_public": null,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z",
"organization_id": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "4d64fa08-20fc-450d-bebd-ebd7c7b6e249",
"credentials": {
"code": {
"type": "code",
"identifiers": [
"hi@example.org"
],
"config": {
"addresses": [
{
"address": "hi@example.org",
"channel": "email"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
},
"schema_id": "",
"schema_url": "",
"state": "",
"traits": null,
"metadata_public": null,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z",
"organization_id": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"id": "4d64fa08-20fc-450d-bebd-ebd7c7b6e249",
"credentials": {
"code": {
"type": "code",
"identifiers": [
"foo@example.org",
"bar@example.org"
],
"config": {
"addresses": [
{
"address": "foo@example.org",
"channel": "email"
},
{
"address": "bar@example.org",
"channel": "email"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
},
"schema_id": "",
"schema_url": "",
"state": "",
"traits": null,
"metadata_public": null,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z",
"organization_id": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "4d64fa08-20fc-450d-bebd-ebd7c7b6e249",
"credentials": {
"code": {
"type": "code",
"identifiers": [
"hi@example.org"
],
"config": {
"addresses": [
{
"address": "hi@example.org",
"channel": "email"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
},
"schema_id": "",
"schema_url": "",
"state": "",
"traits": null,
"metadata_public": null,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z",
"organization_id": null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"id": "4d64fa08-20fc-450d-bebd-ebd7c7b6e249",
"credentials": {
"code": {
"type": "code",
"identifiers": [
"hi@example.org"
],
"config": {
"addresses": [
{
"address": "hi@example.org",
"channel": "email"
}
]
},
"version": 1,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z"
}
},
"schema_id": "",
"schema_url": "",
"state": "",
"traits": null,
"metadata_public": null,
"created_at": "0001-01-01T00:00:00Z",
"updated_at": "0001-01-01T00:00:00Z",
"organization_id": null
}
Loading

0 comments on commit 123e807

Please sign in to comment.