diff --git a/cmd/api/src/api/auth.go b/cmd/api/src/api/auth.go index 3f30c4afb..9655a6af4 100644 --- a/cmd/api/src/api/auth.go +++ b/cmd/api/src/api/auth.go @@ -296,24 +296,23 @@ func (s authenticator) ValidateRequestSignature(tokenID uuid.UUID, request *http } func DeleteBrowserCookie(request *http.Request, response http.ResponseWriter, name string) { - SetSecureBrowserCookie(request, response, name, "", time.Now().UTC(), false) + SetSecureBrowserCookie(request, response, name, "", time.Now().UTC(), false, 0) } -func SetSecureBrowserCookie(request *http.Request, response http.ResponseWriter, name, value string, expires time.Time, httpOnly bool) { +func SetSecureBrowserCookie(request *http.Request, response http.ResponseWriter, name, value string, expires time.Time, httpOnly bool, sameSite http.SameSite) { var ( - hostURL = *ctx.FromRequest(request).Host - sameSiteValue = http.SameSiteDefaultMode - domainValue string + hostURL = *ctx.FromRequest(request).Host + isHttps = hostURL.Scheme == "https" ) - if hostURL.Scheme == "https" { - sameSiteValue = http.SameSiteStrictMode + // If sameSite is not explicitly set, we want to rely on the host scheme + if sameSite == 0 && isHttps { + sameSite = http.SameSiteStrictMode } - // NOTE: Set-Cookie should generally have the Domain field blank to ensure the cookie is only included with requests against the host, excluding subdomains; however, - // most browsers will ignore Set-Cookie headers from localhost responses if the Domain field is not set explicitly. - if strings.Contains(hostURL.Hostname(), "localhost") { - domainValue = hostURL.Hostname() + // NOTE: Browsers will not set localhost cookies with sameSite set to None. This is a local network SSO workaround + if strings.Contains(hostURL.Hostname(), "localhost") && sameSite == http.SameSiteNoneMode { + sameSite = http.SameSiteDefaultMode } // Set the token cookie @@ -321,11 +320,10 @@ func SetSecureBrowserCookie(request *http.Request, response http.ResponseWriter, Name: name, Value: value, Expires: expires, - Secure: hostURL.Scheme == "https", + Secure: isHttps, HttpOnly: httpOnly, - SameSite: sameSiteValue, + SameSite: sameSite, Path: "/", - Domain: domainValue, }) } @@ -356,8 +354,6 @@ func (s authenticator) CreateSSOSession(request *http.Request, response http.Res auditLogFields["oidc_provider_id"] = ssoProvider.OIDCProvider.ID authProvider = *ssoProvider.OIDCProvider } - DeleteBrowserCookie(request, response, AuthStateCookieName) - DeleteBrowserCookie(request, response, AuthPKCECookieName) } // Generate commit ID for audit logging @@ -404,8 +400,8 @@ func (s authenticator) CreateSSOSession(request *http.Request, response http.Res locationURL := URLJoinPath(hostURL, UserInterfacePath) - // Set the token cookie - SetSecureBrowserCookie(request, response, AuthTokenCookieName, sessionJWT, time.Now().UTC().Add(s.cfg.AuthSessionTTL()), false) + // Set the token cookie, httpOnly must be false for the UI to pick up and store token + SetSecureBrowserCookie(request, response, AuthTokenCookieName, sessionJWT, time.Now().UTC().Add(s.cfg.AuthSessionTTL()), false, 0) // Redirect back to the UI landing page response.Header().Add(headers.Location.String(), locationURL.String()) diff --git a/cmd/api/src/api/constant.go b/cmd/api/src/api/constant.go index 1fe172b81..0eab516f3 100644 --- a/cmd/api/src/api/constant.go +++ b/cmd/api/src/api/constant.go @@ -34,14 +34,16 @@ const ( AuthorizationSchemeBHESignature = "bhesignature" AuthorizationSchemeBearer = "bearer" + // Form parameters + FormParameterState = "state" + FormParameterCode = "code" + // Query parameters QueryParameterSortBy = "sort_by" QueryParameterHydrateCounts = "counts" QueryParameterHydrateDomains = "hydrate_domains" QueryParameterHydrateOUs = "hydrate_ous" QueryParameterScope = "scope" - QueryParameterState = "state" - QueryParameterCode = "code" // URI path parameters URIPathVariableApplicationConfigurationParameter = "parameter" diff --git a/cmd/api/src/api/v2/auth/oidc.go b/cmd/api/src/api/v2/auth/oidc.go index 69fb644f2..3a6c78e94 100644 --- a/cmd/api/src/api/v2/auth/oidc.go +++ b/cmd/api/src/api/v2/auth/oidc.go @@ -183,14 +183,13 @@ func (s ManagementResource) OIDCLoginHandler(response http.ResponseWriter, reque verifier := oauth2.GenerateVerifier() // Store PKCE on web browser in secure cookie for retrieval in callback - api.SetSecureBrowserCookie(request, response, api.AuthPKCECookieName, verifier, time.Now().UTC().Add(time.Minute*7), true) + api.SetSecureBrowserCookie(request, response, api.AuthPKCECookieName, verifier, time.Now().UTC().Add(time.Minute*7), true, http.SameSiteNoneMode) // Store State on web browser in secure cookie for retrieval in callback - api.SetSecureBrowserCookie(request, response, api.AuthStateCookieName, state, time.Now().UTC().Add(time.Minute*7), true) - - // Redirect user to consent page to ask for permission for the scopes specified above. - redirectURL := conf.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.S256ChallengeOption(verifier)) + api.SetSecureBrowserCookie(request, response, api.AuthStateCookieName, state, time.Now().UTC().Add(time.Minute*7), true, http.SameSiteNoneMode) + // Redirect user to consent page to ask for permission for the scopes specified above and specify POST callback + redirectURL := conf.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.S256ChallengeOption(verifier), oauth2.SetAuthURLParam("response_mode", "form_post")) response.Header().Add(headers.Location.String(), redirectURL) response.WriteHeader(http.StatusFound) } @@ -198,39 +197,37 @@ func (s ManagementResource) OIDCLoginHandler(response http.ResponseWriter, reque func (s ManagementResource) OIDCCallbackHandler(response http.ResponseWriter, request *http.Request, ssoProvider model.SSOProvider) { var ( - queryParams = request.URL.Query() - state = queryParams[api.QueryParameterState] - code = queryParams[api.QueryParameterCode] + state = request.FormValue(api.FormParameterState) + code = request.FormValue(api.FormParameterCode) ) + // No matter what happens, wipe the auth cookies + api.DeleteBrowserCookie(request, response, api.AuthStateCookieName) + api.DeleteBrowserCookie(request, response, api.AuthPKCECookieName) + if ssoProvider.OIDCProvider == nil { // SSO misconfiguration scenario api.RedirectToLoginURL(response, request, "Your SSO Connection failed, please contact your Administrator") - } else if len(code) == 0 { - // Don't want to log state but do want to know if state was present - hasState := queryParams.Has(api.QueryParameterState) - queryParams.Del(api.QueryParameterState) - log.Warnf("[OIDC] auth code is missing, has state %t %+v", hasState, queryParams) - // Missing authorization code implies a credentials or form issue - // Not explicitly covered, treat as technical issue + } else if code == "" { + log.Warnf("[OIDC] auth code is missing") + api.RedirectToLoginURL(response, request, "We’re having trouble connecting. Please check your internet and try again.") + } else if state == "" { + log.Warnf("[OIDC] state parameter is missing") + // Missing state parameter - treat as technical issue api.RedirectToLoginURL(response, request, "We’re having trouble connecting. Please check your internet and try again.") } else if pkceVerifier, err := request.Cookie(api.AuthPKCECookieName); err != nil { log.Warnf("[OIDC] pkce cookie is missing") // Missing PKCE verifier - likely a technical or config issue api.RedirectToLoginURL(response, request, "We’re having trouble connecting. Please check your internet and try again.") - } else if len(state) == 0 { - log.Warnf("[OIDC] state parameter is missing") - // Missing state parameter - treat as technical issue - api.RedirectToLoginURL(response, request, "We’re having trouble connecting. Please check your internet and try again.") - } else if stateCookie, err := request.Cookie(api.AuthStateCookieName); err != nil || stateCookie.Value != state[0] { - log.Warnf("[OIDC] state cookie does not match %v", err) + } else if stateCookie, err := request.Cookie(api.AuthStateCookieName); err != nil || stateCookie.Value != state { + log.Warnf(fmt.Sprintf("[OIDC] state cookie does not match %v", err)) // Invalid state - treat as technical issue or misconfiguration api.RedirectToLoginURL(response, request, "We’re having trouble connecting. Please check your internet and try again.") } else if provider, err := oidc.NewProvider(request.Context(), ssoProvider.OIDCProvider.Issuer); err != nil { log.Warnf("[OIDC] Failed to create OIDC provider: %v", err) // SSO misconfiguration scenario api.RedirectToLoginURL(response, request, "Your SSO Connection failed, please contact your Administrator") - } else if claims, err := getOIDCClaims(request.Context(), provider, ssoProvider, pkceVerifier, code[0]); err != nil { + } else if claims, err := getOIDCClaims(request.Context(), provider, ssoProvider, pkceVerifier, code); err != nil { log.Warnf("[OIDC] %v", err) api.RedirectToLoginURL(response, request, "Your SSO was unable to authenticate your user, please contact your Administrator") } else if email, err := getEmailFromOIDCClaims(claims); errors.Is(err, ErrEmailMissing) { // Note email claims are not always present so we will check different claim keys for possible email