From 339138b576ace10eea92026fcf4fbacfbee445d8 Mon Sep 17 00:00:00 2001 From: Jesse Suen Date: Thu, 7 Jun 2018 02:07:53 -0700 Subject: [PATCH] Remove hard requirement of initializing OIDC app during server startup (resolves #272) --- server/server.go | 26 ----------------------- util/dex/dex.go | 39 ++++++++++++++++++++++------------ util/session/sessionmanager.go | 6 +++++- 3 files changed, 31 insertions(+), 40 deletions(-) diff --git a/server/server.go b/server/server.go index e2aac4f3d6..448b3b757b 100644 --- a/server/server.go +++ b/server/server.go @@ -184,7 +184,6 @@ func (a *ArgoCDServer) Run(ctx context.Context, port int) { go func() { a.checkServeErr("httpsS", httpsS.Serve(httpsL)) }() go func() { a.checkServeErr("tlsm", tlsm.Serve()) }() } - go a.initializeOIDCClientApp() go a.watchSettings(ctx) go a.rbacPolicyLoader(ctx) go func() { a.checkServeErr("tcpm", tcpm.Serve()) }() @@ -262,31 +261,6 @@ func (a *ArgoCDServer) rbacPolicyLoader(ctx context.Context) { errors.CheckError(err) } -// initializeOIDCClientApp initializes the OIDC Client application, querying the well known oidc -// configuration path. Because ArgoCD is a OIDC client to itself, we have a chicken-and-egg problem -// of (1) serving dex over HTTP, and (2) querying the OIDC provider (ourselves) to initialize the -// app (HTTP GET http://example-argocd.com/api/dex/.well-known/openid-configuration) -// This method is expected to be invoked right after we start listening over HTTP -func (a *ArgoCDServer) initializeOIDCClientApp() { - if !a.settings.IsSSOConfigured() { - return - } - // wait for dex to become ready - dexClient, err := dexutil.NewDexClient() - errors.CheckError(err) - dexClient.WaitUntilReady() - var realErr error - _ = wait.ExponentialBackoff(backoff, func() (bool, error) { - _, realErr = a.sessionMgr.OIDCProvider() - if realErr != nil { - a.log.Warnf("failed to initialize client app: %v", realErr) - return false, nil - } - return true, nil - }) - errors.CheckError(realErr) -} - func (a *ArgoCDServer) useTLS() bool { if a.Insecure || a.settings.Certificate == nil { return false diff --git a/util/dex/dex.go b/util/dex/dex.go index 320e3b8c60..362197fce8 100644 --- a/util/dex/dex.go +++ b/util/dex/dex.go @@ -12,17 +12,18 @@ import ( "os" "time" - "github.com/argoproj/argo-cd/common" - "github.com/argoproj/argo-cd/errors" - "github.com/argoproj/argo-cd/util/cache" - "github.com/argoproj/argo-cd/util/session" - "github.com/argoproj/argo-cd/util/settings" "github.com/coreos/dex/api" oidc "github.com/coreos/go-oidc" jwt "github.com/dgrijalva/jwt-go" log "github.com/sirupsen/logrus" "golang.org/x/oauth2" "google.golang.org/grpc" + + "github.com/argoproj/argo-cd/common" + "github.com/argoproj/argo-cd/errors" + "github.com/argoproj/argo-cd/util/cache" + "github.com/argoproj/argo-cd/util/session" + "github.com/argoproj/argo-cd/util/settings" ) const ( @@ -141,15 +142,18 @@ func NewClientApp(settings *settings.ArgoCDSettings, sessionMgr *session.Session return &a, nil } -func (a *ClientApp) oauth2Config(scopes []string) *oauth2.Config { - provider, _ := a.sessionMgr.OIDCProvider() +func (a *ClientApp) oauth2Config(scopes []string) (*oauth2.Config, error) { + provider, err := a.sessionMgr.OIDCProvider() + if err != nil { + return nil, err + } return &oauth2.Config{ ClientID: a.clientID, ClientSecret: a.clientSecret, Endpoint: provider.Endpoint(), Scopes: scopes, RedirectURL: a.redirectURI, - } + }, nil } var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -196,18 +200,23 @@ func (a *ClientApp) verifyAppState(state string) (*appState, error) { } func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) { - var authCodeURL string + var opts []oauth2.AuthCodeOption returnURL := r.FormValue("return_url") scopes := []string{"openid", "profile", "email", "groups"} appState := a.generateAppState(returnURL) if r.FormValue("offline_access") != "yes" { - authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState) + // no-op } else if a.sessionMgr.OfflineAsScope() { scopes = append(scopes, "offline_access") - authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState) } else { - authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState, oauth2.AccessTypeOffline) + opts = append(opts, oauth2.AccessTypeOffline) } + oauth2Config, err := a.oauth2Config(scopes) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + authCodeURL := oauth2Config.AuthCodeURL(appState, opts...) http.Redirect(w, r, authCodeURL, http.StatusSeeOther) } @@ -219,7 +228,11 @@ func (a *ClientApp) HandleCallback(w http.ResponseWriter, r *http.Request) { ) ctx := oidc.ClientContext(r.Context(), a.client) - oauth2Config := a.oauth2Config(nil) + oauth2Config, err := a.oauth2Config(nil) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } switch r.Method { case "GET": // Authorization redirect callback from OAuth2 auth flow. diff --git a/util/session/sessionmanager.go b/util/session/sessionmanager.go index 7fa2b178b2..e5c1808b92 100644 --- a/util/session/sessionmanager.go +++ b/util/session/sessionmanager.go @@ -197,7 +197,11 @@ func MakeCookieMetadata(key, value string, flags ...string) string { return strings.Join(components, "; ") } -// OIDCProvider lazily returns the OIDC provider +// OIDCProvider lazily initializes and returns the OIDC provider, querying the well known oidc +// configuration path (http://example-argocd.com/api/dex/.well-known/openid-configuration). +// We have to initialize the proviver lazily since ArgoCD is an OIDC client to itself, which +// presents a chicken-and-egg problem of (1) serving dex over HTTP, and (2) querying the OIDC +// provider (ourselves) to initialize the app. func (mgr *SessionManager) OIDCProvider() (*oidc.Provider, error) { if mgr.provider != nil { return mgr.provider, nil