From 8aafa0625991d8d075edccedbb277a85050bf3d6 Mon Sep 17 00:00:00 2001 From: Jens L Date: Wed, 18 Oct 2023 19:43:36 +0200 Subject: [PATCH] providers/radius: TOTP MFA support (#7217) * move CheckPasswordMFA to flow executor Signed-off-by: Jens Langhammer * add mfa support field to radius Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/providers/radius/api.py | 2 + .../0002_radiusprovider_mfa_support.py | 21 ++++++++ authentik/providers/radius/models.py | 11 +++++ blueprints/schema.json | 5 ++ internal/outpost/flow/solvers_mfa.go | 48 ++++++++++++++++++ internal/outpost/ldap/bind/direct/bind.go | 49 ++----------------- internal/outpost/radius/api.go | 7 ++- .../outpost/radius/handle_access_request.go | 3 ++ internal/outpost/radius/radius.go | 1 + schema.yml | 28 +++++++++++ .../providers/radius/RadiusProviderForm.ts | 20 ++++++++ 11 files changed, 145 insertions(+), 50 deletions(-) create mode 100644 authentik/providers/radius/migrations/0002_radiusprovider_mfa_support.py create mode 100644 internal/outpost/flow/solvers_mfa.go diff --git a/authentik/providers/radius/api.py b/authentik/providers/radius/api.py index d9a2578ce..af43424bf 100644 --- a/authentik/providers/radius/api.py +++ b/authentik/providers/radius/api.py @@ -21,6 +21,7 @@ class RadiusProviderSerializer(ProviderSerializer): # an admin might have to view it "shared_secret", "outpost_set", + "mfa_support", ] extra_kwargs = ProviderSerializer.Meta.extra_kwargs @@ -55,6 +56,7 @@ class RadiusOutpostConfigSerializer(ModelSerializer): "auth_flow_slug", "client_networks", "shared_secret", + "mfa_support", ] diff --git a/authentik/providers/radius/migrations/0002_radiusprovider_mfa_support.py b/authentik/providers/radius/migrations/0002_radiusprovider_mfa_support.py new file mode 100644 index 000000000..4a8af9e30 --- /dev/null +++ b/authentik/providers/radius/migrations/0002_radiusprovider_mfa_support.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.6 on 2023-10-18 15:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("authentik_providers_radius", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="radiusprovider", + name="mfa_support", + field=models.BooleanField( + default=True, + help_text="When enabled, code-based multi-factor authentication can be used by appending a semicolon and the TOTP code to the password. This should only be enabled if all users that will bind to this provider have a TOTP device configured, as otherwise a password may incorrectly be rejected if it contains a semicolon.", + verbose_name="MFA Support", + ), + ), + ] diff --git a/authentik/providers/radius/models.py b/authentik/providers/radius/models.py index 7ba39b47e..781023fd6 100644 --- a/authentik/providers/radius/models.py +++ b/authentik/providers/radius/models.py @@ -27,6 +27,17 @@ class RadiusProvider(OutpostModel, Provider): ), ) + mfa_support = models.BooleanField( + default=True, + verbose_name="MFA Support", + help_text=_( + "When enabled, code-based multi-factor authentication can be used by appending a " + "semicolon and the TOTP code to the password. This should only be enabled if all " + "users that will bind to this provider have a TOTP device configured, as otherwise " + "a password may incorrectly be rejected if it contains a semicolon." + ), + ) + @property def launch_url(self) -> Optional[str]: """Radius never has a launch URL""" diff --git a/blueprints/schema.json b/blueprints/schema.json index ca96a5e16..deb177e4d 100644 --- a/blueprints/schema.json +++ b/blueprints/schema.json @@ -4794,6 +4794,11 @@ "minLength": 1, "title": "Shared secret", "description": "Shared secret between clients and server to hash packets." + }, + "mfa_support": { + "type": "boolean", + "title": "MFA Support", + "description": "When enabled, code-based multi-factor authentication can be used by appending a semicolon and the TOTP code to the password. This should only be enabled if all users that will bind to this provider have a TOTP device configured, as otherwise a password may incorrectly be rejected if it contains a semicolon." } }, "required": [] diff --git a/internal/outpost/flow/solvers_mfa.go b/internal/outpost/flow/solvers_mfa.go new file mode 100644 index 000000000..f2952eb3d --- /dev/null +++ b/internal/outpost/flow/solvers_mfa.go @@ -0,0 +1,48 @@ +package flow + +import ( + "regexp" + "strconv" + "strings" +) + +const CodePasswordSeparator = ";" + +var alphaNum = regexp.MustCompile(`^[a-zA-Z0-9]*$`) + +// CheckPasswordInlineMFA For protocols that only support username/password, check if the password +// contains the TOTP code +func (fe *FlowExecutor) CheckPasswordInlineMFA() { + password := fe.Answers[StagePassword] + // We already have an authenticator answer + if fe.Answers[StageAuthenticatorValidate] != "" { + return + } + // password doesn't contain the separator + if !strings.Contains(password, CodePasswordSeparator) { + return + } + // password ends with the separator, so it won't contain an answer + if strings.HasSuffix(password, CodePasswordSeparator) { + return + } + idx := strings.LastIndex(password, CodePasswordSeparator) + authenticator := password[idx+1:] + // Authenticator is either 6 chars (totp code) or 8 chars (long totp or static) + if len(authenticator) == 6 { + // authenticator answer isn't purely numerical, so won't be value + if _, err := strconv.Atoi(authenticator); err != nil { + return + } + } else if len(authenticator) == 8 { + // 8 chars can be a long totp or static token, so it needs to be alphanumerical + if !alphaNum.MatchString(authenticator) { + return + } + } else { + // Any other length, doesn't contain an answer + return + } + fe.Answers[StagePassword] = password[:idx] + fe.Answers[StageAuthenticatorValidate] = authenticator +} diff --git a/internal/outpost/ldap/bind/direct/bind.go b/internal/outpost/ldap/bind/direct/bind.go index ce74d18f8..f6e49ccfb 100644 --- a/internal/outpost/ldap/bind/direct/bind.go +++ b/internal/outpost/ldap/bind/direct/bind.go @@ -2,9 +2,6 @@ package direct import ( "context" - "regexp" - "strconv" - "strings" "beryju.io/ldap" "github.com/getsentry/sentry-go" @@ -16,10 +13,6 @@ import ( "goauthentik.io/internal/outpost/ldap/metrics" ) -const CodePasswordSeparator = ";" - -var alphaNum = regexp.MustCompile(`^[a-zA-Z0-9]*$`) - func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResultCode, error) { fe := flow.NewFlowExecutor(req.Context(), db.si.GetAuthenticationFlowSlug(), db.si.GetAPIClient().GetConfig(), log.Fields{ "bindDN": req.BindDN, @@ -31,7 +24,9 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul fe.Answers[flow.StageIdentification] = username fe.Answers[flow.StagePassword] = req.BindPW - db.CheckPasswordMFA(fe) + if db.si.GetMFASupport() { + fe.CheckPasswordInlineMFA() + } passed, err := fe.Execute() flags := flags.UserFlags{ @@ -141,41 +136,3 @@ func (db *DirectBinder) Bind(username string, req *bind.Request) (ldap.LDAPResul uisp.Finish() return ldap.LDAPResultSuccess, nil } - -func (db *DirectBinder) CheckPasswordMFA(fe *flow.FlowExecutor) { - if !db.si.GetMFASupport() { - return - } - password := fe.Answers[flow.StagePassword] - // We already have an authenticator answer - if fe.Answers[flow.StageAuthenticatorValidate] != "" { - return - } - // password doesn't contain the separator - if !strings.Contains(password, CodePasswordSeparator) { - return - } - // password ends with the separator, so it won't contain an answer - if strings.HasSuffix(password, CodePasswordSeparator) { - return - } - idx := strings.LastIndex(password, CodePasswordSeparator) - authenticator := password[idx+1:] - // Authenticator is either 6 chars (totp code) or 8 chars (long totp or static) - if len(authenticator) == 6 { - // authenticator answer isn't purely numerical, so won't be value - if _, err := strconv.Atoi(authenticator); err != nil { - return - } - } else if len(authenticator) == 8 { - // 8 chars can be a long totp or static token, so it needs to be alphanumerical - if !alphaNum.MatchString(authenticator) { - return - } - } else { - // Any other length, doesn't contain an answer - return - } - fe.Answers[flow.StagePassword] = password[:idx] - fe.Answers[flow.StageAuthenticatorValidate] = authenticator -} diff --git a/internal/outpost/radius/api.go b/internal/outpost/radius/api.go index 5092a9cfb..ce35e7096 100644 --- a/internal/outpost/radius/api.go +++ b/internal/outpost/radius/api.go @@ -40,11 +40,10 @@ func (rs *RadiusServer) Refresh() error { providers := make([]*ProviderInstance, len(outposts.Results)) for idx, provider := range outposts.Results { logger := log.WithField("logger", "authentik.outpost.radius").WithField("provider", provider.Name) - s := *provider.SharedSecret - c := *provider.ClientNetworks providers[idx] = &ProviderInstance{ - SharedSecret: []byte(s), - ClientNetworks: parseCIDRs(c), + SharedSecret: []byte(provider.GetSharedSecret()), + ClientNetworks: parseCIDRs(provider.GetClientNetworks()), + MFASupport: provider.GetMfaSupport(), appSlug: provider.ApplicationSlug, flowSlug: provider.AuthFlowSlug, s: rs, diff --git a/internal/outpost/radius/handle_access_request.go b/internal/outpost/radius/handle_access_request.go index 465f1d267..6ea387f53 100644 --- a/internal/outpost/radius/handle_access_request.go +++ b/internal/outpost/radius/handle_access_request.go @@ -22,6 +22,9 @@ func (rs *RadiusServer) Handle_AccessRequest(w radius.ResponseWriter, r *RadiusR fe.Answers[flow.StageIdentification] = username fe.Answers[flow.StagePassword] = rfc2865.UserPassword_GetString(r.Packet) + if r.pi.MFASupport { + fe.CheckPasswordInlineMFA() + } passed, err := fe.Execute() diff --git a/internal/outpost/radius/radius.go b/internal/outpost/radius/radius.go index d78854e4a..808336e57 100644 --- a/internal/outpost/radius/radius.go +++ b/internal/outpost/radius/radius.go @@ -17,6 +17,7 @@ import ( type ProviderInstance struct { ClientNetworks []*net.IPNet SharedSecret []byte + MFASupport bool appSlug string flowSlug string diff --git a/schema.yml b/schema.yml index a9edeb564..a311785f0 100644 --- a/schema.yml +++ b/schema.yml @@ -37484,6 +37484,13 @@ components: type: string minLength: 1 description: Shared secret between clients and server to hash packets. + mfa_support: + type: boolean + description: When enabled, code-based multi-factor authentication can be + used by appending a semicolon and the TOTP code to the password. This + should only be enabled if all users that will bind to this provider have + a TOTP device configured, as otherwise a password may incorrectly be rejected + if it contains a semicolon. PatchedReputationPolicyRequest: type: object description: Reputation Policy Serializer @@ -39379,6 +39386,13 @@ components: shared_secret: type: string description: Shared secret between clients and server to hash packets. + mfa_support: + type: boolean + description: When enabled, code-based multi-factor authentication can be + used by appending a semicolon and the TOTP code to the password. This + should only be enabled if all users that will bind to this provider have + a TOTP device configured, as otherwise a password may incorrectly be rejected + if it contains a semicolon. required: - application_slug - auth_flow_slug @@ -39454,6 +39468,13 @@ components: items: type: string readOnly: true + mfa_support: + type: boolean + description: When enabled, code-based multi-factor authentication can be + used by appending a semicolon and the TOTP code to the password. This + should only be enabled if all users that will bind to this provider have + a TOTP device configured, as otherwise a password may incorrectly be rejected + if it contains a semicolon. required: - assigned_application_name - assigned_application_slug @@ -39499,6 +39520,13 @@ components: type: string minLength: 1 description: Shared secret between clients and server to hash packets. + mfa_support: + type: boolean + description: When enabled, code-based multi-factor authentication can be + used by appending a semicolon and the TOTP code to the password. This + should only be enabled if all users that will bind to this provider have + a TOTP device configured, as otherwise a password may incorrectly be rejected + if it contains a semicolon. required: - authorization_flow - name diff --git a/web/src/admin/providers/radius/RadiusProviderForm.ts b/web/src/admin/providers/radius/RadiusProviderForm.ts index e38307074..3898ba048 100644 --- a/web/src/admin/providers/radius/RadiusProviderForm.ts +++ b/web/src/admin/providers/radius/RadiusProviderForm.ts @@ -70,6 +70,26 @@ export class RadiusProviderFormPage extends ModelForm { >

${msg("Flow used for users to authenticate.")}

+ + +

+ ${msg( + "When enabled, code-based multi-factor authentication can be used by appending a semicolon and the TOTP code to the password. This should only be enabled if all users that will bind to this provider have a TOTP device configured, as otherwise a password may incorrectly be rejected if it contains a semicolon.", + )} +

+ ${msg("Protocol settings")}