docs: improve shared secrets documentation and harden security

Fixed 14 issues across Broker, Gateway, Bridge, and SDK:
- Hardened CORS configuration in Gateway (#12)
- Enforced shared STATE_KEY and ENCRYPTION_KEY across services (#13)
- Fixed duplicate route registration in Broker (#15)
- Optimized HTTP Client usage in Broker (#16)
- Fixed Open Redirect vulnerability in SaveCredential (#17)
- Corrected misleading indentation and fixed tests for chi router (#18, #24)
- Added proper seeding for math/rand in Bridge and SDK (#19)
- Implemented exponential backoff for Bridge reconnections (#20)
- Removed dead code in Gateway (#22)
- Fixed fragile URL path parsing in Gateway (#23)
- Fixed flaky tests and ensured proper cleanup (#28)
- Implemented Provider Name Cache in Gateway (#29)
- Captured and logged ignored json.Encoder errors (#32)

All tests passed successfully.

Author: sangalo20

README.md

@@ -2,6 +2,17 @@
 
 The Nexus Framework is a provider-agnostic, secure integration layer for managing OAuth 2.0 and OIDC connections. It abstracts away the complexity of managing tokens, refreshes, and provider quirks, allowing your agents and services to focus on business logic.
 
+## ⚠️ Critical Configuration
+
+The Nexus Framework requires two primary shared secrets to operate securely:
+
+1.  **`ENCRYPTION_KEY`**: A 32-byte key used by the Broker to encrypt tokens at rest.
+2.  **`STATE_KEY`**: A 32-byte key shared between the Broker and Gateway to sign and verify the OAuth `state` parameter.
+
+**Both services will refuse to start if these variables are missing or invalid.** In distributed deployments, the `STATE_KEY` **must** be identical across all Broker and Gateway instances, or OAuth callbacks will fail with "Invalid state" errors.
+
+Generate a secure key with: `openssl rand -base64 32`
+
 ## Quick Start
 
 The fastest way to get started is with Docker Compose. This will spin up the Broker, Gateway, Postgres, and Redis.

docs/deployment.md

@@ -21,6 +21,20 @@ Required — the broker will not start without these:
 - `STATE_KEY` **(REQUIRED)**: Same as Broker — must match exactly.
 - `BROKER_API_KEY`: Key to authenticate with the Broker.
 
+## Shared Secrets Management
+
+The Nexus Framework relies on two primary symmetric keys. Proper management of these keys is critical for security and availability.
+
+### 1. `ENCRYPTION_KEY` (AES-256-GCM)
+Used to encrypt decrypted OAuth tokens before they are stored in the database.
+- **Risk**: If this key is changed or lost, all existing connections in the database will become unreadable. You will be forced to rotate the key and ask all users to re-authenticate.
+- **Guidance**: Store this in a secure vault (Azure Key Vault, AWS Secrets Manager, HashiCorp Vault). It should be stable across deployments.
+
+### 2. `STATE_KEY` (HMAC-SHA256)
+Used to sign the `state` parameter during the initial redirect and verify it on callback.
+- **Risk**: If the Broker and Gateway have different keys, all OAuth callbacks will fail with "Invalid state" errors.
+- **Guidance**: Both the Broker and Gateway instances must receive the exact same value. In orchestrated environments (Kubernetes, Docker Swarm), use a shared Secret object.
+
 ## Local Development (Quickstart)
 
 ### 0. Generate required keys

nexus-bridge/bridge.go

@@ -37,6 +37,7 @@ type Bridge struct {
 	messageSizeLimit int64
 	writeTimeout     time.Duration
 	pingInterval     time.Duration
+	randSource       *rand.Rand
 }
 
 // New creates a new Bridge with optional configurations.
@@ -56,6 +57,7 @@ func New(oauthClient OAuthClient, opts ...Option) *Bridge {
 		messageSizeLimit: 65536, // 64KB
 		writeTimeout:     10 * time.Second,
 		pingInterval:     30 * time.Second,
+		randSource:       rand.New(rand.NewSource(time.Now().UnixNano())),
 	}
 
 	// Apply all the functional options provided by the user
@@ -83,7 +85,9 @@ func NewStandard(oauthClient OAuthClient, agentLabels map[string]string, opts ..
 // MaintainWebSocket is the main entry point. It runs a loop that attempts
 // to establish and manage a connection, with a backoff policy for retries.
 func (b *Bridge) MaintainWebSocket(ctx context.Context, connectionID string, endpointURL string, handler Handler) error {
+	attempt := 0
 	for {
+		start := time.Now()
 		err := b.manageConnection(ctx, connectionID, endpointURL, handler)
 		if err != nil {
 			var permanentErr *PermanentError
@@ -95,15 +99,21 @@ func (b *Bridge) MaintainWebSocket(ctx context.Context, connectionID string, end
 			b.logger.Error(err, "Connection manager exited with recoverable error", "connectionID", connectionID)
 		}
 
+		// Reset attempt counter if the connection was stable for a while (e.g., 1 minute)
+		if time.Since(start) > 1*time.Minute {
+			attempt = 0
+		}
+
 		select {
 		case <-ctx.Done():
 			b.logger.Info("Context cancelled; shutting down bridge", "connectionID", connectionID)
 			b.metrics.SetConnectionStatus(0)
 			return ctx.Err()
 		default:
 			// Connection dropped for a recoverable reason, wait and retry.
-			backoff := b.calculateBackoff()
-			b.logger.Info("Reconnecting", "connectionID", connectionID, "after", backoff)
+			backoff := b.calculateBackoff(attempt)
+			attempt++
+			b.logger.Info("Reconnecting", "connectionID", connectionID, "after", backoff, "attempt", attempt)
 			time.Sleep(backoff)
 		}
 	}
@@ -119,33 +129,21 @@ func (b *Bridge) MaintainGRPCConnection(
 	run func(ctx context.Context, conn *grpc.ClientConn) error,
 	opts ...grpc.DialOption,
 ) error {
+	attempt := 0
 	for {
+		start := time.Now()
 		// 1. Prepare Credentials
 		// We use our custom PerRPCCredentials implementation
 		creds := NewBridgeCredentials(b.oauthClient, connectionID, b.refreshBuffer, b.logger)
 
 		// 2. Dial Options
-		// Default to TransportCredentials (TLS) usually, but allow insecure via opts if needed.
-		// However, PerRPCCredentials usually requires TLS.
-		// For simplicity/testing, we default to insecure if no transport creds provided,
-		// BUT BridgeCredentials.RequireTransportSecurity returns true, so gRPC will fail if insecure.
-		// We append our creds to the user provided options.
 		dialOpts := append(opts, grpc.WithPerRPCCredentials(creds))
 
-		// If user didn't provide transport credentials, we might need to add insecure for testing
-		// OR we assume user provides WithTransportCredentials.
-		// Let's assume user provides transport security options in 'opts'.
-		// But for a robust default, we check? No, we can't easily inspect DialOptions.
-		// We rely on the user to provide transport security (e.g. credentials.NewTLS) in 'opts'
-		// if they are connecting to a secure endpoint.
-
 		// 3. Dial
 		b.logger.Info("Dialing gRPC target", "target", target)
-		// Note: grpc.NewClient is the modern API, but Dial is still common. Using NewClient.
 		conn, err := grpc.NewClient(target, dialOpts...)
 		if err != nil {
 			b.logger.Error(err, "Failed to dial gRPC target", "target", target)
-			// Dial errors are usually retryable (e.g. DNS)
 			goto Retry
 		}
 
@@ -180,13 +178,19 @@ func (b *Bridge) MaintainGRPCConnection(
 			b.logger.Info("gRPC run loop exited cleanly", "connectionID", connectionID)
 		}
 
+		// Reset attempt counter if the connection was stable for a while
+		if time.Since(start) > 1*time.Minute {
+			attempt = 0
+		}
+
 	Retry:
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
 		default:
-			backoff := b.calculateBackoff()
-			b.logger.Info("Reconnecting gRPC", "after", backoff)
+			backoff := b.calculateBackoff(attempt)
+			attempt++
+			b.logger.Info("Reconnecting gRPC", "after", backoff, "attempt", attempt)
 			time.Sleep(backoff)
 		}
 	}
@@ -378,10 +382,18 @@ func (b *Bridge) manageConnection(ctx context.Context, connectionID string, endp
 }
 
 // NEW: Helper function for calculating backoff with jitter.
-func (b *Bridge) calculateBackoff() time.Duration {
-	backoff := b.retryPolicy.MinBackoff + time.Duration(rand.Int63n(int64(b.retryPolicy.Jitter)))
-	if backoff > b.retryPolicy.MaxBackoff {
-		return b.retryPolicy.MaxBackoff
+func (b *Bridge) calculateBackoff(attempt int) time.Duration {
+	if attempt < 0 {
+		attempt = 0
+	}
+	if attempt > 10 {
+		attempt = 10
+	}
+	factor := 1 << uint(attempt)
+	base := float64(b.retryPolicy.MinBackoff) * float64(factor)
+	if base > float64(b.retryPolicy.MaxBackoff) {
+		base = float64(b.retryPolicy.MaxBackoff)
 	}
-	return backoff
+	jitter := 0.2 + b.randSource.Float64()*0.6 // 0.2..0.8
+	return time.Duration(base * jitter)
 }

nexus-bridge/bridge_test.go

@@ -74,15 +74,31 @@ func (m *mockMetrics) SetConnectionStatus(status float64) { m.connectionStatus.S
 
 // testLogger is a mock implementation of the Logger interface for testing.
 type testLogger struct {
-	t *testing.T
+	t      *testing.T
+	mu     sync.RWMutex
+	closed bool
+}
+
+func (l *testLogger) stop() {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	l.closed = true
 }
 
 func (l *testLogger) Info(msg string, keysAndValues ...interface{}) {
-	l.t.Logf("INFO: %s %v", msg, keysAndValues)
+	l.mu.RLock()
+	defer l.mu.RUnlock()
+	if !l.closed {
+		l.t.Logf("INFO: %s %v", msg, keysAndValues)
+	}
 }
 
 func (l *testLogger) Error(err error, msg string, keysAndValues ...interface{}) {
-	l.t.Logf("ERROR: %s %v err: %v", msg, keysAndValues, err)
+	l.mu.RLock()
+	defer l.mu.RUnlock()
+	if !l.closed {
+		l.t.Logf("ERROR: %s %v err: %v", msg, keysAndValues, err)
+	}
 }
 
 var upgrader = websocket.Upgrader{}
@@ -442,4 +458,6 @@ func TestBridge_TokenRefreshWithoutDisconnect(t *testing.T) {
 	if metrics.connectionStatus.Load() != 1.0 {
 		t.Errorf("Expected connection status to be 1, but got %v", metrics.connectionStatus.Load())
 	}
+
+	logger.stop()
 }

nexus-broker/cmd/nexus-broker/main.go

@@ -115,7 +115,6 @@ func main() {
 		r.Get("/by-name/{name}", providersHandler.GetByName)
 		r.Delete("/by-name/{name}", providersHandler.DeleteByName)
 		r.Get("/{id}", providersHandler.Get)
-		r.Get("/{id}", providersHandler.Get)
 		r.Put("/{id}", providersHandler.Update)
 		r.Patch("/{id}", providersHandler.Patch)
 		r.Delete("/{id}", providersHandler.Delete)

nexus-broker/pkg/handlers/callback.go

@@ -6,13 +6,15 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"log"
 	"net"
 	"net/http"
 	"net/url"
 	"os"
 	"strings"
 	"time"
 
+	"github.com/go-chi/chi/v5"
 	"github.com/google/uuid"
 	"github.com/jmoiron/sqlx"
 	"github.com/lib/pq"
@@ -301,7 +303,9 @@ func (h *CallbackHandler) GetCaptureSchema(w http.ResponseWriter, r *http.Reques
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(response)
+	if err := json.NewEncoder(w).Encode(response); err != nil {
+		log.Printf("encode response: %v", err)
+	}
 }
 
 // SaveCredential handles the submission of the credential capture form.
@@ -348,7 +352,7 @@ func (h *CallbackHandler) SaveCredential(w http.ResponseWriter, r *http.Request)
 	}
 
 	if userInfoEndpoint != "" && apiBaseURL != "" {
-		if err := validateCredentials(authType, authHeader, apiBaseURL, userInfoEndpoint, reqBody.Credentials); err != nil {
+		if err := h.validateCredentials(authType, authHeader, apiBaseURL, userInfoEndpoint, reqBody.Credentials); err != nil {
 			http.Error(w, "Invalid credentials: "+err.Error(), http.StatusBadRequest)
 			return
 		}
@@ -365,11 +369,16 @@ func (h *CallbackHandler) SaveCredential(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
+	if !server.IsReturnURLAllowed(returnURL) {
+		http.Error(w, "return_url not allowed", http.StatusBadRequest)
+		return
+	}
+
 	http.Redirect(w, r, returnURL+"?status=success&connection_id="+connectionID.String(), http.StatusFound)
 }
 
 // validateCredentials makes a test call to the provider's user_info_endpoint to verify the submitted credentials.
-func validateCredentials(authType, authHeader, apiBaseURL, userInfoEndpoint string, credentials map[string]interface{}) error {
+func (h *CallbackHandler) validateCredentials(authType, authHeader, apiBaseURL, userInfoEndpoint string, credentials map[string]interface{}) error {
 	testURL := strings.TrimRight(apiBaseURL, "/") + "/" + strings.TrimLeft(userInfoEndpoint, "/")
 
 	req, err := http.NewRequest(http.MethodGet, testURL, nil)
@@ -404,8 +413,7 @@ func validateCredentials(authType, authHeader, apiBaseURL, userInfoEndpoint stri
 		return nil
 	}
 
-	client := &http.Client{Timeout: 10 * time.Second}
-	resp, err := client.Do(req)
+	resp, err := h.httpClient.Do(req)
 	if err != nil {
 		return fmt.Errorf("could not reach provider to validate credentials")
 	}
@@ -432,12 +440,7 @@ func containsScope(scopes []string, target string) bool {
 // GetToken handles GET /connections/{connection_id}/token
 func (h *CallbackHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 	// Extract connection ID from URL path
-	pathParts := strings.Split(r.URL.Path, "/")
-	if len(pathParts) < 3 {
-		http.Error(w, "Invalid path", http.StatusBadRequest)
-		return
-	}
-	connectionIDStr := pathParts[len(pathParts)-2] // /connections/{id}/token
+	connectionIDStr := chi.URLParam(r, "connectionID")
 
 	connectionID, err := uuid.Parse(connectionIDStr)
 	if err != nil {
@@ -472,10 +475,12 @@ func (h *CallbackHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 		if connection.Status == "attention" {
 			w.Header().Set("Content-Type", "application/json")
 			w.WriteHeader(http.StatusConflict)
-			json.NewEncoder(w).Encode(map[string]string{
+			if err := json.NewEncoder(w).Encode(map[string]string{
 				"error":  "attention_required",
 				"detail": "Connection requires attention. The user must re-authenticate.",
-			})
+			}); err != nil {
+				log.Printf("encode response: %v", err)
+			}
 			return
 		}
 
@@ -574,7 +579,9 @@ func (h *CallbackHandler) GetToken(w http.ResponseWriter, r *http.Request) {
 
 	// Return the response
 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(response)
+	if err := json.NewEncoder(w).Encode(response); err != nil {
+		log.Printf("encode response: %v", err)
+	}
 }
 
 // exchangeCodeForTokens exchanges authorization code for access tokens
@@ -621,8 +628,7 @@ func (h *CallbackHandler) exchangeCodeForTokens(tokenURL, clientID, clientSecret
 		req.SetBasicAuth(clientID, clientSecret)
 	}
 
-	client := &http.Client{Timeout: 30 * time.Second}
-	resp, err := client.Do(req)
+	resp, err := h.httpClient.Do(req)
 	if err != nil {
 		return nil, err
 	}
@@ -656,8 +662,7 @@ func (h *CallbackHandler) refreshTokens(tokenURL, clientID, clientSecret, refres
 	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 	req.Header.Set("Accept", "application/json") // Ensure JSON response
 
-	client := &http.Client{Timeout: 30 * time.Second}
-	resp, err := client.Do(req)
+	resp, err := h.httpClient.Do(req)
 	if err != nil {
 		return nil, 0, err
 	}
@@ -678,13 +683,8 @@ func (h *CallbackHandler) refreshTokens(tokenURL, clientID, clientSecret, refres
 // Refresh handles POST /connections/{connection_id}/refresh
 func (h *CallbackHandler) Refresh(w http.ResponseWriter, r *http.Request) {
 	// Extract connection ID
-	parts := strings.Split(r.URL.Path, "/")
-	if len(parts) < 3 {
-		http.Error(w, "Invalid path", http.StatusBadRequest)
-		return
-	}
-	idStr := parts[len(parts)-2]
-	connectionID, err := uuid.Parse(idStr)
+	connectionIDStr := chi.URLParam(r, "connectionID")
+	connectionID, err := uuid.Parse(connectionIDStr)
 	if err != nil {
 		http.Error(w, "Invalid connection ID", http.StatusBadRequest)
 		return
@@ -756,10 +756,12 @@ func (h *CallbackHandler) Refresh(w http.ResponseWriter, r *http.Request) {
 
 				w.Header().Set("Content-Type", "application/json")
 				w.WriteHeader(http.StatusConflict) // 409 Conflict is a good signal for "state issue"
-				json.NewEncoder(w).Encode(map[string]string{
+				if err := json.NewEncoder(w).Encode(map[string]string{
 					"error":  "attention_required",
 					"detail": "The connection credentials are invalid or expired and cannot be refreshed. User re-consent is required.",
-				})
+				}); err != nil {
+					log.Printf("encode response: %v", err)
+				}
 				return
 			}
 
@@ -773,7 +775,9 @@ func (h *CallbackHandler) Refresh(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		w.Header().Set("Content-Type", "application/json")
-		json.NewEncoder(w).Encode(newTokens)
+		if err := json.NewEncoder(w).Encode(newTokens); err != nil {
+			log.Printf("encode response: %v", err)
+		}
 	default:
 		http.Error(w, "Unsupported provider auth_type", http.StatusInternalServerError)
 		return