diff --git a/api/login.go b/api/login.go index fb15b27d1..eae74702b 100644 --- a/api/login.go +++ b/api/login.go @@ -673,36 +673,53 @@ func getSecretFromFile(source string) (string, error) { func oidcRedirect(w http.ResponseWriter, r *http.Request) { pid := mux.Vars(r)["provider"] - oauthState, err := r.Cookie("oauthstate") loginURL, _ := url.JoinPath(util.Config.WebHost, "auth/login") - if err != nil { - log.Error(err.Error()) + provider, ok := util.Config.OidcProviders[pid] + if !ok { + log.Error(fmt.Errorf("no such provider: %s", pid)) http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) return } - s := r.FormValue("state") - b, err := base64.URLEncoding.DecodeString(s) - - if err != nil { - log.Error(err.Error()) - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return - } + // IdP-initiated flow is detected by the absence of a state parameter. + // In SP-initiated flow, oidcLogin always sets state before redirecting to the IdP. + stateParam := r.FormValue("state") + idpInitiated := stateParam == "" var stateData oAuthState - err = json.Unmarshal(b, &stateData) - if err != nil { - log.Error(err.Error()) - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return - } + if idpInitiated { + if !provider.AllowIdpInitiated { + log.Warn("IdP-initiated OIDC login attempted but not allowed for provider: " + pid) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } + } else { + oauthState, err := r.Cookie("oauthstate") + if err != nil { + log.Error(err.Error()) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } - if stateData.Csrf != oauthState.Value { - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return + b, err := base64.URLEncoding.DecodeString(stateParam) + if err != nil { + log.Error(err.Error()) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } + + if err = json.Unmarshal(b, &stateData); err != nil { + log.Error(err.Error()) + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } + + if stateData.Csrf != oauthState.Value { + http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) + return + } } ctx := context.Background() @@ -714,13 +731,6 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { return } - provider, ok := util.Config.OidcProviders[pid] - if !ok { - log.Error(fmt.Errorf("no such provider: %s", pid)) - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return - } - verifier := _oidc.Verifier(&oidc.Config{ClientID: oauth.ClientID}) code := r.URL.Query().Get("code") @@ -734,12 +744,10 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { var claims claimResult - // Extract the ID Token from OAuth2 token. - rawIDToken, ok := oauth2Token.Extra("id_token").(string) + rawIDToken, tokenOk := oauth2Token.Extra("id_token").(string) - if ok && rawIDToken != "" { + if tokenOk && rawIDToken != "" { var idToken *oidc.IDToken - // Parse and verify ID Token payload. idToken, err = verifier.Verify(ctx, rawIDToken) if err == nil { @@ -770,7 +778,7 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { return } - user, err := helpers.Store(r).GetUserByLoginOrEmail("", claims.email) // ignore username because it creates a lot of problems + user, err := helpers.Store(r).GetUserByLoginOrEmail("", claims.email) if err != nil { user = db.User{ Username: claims.username, @@ -795,18 +803,13 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { createSession(w, r, user, true) - config, ok := util.Config.OidcProviders[pid] - if !ok { - log.Error(fmt.Errorf("no such provider: %s", pid)) - http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) - return - } - redirectPath := "" - if config.ReturnViaState { - redirectPath = stateData.Return - } else { - redirectPath = mux.Vars(r)["redirect_path"] + if !idpInitiated { + if provider.ReturnViaState { + redirectPath = stateData.Return + } else { + redirectPath = mux.Vars(r)["redirect_path"] + } } if !strings.HasPrefix(redirectPath, "/") { diff --git a/util/OdbcProvider.go b/util/OdbcProvider.go index 308cc6bad..0f2dc85c0 100644 --- a/util/OdbcProvider.go +++ b/util/OdbcProvider.go @@ -17,7 +17,11 @@ type OidcProvider struct { EmailClaim string `json:"email_claim" default:"email"` Order int `json:"order"` // ReturnViaState when true, passes the return path via the OAuth state parameter instead of the redirect URL path. This is useful for OAuth providers that have strict redirect URL validation. - ReturnViaState bool `json:"return_via_state" default:"true"` + ReturnViaState bool `json:"return_via_state" default:"true"` + // AllowIdpInitiated when true, permits IdP-initiated OIDC login (e.g. clicking the app + // tile in Okta). This skips CSRF state validation since the flow does not originate from + // Semaphore. Only enable this for trusted identity providers. + AllowIdpInitiated bool `json:"allow_idp_initiated"` } type ClaimsProvider interface {