Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions cmd/bridge/config/auth/authoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,15 @@ func (c *completedOptions) getAuthenticator(

// Config for logging into console.
oidcClientConfig := &oauth2.Config{
AuthSource: authSource,
IssuerURL: userAuthOIDCIssuerURL.String(),
IssuerCA: c.CAFilePath,
ClientID: c.ClientID,
ClientSecret: oidcClientSecret,
RedirectURL: proxy.SingleJoiningSlash(baseURL.String(), server.AuthLoginCallbackEndpoint),
Scope: scopes,
OCLoginCommand: c.OCLoginCommand,
AuthSource: authSource,
IssuerURL: userAuthOIDCIssuerURL.String(),
IssuerCA: c.CAFilePath,
ClientID: c.ClientID,
ClientSecret: oidcClientSecret,
RedirectURL: proxy.SingleJoiningSlash(baseURL.String(), server.AuthLoginCallbackEndpoint),
ConsoleBaseAddress: baseURL.String(),
Scope: scopes,
OCLoginCommand: c.OCLoginCommand,

// Use the k8s CA file for OpenShift OAuth metadata discovery.
// This might be different than IssuerCA.
Expand Down
26 changes: 20 additions & 6 deletions frontend/public/module/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,25 @@ export const authSvc = {
setNext(next);
clearLocalStorage(clearLocalStorageKeys);
coFetch(logoutURL, { method: 'POST' })
// eslint-disable-next-line no-console
.catch((e) => console.error('Error logging out', e))
.then(() => {
.then(async (response) => {
let dynamicLogoutURL: string | undefined;
try {
const data = await response.json();
dynamicLogoutURL = data?.logoutRedirectURL;
} catch {
// Response may be empty (204) for non-OIDC auth — ignore
}

if (isKubeAdmin) {
authSvc.logoutKubeAdmin();
} else if (dynamicLogoutURL) {
window.location.assign(dynamicLogoutURL);
} else {
authSvc.logoutRedirect(next);
}
})
.catch(() => {
authSvc.logoutRedirect(next);
});
}),

Expand Down Expand Up @@ -164,8 +175,11 @@ export const authSvc = {
}
},

// Handle 401 responses with redirect loop detection
handle401: (next) => {
// Handle 401 responses with redirect loop detection.
// Wrapped in _.once so that only the first 401 per page load triggers the
// flow — the SPA fires many API calls concurrently and each would otherwise
// increment the redirect counter past MAX_AUTH_REDIRECTS within a single load.
handle401: _.once((next) => {
const redirectCount = incrementAuthRedirectCount();

// If we've exceeded the max redirects, redirect to the error page
Expand All @@ -190,7 +204,7 @@ export const authSvc = {

// Proceed with normal logout flow
authSvc.logout(next);
},
}),

// Reset redirect counter (called on successful k8s requests)
resetRedirectCount: () => {
Expand Down
30 changes: 23 additions & 7 deletions pkg/auth/oauth2/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
const (
stateCookieName = "login-state"
errorOAuth = "oauth_error"
stateLength = 32 // hex-encoded 16 random bytes
errorLoginState = "login_state_error"
errorCookie = "cookie_error"
errorInternal = "internal_error"
Expand Down Expand Up @@ -107,6 +108,7 @@ type Config struct {
LogoutRedirectOverride string // overrides the OIDC provider's front-channel logout URL
IssuerCA string
RedirectURL string
ConsoleBaseAddress string // used as post_logout_redirect_uri in OIDC RP-Initiated Logout
ClientID string
ClientSecret string
Scope []string
Expand Down Expand Up @@ -200,6 +202,7 @@ func NewOAuth2Authenticator(ctx context.Context, config *Config) (*OAuth2Authent
issuerURL: c.IssuerURL,
logoutRedirectOverride: c.LogoutRedirectOverride,
clientID: c.ClientID,
consoleBaseAddress: c.ConsoleBaseAddress,
cookiePath: c.CookiePath,
secureCookies: c.SecureCookies,
constructOAuth2Config: a.oauth2ConfigConstructor,
Expand Down Expand Up @@ -287,10 +290,12 @@ func (a *OAuth2Authenticator) LoginFunc(w http.ResponseWriter, r *http.Request)
state := hex.EncodeToString(randData[:])

cookie := http.Cookie{
Name: stateCookieName,
Name: stateCookieName + "-" + state[:8],
Value: state,
HttpOnly: true,
Secure: a.secureCookies,
Path: "/auth",
MaxAge: 300,
}
http.SetCookie(w, &cookie)
http.Redirect(w, r, a.oauth2Config().AuthCodeURL(state), http.StatusSeeOther)
Expand Down Expand Up @@ -321,11 +326,20 @@ func (a *OAuth2Authenticator) CallbackFunc(fn func(loginInfo sessions.LoginJSON,
return
}

cookieState, err := r.Cookie(stateCookieName)
if err != nil {
klog.Errorf("failed to parse state cookie: %v", err)
a.redirectAuthError(w, errorMissingState)
return
// Look up the per-state cookie for this login flow.
var cookieState *http.Cookie
if len(urlState) == stateLength {
cookieState, _ = r.Cookie(stateCookieName + "-" + urlState[:8])
}
if cookieState == nil {
// Fall back to legacy single cookie for rolling-update compat.
var cookieErr error
cookieState, cookieErr = r.Cookie(stateCookieName)
if cookieErr != nil {
klog.Errorf("failed to parse state cookie: %v", cookieErr)
a.redirectAuthError(w, errorMissingState)
return
}
}

// Lack of both `error` and `code` indicates some other redirect with no params.
Expand All @@ -341,10 +355,12 @@ func (a *OAuth2Authenticator) CallbackFunc(fn func(loginInfo sessions.LoginJSON,
}

if urlState != cookieState.Value {
klog.Error("state in url does not match State cookie")
klog.Errorf("state in url does not match state cookie %s", cookieState.Name)
a.redirectAuthError(w, errorInvalidState)
return
}
http.SetCookie(w, &http.Cookie{Name: cookieState.Name, Path: "/auth", MaxAge: -1})

ctx := oidc.ClientContext(r.Context(), a.clientFunc())
oauthConfig := a.oauth2Config()
token, err := oauthConfig.Exchange(ctx, code)
Expand Down
61 changes: 60 additions & 1 deletion pkg/auth/oauth2/auth_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package oauth2

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/url"
"sync"
"time"

oidc "github.com/coreos/go-oidc"
"golang.org/x/oauth2"
"k8s.io/klog/v2"

"github.com/openshift/console/pkg/auth"
"github.com/openshift/console/pkg/auth/sessions"
Expand All @@ -35,6 +38,7 @@ type oidcConfig struct {
issuerURL string
logoutRedirectOverride string
clientID string
consoleBaseAddress string
cookiePath string
secureCookies bool
constructOAuth2Config oauth2ConfigConstructor
Expand Down Expand Up @@ -102,7 +106,7 @@ func (o *oidcAuth) refreshSession(ctx context.Context, w http.ResponseWriter, r
&oauth2.Token{RefreshToken: cookieRefreshToken},
).Token()
if err != nil {
return nil, fmt.Errorf("failed to refresh a token %s: %w", cookieRefreshToken, err)
return nil, fmt.Errorf("failed to refresh a token: %w", err)
}

ls, err := o.sessions.UpdateTokens(w, r, o.verify, newTokens)
Expand All @@ -122,11 +126,66 @@ func (o *oidcAuth) DeleteSession(w http.ResponseWriter, r *http.Request) {
o.sessions.DeleteSession(w, r)
}

// logout handles the OIDC logout flow by constructing a logout redirect URL
// with an id_token_hint appended (when available) and returning it as JSON.
func (o *oidcAuth) logout(w http.ResponseWriter, r *http.Request) {
logoutURL := o.logoutRedirectURLWithIDTokenHint(w, r)
o.DeleteSession(w, r)

if logoutURL != "" {
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(map[string]string{
"logoutRedirectURL": logoutURL,
}); err != nil {
klog.Errorf("failed to write logout redirect response: %v", err)
}
return
}

w.WriteHeader(http.StatusNoContent)
}

// logoutRedirectURLWithIDTokenHint builds the logout redirect URL with the
// id_token_hint query parameter appended, per the OIDC RP-Initiated Logout spec.
// Returns empty when the session or id_token is unavailable — the caller should
// fall back to 204 so the frontend uses the static logout redirect instead of
// navigating to the IdP logout endpoint without a hint (which would cause a
// redirect loop via post_logout_redirect_uri).
func (o *oidcAuth) logoutRedirectURLWithIDTokenHint(w http.ResponseWriter, r *http.Request) string {
baseURL := o.LogoutRedirectURL()
if baseURL == "" {
return ""
}

ls, err := o.sessions.GetSession(w, r)
if err != nil || ls == nil {
klog.V(4).Infof("could not retrieve session for id_token_hint: %v", err)
return ""
}

idToken := ls.AccessToken()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be a little confused here - but isn't the access_token and id_token different? Can id_token_hint param accept access_token if it was used to authenticate?

My understanding was id_token_hint needs to take id_token . Since id_token needs to use OIDC flow would this work in OAuth2 ? I believe the oauth-server or osin when making the request gets responseData which would contain id_token but I believe id_token is never specified as something stored to use in the same way access_token is.

I'm asking because I'm curious as to how console works. I was looking at this work and thought of using id_token_hint but ran across this issue. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question! The method name AccessToken() is indeed misleading here. It returns ls.rawToken, and what rawToken contains depends on the auth path:

OIDC path (this fix's scope): When tokenVerifier != nil, rawToken is set to token.Extra("id_token").(string) — the raw id_token JWT from the OIDC provider (loginstate.go L75-88). So AccessToken() actually returns the id_token in this path, which is exactly what id_token_hint needs.

OpenShift OAuth path: When tokenVerifier == nil, rawToken is set to token.AccessToken — the OCP OAuth bearer token (e.g. sha256~...) (loginstate.go L64-72). This is NOT an id_token and can't be used as id_token_hint. But this fix only runs in the oidcAuth.logout() handler, which is never called in the OpenShift auth path — openShiftAuth.logout() has its own separate implementation.

So to directly answer your question: this won't work in pure OAuth2 (OpenShift auth mode) because the id_token isn't stored there. It only works in the OIDC auth path where the console talks directly to an OIDC provider like Keycloak.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting back to me so quickly. That makes perfect sense. Thanks for the explanation!

if idToken == "" {
return ""
}

parsed, err := url.Parse(baseURL)
if err != nil {
klog.Errorf("failed to parse logout redirect URL: %v", err)
return ""
}

q := parsed.Query()
q.Set("id_token_hint", idToken)
if !q.Has("client_id") && o.clientID != "" {
q.Set("client_id", o.clientID)
}
if !q.Has("post_logout_redirect_uri") && o.consoleBaseAddress != "" {
q.Set("post_logout_redirect_uri", o.consoleBaseAddress)
}
parsed.RawQuery = q.Encode()
return parsed.String()
}

func (o *oidcAuth) getLoginState(w http.ResponseWriter, r *http.Request) (*sessions.LoginState, error) {
ls, err := o.sessions.GetSession(w, r)
if err != nil {
Expand Down
Loading