From 6977ade5df5724b38af5348c46bace0f6055a45f Mon Sep 17 00:00:00 2001 From: Andre Perry Date: Sat, 24 Jul 2021 05:43:19 +1000 Subject: [PATCH] Add ValidConfiguration and test for external auth provider --- app/boot_levels.go | 71 ++++++++++++------------------- app/boot_levels_test.go | 9 ++-- auth/external/external.go | 3 +- auth/settings/settings.go | 67 +++++++++++++++-------------- system/types/app_settings.go | 26 ++++++++--- system/types/app_settings_test.go | 20 ++++++++- 6 files changed, 109 insertions(+), 87 deletions(-) diff --git a/app/boot_levels.go b/app/boot_levels.go index 4e1780054..02113d94b 100644 --- a/app/boot_levels.go +++ b/app/boot_levels.go @@ -7,7 +7,6 @@ import ( "strings" authService "github.com/cortezaproject/corteza-server/auth" - "github.com/cortezaproject/corteza-server/auth/external" authHandlers "github.com/cortezaproject/corteza-server/auth/handlers" "github.com/cortezaproject/corteza-server/auth/saml" authSettings "github.com/cortezaproject/corteza-server/auth/settings" @@ -517,51 +516,37 @@ func (app *CortezaApp) initSystemEntities(ctx context.Context) (err error) { } func updateAuthSettings(svc authServicer, current *types.AppSettings) { - var ( - // current auth settings - cas = current.Auth - providers []authSettings.Provider - ) - - for _, p := range cas.External.Providers { - if !p.Enabled || p.Handle == "" || p.Key == "" || p.Secret == "" { - continue - } - - if strings.HasPrefix(p.Handle, external.OIDC_PROVIDER_PREFIX) { - if p.IssuerUrl == "" { - // OIDC IdPs need to have issuer URL - continue - } - } else { - // non-OIDC provider do not need issuer URL - p.IssuerUrl = "" - } - - providers = append(providers, authSettings.Provider{ - Handle: p.Handle, - Label: p.Label, - IssuerUrl: p.IssuerUrl, - Key: p.Key, - RedirectUrl: p.RedirectUrl, - Secret: p.Secret, - }) - } - as := &authSettings.Settings{ - LocalEnabled: cas.Internal.Enabled, - SignupEnabled: cas.Internal.Signup.Enabled, - EmailConfirmationRequired: cas.Internal.Signup.EmailConfirmationRequired, - PasswordResetEnabled: cas.Internal.PasswordReset.Enabled, - ExternalEnabled: cas.External.Enabled, - Providers: providers, + LocalEnabled: current.Auth.Internal.Enabled, + SignupEnabled: current.Auth.Internal.Signup.Enabled, + EmailConfirmationRequired: current.Auth.Internal.Signup.EmailConfirmationRequired, + PasswordResetEnabled: current.Auth.Internal.PasswordReset.Enabled, + ExternalEnabled: current.Auth.External.Enabled, + MultiFactor: authSettings.MultiFactor{ + TOTP: authSettings.TOTP{ + Enabled: current.Auth.MultiFactor.TOTP.Enabled, + Enforced: current.Auth.MultiFactor.TOTP.Enforced, + Issuer: current.Auth.MultiFactor.TOTP.Issuer, + }, + EmailOTP: authSettings.EmailOTP{ + Enabled: current.Auth.MultiFactor.EmailOTP.Enabled, + Enforced: current.Auth.MultiFactor.EmailOTP.Enforced, + }, + }, } - as.MultiFactor.TOTP.Enabled = cas.MultiFactor.TOTP.Enabled - as.MultiFactor.TOTP.Enforced = cas.MultiFactor.TOTP.Enforced - as.MultiFactor.TOTP.Issuer = cas.MultiFactor.TOTP.Issuer - as.MultiFactor.EmailOTP.Enabled = cas.MultiFactor.EmailOTP.Enabled - as.MultiFactor.EmailOTP.Enforced = cas.MultiFactor.EmailOTP.Enforced + for _, p := range current.Auth.External.Providers { + if p.ValidConfiguration() { + as.Providers = append(as.Providers, authSettings.Provider{ + Handle: p.Handle, + Label: p.Label, + IssuerUrl: p.IssuerUrl, + Key: p.Key, + RedirectUrl: p.RedirectUrl, + Secret: p.Secret, + }) + } + } // SAML saml.UpdateSettings(current, as) diff --git a/app/boot_levels_test.go b/app/boot_levels_test.go index f0fc4f533..1f9158887 100644 --- a/app/boot_levels_test.go +++ b/app/boot_levels_test.go @@ -70,11 +70,10 @@ func Test_updateAuthSettings(t *testing.T) { Secret: "sec", }, { // add (w/o issuer) - Enabled: true, - Handle: "google", - IssuerUrl: "issuer", - Key: "key", - Secret: "sec", + Enabled: true, + Handle: "google", + Key: "key", + Secret: "sec", }, { // add Enabled: true, diff --git a/auth/external/external.go b/auth/external/external.go index 0c2571f5c..4c6f4cae9 100644 --- a/auth/external/external.go +++ b/auth/external/external.go @@ -6,7 +6,8 @@ import ( ) const ( - OIDC_PROVIDER_PREFIX = "openid-connect." + OIDC_PROVIDER_PREFIX = "openid-connect." // must match const in "github.com/cortezaproject/corteza-server/system/types" app_settings.go + ) func Init(store sessions.Store) { diff --git a/auth/settings/settings.go b/auth/settings/settings.go index 41272f46c..e417b5895 100644 --- a/auth/settings/settings.go +++ b/auth/settings/settings.go @@ -8,48 +8,53 @@ type ( PasswordResetEnabled bool ExternalEnabled bool Providers []Provider + Saml SAML + MultiFactor MultiFactor + } - Saml struct { - Enabled bool + SAML struct { + Enabled bool - // SAML certificate - Cert string + // SAML certificate + Cert string - // SAML certificate private key - Key string + // SAML certificate private key + Key string - // Identity provider hostname - IDP struct { - URL string - Name string + // Identity provider hostname + IDP struct { + URL string + Name string - // identifier payload from idp - IdentName string - IdentHandle string - IdentIdentifier string - } + // identifier payload from idp + IdentName string + IdentHandle string + IdentIdentifier string } + } - MultiFactor struct { - EmailOTP struct { - // Can users use email for MFA - Enabled bool + MultiFactor struct { + EmailOTP EmailOTP + TOTP TOTP + } - // Is MFA with email enforced? - Enforced bool - } + EmailOTP struct { + // Can users use email for MFA + Enabled bool - TOTP struct { - // Can users use TOTP MFA? - Enabled bool + // Is MFA with email enforced? + Enforced bool + } - // Is TOTP MFA enforced? - Enforced bool + TOTP struct { + // Can users use TOTP MFA? + Enabled bool - // TOTP issuer - Issuer string - } - } + // Is TOTP MFA enforced? + Enforced bool + + // TOTP issuer + Issuer string } Provider struct { diff --git a/system/types/app_settings.go b/system/types/app_settings.go index 0ce5894e7..6ab9602f9 100644 --- a/system/types/app_settings.go +++ b/system/types/app_settings.go @@ -5,6 +5,10 @@ import ( "strings" ) +const ( + oidcProviderPrefix = "openid-connect." // must match const in "github.com/cortezaproject/corteza-server/auth/external" external.go +) + type ( // AppSettings type is structured representation of all application settings // @@ -157,6 +161,19 @@ type ( } ) +func (set *ExternalAuthProvider) ValidConfiguration() bool { + if !set.Enabled || set.Handle == "" || set.Key == "" || set.Secret == "" { + return false + } + + if strings.HasPrefix(set.Handle, oidcProviderPrefix) && set.IssuerUrl == "" { + // OIDC IdPs need to have issuer URL + return false + } + + return true +} + // DecodeKV translates settings' KV into internal system external auth settings func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err error) { if *set == nil { @@ -170,13 +187,12 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err kv = kv.CutPrefix(prefix + ".") // add all additional providers (prefixed with "openid-connect.") - oidcPrefix := "openid-connect." for p := range kv { - if !strings.HasPrefix(p, oidcPrefix) { + if !strings.HasPrefix(p, oidcProviderPrefix) { continue } - l := len(oidcPrefix) + l := len(oidcProviderPrefix) dotPos := strings.Index(p[l:], ".") + l if dotPos > 0 { providers[p[:dotPos]] = true @@ -207,8 +223,8 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err case "crust-iam", "crust", "crust-unify": p.Label = "Crust IAM" default: - if strings.HasPrefix(p.Handle, oidcPrefix) { - p.Label = strings.Title(p.Handle[len(oidcPrefix):]) + if strings.HasPrefix(p.Handle, oidcProviderPrefix) { + p.Label = strings.Title(p.Handle[len(oidcProviderPrefix):]) } else { p.Label = strings.Title(p.Handle) } diff --git a/system/types/app_settings_test.go b/system/types/app_settings_test.go index a548bc7bc..b6a11a3b2 100644 --- a/system/types/app_settings_test.go +++ b/system/types/app_settings_test.go @@ -1,14 +1,30 @@ package types import ( - sqlTypes "github.com/jmoiron/sqlx/types" - "github.com/stretchr/testify/require" "sort" "testing" + + sqlTypes "github.com/jmoiron/sqlx/types" + "github.com/stretchr/testify/require" ) // Hello! This file is auto-generated. +func Test_settingsExtAuthProvidersValidConfiguration(t *testing.T) { + + var ( + empty = ExternalAuthProvider{} + google = ExternalAuthProvider{Enabled: true, Handle: "google", Key: "some-guid", Secret: "s3cret"} + noIssuerOIDC = ExternalAuthProvider{Enabled: true, Handle: "openid-connect.bar", Key: "some-guid", Secret: "s3cret"} + goodOIDC = ExternalAuthProvider{Enabled: true, Handle: "openid-connect.bar", Key: "some-guid", Secret: "s3cret", IssuerUrl: "https://example.org"} + ) + + require.False(t, noIssuerOIDC.ValidConfiguration()) + require.True(t, goodOIDC.ValidConfiguration()) + require.False(t, empty.ValidConfiguration()) + require.True(t, google.ValidConfiguration()) +} + func Test_settingsExtAuthProvidersDecode(t *testing.T) { type ( Dst struct {