From c6bee31af1086d7214bd9fa7343a96e319783ea5 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 14 Jul 2021 21:23:50 +0200 Subject: [PATCH] Fix external auth provider management --- app/boot_levels.go | 5 ++- auth/auth.go | 17 +++++---- auth/commands/commands.go | 2 + auth/external/{federated.go => external.go} | 8 +--- .../{federated_test.go => external_test.go} | 0 auth/external/goth.go | 7 ++-- auth/external/register.go | 38 ++++++++++--------- auth/handlers/handler.go | 29 +++++++------- pkg/provision/oidc.go | 7 ++-- 9 files changed, 56 insertions(+), 57 deletions(-) rename auth/external/{federated.go => external.go} (56%) rename auth/external/{federated_test.go => external_test.go} (100%) diff --git a/app/boot_levels.go b/app/boot_levels.go index 993ddbbc9..2df8e2411 100644 --- a/app/boot_levels.go +++ b/app/boot_levels.go @@ -4,6 +4,8 @@ import ( "context" "crypto/tls" "fmt" + "strings" + authService "github.com/cortezaproject/corteza-server/auth" authSettings "github.com/cortezaproject/corteza-server/auth/settings" autService "github.com/cortezaproject/corteza-server/automation/service" @@ -30,7 +32,6 @@ import ( "github.com/cortezaproject/corteza-server/system/types" "go.uber.org/zap" gomail "gopkg.in/mail.v2" - "strings" ) const ( @@ -419,7 +420,7 @@ func updateAuthSettings(svc authServicer, current *types.AppSettings) { ) for _, p := range cas.External.Providers { - if !p.Enabled { + if !p.Enabled || p.Handle == "" || p.IssuerUrl == "" || p.Key == "" || p.Secret == "" { continue } diff --git a/auth/auth.go b/auth/auth.go index 8775ad6e9..47e89eb82 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -4,6 +4,13 @@ import ( "context" "embed" "fmt" + "html/template" + "net/http" + "os" + "strconv" + "strings" + "time" + "github.com/Masterminds/sprig" "github.com/cortezaproject/corteza-server/auth/external" "github.com/cortezaproject/corteza-server/auth/handlers" @@ -20,12 +27,6 @@ import ( "github.com/go-chi/chi" oauth2def "github.com/go-oauth2/oauth2/v4" "go.uber.org/zap" - "html/template" - "net/http" - "os" - "strconv" - "strings" - "time" ) type ( @@ -216,7 +217,7 @@ func New(ctx context.Context, log *zap.Logger, s store.Storer, opt options.AuthO Settings: svc.settings, } - external.Init(log, sesManager.Store()) + external.Init(sesManager.Store()) return } @@ -248,7 +249,7 @@ func (svc *service) UpdateSettings(s *settings.Settings) { if len(svc.settings.Providers) != len(s.Providers) { svc.log.Debug("setting changed", zap.Int("providers", len(s.Providers))) - external.SetupGothProviders(svc.opt.ExternalRedirectURL, s.Providers...) + external.SetupGothProviders(svc.log, svc.opt.ExternalRedirectURL, s.Providers...) } svc.settings = s diff --git a/auth/commands/commands.go b/auth/commands/commands.go index 1aea9f589..c15b3d56c 100644 --- a/auth/commands/commands.go +++ b/auth/commands/commands.go @@ -5,6 +5,7 @@ import ( "github.com/cortezaproject/corteza-server/auth/external" "github.com/cortezaproject/corteza-server/pkg/auth" "github.com/cortezaproject/corteza-server/pkg/cli" + "github.com/cortezaproject/corteza-server/pkg/logger" "github.com/cortezaproject/corteza-server/pkg/options" "github.com/cortezaproject/corteza-server/system/service" "github.com/cortezaproject/corteza-server/system/types" @@ -44,6 +45,7 @@ func General(app serviceInitializer, opt options.AuthOpt) *cobra.Command { ctx := auth.SetSuperUserContext(cli.Context()) _, err := external.RegisterOidcProvider( ctx, + logger.Default(), opt, args[0], args[1], diff --git a/auth/external/federated.go b/auth/external/external.go similarity index 56% rename from auth/external/federated.go rename to auth/external/external.go index 71a9ac516..0c2571f5c 100644 --- a/auth/external/federated.go +++ b/auth/external/external.go @@ -3,18 +3,12 @@ package external import ( "github.com/gorilla/sessions" "github.com/markbates/goth/gothic" - "go.uber.org/zap" -) - -var ( - log = zap.NewNop() ) const ( OIDC_PROVIDER_PREFIX = "openid-connect." ) -func Init(logger *zap.Logger, store sessions.Store) { - log = logger.Named("external") +func Init(store sessions.Store) { gothic.Store = store } diff --git a/auth/external/federated_test.go b/auth/external/external_test.go similarity index 100% rename from auth/external/federated_test.go rename to auth/external/external_test.go diff --git a/auth/external/goth.go b/auth/external/goth.go index 60e9c67b7..891509b8c 100644 --- a/auth/external/goth.go +++ b/auth/external/goth.go @@ -1,6 +1,8 @@ package external import ( + "strings" + "github.com/cortezaproject/corteza-server/auth/settings" "github.com/markbates/goth" "github.com/markbates/goth/providers/facebook" @@ -9,7 +11,6 @@ import ( "github.com/markbates/goth/providers/linkedin" "github.com/markbates/goth/providers/openidConnect" "go.uber.org/zap" - "strings" ) // We're expecting that our users will be able to complete @@ -18,7 +19,7 @@ const ( WellKnown = "/.well-known/openid-configuration" ) -func SetupGothProviders(redirectUrl string, ep ...settings.Provider) { +func SetupGothProviders(log *zap.Logger, redirectUrl string, ep ...settings.Provider) { var ( err error ) @@ -43,7 +44,7 @@ func SetupGothProviders(redirectUrl string, ep ...settings.Provider) { redirect = strings.Replace(redirectUrl, "{provider}", pc.Handle, 1) } - if strings.Index(pc.Handle, OIDC_PROVIDER_PREFIX) == 0 { + if strings.HasPrefix(pc.Handle, OIDC_PROVIDER_PREFIX) { if pc.IssuerUrl == "" { log.Error("failed to discover OIDC provider, URL empty") continue diff --git a/auth/external/register.go b/auth/external/register.go index ae0200290..1b2d93105 100644 --- a/auth/external/register.go +++ b/auth/external/register.go @@ -2,26 +2,28 @@ package external import ( "context" + "io/ioutil" + "net/http" + "net/url" + "strings" + "github.com/cortezaproject/corteza-server/pkg/options" "github.com/cortezaproject/corteza-server/system/service" "github.com/cortezaproject/corteza-server/system/types" "github.com/crusttech/go-oidc" "github.com/pkg/errors" "go.uber.org/zap" - "io/ioutil" - "net/http" - "net/url" - "strings" ) -func AddProvider(ctx context.Context, eap *types.ExternalAuthProvider, force bool) error { +func AddProvider(ctx context.Context, log *zap.Logger, eap *types.ExternalAuthProvider, force bool) error { var ( - s = service.CurrentSettings - log = log.With( - zap.Bool("force", force), - zap.String("handle", eap.Handle), - zap.String("key", eap.Key), - ) + s = service.CurrentSettings + ) + + log = log.With( + zap.Bool("force", force), + zap.String("handle", eap.Handle), + zap.String("key", eap.Key), ) if eap.IssuerUrl != "" { @@ -50,16 +52,16 @@ func AddProvider(ctx context.Context, eap *types.ExternalAuthProvider, force boo // @todo remove dependency on github.com/crusttech/go-oidc (and github.com/coreos/go-oidc) // and move client registration to corteza codebase -func DiscoverOidcProvider(ctx context.Context, opt options.AuthOpt, name, url string) (eap *types.ExternalAuthProvider, err error) { +func DiscoverOidcProvider(ctx context.Context, log *zap.Logger, opt options.AuthOpt, name, url string) (eap *types.ExternalAuthProvider, err error) { var ( provider *oidc.Provider client *oidc.Client redirectUrl = strings.Replace(opt.ExternalRedirectURL, "{provider}", OIDC_PROVIDER_PREFIX+name, 1) + ) - log = log.With( - zap.String("name", name), - zap.String("url", url), - ) + log = log.With( + zap.String("name", name), + zap.String("url", url), ) if provider, err = oidc.NewProvider(ctx, url); err != nil { @@ -90,7 +92,7 @@ func DiscoverOidcProvider(ctx context.Context, opt options.AuthOpt, name, url st return } -func RegisterOidcProvider(ctx context.Context, opt options.AuthOpt, name, providerUrl string, force, validate, enable bool) (eap *types.ExternalAuthProvider, err error) { +func RegisterOidcProvider(ctx context.Context, log *zap.Logger, opt options.AuthOpt, name, providerUrl string, force, validate, enable bool) (eap *types.ExternalAuthProvider, err error) { var ( s = service.CurrentSettings ) @@ -123,7 +125,7 @@ func RegisterOidcProvider(ctx context.Context, opt options.AuthOpt, name, provid return } - eap, err = DiscoverOidcProvider(ctx, opt, name, p.String()) + eap, err = DiscoverOidcProvider(ctx, log, opt, name, p.String()) if err != nil { return } diff --git a/auth/handlers/handler.go b/auth/handlers/handler.go index 0d281050e..a15c07d88 100644 --- a/auth/handlers/handler.go +++ b/auth/handlers/handler.go @@ -6,7 +6,6 @@ import ( "io" "net/http" "net/url" - "sort" "strings" "github.com/cortezaproject/corteza-server/auth/external" @@ -269,33 +268,31 @@ func (h *AuthHandlers) enrichTmplData(req *request.AuthReq) interface{} { d["alerts"] = append(req.PrevAlerts, req.NewAlerts...) dSettings := *h.Settings - dSettings.Providers = nil - d["settings"] = dSettings - providers := h.AuthService.GetProviders() - sort.Sort(providers) - - var pp = make([]provider, 0, len(providers)) - for i := range providers { - if !providers[i].Enabled { + var pp = make([]provider, 0, len(dSettings.Providers)) + for _, p := range dSettings.Providers { + if _, err := goth.GetProvider(p.Handle); err != nil { continue } - p := provider{ - Label: providers[i].Label, - Handle: providers[i].Handle, - Icon: providers[i].Handle, + out := provider{ + Label: p.Label, + Handle: p.Handle, + Icon: p.Handle, } - if strings.HasPrefix(p.Icon, external.OIDC_PROVIDER_PREFIX) { - p.Icon = "key" + if strings.HasPrefix(out.Icon, external.OIDC_PROVIDER_PREFIX) { + out.Icon = "key" } - pp = append(pp, p) + pp = append(pp, out) } d["providers"] = pp + dSettings.Providers = nil + d["settings"] = dSettings + return d } diff --git a/pkg/provision/oidc.go b/pkg/provision/oidc.go index baca13b78..76bbc6cde 100644 --- a/pkg/provision/oidc.go +++ b/pkg/provision/oidc.go @@ -3,12 +3,13 @@ package provision import ( "context" "fmt" + "strings" + "github.com/cortezaproject/corteza-server/auth/external" "github.com/cortezaproject/corteza-server/pkg/auth" "github.com/cortezaproject/corteza-server/pkg/options" "github.com/cortezaproject/corteza-server/system/types" "go.uber.org/zap" - "strings" ) // Provisions OIDC providers from PROVISION_OIDC_PROVIDER env variable @@ -48,7 +49,7 @@ func oidcAutoDiscovery(ctx context.Context, log *zap.Logger, opt options.AuthOpt // // enable: true // we want provider & the entire external auth to be validated - eap, err = external.RegisterOidcProvider(ctx, opt, name, purl, false, false, true) + eap, err = external.RegisterOidcProvider(ctx, log, opt, name, purl, false, false, true) if err != nil { log.Error( @@ -115,7 +116,7 @@ func authAddExternals(ctx context.Context, log *zap.Logger) (err error) { ctx = auth.SetSuperUserContext(ctx) - _ = external.AddProvider(ctx, eap, false) + _ = external.AddProvider(ctx, log, eap, false) } return