implement PKCE for OIDC auth flow

backend/auth/handler.go

@@ -18,6 +18,8 @@ package auth
 
 import (
 	"context"
+	"crypto/rand"
+	"crypto/sha256"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -85,6 +87,8 @@ func (ah *AuthHandler) HandleAuthorize(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	logger.V(2).Info("AuthorizeRequest prepared", "authReq", authReq, "params", params)
+
 	if authReq.RedirectURL == "" || authReq.SessionID == "" {
 		logger.Error(errors.New("missing required parameters"), "failed to authorize")
 		ah.respondWithError(w, authReq.ClientType, "missing redirect_url or session_id", http.StatusBadRequest)
@@ -94,20 +98,37 @@ func (ah *AuthHandler) HandleAuthorize(w http.ResponseWriter, r *http.Request) {
 	scopes := []string{"openid", "profile", "email", "offline_access", "groups"}
 	dataCode, err := json.Marshal(authReq)
 	if err != nil {
-		logger.Info("failed to marshal auth code", "error", err)
+		logger.Error(err, "failed to marshal auth code")
 		ah.respondWithError(w, authReq.ClientType, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	provider, err := ah.oidc.GetOIDCProvider(r.Context())
 	if err != nil {
-		logger.Info("failed to get OIDC provider", "error", err)
+		logger.Error(err, "failed to get OIDC provider")
 		ah.respondWithError(w, authReq.ClientType, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
 	encoded := base64.URLEncoding.EncodeToString(dataCode)
-	authURL := provider.OIDCProviderConfig(scopes).AuthCodeURL(encoded)
+
+	verifier, challenge, err := generatePKCE()
+	if err != nil {
+		logger.Error(err, "failed to generate PKCE")
+		ah.respondWithError(w, authReq.ClientType, "failed to generate PKCE", http.StatusInternalServerError)
+		return
+	}
+	if err := ah.sessionStore.SavePKCEVerifier(authReq.SessionID, verifier); err != nil {
+		logger.Error(err, "failed to store PKCE verifier")
+		ah.respondWithError(w, authReq.ClientType, "failed to store PKCE verifier", http.StatusInternalServerError)
+		return
+	}
+
+	opts := []oauth2.AuthCodeOption{
+		oauth2.SetAuthURLParam("code_challenge", challenge),
+		oauth2.SetAuthURLParam("code_challenge_method", "S256"),
+	}
+	authURL := provider.OIDCProviderConfig(scopes).AuthCodeURL(encoded, opts...)
 
 	http.Redirect(w, r, authURL, http.StatusFound)
 }
@@ -153,10 +174,11 @@ func (ah *AuthHandler) HandleCallback(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
+	logger.V(2).Info("HandleCallback state unmarshaled", "authCode", authCode)
 
 	provider, err := ah.oidc.GetOIDCProvider(r.Context())
 	if err != nil {
-		logger.Info("failed to get OIDC provider", "error", err)
+		logger.Error(err, "failed to get OIDC provider")
 		ah.respondWithError(w, authCode.ClientType, err.Error(), http.StatusInternalServerError)
 		return
 	}
@@ -172,7 +194,16 @@ func (ah *AuthHandler) HandleCallback(w http.ResponseWriter, r *http.Request) {
 		ctx = context.WithValue(ctx, oauth2.HTTPClient, client)
 	}
 
-	token, err := provider.OIDCProviderConfig(nil).Exchange(ctx, code)
+	verifier, err := ah.sessionStore.LoadAndDeletePKCEVerifier(authCode.SessionID)
+	if err != nil || verifier == "" {
+		logger.Error(err, "PKCE verifier not found for session; cannot exchange code", "sessionID", authCode.SessionID)
+		msg := "PKCE verifier not found. If you run multiple backend instances, use a shared session store (e.g. Redis) so the instance handling the callback can read the verifier stored at authorize time."
+		ah.respondWithError(w, authCode.ClientType, msg, http.StatusBadRequest)
+		return
+	}
+
+	exchangeOpts := []oauth2.AuthCodeOption{oauth2.VerifierOption(verifier)}
+	token, err := provider.OIDCProviderConfig(nil).Exchange(ctx, code, exchangeOpts...)
 	if err != nil {
 		logger.Error(err, "failed to exchange token")
 		http.Error(w, "internal error", http.StatusInternalServerError)
@@ -204,6 +235,7 @@ func (ah *AuthHandler) HandleCallback(w http.ResponseWriter, r *http.Request) {
 			sessionState.SessionID,
 			sessionState.ClusterID,
 			sessionState.RedirectURL,
+			sessionState.Token.Groups,
 			24*time.Hour, // 24 hours expiration
 		)
 		if err != nil {
@@ -326,3 +358,14 @@ func (ah *AuthHandler) unwrapJWT(p string) ([]byte, error) {
 	}
 	return payload, nil
 }
+
+func generatePKCE() (string, string, error) {
+	data := make([]byte, 32)
+	if _, err := rand.Read(data); err != nil {
+		return "", "", err
+	}
+	verifier := base64.RawURLEncoding.EncodeToString(data)
+	hash := sha256.Sum256([]byte(verifier))
+	challenge := base64.RawURLEncoding.EncodeToString(hash[:])
+	return verifier, challenge, nil
+}

backend/auth/jwt.go

@@ -32,11 +32,12 @@ type JWTService struct {
 }
 
 type Claims struct {
-	Subject     string `json:"sub"`
-	Issuer      string `json:"iss"`
-	SessionID   string `json:"sid"`
-	ClusterID   string `json:"cid"`
-	RedirectURL string `json:"red,omitempty"`
+	Subject     string   `json:"sub"`
+	Issuer      string   `json:"iss"`
+	SessionID   string   `json:"sid"`
+	ClusterID   string   `json:"cid"`
+	Groups      []string `json:"groups,omitempty"`
+	RedirectURL string   `json:"red,omitempty"`
 	jwt.RegisteredClaims
 }
 
@@ -53,13 +54,14 @@ func NewJWTService(issuer string) (*JWTService, error) {
 	}, nil
 }
 
-func (js *JWTService) GenerateToken(subject, oidcIssuer, sessionID, clusterID, redirectURL string, expiration time.Duration) (string, error) {
+func (js *JWTService) GenerateToken(subject, oidcIssuer, sessionID, clusterID, redirectURL string, groups []string, expiration time.Duration) (string, error) {
 	now := time.Now()
 	claims := &Claims{
 		Subject:     subject,
 		Issuer:      oidcIssuer,
 		SessionID:   sessionID,
 		ClusterID:   clusterID,
+		Groups:      groups,
 		RedirectURL: redirectURL,
 		RegisteredClaims: jwt.RegisteredClaims{
 			Subject:   subject,

backend/auth/middleware.go

@@ -19,6 +19,7 @@ package auth
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
@@ -111,6 +112,7 @@ func (am *AuthMiddleware) authenticate(next http.Handler) http.Handler {
 						Token: session.TokenInfo{
 							Subject: claims.Subject,
 							Issuer:  claims.Issuer,
+							Groups:  claims.Groups,
 						},
 						SessionID:   claims.SessionID,
 						ClusterID:   claims.ClusterID,
@@ -219,7 +221,8 @@ func (am *AuthMiddleware) authorizeK8S(next http.Handler) http.Handler {
 		if err != nil {
 			logger.V(2).Info("Kubernetes RBAC authorization failed", "error", err)
 			statusCode, code, details := mapErrorToCode(err)
-			writeErrorResponse(w, statusCode, code, "Cluster authorization failed. Missing required permissions in the cluster to access bindings.", details)
+			hint := fmt.Sprintf("%s Start the backend with --oidc-allowed-users=%s or --oidc-allowed-groups=<group> (user must be in one of the allowed groups).", details, authCtx.SessionState.Token.Subject)
+			writeErrorResponse(w, statusCode, code, "Cluster authorization failed. Missing required permissions in the cluster to access bindings.", hint)
 			return
 		}
 

backend/auth/types.go

@@ -121,11 +121,14 @@ func NewOIDCServiceProviderWithTLS(
 
 func (o *OIDCServiceProvider) OIDCProviderConfig(scopes []string) *oauth2.Config {
 	config := &oauth2.Config{
-		ClientID:     o.clientID,
-		ClientSecret: o.clientSecret,
-		Endpoint:     o.provider.Endpoint(),
-		RedirectURL:  o.redirectURI,
-		Scopes:       scopes,
+		ClientID:    o.clientID,
+		Endpoint:    o.provider.Endpoint(),
+		RedirectURL: o.redirectURI,
+		Scopes:      scopes,
+	}
+
+	if o.clientSecret != "" {
+		config.ClientSecret = o.clientSecret
 	}
 
 	return config
@@ -134,3 +137,15 @@ func (o *OIDCServiceProvider) OIDCProviderConfig(scopes []string) *oauth2.Config
 func (o *OIDCServiceProvider) GetTLSConfig() *tls.Config {
 	return o.tlsConfig
 }
+
+func (o *OIDCServiceProvider) ClientID() string {
+	return o.clientID
+}
+
+func (o *OIDCServiceProvider) ClientSecret() string {
+	return o.clientSecret
+}
+
+func (o *OIDCServiceProvider) IssuerURL() string {
+	return o.issuerURL
+}

backend/options/oidc.go

@@ -112,9 +112,6 @@ func (options *OIDC) Validate() error {
 	if options.IssuerClientID == "" {
 		return fmt.Errorf("OIDC issuer client ID cannot be empty")
 	}
-	if options.IssuerClientSecret == "" && os.Getenv("OIDC_CLIENT_SECRET") == "" {
-		return fmt.Errorf("OIDC issuer client secret cannot be empty")
-	}
 	if os.Getenv("OIDC_CLIENT_SECRET") != "" && options.Type == string(kubebindv1alpha2.OIDCProviderTypeEmbedded) {
 		return fmt.Errorf("OIDC issuer client secret cannot be provided via environment variable when using embedded OIDC provider")
 	}

backend/server.go

@@ -270,22 +270,33 @@ func NewServer(ctx context.Context, c *Config) (*Server, error) {
 func (s *Server) initializeOIDCProvider(ctx context.Context, callback string) (*auth.OIDCServiceProvider, error) {
 	logger := klog.FromContext(ctx)
 	logger.Info("Initializing OIDC Service Provider", "issuerURL", s.Config.Options.OIDC.IssuerURL)
-	if s.Config.Options.OIDC.TLSConfig != nil {
+
+	clientID := s.Config.Options.OIDC.IssuerClientID
+	clientSecret := s.Config.Options.OIDC.IssuerClientSecret
+	issuerURL := s.Config.Options.OIDC.IssuerURL
+	tlsConfig := s.Config.Options.OIDC.TLSConfig
+
+	if clientSecret != "" {
+		logger.Info("WARNING: OIDC client secret is configured. For CLI applications, consider using PKCE.")
+	}
+
+	if tlsConfig != nil {
 		return auth.NewOIDCServiceProviderWithTLS(
 			ctx,
-			s.Config.Options.OIDC.IssuerClientID,
-			s.Config.Options.OIDC.IssuerClientSecret,
+			clientID,
+			clientSecret,
 			callback,
-			s.Config.Options.OIDC.IssuerURL,
-			s.Config.Options.OIDC.TLSConfig,
+			issuerURL,
+			tlsConfig,
 		)
 	}
+
 	return auth.NewOIDCServiceProvider(
 		ctx,
-		s.Config.Options.OIDC.IssuerClientID,
-		s.Config.Options.OIDC.IssuerClientSecret,
+		clientID,
+		clientSecret,
 		callback,
-		s.Config.Options.OIDC.IssuerURL,
+		issuerURL,
 	)
 }
 

backend/session/store.go

@@ -17,26 +17,32 @@ limitations under the License.
 package session
 
 import (
+	"errors"
 	"fmt"
 	"sync"
 )
 
 var ErrSessionNotFound = fmt.Errorf("session not found")
+var ErrPKCEVerifierNotFound = fmt.Errorf("pkce verifier not found")
 
 type Store interface {
 	Save(state *State) error
 	Load(sessionID string) (*State, error)
 	Delete(sessionID string) error
+	SavePKCEVerifier(sessionID, verifier string) error
+	LoadAndDeletePKCEVerifier(sessionID string) (string, error)
 }
 
 type InMemoryStore struct {
-	lock     sync.RWMutex
-	sessions map[string]*State
+	lock          sync.RWMutex
+	sessions      map[string]*State
+	pkceVerifiers map[string]string
 }
 
 func NewInMemoryStore() *InMemoryStore {
 	return &InMemoryStore{
-		sessions: make(map[string]*State),
+		sessions:      make(map[string]*State),
+		pkceVerifiers: make(map[string]string),
 	}
 }
 
@@ -63,3 +69,27 @@ func (s *InMemoryStore) Delete(sessionID string) error {
 	delete(s.sessions, sessionID)
 	return nil
 }
+
+func (s *InMemoryStore) SavePKCEVerifier(sessionID, verifier string) error {
+	if sessionID == "" || verifier == "" {
+		return errors.New("sessionID and verifier cannot be empty")
+	}
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	s.pkceVerifiers[sessionID] = verifier
+	return nil
+}
+
+func (s *InMemoryStore) LoadAndDeletePKCEVerifier(sessionID string) (string, error) {
+	if sessionID == "" {
+		return "", ErrPKCEVerifierNotFound
+	}
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	verifier, ok := s.pkceVerifiers[sessionID]
+	if !ok {
+		return "", ErrPKCEVerifierNotFound
+	}
+	delete(s.pkceVerifiers, sessionID)
+	return verifier, nil
+}

cli/pkg/kubectl/bind/plugin/authenticate_test.go

@@ -30,8 +30,8 @@ func TestValidateVersion(t *testing.T) {
 		{"v0.0.0", "v0.0.0", false},
 		{"development version", "v0.0.0-master+$Format:%H$", false},
 		{"old", "v0.2.3", true},
-		{"minimum", "v0.3.0", false},
-		{"newer minor", "v0.3.5", false},
+		{"minimum", "v0.6.0", false},
+		{"newer minor", "v0.6.1", false},
 		{"newer major", "v1.2.5", false},
 	}
 	for _, tt := range tests {