refactor: prevent internal error exposure in API responses

server/api/callback/index.go

@@ -42,53 +42,53 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 
 	oauthClient, err := oauth.GetClient()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "OAuth configuration missing")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "OAuth service unavailable")
 		return
 	}
 
 	token, err := oauthClient.ExchangeCode(code)
 	if err != nil {
-		httputil.WriteError(w, http.StatusBadRequest, "exchange_failed", "Failed to exchange authorization code")
+		httputil.WriteErrorWithLog(w, err, http.StatusBadRequest, "exchange_failed", "Failed to exchange authorization code")
 		return
 	}
 
 	redisClient, err := redis.GetClient()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Redis connection failed")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Storage service unavailable")
 		return
 	}
 
 	sessionID, err := crypto.GenerateSessionID()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate session ID")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create session")
 		return
 	}
 
 	if err := redisClient.Set(redis.SessionKeyPrefix+sessionID, token.AccessToken, redis.SessionTTL); err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to store session")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to store session")
 		return
 	}
 
 	refreshTokenID, err := crypto.GenerateRefreshTokenID()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate refresh token ID")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create session")
 		return
 	}
 
 	if err := redisClient.Set(redis.RefreshTokenKeyPrefix+refreshTokenID, sessionID, redis.RefreshTokenTTL); err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to store refresh token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to store session")
 		return
 	}
 
 	accessToken, err := jwt.GenerateAccessToken(sessionID)
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate access token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create access token")
 		return
 	}
 
 	refreshToken, err := jwt.GenerateRefreshToken(refreshTokenID, sessionID)
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate refresh token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create refresh token")
 		return
 	}
 

server/api/callback/index_test.go

@@ -0,0 +1,143 @@
+package handler
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github-project-status-viewer-server/pkg/httputil"
+)
+
+func TestHandler_ErrorResponseSanitization(t *testing.T) {
+	tests := []struct {
+		expectedCode        string
+		expectedDescription string
+		name                string
+		queryParams         string
+		shouldNotContain    []string
+	}{
+		{
+			name:                "missing code parameter should return sanitized error",
+			queryParams:         "?state=abc123",
+			expectedCode:        "missing_code",
+			expectedDescription: "Authorization code is required",
+			shouldNotContain:    []string{"internal", "stack", "nil"},
+		},
+		{
+			name:                "missing state parameter should return sanitized error",
+			queryParams:         "?code=test_code",
+			expectedCode:        "missing_state",
+			expectedDescription: "State parameter is required for CSRF protection",
+			shouldNotContain:    []string{"internal", "stack", "nil"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, "/api/callback"+tt.queryParams, nil)
+			w := httptest.NewRecorder()
+
+			Handler(w, req)
+
+			if w.Code != http.StatusBadRequest {
+				t.Errorf("Status code = %v, want %v", w.Code, http.StatusBadRequest)
+			}
+
+			var apiError httputil.APIError
+			if err := json.NewDecoder(w.Body).Decode(&apiError); err != nil {
+				t.Fatalf("Failed to decode error response: %v", err)
+			}
+
+			if apiError.Code != tt.expectedCode {
+				t.Errorf("Error code = %v, want %v", apiError.Code, tt.expectedCode)
+			}
+
+			if apiError.Description != tt.expectedDescription {
+				t.Errorf("Error description = %v, want %v", apiError.Description, tt.expectedDescription)
+			}
+
+			responseBody := w.Body.String()
+			for _, forbidden := range tt.shouldNotContain {
+				if containsString(responseBody, forbidden) {
+					t.Errorf("Response should not contain '%s' but body contains: %s", forbidden, responseBody)
+				}
+			}
+		})
+	}
+}
+
+func TestHandler_MethodValidation(t *testing.T) {
+	tests := []struct {
+		method     string
+		name       string
+		wantStatus int
+	}{
+		{
+			name:       "GET method should be accepted",
+			method:     http.MethodGet,
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "POST method should be rejected",
+			method:     http.MethodPost,
+			wantStatus: http.StatusMethodNotAllowed,
+		},
+		{
+			name:       "PUT method should be rejected",
+			method:     http.MethodPut,
+			wantStatus: http.StatusMethodNotAllowed,
+		},
+		{
+			name:       "DELETE method should be rejected",
+			method:     http.MethodDelete,
+			wantStatus: http.StatusMethodNotAllowed,
+		},
+		{
+			name:       "OPTIONS method should be accepted for CORS",
+			method:     http.MethodOptions,
+			wantStatus: http.StatusOK,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest(tt.method, "/api/callback?code=test&state=test", nil)
+			w := httptest.NewRecorder()
+
+			Handler(w, req)
+
+			if w.Code != tt.wantStatus {
+				t.Errorf("Status code = %v, want %v", w.Code, tt.wantStatus)
+			}
+		})
+	}
+}
+
+func TestHandler_CORSHeaders(t *testing.T) {
+	t.Setenv("CHROME_EXTENSION_ID", "test-extension-id")
+
+	req := httptest.NewRequest(http.MethodGet, "/api/callback?code=test&state=test", nil)
+	w := httptest.NewRecorder()
+
+	Handler(w, req)
+
+	corsHeader := w.Header().Get("Access-Control-Allow-Origin")
+	expectedOrigin := "chrome-extension://test-extension-id"
+	if corsHeader != expectedOrigin {
+		t.Errorf("Expected CORS header to be %s, got %s", expectedOrigin, corsHeader)
+	}
+}
+
+func containsString(s, substr string) bool {
+	return len(s) >= len(substr) && stringContains(s, substr)
+}
+
+func stringContains(s, substr string) bool {
+	for i := 0; i <= len(s)-len(substr); i++ {
+		if s[i:i+len(substr)] == substr {
+			return true
+		}
+	}
+	return false
+}

