diff --git a/app/boot_levels.go b/app/boot_levels.go index 7e2a27053..688dba9cd 100644 --- a/app/boot_levels.go +++ b/app/boot_levels.go @@ -4,9 +4,6 @@ import ( "context" "crypto/tls" "fmt" - "github.com/cortezaproject/corteza-server/seeder" - "strings" - authService "github.com/cortezaproject/corteza-server/auth" authHandlers "github.com/cortezaproject/corteza-server/auth/handlers" "github.com/cortezaproject/corteza-server/auth/saml" @@ -32,12 +29,14 @@ import ( "github.com/cortezaproject/corteza-server/pkg/scheduler" "github.com/cortezaproject/corteza-server/pkg/sentry" "github.com/cortezaproject/corteza-server/pkg/websocket" + "github.com/cortezaproject/corteza-server/seeder" "github.com/cortezaproject/corteza-server/store" sysService "github.com/cortezaproject/corteza-server/system/service" sysEvent "github.com/cortezaproject/corteza-server/system/service/event" "github.com/cortezaproject/corteza-server/system/types" "go.uber.org/zap" gomail "gopkg.in/mail.v2" + "strings" ) const ( @@ -499,7 +498,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 91e404faf..590e48797 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -219,7 +219,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()) svc.log.Info( "auth server ready", @@ -341,7 +341,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 bcca5ee68..1a79f3647 100644 --- a/auth/commands/commands.go +++ b/auth/commands/commands.go @@ -6,6 +6,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/store" "github.com/cortezaproject/corteza-server/system/service" @@ -50,6 +51,7 @@ func Command(ctx context.Context, app serviceInitializer, storeInit func(ctx con _, err = external.RegisterOidcProvider( ctx, + logger.Default(), s, app.Options().Auth, args[0], 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 496fe1afc..5be9c1ff5 100644 --- a/auth/external/register.go +++ b/auth/external/register.go @@ -16,11 +16,12 @@ import ( "go.uber.org/zap" ) -// AddProvider is used by provisioning process -func AddProvider(ctx context.Context, s store.Settings, eap *types.ExternalAuthProvider, force bool) error { - prefix := "auth.external.providers." + eap.Key + "." +func AddProvider(ctx context.Context, log *zap.Logger, s store.Settings, eap *types.ExternalAuthProvider, force bool) error { + var ( + prefix = "auth.external.providers." + eap.Key + "." + ) - log := log.With( + log = log.With( zap.Bool("force", force), zap.String("handle", eap.Handle), zap.String("key", eap.Key), @@ -61,16 +62,16 @@ func AddProvider(ctx context.Context, s store.Settings, eap *types.ExternalAuthP // @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 { @@ -101,7 +102,7 @@ func DiscoverOidcProvider(ctx context.Context, opt options.AuthOpt, name, url st return } -func RegisterOidcProvider(ctx context.Context, s store.Settings, opt options.AuthOpt, name, providerUrl string, force, validate, enable bool) (eap *types.ExternalAuthProvider, err error) { +func RegisterOidcProvider(ctx context.Context, log *zap.Logger, s store.Settings, opt options.AuthOpt, name, providerUrl string, force, validate, enable bool) (eap *types.ExternalAuthProvider, err error) { if !force { prefix := "auth.external.providers." + eap.Key + "." @@ -136,7 +137,7 @@ func RegisterOidcProvider(ctx context.Context, s store.Settings, opt options.Aut 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 b60d9b7b8..e355d2e95 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" @@ -20,6 +19,7 @@ import ( "github.com/go-oauth2/oauth2/v4/server" "github.com/gorilla/csrf" "github.com/gorilla/sessions" + "github.com/markbates/goth" "go.uber.org/zap" ) @@ -273,38 +273,36 @@ 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)) + var pp = make([]provider, 0, len(dSettings.Providers)) if h.Settings.Saml.Enabled { pp = append(pp, provider(saml.TemplateProvider(h.Settings.Saml.IDP.URL, h.Settings.Saml.IDP.Name))) } - for i := range providers { - if !providers[i].Enabled { + 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 596c45757..5a0968bd9 100644 --- a/pkg/provision/oidc.go +++ b/pkg/provision/oidc.go @@ -50,7 +50,7 @@ func oidcAutoDiscovery(ctx context.Context, log *zap.Logger, s store.Settings, o // // enable: true // we want provider & the entire external auth to be validated - eap, err = external.RegisterOidcProvider(ctx, s, opt, name, purl, false, false, true) + eap, err = external.RegisterOidcProvider(ctx, log, s, opt, name, purl, false, false, true) if err != nil { log.Error( @@ -126,7 +126,7 @@ func authAddExternals(ctx context.Context, log *zap.Logger, s store.Settings) (e eap.Handle = kind } - _ = external.AddProvider(ctx, s, eap, false) + _ = external.AddProvider(ctx, log, s, eap, false) } return