From a94ca6eaa2d36f88b0764d3ec05671606d72dd5e Mon Sep 17 00:00:00 2001 From: Harry Anderson Date: Thu, 18 Jun 2026 01:33:49 +0000 Subject: [PATCH 1/3] feat(oidc): add CLI login via device authorization grant + PKCE Extend the existing OIDC operator-UI auth to the CLI using the OAuth 2.0 device authorization grant (RFC 8628), brokered by the node so the provider device_code never reaches the CLI. - Always send PKCE (RFC 7636) on the authorization-code flow; make the OIDC ClientSecret optional so a public/native client works while confidential deployments stay unchanged. - Extract issueSessionFromIDToken so the browser exchange and the device flow share one verification + role-mapping + session-creation path. - Add /oidc-device/start and /oidc-device/poll endpoints and a CLI device-flow cookie authenticator; RemoteLogin branches on OIDC and keeps the local email/password path as break-glass. --- .changeset/oidc-cli-device-flow.md | 7 + core/cmd/app.go | 5 +- core/cmd/oidc_device_auth.go | 201 +++++++++++++++ core/cmd/oidc_device_auth_test.go | 129 ++++++++++ core/cmd/shell.go | 6 + core/cmd/shell_remote.go | 18 ++ core/sessions/oidcauth/device_flow.go | 278 +++++++++++++++++++++ core/sessions/oidcauth/device_flow_test.go | 159 ++++++++++++ core/sessions/oidcauth/helpers_test.go | 1 + core/sessions/oidcauth/oidc.go | 159 ++++++++---- 10 files changed, 911 insertions(+), 52 deletions(-) create mode 100644 .changeset/oidc-cli-device-flow.md create mode 100644 core/cmd/oidc_device_auth.go create mode 100644 core/cmd/oidc_device_auth_test.go create mode 100644 core/sessions/oidcauth/device_flow.go create mode 100644 core/sessions/oidcauth/device_flow_test.go diff --git a/.changeset/oidc-cli-device-flow.md b/.changeset/oidc-cli-device-flow.md new file mode 100644 index 00000000000..09aa9ae70e8 --- /dev/null +++ b/.changeset/oidc-cli-device-flow.md @@ -0,0 +1,7 @@ +--- +"chainlink": minor +--- + +#added Added OIDC login support to the CLI via the OAuth 2.0 device authorization grant (RFC 8628). When a node is configured with `AuthenticationMethod = 'oidc'`, `chainlink admin login` (with no credentials file) now performs a browser-based device flow against the identity provider, brokered by the node. The local email/password path is unchanged and remains available as a break-glass admin. + +#changed The OIDC authorization-code (operator UI) flow now always uses PKCE (RFC 7636). The OIDC `ClientSecret` is now optional: confidential clients still send it, while public clients (required for the device flow) rely on PKCE instead. Existing confidential-client deployments are unaffected. diff --git a/core/cmd/app.go b/core/cmd/app.go index 3045e71c2a9..a9c2446a720 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -107,7 +107,8 @@ func NewApp(s *Shell) *cli.App { insecureSkipVerify := c.Bool("insecure-skip-verify") clientOpts := ClientOpts{RemoteNodeURL: *remoteNodeURL, InsecureSkipVerify: insecureSkipVerify} - cookieAuth := NewSessionCookieAuthenticator(clientOpts, DiskCookieStore{Config: cookieJar}, s.Logger) + cookieStore := DiskCookieStore{Config: cookieJar} + cookieAuth := NewSessionCookieAuthenticator(clientOpts, cookieStore, s.Logger) sessionRequestBuilder := NewFileSessionRequestBuilder(s.Logger) credentialsFile := c.String("admin-credentials-file") @@ -119,6 +120,8 @@ func NewApp(s *Shell) *cli.App { s.HTTP = NewAuthenticatedHTTPClient(s.Logger, clientOpts, cookieAuth, sr) s.CookieAuthenticator = cookieAuth s.FileSessionRequestBuilder = sessionRequestBuilder + s.clientOpts = clientOpts + s.cookieStore = cookieStore // Allow for initServerConfig to be called if the flag is provided. if c.Bool("applyInitServerConfig") { diff --git a/core/cmd/oidc_device_auth.go b/core/cmd/oidc_device_auth.go new file mode 100644 index 00000000000..bfda0d10162 --- /dev/null +++ b/core/cmd/oidc_device_auth.go @@ -0,0 +1,201 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "time" + + "github.com/pkg/errors" + + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/web" +) + +// OIDC device authorization grant client for the CLI. +// +// When a node is configured with AuthenticationMethod = 'oidc', the local +// users table only holds break-glass admins; interactive operators authenticate +// through the identity provider. The CLI cannot drive a browser redirect, so it +// uses the RFC 8628 device flow the node brokers at /oidc-device/start and +// /oidc-device/poll. The CLI never talks to the identity provider directly and +// never holds any token; it receives only an opaque handle and, on success, the +// same session cookie the browser flow produces. + +// oidcDeviceStartResponse mirrors oidcauth.DeviceStartResponse. +type oidcDeviceStartResponse struct { + DeviceHandle string `json:"device_handle"` + UserCode string `json:"user_code"` + VerificationURI string `json:"verification_uri"` + VerificationURIComplete string `json:"verification_uri_complete,omitempty"` + ExpiresIn int64 `json:"expires_in"` + Interval int64 `json:"interval"` +} + +// oidcDevicePollResponse mirrors oidcauth.DevicePollResponse. +type oidcDevicePollResponse struct { + Status string `json:"status"` + Message string `json:"message,omitempty"` +} + +// OIDCDeviceCookieAuthenticator obtains a session cookie via the node-brokered +// device authorization flow and persists it to the same cookie store the +// password authenticator uses. +type OIDCDeviceCookieAuthenticator struct { + config ClientOpts + store CookieStore + lggr logger.SugaredLogger + // out is where user-facing prompts are written. Defaults to os.Stdout in + // production; overridable in tests. + out io.Writer +} + +func NewOIDCDeviceCookieAuthenticator(config ClientOpts, store CookieStore, out io.Writer, lggr logger.Logger) *OIDCDeviceCookieAuthenticator { + return &OIDCDeviceCookieAuthenticator{ + config: config, + store: store, + lggr: logger.Sugared(lggr), + out: out, + } +} + +// NodeHasOIDCEnabled probes the node's unauthenticated /oidc-enabled endpoint to +// decide whether the device flow applies. A non-OIDC node 404s the route. +func NodeHasOIDCEnabled(ctx context.Context, config ClientOpts, lggr logger.Logger) bool { + client := newHttpClient(lggr, config.InsecureSkipVerify) + u := config.RemoteNodeURL.String() + "/oidc-enabled" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return false + } + resp, err := client.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() + return resp.StatusCode == http.StatusOK +} + +// Login runs the full device flow: start, prompt the operator, poll until the +// node reports completion, then save the returned session cookie. +func (o *OIDCDeviceCookieAuthenticator) Login(ctx context.Context) error { + start, err := o.start(ctx) + if err != nil { + return err + } + + o.promptUser(start) + + interval := time.Duration(start.Interval) * time.Second + if interval <= 0 { + interval = 5 * time.Second + } + deadline := time.Now().Add(time.Duration(start.ExpiresIn) * time.Second) + + ticker := time.NewTicker(interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + if time.Now().After(deadline) { + return errors.New("device authorization timed out before approval") + } + cookie, status, perr := o.poll(ctx, start.DeviceHandle) + if perr != nil { + return perr + } + switch status { + case "complete": + if cookie == nil { + return errors.New("node reported login complete but returned no session cookie") + } + return o.store.Save(cookie) + case "denied": + return errors.New("device authorization was denied or expired") + default: // "pending" + continue + } + } + } +} + +func (o *OIDCDeviceCookieAuthenticator) start(ctx context.Context) (*oidcDeviceStartResponse, error) { + u := o.config.RemoteNodeURL.String() + "/oidc-device/start" + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, nil) + if err != nil { + return nil, err + } + client := newHttpClient(o.lggr, o.config.InsecureSkipVerify) + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + return nil, errors.Errorf("failed to start device authorization (status %d): %s", resp.StatusCode, string(body)) + } + var sr oidcDeviceStartResponse + if err := json.Unmarshal(body, &sr); err != nil { + return nil, errors.Wrap(err, "failed to parse device authorization response") + } + return &sr, nil +} + +// poll asks the node for the flow status. On "complete" the node sets the +// session cookie on the response, which is extracted and returned. +func (o *OIDCDeviceCookieAuthenticator) poll(ctx context.Context, handle string) (*http.Cookie, string, error) { + b, err := json.Marshal(map[string]string{"device_handle": handle}) + if err != nil { + return nil, "", err + } + u := o.config.RemoteNodeURL.String() + "/oidc-device/poll" + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, bytes.NewReader(b)) + if err != nil { + return nil, "", err + } + req.Header.Set("Content-Type", "application/json") + client := newHttpClient(o.lggr, o.config.InsecureSkipVerify) + resp, err := client.Do(req) + if err != nil { + return nil, "", err + } + defer resp.Body.Close() + + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode == http.StatusNotFound { + return nil, "denied", nil + } + if resp.StatusCode != http.StatusOK { + return nil, "", errors.Errorf("device poll failed (status %d): %s", resp.StatusCode, string(body)) + } + var pr oidcDevicePollResponse + if err := json.Unmarshal(body, &pr); err != nil { + return nil, "", errors.Wrap(err, "failed to parse device poll response") + } + if pr.Status == "complete" { + return web.FindSessionCookie(resp.Cookies()), "complete", nil + } + return nil, pr.Status, nil +} + +func (o *OIDCDeviceCookieAuthenticator) promptUser(start *oidcDeviceStartResponse) { + uri := start.VerificationURIComplete + if uri == "" { + uri = start.VerificationURI + } + fmt.Fprintln(o.out, "To log in, open the following URL in a browser and sign in with your identity provider:") + fmt.Fprintf(o.out, "\n %s\n\n", uri) + if start.VerificationURIComplete == "" { + fmt.Fprintf(o.out, "When prompted, enter the code: %s\n\n", start.UserCode) + } else { + fmt.Fprintf(o.out, "Verify the code shown matches: %s\n\n", start.UserCode) + } + fmt.Fprintln(o.out, "Waiting for approval...") +} diff --git a/core/cmd/oidc_device_auth_test.go b/core/cmd/oidc_device_auth_test.go new file mode 100644 index 00000000000..d52cf30b758 --- /dev/null +++ b/core/cmd/oidc_device_auth_test.go @@ -0,0 +1,129 @@ +package cmd_test + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/cmd" + "github.com/smartcontractkit/chainlink/v2/core/logger" +) + +// mockNode stands in for an OIDC-enabled chainlink node, serving the three +// endpoints the CLI device flow touches. +type mockNode struct { + oidcEnabled bool + pollsPending int32 // number of "pending" polls to return before "complete" + denyAfter bool // if true, return "denied" instead of completing +} + +func (m *mockNode) handler(t *testing.T) http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("/oidc-enabled", func(w http.ResponseWriter, r *http.Request) { + if !m.oidcEnabled { + w.WriteHeader(http.StatusNotFound) + return + } + w.WriteHeader(http.StatusOK) + }) + mux.HandleFunc("/oidc-device/start", func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + _ = json.NewEncoder(w).Encode(map[string]any{ + "device_handle": "test-handle", + "user_code": "WDJB-MJHT", + "verification_uri": "https://sso.example.com/activate", + "verification_uri_complete": "https://sso.example.com/activate?user_code=WDJB-MJHT", + "expires_in": 300, + "interval": 1, + }) + }) + mux.HandleFunc("/oidc-device/poll", func(w http.ResponseWriter, r *http.Request) { + var req map[string]string + require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) + assert.Equal(t, "test-handle", req["device_handle"], "CLI must echo back the opaque handle") + + if atomic.AddInt32(&m.pollsPending, -1) >= 0 { + _ = json.NewEncoder(w).Encode(map[string]string{"status": "pending"}) + return + } + if m.denyAfter { + _ = json.NewEncoder(w).Encode(map[string]string{"status": "denied", "message": "expired"}) + return + } + http.SetCookie(w, &http.Cookie{Name: "clsession", Value: "abc123", Path: "/"}) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "complete"}) + }) + return mux +} + +func newClientOpts(t *testing.T, srv *httptest.Server) cmd.ClientOpts { + u, err := url.Parse(srv.URL) + require.NoError(t, err) + return cmd.ClientOpts{RemoteNodeURL: *u} +} + +func TestNodeHasOIDCEnabled(t *testing.T) { + t.Parallel() + lggr := logger.TestLogger(t) + + enabled := &mockNode{oidcEnabled: true} + srv := httptest.NewServer(enabled.handler(t)) + defer srv.Close() + assert.True(t, cmd.NodeHasOIDCEnabled(context.Background(), newClientOpts(t, srv), lggr)) + + disabled := &mockNode{oidcEnabled: false} + srv2 := httptest.NewServer(disabled.handler(t)) + defer srv2.Close() + assert.False(t, cmd.NodeHasOIDCEnabled(context.Background(), newClientOpts(t, srv2), lggr)) +} + +func TestOIDCDeviceLogin_Success(t *testing.T) { + t.Parallel() + node := &mockNode{oidcEnabled: true, pollsPending: 2} + srv := httptest.NewServer(node.handler(t)) + defer srv.Close() + + store := &cmd.MemoryCookieStore{} + var out bytes.Buffer + auth := cmd.NewOIDCDeviceCookieAuthenticator(newClientOpts(t, srv), store, &out, logger.TestLogger(t)) + + require.NoError(t, auth.Login(context.Background())) + + // The session cookie produced by the node must land in the cookie jar. + cookie, err := store.Retrieve() + require.NoError(t, err) + require.NotNil(t, cookie) + assert.Equal(t, "clsession", cookie.Name) + assert.Equal(t, "abc123", cookie.Value) + + // The operator must be shown the verification URI and code. + assert.Contains(t, out.String(), "https://sso.example.com/activate") + assert.Contains(t, out.String(), "WDJB-MJHT") +} + +func TestOIDCDeviceLogin_Denied(t *testing.T) { + t.Parallel() + node := &mockNode{oidcEnabled: true, pollsPending: 1, denyAfter: true} + srv := httptest.NewServer(node.handler(t)) + defer srv.Close() + + store := &cmd.MemoryCookieStore{} + var out bytes.Buffer + auth := cmd.NewOIDCDeviceCookieAuthenticator(newClientOpts(t, srv), store, &out, logger.TestLogger(t)) + + err := auth.Login(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "denied") + + // On denial nothing must be written to the cookie jar. + cookie, _ := store.Retrieve() + assert.Nil(t, cookie) +} diff --git a/core/cmd/shell.go b/core/cmd/shell.go index 347bdf22e0b..a98b4c95393 100644 --- a/core/cmd/shell.go +++ b/core/cmd/shell.go @@ -177,6 +177,12 @@ type Shell struct { ChangePasswordPrompter ChangePasswordPrompter PasswordPrompter PasswordPrompter + // clientOpts and cookieStore are retained so RemoteLogin can construct an + // OIDC device-flow authenticator on demand, reusing the same node URL and + // cookie jar as the password authenticator. + clientOpts ClientOpts + cookieStore CookieStore + configFiles []string configFilesIsSet bool secretsFiles []string diff --git a/core/cmd/shell_remote.go b/core/cmd/shell_remote.go index 2b0fe2a8867..198b5d49d57 100644 --- a/core/cmd/shell_remote.go +++ b/core/cmd/shell_remote.go @@ -175,8 +175,26 @@ func (s *Shell) getPage(requestURI string, page int, model any) (err error) { } // RemoteLogin creates a cookie session to run remote commands. +// +// With no credentials file and an OIDC-enabled node, login runs the browser +// device authorization flow against the identity provider. A supplied +// credentials file always uses the local email/password path (the OIDC +// break-glass admin), so automation and recovery keep working unchanged. func (s *Shell) RemoteLogin(c *cli.Context) error { lggr := s.Logger.Named("RemoteLogin") + + if c.String("file") == "" && NodeHasOIDCEnabled(s.ctx(), s.clientOpts, lggr) { + auth := NewOIDCDeviceCookieAuthenticator(s.clientOpts, s.cookieStore, os.Stdout, lggr) + if err := auth.Login(s.ctx()); err != nil { + return s.errorOut(err) + } + if err := s.checkRemoteBuildCompatibility(lggr, c.Bool("bypass-version-check"), static.Version, static.Sha); err != nil { + return s.errorOut(err) + } + fmt.Println("Successfully Logged In.") + return nil + } + sessionRequest, err := s.buildSessionRequest(c.String("file")) if err != nil { return s.errorOut(err) diff --git a/core/sessions/oidcauth/device_flow.go b/core/sessions/oidcauth/device_flow.go new file mode 100644 index 00000000000..6ad20001802 --- /dev/null +++ b/core/sessions/oidcauth/device_flow.go @@ -0,0 +1,278 @@ +package oidcauth + +import ( + "context" + "crypto/rand" + "encoding/base64" + "errors" + "io" + "net/http" + "sync" + "time" + + "github.com/gin-contrib/sessions" + "github.com/gin-gonic/gin" + "golang.org/x/oauth2" + + "github.com/smartcontractkit/chainlink/v2/core/logger/audit" + webauth "github.com/smartcontractkit/chainlink/v2/core/web/auth" +) + +// Device authorization grant (RFC 8628) for headless clients (the CLI). +// +// The node brokers the flow as a confidential or public OAuth client: it calls +// the provider's device authorization endpoint, hands the user-facing code and +// verification URI back to the CLI, then polls the provider's token endpoint on +// the CLI's behalf. The raw provider device_code never leaves the node; the CLI +// only ever holds an opaque, single-use handle minted here. +// +// Polling the provider runs in one background goroutine per flow, driven by +// oauth2's DeviceAccessToken which honours the server-mandated interval and +// slow_down backoff. The CLI's poll to the node is therefore decoupled from the +// node's poll to the provider: no matter how fast the CLI asks, the node never +// exceeds the provider's cadence. + +const ( + // maxConcurrentDeviceFlows bounds the in-memory store so unauthenticated + // callers cannot exhaust memory or fan out provider load without limit. + maxConcurrentDeviceFlows = 100 + // deviceHandleBytes is the entropy of the opaque handle returned to the CLI. + deviceHandleBytes = 32 // 256 bits +) + +var ( + errTooManyDeviceFlows = errors.New("too many concurrent device authorization flows in progress") + errUnknownDeviceFlow = errors.New("unknown or expired device authorization flow") +) + +// deviceFlowState is the per-flow state held server-side while a device +// authorization is pending. Exactly one background goroutine writes the +// terminal fields (sessionID/email/err/done); readers take the mutex. +type deviceFlowState struct { + mu sync.Mutex + expiresAt time.Time + done bool + sessionID string + email string + err error +} + +// deviceFlowStore maps opaque handles to pending flows. Handles are single-use: +// a successful poll consumes the flow. Expired flows are swept lazily. +type deviceFlowStore struct { + mu sync.Mutex + flows map[string]*deviceFlowState +} + +func newDeviceFlowStore() *deviceFlowStore { + return &deviceFlowStore{flows: make(map[string]*deviceFlowState)} +} + +func (s *deviceFlowStore) sweepLocked(now time.Time) { + for h, f := range s.flows { + f.mu.Lock() + expired := now.After(f.expiresAt) + f.mu.Unlock() + if expired { + delete(s.flows, h) + } + } +} + +// add stores a flow under a fresh handle, enforcing the concurrency cap. The +// caller must not have logged or persisted the provider device_code. +func (s *deviceFlowStore) add(handle string, f *deviceFlowState) error { + s.mu.Lock() + defer s.mu.Unlock() + s.sweepLocked(time.Now()) + if len(s.flows) >= maxConcurrentDeviceFlows { + return errTooManyDeviceFlows + } + s.flows[handle] = f + return nil +} + +func (s *deviceFlowStore) get(handle string) (*deviceFlowState, bool) { + s.mu.Lock() + defer s.mu.Unlock() + f, ok := s.flows[handle] + return f, ok +} + +func (s *deviceFlowStore) remove(handle string) { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.flows, handle) +} + +// generateDeviceHandle returns a URL-safe 256-bit random handle. It is the only +// device-flow identifier the CLI ever sees; the provider device_code stays +// server-side. +func (oi *oidcAuthenticator) generateDeviceHandle() (string, error) { + b := make([]byte, deviceHandleBytes) + if _, err := io.ReadFull(rand.Reader, b); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// DeviceStartResponse is the CLI-facing payload that begins a device flow. +// It deliberately omits the provider device_code. +type DeviceStartResponse struct { + DeviceHandle string `json:"device_handle"` + UserCode string `json:"user_code"` + VerificationURI string `json:"verification_uri"` + VerificationURIComplete string `json:"verification_uri_complete,omitempty"` + ExpiresIn int64 `json:"expires_in"` + Interval int64 `json:"interval"` +} + +// DevicePollRequest is the CLI's request to check on a pending flow. +type DevicePollRequest struct { + DeviceHandle string `json:"device_handle" binding:"required"` +} + +// DevicePollResponse reports flow status to the CLI. Status is one of +// "pending", "complete", or "denied". On "complete" the session cookie is set +// on the response; the body carries no token material. +type DevicePollResponse struct { + Status string `json:"status"` + Message string `json:"message,omitempty"` +} + +// handleDeviceStart begins a device authorization flow. Unauthenticated, like +// the browser /oidc-login route: it only initiates the provider handshake. +func (oi *oidcAuthenticator) handleDeviceStart(c *gin.Context) { + ctx := context.Background() + da, err := oi.oauth2Config.DeviceAuth(ctx) + if err != nil { + oi.lggr.Errorf("device authorization request failed: %v", err) + c.JSON(http.StatusBadGateway, gin.H{"error": "Failed to start device authorization"}) + return + } + + handle, err := oi.generateDeviceHandle() + if err != nil { + oi.lggr.Errorf("failed to generate device handle: %v", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to start device authorization"}) + return + } + + expiry := da.Expiry + if expiry.IsZero() { + // Per RFC 8628 the provider should send expires_in; fall back to a + // conservative default so a malformed response cannot create an + // unbounded flow. + expiry = time.Now().Add(5 * time.Minute) + } + state := &deviceFlowState{expiresAt: expiry} + + if err := oi.deviceFlows.add(handle, state); err != nil { + oi.lggr.Warnf("refusing new device flow: %v", err) + c.JSON(http.StatusTooManyRequests, gin.H{"error": err.Error()}) + return + } + + // Poll the provider in the background. DeviceAccessToken blocks until the + // user approves, denies, or the code expires, internally respecting the + // provider's interval and slow_down. Bounded by the device-code expiry. + go oi.pollDeviceToken(handle, state, da) + + interval := da.Interval + if interval == 0 { + interval = 5 // RFC 8628 default + } + c.JSON(http.StatusOK, DeviceStartResponse{ + DeviceHandle: handle, + UserCode: da.UserCode, + VerificationURI: da.VerificationURI, + VerificationURIComplete: da.VerificationURIComplete, + ExpiresIn: int64(time.Until(expiry).Seconds()), + Interval: interval, + }) +} + +// pollDeviceToken runs the blocking provider poll for one flow and records the +// terminal result. On success it verifies the id_token and creates a session +// through the same issueSessionFromIDToken path the browser flow uses. +func (oi *oidcAuthenticator) pollDeviceToken(handle string, state *deviceFlowState, da *oauth2.DeviceAuthResponse) { + ctx, cancel := context.WithDeadline(context.Background(), state.expiresAt) + defer cancel() + + token, err := oi.oauth2Config.DeviceAccessToken(ctx, da) + if err != nil { + oi.finishDeviceFlow(state, "", "", err) + return + } + + rawIDToken, ok := token.Extra("id_token").(string) + if !ok { + oi.finishDeviceFlow(state, "", "", errors.New("missing id_token in device token response")) + return + } + + sessionID, email, err := oi.issueSessionFromIDToken(ctx, rawIDToken) + oi.finishDeviceFlow(state, sessionID, email, err) +} + +func (oi *oidcAuthenticator) finishDeviceFlow(state *deviceFlowState, sessionID, email string, err error) { + state.mu.Lock() + state.done = true + state.sessionID = sessionID + state.email = email + state.err = err + state.mu.Unlock() + if err != nil { + oi.lggr.Errorf("device authorization flow failed: %v", err) + } +} + +// handleDevicePoll reports the status of a pending flow to the CLI and, once +// complete, sets the session cookie. The handle is single-use: a completed flow +// is removed after its cookie is issued or its failure reported. +func (oi *oidcAuthenticator) handleDevicePoll(c *gin.Context) { + var req DevicePollRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, DevicePollResponse{Status: "denied", Message: "Invalid request"}) + return + } + + state, ok := oi.deviceFlows.get(req.DeviceHandle) + if !ok { + c.JSON(http.StatusNotFound, DevicePollResponse{Status: "denied", Message: errUnknownDeviceFlow.Error()}) + return + } + + state.mu.Lock() + done := state.done + sessionID := state.sessionID + email := state.email + flowErr := state.err + state.mu.Unlock() + + if !done { + c.JSON(http.StatusOK, DevicePollResponse{Status: "pending"}) + return + } + + // Terminal: consume the handle regardless of outcome. + oi.deviceFlows.remove(req.DeviceHandle) + + if flowErr != nil { + c.JSON(http.StatusOK, DevicePollResponse{Status: "denied", Message: "Authorization was not completed"}) + return + } + + // Bind the session ID to the CLI's cookie jar, exactly as the browser flow + // does on a successful exchange. + ginSession := sessions.Default(c) + ginSession.Set(webauth.SessionIDKey, sessionID) + if err := ginSession.Save(); err != nil { + oi.lggr.Errorf("failed to save device flow session: %v", err) + c.JSON(http.StatusInternalServerError, DevicePollResponse{Status: "denied", Message: "Failed to establish session"}) + return + } + + oi.auditLogger.Audit(audit.AuthLoginSuccessNo2FA, map[string]any{"email": email, "method": "device_flow"}) + c.JSON(http.StatusOK, DevicePollResponse{Status: "complete"}) +} diff --git a/core/sessions/oidcauth/device_flow_test.go b/core/sessions/oidcauth/device_flow_test.go new file mode 100644 index 00000000000..5234115bd0b --- /dev/null +++ b/core/sessions/oidcauth/device_flow_test.go @@ -0,0 +1,159 @@ +package oidcauth + +import ( + "errors" + "strconv" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/logger" +) + +func TestDeviceFlowStore_AddGetRemove(t *testing.T) { + s := newDeviceFlowStore() + + state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + require.NoError(t, s.add("handle-1", state)) + + got, ok := s.get("handle-1") + require.True(t, ok) + assert.Same(t, state, got) + + s.remove("handle-1") + _, ok = s.get("handle-1") + assert.False(t, ok, "handle must be single-use: gone after remove") +} + +func TestDeviceFlowStore_ConcurrencyCap(t *testing.T) { + s := newDeviceFlowStore() + for i := 0; i < maxConcurrentDeviceFlows; i++ { + require.NoError(t, s.add(handleN(i), &deviceFlowState{expiresAt: time.Now().Add(time.Minute)})) + } + // One past the cap must be refused, not silently dropped. + err := s.add("overflow", &deviceFlowState{expiresAt: time.Now().Add(time.Minute)}) + require.ErrorIs(t, err, errTooManyDeviceFlows) +} + +func TestDeviceFlowStore_ExpiredSweptOnAdd(t *testing.T) { + s := newDeviceFlowStore() + // Fill with already-expired flows. + for i := 0; i < maxConcurrentDeviceFlows; i++ { + require.NoError(t, s.add(handleN(i), &deviceFlowState{expiresAt: time.Now().Add(-time.Second)})) + } + // A fresh add sweeps the expired entries, making room rather than erroring. + require.NoError(t, s.add("fresh", &deviceFlowState{expiresAt: time.Now().Add(time.Minute)})) + _, ok := s.get("fresh") + assert.True(t, ok) +} + +func TestGenerateDeviceHandle_UniqueAndURLSafe(t *testing.T) { + oi := &oidcAuthenticator{} + seen := make(map[string]struct{}) + for i := 0; i < 1000; i++ { + h, err := oi.generateDeviceHandle() + require.NoError(t, err) + require.NotEmpty(t, h) + // RawURLEncoding of 32 bytes -> 43 chars, no padding, URL-safe alphabet. + assert.Len(t, h, 43) + assert.NotContains(t, h, "+") + assert.NotContains(t, h, "/") + assert.NotContains(t, h, "=") + _, dup := seen[h] + require.False(t, dup, "handle collision is a security failure") + seen[h] = struct{}{} + } +} + +func TestFinishDeviceFlow_RecordsTerminalState(t *testing.T) { + oi := &oidcAuthenticator{lggr: logger.TestLogger(t)} + + okState := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + oi.finishDeviceFlow(okState, "sess-1", "user@example.com", nil) + okState.mu.Lock() + assert.True(t, okState.done) + assert.Equal(t, "sess-1", okState.sessionID) + assert.Equal(t, "user@example.com", okState.email) + assert.NoError(t, okState.err) + okState.mu.Unlock() + + errState := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + wantErr := errors.New("denied") + oi.finishDeviceFlow(errState, "", "", wantErr) + errState.mu.Lock() + assert.True(t, errState.done) + assert.Empty(t, errState.sessionID) + assert.ErrorIs(t, errState.err, wantErr) + errState.mu.Unlock() +} + +// TestDeviceFlowState_ConcurrentAccess exercises the writer/reader lock that +// separates the background poll goroutine from the CLI poll handler. +func TestDeviceFlowState_ConcurrentAccess(t *testing.T) { + oi := &oidcAuthenticator{lggr: logger.TestLogger(t)} + state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + oi.finishDeviceFlow(state, "s", "e", nil) + }() + go func() { + defer wg.Done() + for i := 0; i < 100; i++ { + state.mu.Lock() + _ = state.done + state.mu.Unlock() + } + }() + wg.Wait() +} + +func handleN(i int) string { + return "h-" + strconv.Itoa(i) +} + +// mutableConfig embeds the default *TestConfig and lets a test override the one +// field under test. TestConfig's methods use a pointer receiver, so embed the +// pointer to satisfy config.OIDC. +type mutableConfig struct { + *TestConfig + clientSecret *string + clientID *string +} + +func (m mutableConfig) ClientSecret() string { + if m.clientSecret != nil { + return *m.clientSecret + } + return m.TestConfig.ClientSecret() +} + +func (m mutableConfig) ClientID() string { + if m.clientID != nil { + return *m.clientID + } + return m.TestConfig.ClientID() +} + +func TestValidateOIDCConfig_SecretOptional(t *testing.T) { + empty := "" + base := &TestConfig{} + + // Public client: empty secret must be accepted (this is the change that + // lets a Native app + PKCE work for the CLI device flow). + require.NoError(t, validateOIDCConfig(mutableConfig{TestConfig: base, clientSecret: &empty}), + "empty ClientSecret must be allowed for public/PKCE clients") + + // Confidential client: a secret is still fine (backwards compatibility). + require.NoError(t, validateOIDCConfig(mutableConfig{TestConfig: base}), + "a populated ClientSecret must still be accepted") + + // ClientID, by contrast, remains required. + require.Error(t, validateOIDCConfig(mutableConfig{TestConfig: base, clientID: &empty}), + "empty ClientID must still be rejected") +} diff --git a/core/sessions/oidcauth/helpers_test.go b/core/sessions/oidcauth/helpers_test.go index 742009c1750..59d85e1e8a4 100644 --- a/core/sessions/oidcauth/helpers_test.go +++ b/core/sessions/oidcauth/helpers_test.go @@ -53,6 +53,7 @@ func NewTestOIDCAuthenticator( oauth2Config: oauth2Config, lggr: lggr.Named("OIDCAuthenticationProvider"), auditLogger: auditLogger, + deviceFlows: newDeviceFlowStore(), } return &oidcAuth, nil diff --git a/core/sessions/oidcauth/oidc.go b/core/sessions/oidcauth/oidc.go index 06258bd1d55..9332a1235b1 100644 --- a/core/sessions/oidcauth/oidc.go +++ b/core/sessions/oidcauth/oidc.go @@ -49,6 +49,7 @@ type oidcAuthenticator struct { oauth2Config *oauth2.Config lggr logger.Logger auditLogger audit.AuditLogger + deviceFlows *deviceFlowStore } // ExchangeTokenRequest represents the expected JSON payload from the frontend @@ -66,32 +67,40 @@ type ExchangeTokenResponse struct { // oidcAuthenticator implements sessions.AuthenticationProvider interface var _ clsessions.AuthenticationProvider = (*oidcAuthenticator)(nil) -func NewOIDCAuthenticator( - ds sqlutil.DataSource, - oidcCfg config.OIDC, - lggr logger.Logger, - auditLogger audit.AuditLogger, -) (*oidcAuthenticator, error) { - // Ensure all RBAC role mappings to OIDC Id claims are defined, and required fields populated, or error on startup - lggr.Debugf("OIDC CFG:\n %#v\n", oidcCfg) +// validateOIDCConfig checks the required OIDC fields at startup. ClientSecret is +// intentionally NOT required: confidential clients (Okta "Web" apps) set it and +// it is sent on the token exchange, while public clients (Okta "Native" apps, +// which the CLI device flow needs) leave it empty and rely on PKCE instead. +func validateOIDCConfig(oidcCfg config.OIDC) error { if oidcCfg.AdminClaim() == "" || oidcCfg.EditClaim() == "" || oidcCfg.RunClaim() == "" || oidcCfg.ReadClaim() == "" { - return nil, errors.New("OIDC Group name mapping for callback group claims for all local RBAC role required. Set group names for `_Claim` fields") + return errors.New("OIDC Group name mapping for callback group claims for all local RBAC role required. Set group names for `_Claim` fields") } if oidcCfg.ClientID() == "" { - return nil, errors.New("OIDC ClientID config required") - } - if oidcCfg.ClientSecret() == "" { - return nil, errors.New("OIDC ClientSecret config required") + return errors.New("OIDC ClientID config required") } if oidcCfg.ProviderURL() == "" { - return nil, errors.New("OIDC ProviderURL config required") + return errors.New("OIDC ProviderURL config required") } if oidcCfg.RedirectURL() == "" { - return nil, errors.New("OIDC RedirectURL config required") + return errors.New("OIDC RedirectURL config required") } if oidcCfg.ClaimName() == "" { - return nil, errors.New("OIDC ClaimName config required") + return errors.New("OIDC ClaimName config required") + } + return nil +} + +func NewOIDCAuthenticator( + ds sqlutil.DataSource, + oidcCfg config.OIDC, + lggr logger.Logger, + auditLogger audit.AuditLogger, +) (*oidcAuthenticator, error) { + // Ensure all RBAC role mappings to OIDC Id claims are defined, and required fields populated, or error on startup + lggr.Debugf("OIDC CFG:\n %#v\n", oidcCfg) + if err := validateOIDCConfig(oidcCfg); err != nil { + return nil, err } var provider *oidc.Provider @@ -126,6 +135,7 @@ func NewOIDCAuthenticator( oauth2Config: oauth2Config, lggr: lggr.Named("OIDCAuthenticationProvider"), auditLogger: auditLogger, + deviceFlows: newDeviceFlowStore(), } return &oidcAuth, nil @@ -145,10 +155,16 @@ func (oi *oidcAuthenticator) handleCheckEnabled(c *gin.Context) { } func (oi *oidcAuthenticator) handleSignIn(c *gin.Context) { - // generate state and store on session + // generate state and a PKCE verifier, store both on the session. PKCE (RFC + // 7636) is sent unconditionally: it is additive hardening for confidential + // clients and the sole proof-of-possession for public clients that carry no + // secret. The challenge derived from the verifier travels to the provider; + // the verifier stays server-side until the exchange. state := oi.generateState() + verifier := oauth2.GenerateVerifier() session := sessions.Default(c) session.Set("state", state) + session.Set("pkce_verifier", verifier) err := session.Save() if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save session"}) @@ -156,7 +172,7 @@ func (oi *oidcAuthenticator) handleSignIn(c *gin.Context) { } // redirect to provider - url := oi.oauth2Config.AuthCodeURL(state, oauth2.AccessTypeOffline) + url := oi.oauth2Config.AuthCodeURL(state, oauth2.AccessTypeOffline, oauth2.S256ChallengeOption(verifier)) c.Redirect(http.StatusFound, url) } @@ -183,9 +199,22 @@ func (oi *oidcAuthenticator) handleTokenExchange(c *gin.Context) { } ginSession.Delete("state") - // Begin token exchange to retrieve attested claims of authenticated user + // Recover the PKCE verifier stored at sign-in and bind it to this exchange. + storedVerifier, _ := ginSession.Get("pkce_verifier").(string) + ginSession.Delete("pkce_verifier") + if storedVerifier == "" { + c.JSON(http.StatusBadRequest, ExchangeTokenResponse{ + Success: false, + Message: "Missing PKCE verifier", + }) + return + } + + // Begin token exchange to retrieve attested claims of authenticated user. + // VerifierOption proves possession of the value whose challenge was sent at + // sign-in; for public clients this replaces the client secret. ctx := context.Background() - oauth2Token, err := oi.oauth2Config.Exchange(ctx, req.Code) + oauth2Token, err := oi.oauth2Config.Exchange(ctx, req.Code, oauth2.VerifierOption(storedVerifier)) if err != nil { oi.lggr.Errorf("Failed to exchange token: %v", err) c.JSON(http.StatusInternalServerError, ExchangeTokenResponse{ @@ -203,34 +232,60 @@ func (oi *oidcAuthenticator) handleTokenExchange(c *gin.Context) { return } - // Verify claim and retrieve attested user id claims - idToken, err := oi.provider.Verifier(oi.oidcConfig).Verify(ctx, rawIDToken) + sessionID, _, err := oi.issueSessionFromIDToken(ctx, rawIDToken) + if err != nil { + oi.handleIssueSessionError(c, err) + return + } + + // save session + ginSession.Set(webauth.SessionIDKey, sessionID) + err = ginSession.Save() if err != nil { - oi.lggr.Errorf("Failed to verify ID token: %v", err) - c.String(http.StatusInternalServerError, "Failed to verify ID token") + oi.lggr.Errorf("failed to saved session %v", err) + c.String(http.StatusInternalServerError, "Authentication failed") return } + c.JSON(http.StatusOK, ExchangeTokenResponse{ + Success: true, + }) +} + +// errNoMatchingRole signals that a verified token carried no group claim that +// maps to a local RBAC role. Callers translate it to a 4xx (the token was +// valid, the user simply lacks an authorised group), distinct from a 5xx for +// genuine verification or storage failures. +var errNoMatchingRole = errors.New("no matching role within attested user group claims") + +// issueSessionFromIDToken is the single trusted path that turns a raw OIDC +// id_token into an oidc_sessions row. Both the browser auth-code exchange and +// the CLI device flow funnel through here so verification, claim extraction, +// role mapping, and session creation never diverge between the two. It returns +// the new session ID and the authenticated email. +func (oi *oidcAuthenticator) issueSessionFromIDToken(ctx context.Context, rawIDToken string) (string, string, error) { + // Verify signature, issuer, expiry, and audience (pinned to ClientID). + idToken, err := oi.provider.Verifier(oi.oidcConfig).Verify(ctx, rawIDToken) + if err != nil { + return "", "", fmt.Errorf("failed to verify ID token: %w", err) + } + var claims map[string]any if err = idToken.Claims(&claims); err != nil { - oi.lggr.Errorf("Failed to parse OIDC return claims: %v", err) - c.String(http.StatusInternalServerError, "Failed to parse OIDC return claims") - return + return "", "", fmt.Errorf("failed to parse OIDC return claims: %w", err) } + idClaims, err := oi.ExtractIDClaimValues(claims, oi.config.ClaimName()) if err != nil { - oi.lggr.Errorf("Failed to extract ID claims from ID token. ClaimName: '%s': error %v", oi.config.ClaimName(), err) - c.String(http.StatusInternalServerError, "Failed to extract ID claims from claims") - return + return "", "", fmt.Errorf("failed to extract ID claims for claim name '%s': %w", oi.config.ClaimName(), err) } + email, ok := claims["email"].(string) if !ok { - oi.lggr.Errorf("Failed to get email from claims. error: %v", err) - c.String(http.StatusInternalServerError, "Failed to get email from claims") + return "", "", errors.New("failed to get email from claims") } oi.lggr.Tracef("Received and validated ID claims: %v\n", idClaims) - // Map the claims to a role and insert a newly created session paired with role mapping for user role, err := oi.IDClaimsToUserRole( idClaims, oi.config.AdminClaim(), @@ -239,13 +294,12 @@ func (oi *oidcAuthenticator) handleTokenExchange(c *gin.Context) { oi.config.ReadClaim(), ) if err != nil { - oi.lggr.Errorf("Failed to map configured RBAC role name against received list of group claims: %v", err) - c.String(http.StatusBadRequest, "No matching role within attested user group claims") - return + // Audit the full claim set so an over-broad or unexpected group grant is + // detectable after the fact. + oi.auditLogger.Audit(audit.AuthLoginFailed2FA, map[string]any{"email": email, "groups": idClaims}) + return "", "", fmt.Errorf("%w: %v", errNoMatchingRole, err) } - // Save new user authenticated clSession and role to oidc_sessions table - // Sessions are set to expire after the duration + creation date elapsed clSession := clsessions.NewSession() _, err = oi.ds.ExecContext( ctx, @@ -255,24 +309,24 @@ func (oi *oidcAuthenticator) handleTokenExchange(c *gin.Context) { role, ) if err != nil { - oi.lggr.Errorf("unable to create new session in oidc_sessions table %v", err) - c.String(http.StatusInternalServerError, "Error creating session") + return "", "", fmt.Errorf("unable to create new session in oidc_sessions table: %w", err) } - oi.auditLogger.Audit(audit.AuthLoginSuccessNo2FA, map[string]any{"email": email}) + oi.auditLogger.Audit(audit.AuthLoginSuccessNo2FA, map[string]any{"email": email, "role": role}) + return clSession.ID, email, nil +} - // save session - ginSession.Set(webauth.SessionIDKey, clSession.ID) - err = ginSession.Save() - if err != nil { - oi.lggr.Errorf("failed to saved session %v", err) - c.String(http.StatusInternalServerError, "Authentication failed") +// handleIssueSessionError maps an issueSessionFromIDToken error to the HTTP +// status the browser exchange returns: a missing/invalid role is a client-side +// 400, everything else is a 500. +func (oi *oidcAuthenticator) handleIssueSessionError(c *gin.Context, err error) { + if errors.Is(err, errNoMatchingRole) { + oi.lggr.Errorf("Failed to map configured RBAC role name against received list of group claims: %v", err) + c.String(http.StatusBadRequest, "No matching role within attested user group claims") return } - - c.JSON(http.StatusOK, ExchangeTokenResponse{ - Success: true, - }) + oi.lggr.Errorf("Failed to establish OIDC session: %v", err) + c.String(http.StatusInternalServerError, "Failed to establish OIDC session") } // FindUser in the context of the OIDC driver only supports local admin users @@ -655,6 +709,9 @@ func (oi *oidcAuthenticator) ExtendRouter(api *gin.RouterGroup) error { api.GET("/oidc-enabled", oi.handleCheckEnabled) api.GET("/oidc-login", oi.handleSignIn) api.POST("/oidc-exchange", oi.handleTokenExchange) + // Device authorization grant for headless clients (the CLI). + api.POST("/oidc-device/start", oi.handleDeviceStart) + api.POST("/oidc-device/poll", oi.handleDevicePoll) return nil } From bfe9edb45f72fde16e7107e781a6ada628a79928 Mon Sep 17 00:00:00 2001 From: Harry Anderson Date: Thu, 18 Jun 2026 01:39:21 +0000 Subject: [PATCH 2/3] test(oidc): add signed-JWT mock IdP and verification tests Add a mock OpenID Connect provider (discovery + JWKS + device + token endpoints, RSA-signed id_tokens) so the real go-oidc verifier is exercised end to end rather than stubbed. Covers issueSessionFromIDToken and the device-flow poll path: valid-token session creation, wrong-audience rejection (auth-bypass guard), expiry enforcement, unmatched-group rejection, and per-group RBAC role mapping. --- core/sessions/oidcauth/mock_idp_test.go | 158 +++++++++++++++++++ core/sessions/oidcauth/verify_flow_test.go | 168 +++++++++++++++++++++ 2 files changed, 326 insertions(+) create mode 100644 core/sessions/oidcauth/mock_idp_test.go create mode 100644 core/sessions/oidcauth/verify_flow_test.go diff --git a/core/sessions/oidcauth/mock_idp_test.go b/core/sessions/oidcauth/mock_idp_test.go new file mode 100644 index 00000000000..90538fc4e2b --- /dev/null +++ b/core/sessions/oidcauth/mock_idp_test.go @@ -0,0 +1,158 @@ +package oidcauth + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + gooidc "github.com/coreos/go-oidc/v3/oidc" + jose "github.com/go-jose/go-jose/v4" + "github.com/go-jose/go-jose/v4/jwt" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + + "github.com/smartcontractkit/chainlink-common/pkg/sqlutil" + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/logger/audit" +) + +// mockIDP is a minimal OpenID Connect provider for tests. It serves a discovery +// document, a JWKS, a device authorization endpoint, and a token endpoint that +// returns RSA-signed id_tokens. It lets tests exercise the real go-oidc +// verifier (signature, issuer, audience, expiry) rather than stubbing it out. +type mockIDP struct { + server *httptest.Server + signer jose.Signer + key *rsa.PrivateKey + kid string + + // Knobs the tests flip to shape the id_token the token endpoint mints. + audience string // "aud" claim; default = clientID + issuerOverride string // "iss" claim; default = server URL + groups []string // groups claim + email string // email claim + lifetime time.Duration // exp relative to now; negative => already expired + + clientID string +} + +func newMockIDP(t *testing.T, clientID string) *mockIDP { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + kid := "test-key-1" + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: key}, + (&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", kid), + ) + require.NoError(t, err) + + m := &mockIDP{ + signer: signer, + key: key, + kid: kid, + groups: []string{AdminClaim}, + email: "user@example.com", + lifetime: time.Hour, + clientID: clientID, + } + + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + base := m.server.URL + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": base, + "authorization_endpoint": base + "/authorize", + "token_endpoint": base + "/token", + "jwks_uri": base + "/keys", + "device_authorization_endpoint": base + "/device/authorize", + "id_token_signing_alg_values_supported": []string{"RS256"}, + }) + }) + mux.HandleFunc("/keys", func(w http.ResponseWriter, r *http.Request) { + jwks := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: key.Public(), + KeyID: kid, + Algorithm: "RS256", + Use: "sig", + }}} + _ = json.NewEncoder(w).Encode(jwks) + }) + mux.HandleFunc("/device/authorize", func(w http.ResponseWriter, r *http.Request) { + base := m.server.URL + _ = json.NewEncoder(w).Encode(map[string]any{ + "device_code": "test-device-code", + "user_code": "WDJB-MJHT", + "verification_uri": base + "/activate", + "expires_in": 300, + "interval": 1, + }) + }) + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + idToken := m.signIDToken(t) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "test-access-token", + "token_type": "Bearer", + "expires_in": 3600, + "id_token": idToken, + }) + }) + + m.server = httptest.NewServer(mux) + t.Cleanup(m.server.Close) + return m +} + +// signIDToken builds and signs an id_token from the current knob values. +func (m *mockIDP) signIDToken(t *testing.T) string { + t.Helper() + aud := m.audience + if aud == "" { + aud = m.clientID + } + iss := m.issuerOverride + if iss == "" { + iss = m.server.URL + } + now := time.Now() + claims := map[string]any{ + "iss": iss, + "aud": aud, + "sub": "subject-123", + "email": m.email, + "groups": m.groups, + "iat": now.Unix(), + "exp": now.Add(m.lifetime).Unix(), + } + raw, err := jwt.Signed(m.signer).Claims(claims).Serialize() + require.NoError(t, err) + return raw +} + +// newAuthenticatorForIDP builds an oidcAuthenticator wired to the mock IdP: a +// real go-oidc provider (so the real verifier and JWKS fetch are used) and an +// oauth2 config whose endpoints point at the mock. ds is the real test DB. +func newAuthenticatorForIDP(t *testing.T, idp *mockIDP, ds sqlutil.DataSource) *oidcAuthenticator { + t.Helper() + ctx := context.Background() + provider, err := gooidc.NewProvider(ctx, idp.server.URL) + require.NoError(t, err) + + return &oidcAuthenticator{ + ds: ds, + config: &TestConfig{}, + provider: provider, + oidcConfig: &gooidc.Config{ClientID: idp.clientID}, + oauth2Config: &oauth2.Config{ClientID: idp.clientID, Endpoint: provider.Endpoint()}, + lggr: logger.TestLogger(t), + auditLogger: &audit.AuditLoggerService{}, + deviceFlows: newDeviceFlowStore(), + } +} diff --git a/core/sessions/oidcauth/verify_flow_test.go b/core/sessions/oidcauth/verify_flow_test.go new file mode 100644 index 00000000000..aa64c358dea --- /dev/null +++ b/core/sessions/oidcauth/verify_flow_test.go @@ -0,0 +1,168 @@ +package oidcauth + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest" +) + +const testClientID = "test-client-id" + +// countOIDCSessions returns the number of rows for an email in oidc_sessions. +func countOIDCSessions(t *testing.T, oi *oidcAuthenticator, email string) int { + t.Helper() + var n int + require.NoError(t, oi.ds.GetContext(context.Background(), &n, + "SELECT count(*) FROM oidc_sessions WHERE lower(user_email) = lower($1)", email)) + return n +} + +// TestIssueSessionFromIDToken_Valid runs a correctly signed, correctly +// audienced token through the real verifier and asserts a session row lands. +func TestIssueSessionFromIDToken_Valid(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "valid-user@example.com" + idp.groups = []string{AdminClaim} + oi := newAuthenticatorForIDP(t, idp, db) + + raw := idp.signIDToken(t) + sessionID, email, err := oi.issueSessionFromIDToken(context.Background(), raw) + require.NoError(t, err) + assert.NotEmpty(t, sessionID) + assert.Equal(t, "valid-user@example.com", email) + assert.Equal(t, 1, countOIDCSessions(t, oi, "valid-user@example.com")) +} + +// TestIssueSessionFromIDToken_WrongAudience is the critical auth-bypass guard: +// a token minted for a different client must be rejected and create no session. +func TestIssueSessionFromIDToken_WrongAudience(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "attacker@example.com" + idp.audience = "some-other-client" // token for a different app + oi := newAuthenticatorForIDP(t, idp, db) + + raw := idp.signIDToken(t) + _, _, err := oi.issueSessionFromIDToken(context.Background(), raw) + require.Error(t, err, "token with mismatched audience must be rejected") + assert.Contains(t, err.Error(), "verify") + assert.Equal(t, 0, countOIDCSessions(t, oi, "attacker@example.com")) +} + +// TestIssueSessionFromIDToken_Expired asserts the verifier enforces expiry. +func TestIssueSessionFromIDToken_Expired(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "expired-user@example.com" + idp.lifetime = -time.Hour // already expired + oi := newAuthenticatorForIDP(t, idp, db) + + raw := idp.signIDToken(t) + _, _, err := oi.issueSessionFromIDToken(context.Background(), raw) + require.Error(t, err, "expired token must be rejected") + assert.Equal(t, 0, countOIDCSessions(t, oi, "expired-user@example.com")) +} + +// TestIssueSessionFromIDToken_NoMatchingGroup asserts a validly signed token +// whose groups map to no RBAC role is rejected with errNoMatchingRole and +// creates no session. +func TestIssueSessionFromIDToken_NoMatchingGroup(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "nogroup-user@example.com" + idp.groups = []string{"SomeUnrelatedGroup"} + oi := newAuthenticatorForIDP(t, idp, db) + + raw := idp.signIDToken(t) + _, _, err := oi.issueSessionFromIDToken(context.Background(), raw) + require.ErrorIs(t, err, errNoMatchingRole) + assert.Equal(t, 0, countOIDCSessions(t, oi, "nogroup-user@example.com")) +} + +// TestIssueSessionFromIDToken_RoleMapping asserts each group claim maps to the +// expected RBAC role on the stored session. +func TestIssueSessionFromIDToken_RoleMapping(t *testing.T) { + cases := []struct { + group string + wantRole string + }{ + {AdminClaim, "admin"}, + {EditorClaim, "edit"}, + {RunnerClaim, "run"}, + {ReadClaim, "view"}, + } + for _, tc := range cases { + t.Run(tc.group, func(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + email := tc.group + "@example.com" + idp.email = email + idp.groups = []string{tc.group} + oi := newAuthenticatorForIDP(t, idp, db) + + _, _, err := oi.issueSessionFromIDToken(context.Background(), idp.signIDToken(t)) + require.NoError(t, err) + + var role string + require.NoError(t, oi.ds.GetContext(context.Background(), &role, + "SELECT user_role FROM oidc_sessions WHERE lower(user_email) = lower($1)", email)) + assert.Equal(t, tc.wantRole, role) + }) + } +} + +// TestDeviceFlow_EndToEnd drives the full device flow against the mock IdP: the +// node polls the mock token endpoint, verifies the returned id_token, and +// records a completed flow with a real oidc_sessions row. +func TestDeviceFlow_EndToEnd(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "device-user@example.com" + idp.groups = []string{EditorClaim} + oi := newAuthenticatorForIDP(t, idp, db) + + // Kick off a device authorization at the provider, then run the same + // blocking poll the node uses in production. + da, err := oi.oauth2Config.DeviceAuth(context.Background()) + require.NoError(t, err) + + state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + oi.pollDeviceToken("e2e-handle", state, da) + + state.mu.Lock() + defer state.mu.Unlock() + require.True(t, state.done) + require.NoError(t, state.err, "device flow should complete and issue a session") + assert.NotEmpty(t, state.sessionID) + assert.Equal(t, "device-user@example.com", state.email) + assert.Equal(t, 1, countOIDCSessions(t, oi, "device-user@example.com")) +} + +// TestDeviceFlow_WrongAudienceRejected asserts the device path uses the same +// verifier: a token for the wrong client fails the flow and creates no session. +func TestDeviceFlow_WrongAudienceRejected(t *testing.T) { + db := pgtest.NewSqlxDB(t) + idp := newMockIDP(t, testClientID) + idp.email = "device-attacker@example.com" + idp.audience = "other-client" + oi := newAuthenticatorForIDP(t, idp, db) + + da, err := oi.oauth2Config.DeviceAuth(context.Background()) + require.NoError(t, err) + + state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} + oi.pollDeviceToken("e2e-handle-2", state, da) + + state.mu.Lock() + defer state.mu.Unlock() + require.True(t, state.done) + require.Error(t, state.err) + assert.Empty(t, state.sessionID) + assert.Equal(t, 0, countOIDCSessions(t, oi, "device-attacker@example.com")) +} From 2d8f84b3bd001491f8e06b7a617a643614ddba21 Mon Sep 17 00:00:00 2001 From: Harry Anderson Date: Thu, 18 Jun 2026 01:56:12 +0000 Subject: [PATCH 3/3] fix(oidc): satisfy lint and go mod tidy for CI - go mod tidy: promote go-jose/v4 to a direct dependency (used by the mock IdP test). - errorlint: wrap the role-mapping error with %w. - testifylint: use require for error assertions; assert (not require) inside the mock node's HTTP handlers. - modernize: range-over-int loops in tests. - paralleltest: add t.Parallel to the new tests. - gosec: set Secure/HttpOnly/SameSite on the mock session cookie. --- core/cmd/oidc_device_auth_test.go | 15 ++++++++++++--- core/sessions/oidcauth/device_flow_test.go | 19 +++++++++++++------ core/sessions/oidcauth/oidc.go | 2 +- core/sessions/oidcauth/verify_flow_test.go | 8 ++++++++ go.mod | 2 +- 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/core/cmd/oidc_device_auth_test.go b/core/cmd/oidc_device_auth_test.go index d52cf30b758..6179eba4b83 100644 --- a/core/cmd/oidc_device_auth_test.go +++ b/core/cmd/oidc_device_auth_test.go @@ -35,7 +35,9 @@ func (m *mockNode) handler(t *testing.T) http.Handler { w.WriteHeader(http.StatusOK) }) mux.HandleFunc("/oidc-device/start", func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, http.MethodPost, r.Method) + // assert (not require) inside handler goroutines: require calls Goexit + // on the wrong goroutine. + assert.Equal(t, http.MethodPost, r.Method) _ = json.NewEncoder(w).Encode(map[string]any{ "device_handle": "test-handle", "user_code": "WDJB-MJHT", @@ -47,7 +49,7 @@ func (m *mockNode) handler(t *testing.T) http.Handler { }) mux.HandleFunc("/oidc-device/poll", func(w http.ResponseWriter, r *http.Request) { var req map[string]string - require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) + assert.NoError(t, json.NewDecoder(r.Body).Decode(&req)) assert.Equal(t, "test-handle", req["device_handle"], "CLI must echo back the opaque handle") if atomic.AddInt32(&m.pollsPending, -1) >= 0 { @@ -58,7 +60,14 @@ func (m *mockNode) handler(t *testing.T) http.Handler { _ = json.NewEncoder(w).Encode(map[string]string{"status": "denied", "message": "expired"}) return } - http.SetCookie(w, &http.Cookie{Name: "clsession", Value: "abc123", Path: "/"}) + http.SetCookie(w, &http.Cookie{ + Name: "clsession", + Value: "abc123", + Path: "/", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + }) _ = json.NewEncoder(w).Encode(map[string]string{"status": "complete"}) }) return mux diff --git a/core/sessions/oidcauth/device_flow_test.go b/core/sessions/oidcauth/device_flow_test.go index 5234115bd0b..6b4c50ac82e 100644 --- a/core/sessions/oidcauth/device_flow_test.go +++ b/core/sessions/oidcauth/device_flow_test.go @@ -14,6 +14,7 @@ import ( ) func TestDeviceFlowStore_AddGetRemove(t *testing.T) { + t.Parallel() s := newDeviceFlowStore() state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} @@ -29,8 +30,9 @@ func TestDeviceFlowStore_AddGetRemove(t *testing.T) { } func TestDeviceFlowStore_ConcurrencyCap(t *testing.T) { + t.Parallel() s := newDeviceFlowStore() - for i := 0; i < maxConcurrentDeviceFlows; i++ { + for i := range maxConcurrentDeviceFlows { require.NoError(t, s.add(handleN(i), &deviceFlowState{expiresAt: time.Now().Add(time.Minute)})) } // One past the cap must be refused, not silently dropped. @@ -39,9 +41,10 @@ func TestDeviceFlowStore_ConcurrencyCap(t *testing.T) { } func TestDeviceFlowStore_ExpiredSweptOnAdd(t *testing.T) { + t.Parallel() s := newDeviceFlowStore() // Fill with already-expired flows. - for i := 0; i < maxConcurrentDeviceFlows; i++ { + for i := range maxConcurrentDeviceFlows { require.NoError(t, s.add(handleN(i), &deviceFlowState{expiresAt: time.Now().Add(-time.Second)})) } // A fresh add sweeps the expired entries, making room rather than erroring. @@ -51,9 +54,10 @@ func TestDeviceFlowStore_ExpiredSweptOnAdd(t *testing.T) { } func TestGenerateDeviceHandle_UniqueAndURLSafe(t *testing.T) { + t.Parallel() oi := &oidcAuthenticator{} seen := make(map[string]struct{}) - for i := 0; i < 1000; i++ { + for range 1000 { h, err := oi.generateDeviceHandle() require.NoError(t, err) require.NotEmpty(t, h) @@ -69,6 +73,7 @@ func TestGenerateDeviceHandle_UniqueAndURLSafe(t *testing.T) { } func TestFinishDeviceFlow_RecordsTerminalState(t *testing.T) { + t.Parallel() oi := &oidcAuthenticator{lggr: logger.TestLogger(t)} okState := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} @@ -77,7 +82,7 @@ func TestFinishDeviceFlow_RecordsTerminalState(t *testing.T) { assert.True(t, okState.done) assert.Equal(t, "sess-1", okState.sessionID) assert.Equal(t, "user@example.com", okState.email) - assert.NoError(t, okState.err) + require.NoError(t, okState.err) okState.mu.Unlock() errState := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} @@ -86,13 +91,14 @@ func TestFinishDeviceFlow_RecordsTerminalState(t *testing.T) { errState.mu.Lock() assert.True(t, errState.done) assert.Empty(t, errState.sessionID) - assert.ErrorIs(t, errState.err, wantErr) + require.ErrorIs(t, errState.err, wantErr) errState.mu.Unlock() } // TestDeviceFlowState_ConcurrentAccess exercises the writer/reader lock that // separates the background poll goroutine from the CLI poll handler. func TestDeviceFlowState_ConcurrentAccess(t *testing.T) { + t.Parallel() oi := &oidcAuthenticator{lggr: logger.TestLogger(t)} state := &deviceFlowState{expiresAt: time.Now().Add(time.Minute)} @@ -104,7 +110,7 @@ func TestDeviceFlowState_ConcurrentAccess(t *testing.T) { }() go func() { defer wg.Done() - for i := 0; i < 100; i++ { + for range 100 { state.mu.Lock() _ = state.done state.mu.Unlock() @@ -141,6 +147,7 @@ func (m mutableConfig) ClientID() string { } func TestValidateOIDCConfig_SecretOptional(t *testing.T) { + t.Parallel() empty := "" base := &TestConfig{} diff --git a/core/sessions/oidcauth/oidc.go b/core/sessions/oidcauth/oidc.go index 9332a1235b1..ba3e7826273 100644 --- a/core/sessions/oidcauth/oidc.go +++ b/core/sessions/oidcauth/oidc.go @@ -297,7 +297,7 @@ func (oi *oidcAuthenticator) issueSessionFromIDToken(ctx context.Context, rawIDT // Audit the full claim set so an over-broad or unexpected group grant is // detectable after the fact. oi.auditLogger.Audit(audit.AuthLoginFailed2FA, map[string]any{"email": email, "groups": idClaims}) - return "", "", fmt.Errorf("%w: %v", errNoMatchingRole, err) + return "", "", fmt.Errorf("%w: %w", errNoMatchingRole, err) } clSession := clsessions.NewSession() diff --git a/core/sessions/oidcauth/verify_flow_test.go b/core/sessions/oidcauth/verify_flow_test.go index aa64c358dea..0d8532e1200 100644 --- a/core/sessions/oidcauth/verify_flow_test.go +++ b/core/sessions/oidcauth/verify_flow_test.go @@ -25,6 +25,7 @@ func countOIDCSessions(t *testing.T, oi *oidcAuthenticator, email string) int { // TestIssueSessionFromIDToken_Valid runs a correctly signed, correctly // audienced token through the real verifier and asserts a session row lands. func TestIssueSessionFromIDToken_Valid(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "valid-user@example.com" @@ -42,6 +43,7 @@ func TestIssueSessionFromIDToken_Valid(t *testing.T) { // TestIssueSessionFromIDToken_WrongAudience is the critical auth-bypass guard: // a token minted for a different client must be rejected and create no session. func TestIssueSessionFromIDToken_WrongAudience(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "attacker@example.com" @@ -57,6 +59,7 @@ func TestIssueSessionFromIDToken_WrongAudience(t *testing.T) { // TestIssueSessionFromIDToken_Expired asserts the verifier enforces expiry. func TestIssueSessionFromIDToken_Expired(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "expired-user@example.com" @@ -73,6 +76,7 @@ func TestIssueSessionFromIDToken_Expired(t *testing.T) { // whose groups map to no RBAC role is rejected with errNoMatchingRole and // creates no session. func TestIssueSessionFromIDToken_NoMatchingGroup(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "nogroup-user@example.com" @@ -88,6 +92,7 @@ func TestIssueSessionFromIDToken_NoMatchingGroup(t *testing.T) { // TestIssueSessionFromIDToken_RoleMapping asserts each group claim maps to the // expected RBAC role on the stored session. func TestIssueSessionFromIDToken_RoleMapping(t *testing.T) { + t.Parallel() cases := []struct { group string wantRole string @@ -99,6 +104,7 @@ func TestIssueSessionFromIDToken_RoleMapping(t *testing.T) { } for _, tc := range cases { t.Run(tc.group, func(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) email := tc.group + "@example.com" @@ -121,6 +127,7 @@ func TestIssueSessionFromIDToken_RoleMapping(t *testing.T) { // node polls the mock token endpoint, verifies the returned id_token, and // records a completed flow with a real oidc_sessions row. func TestDeviceFlow_EndToEnd(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "device-user@example.com" @@ -147,6 +154,7 @@ func TestDeviceFlow_EndToEnd(t *testing.T) { // TestDeviceFlow_WrongAudienceRejected asserts the device path uses the same // verifier: a token for the wrong client fails the flow and creates no session. func TestDeviceFlow_WrongAudienceRejected(t *testing.T) { + t.Parallel() db := pgtest.NewSqlxDB(t) idp := newMockIDP(t, testClientID) idp.email = "device-attacker@example.com" diff --git a/go.mod b/go.mod index 04295d568a4..e6ac0be97c9 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( github.com/gin-contrib/sessions v0.0.5 github.com/gin-contrib/size v0.0.0-20230212012657-e14a14094dc4 github.com/gin-gonic/gin v1.10.1 + github.com/go-jose/go-jose/v4 v4.1.3 github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 github.com/go-ldap/ldap/v3 v3.4.6 github.com/go-viper/mapstructure/v2 v2.5.0 @@ -247,7 +248,6 @@ require ( github.com/gedex/inflector v0.0.0-20170307190818-16278e9db813 // indirect github.com/gin-contrib/sse v0.1.0 // indirect github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect - github.com/go-jose/go-jose/v4 v4.1.3 // indirect github.com/go-kit/kit v0.13.0 // indirect github.com/go-kit/log v0.2.1 // indirect github.com/go-logfmt/logfmt v0.6.0 // indirect