server/api/refresh/index.go

@@ -33,67 +33,67 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 	refreshToken := tokenString[7:]
 	claims, err := jwt.ValidateRefreshToken(refreshToken)
 	if err != nil {
-		httputil.WriteError(w, http.StatusUnauthorized, "invalid_refresh_token", err.Error())
+		httputil.WriteErrorWithLog(w, err, http.StatusUnauthorized, "invalid_refresh_token", "Invalid or expired refresh token")
 		return
 	}
 
 	redisClient, err := redis.GetClient()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Redis connection failed")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Storage service unavailable")
 		return
 	}
 
 	storedSessionID, err := redisClient.Get(redis.RefreshTokenKeyPrefix + claims.RefreshTokenID)
 	if err != nil {
 		if errors.Is(err, pkgerrors.ErrKeyNotFound) {
-			httputil.WriteError(w, http.StatusUnauthorized, "refresh_token_revoked", "Refresh token has been revoked or expired")
+			httputil.WriteErrorWithLog(w, pkgerrors.ErrRefreshTokenRevoked, http.StatusUnauthorized, "refresh_token_revoked", "Refresh token has been revoked or expired")
 		} else {
-			httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to verify refresh token")
+			httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to verify refresh token")
 		}
 		return
 	}
 
 	if storedSessionID != claims.SessionID {
-		httputil.WriteError(w, http.StatusUnauthorized, "session_mismatch", "Session mismatch detected")
+		httputil.WriteErrorWithLog(w, pkgerrors.ErrSessionMismatch, http.StatusUnauthorized, "session_mismatch", "Session mismatch detected")
 		return
 	}
 
 	exists, err := redisClient.Exists(redis.SessionKeyPrefix + claims.SessionID)
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to check session")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to verify session")
 		return
 	}
 
 	if !exists {
-		httputil.WriteError(w, http.StatusUnauthorized, "session_not_found", "Session expired or invalid")
+		httputil.WriteErrorWithLog(w, pkgerrors.ErrSessionNotFound, http.StatusUnauthorized, "session_not_found", "Session expired or invalid")
 		return
 	}
 
 	if err := redisClient.Delete(redis.RefreshTokenKeyPrefix + claims.RefreshTokenID); err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to revoke old refresh token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to revoke old refresh token")
 		return
 	}
 
 	newRefreshTokenID, err := crypto.GenerateRefreshTokenID()
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate refresh token ID")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create refresh token")
 		return
 	}
 
 	if err := redisClient.Set(redis.RefreshTokenKeyPrefix+newRefreshTokenID, claims.SessionID, redis.RefreshTokenTTL); err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to store new refresh token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to store refresh token")
 		return
 	}
 
 	newAccessToken, err := jwt.GenerateAccessToken(claims.SessionID)
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate access token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create access token")
 		return
 	}
 
 	newRefreshToken, err := jwt.GenerateRefreshToken(newRefreshTokenID, claims.SessionID)
 	if err != nil {
-		httputil.WriteError(w, http.StatusInternalServerError, "server_error", "Failed to generate refresh token")
+		httputil.WriteErrorWithLog(w, err, http.StatusInternalServerError, "server_error", "Failed to create refresh token")
 		return
 	}