From b83be6d0a009227ed44ae1bfcce66407f74f30c6 Mon Sep 17 00:00:00 2001 From: buua436 Date: Fri, 6 Nov 2026 14:55:25 +0800 Subject: [PATCH] fix: harden OAuth, gateway auth, and provider parsing (#22072) --- internal/channels/whatsapp-gateway/README.md | 4 + internal/channels/whatsapp-gateway/auth.js | 13 ++ .../channels/whatsapp-gateway/auth.test.js | 25 ++++ internal/channels/whatsapp-gateway/index.js | 13 +- internal/entity/models/cometapi.go | 37 +++++- internal/entity/models/cometapi_test.go | 55 ++++++++ internal/entity/models/gitee.go | 109 +++++++++------- internal/entity/models/gitee_test.go | 123 ++++++++++++++++++ internal/service/connector.go | 9 +- .../service/connector_google_oauth_test.go | 48 +++++++ 10 files changed, 373 insertions(+), 63 deletions(-) create mode 100644 internal/channels/whatsapp-gateway/auth.js create mode 100644 internal/channels/whatsapp-gateway/auth.test.js create mode 100644 internal/service/connector_google_oauth_test.go diff --git a/internal/channels/whatsapp-gateway/README.md b/internal/channels/whatsapp-gateway/README.md index 89d0cdff0d..30304b4558 100644 --- a/internal/channels/whatsapp-gateway/README.md +++ b/internal/channels/whatsapp-gateway/README.md @@ -13,6 +13,7 @@ npm install ## Run ```bash +WHATSAPP_GATEWAY_TOKEN=change-me \ WHATSAPP_GATEWAY_PORT=3005 \ WHATSAPP_GATEWAY_DATA_DIR=~/.ragflow/whatsapp-gateway \ npm start @@ -30,6 +31,9 @@ npm start ## Notes - Authentication state is persisted under `WHATSAPP_GATEWAY_DATA_DIR`. +- Set `WHATSAPP_GATEWAY_TOKEN` and configure the same token for the RAGFlow + WhatsApp channel. Requests to `/whatsapp/*` require this bearer token; only + `/health` is public for readiness checks. - Scan the QR code exposed in `status.qr_data_url`. - RAGFlow polls `status` and `events` and forwards inbound messages to the connected assistant. diff --git a/internal/channels/whatsapp-gateway/auth.js b/internal/channels/whatsapp-gateway/auth.js new file mode 100644 index 0000000000..6e4e2c411d --- /dev/null +++ b/internal/channels/whatsapp-gateway/auth.js @@ -0,0 +1,13 @@ +import { timingSafeEqual } from 'node:crypto'; + +export function isAuthorized(req, authToken) { + const expectedToken = String(authToken || '').trim(); + if (!expectedToken) { + return false; + } + + const authorization = Buffer.from(String(req.headers.authorization || '').trim()); + const expectedAuthorization = Buffer.from(`Bearer ${expectedToken}`); + return authorization.length === expectedAuthorization.length + && timingSafeEqual(authorization, expectedAuthorization); +} diff --git a/internal/channels/whatsapp-gateway/auth.test.js b/internal/channels/whatsapp-gateway/auth.test.js new file mode 100644 index 0000000000..29b2212afd --- /dev/null +++ b/internal/channels/whatsapp-gateway/auth.test.js @@ -0,0 +1,25 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { isAuthorized } from './auth.js'; + +test('rejects control requests when the gateway token is unset', () => { + assert.equal(isAuthorized({ headers: {} }, ''), false); + assert.equal(isAuthorized({ headers: { authorization: 'Bearer anything' } }, ''), false); +}); + +test('requires the configured bearer token for control requests', () => { + assert.equal(isAuthorized({ headers: {} }, 'gateway-secret'), false); + assert.equal( + isAuthorized({ headers: { authorization: 'Bearer wrong' } }, 'gateway-secret'), + false, + ); + assert.equal( + isAuthorized({ headers: { authorization: 'Bearer gateway-secreu' } }, 'gateway-secret'), + false, + ); + assert.equal( + isAuthorized({ headers: { authorization: 'Bearer gateway-secret' } }, 'gateway-secret'), + true, + ); +}); diff --git a/internal/channels/whatsapp-gateway/index.js b/internal/channels/whatsapp-gateway/index.js index 195e1dfc3f..f64c91b021 100644 --- a/internal/channels/whatsapp-gateway/index.js +++ b/internal/channels/whatsapp-gateway/index.js @@ -3,6 +3,7 @@ import crypto from 'node:crypto'; import os from 'node:os'; import path from 'node:path'; import { mkdir } from 'node:fs/promises'; +import { isAuthorized } from './auth.js'; import { decodeSessionKey, resolveSessionDir } from './session-path.js'; import makeWASocket, { @@ -98,14 +99,6 @@ function sendWsText(socket, payload) { socket.write(buildWsFrame(JSON.stringify(payload))); } -function isAuthorized(req) { - if (!AUTH_TOKEN) { - return true; - } - const auth = String(req.headers.authorization || '').trim(); - return auth === `Bearer ${AUTH_TOKEN}`; -} - class WhatsAppSession { constructor(sessionKey) { this.sessionKey = sessionKey; @@ -431,7 +424,7 @@ const server = http.createServer(async (req, res) => { return sendError(res, 404, 'not found'); } - if (!isAuthorized(req)) { + if (!isAuthorized(req, AUTH_TOKEN)) { return sendError(res, 401, 'unauthorized'); } @@ -492,7 +485,7 @@ server.on('upgrade', (req, socket) => { socket.destroy(); return; } - if (!isAuthorized(req)) { + if (!isAuthorized(req, AUTH_TOKEN)) { socket.destroy(); return; } diff --git a/internal/entity/models/cometapi.go b/internal/entity/models/cometapi.go index e2a54d0d2d..a0697177bc 100644 --- a/internal/entity/models/cometapi.go +++ b/internal/entity/models/cometapi.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "mime/multipart" @@ -93,6 +94,38 @@ func (c *CometAPIModel) balanceURL(apiKey string) string { return parsed.String() } +type redactedCometAPIRequestCause struct { + cause error +} + +func (e redactedCometAPIRequestCause) Error() string { return "request failed" } + +func (e redactedCometAPIRequestCause) Unwrap() error { return e.cause } + +func redactCometAPIBalanceRequestError(err error) error { + var requestErr *url.Error + if !errors.As(err, &requestErr) { + return redactedCometAPIRequestCause{cause: err} + } + + requestURL, parseErr := url.Parse(requestErr.URL) + if parseErr != nil { + return &url.Error{ + Op: requestErr.Op, + Err: redactedCometAPIRequestCause{cause: requestErr.Err}, + } + } + query := requestURL.Query() + query.Del("key") + requestURL.RawQuery = query.Encode() + + return &url.Error{ + Op: requestErr.Op, + URL: requestURL.String(), + Err: redactedCometAPIRequestCause{cause: requestErr.Err}, + } +} + // ChatWithMessages sends multiple messages with roles and returns the response. func (c *CometAPIModel) ChatWithMessages(ctx context.Context, modelName string, messages []Message, apiConfig *APIConfig, chatModelConfig *ChatConfig, modelUsage *common.ModelUsage) (*ChatResponse, error) { if err := c.baseModel.APIConfigCheck(apiConfig); err != nil { @@ -307,12 +340,12 @@ func (c *CometAPIModel) Balance(ctx context.Context, apiConfig *APIConfig) (map[ req, err := http.NewRequestWithContext(ctx, "GET", c.balanceURL(*apiConfig.ApiKey), nil) if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) + return nil, fmt.Errorf("failed to create request: %w", redactCometAPIBalanceRequestError(err)) } resp, err := c.baseModel.httpClient.Do(req) if err != nil { - return nil, fmt.Errorf("failed to send request: %w", err) + return nil, fmt.Errorf("failed to send request: %w", redactCometAPIBalanceRequestError(err)) } defer resp.Body.Close() diff --git a/internal/entity/models/cometapi_test.go b/internal/entity/models/cometapi_test.go index f7866b8540..00142d6b59 100644 --- a/internal/entity/models/cometapi_test.go +++ b/internal/entity/models/cometapi_test.go @@ -2,9 +2,11 @@ package models import ( "encoding/json" + "errors" "io" "net/http" "net/http/httptest" + "net/url" "strings" "sync/atomic" "testing" @@ -588,6 +590,59 @@ func TestCometAPIBalanceRequiresConfiguredURL(t *testing.T) { } } +func TestCometAPIBalanceRedactsAPIKeyOnTransportError(t *testing.T) { + withSSRFBypass(t) + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + server.Close() + + model := newCometAPIForTest("http://unused") + model.baseModel.URLSuffix.Balance = server.URL + "/user/quota" + apiKey := "cometapi-secret-key" + _, err := model.Balance(t.Context(), &APIConfig{ApiKey: &apiKey}) + if err == nil { + t.Fatal("Balance error = nil, want transport error") + } + if strings.Contains(err.Error(), apiKey) { + t.Fatalf("Balance error exposed API key: %v", err) + } + + var requestErr *url.Error + if !errors.As(err, &requestErr) { + t.Fatalf("Balance error %T does not preserve the request error", err) + } + parsedURL, parseErr := url.Parse(requestErr.URL) + if parseErr != nil { + t.Fatalf("parse sanitized request URL: %v", parseErr) + } + if got := parsedURL.Query().Get("key"); got != "" { + t.Fatalf("transport error URL contains API key query value %q", got) + } +} + +func TestRedactCometAPIBalanceRequestErrorHidesUnparseableURL(t *testing.T) { + const apiKey = "cometapi-secret-key" + cause := errors.New("connection refused") + err := redactCometAPIBalanceRequestError(&url.Error{ + Op: "Get", + URL: "http://[::1?key=" + apiKey, + Err: cause, + }) + + if strings.Contains(err.Error(), apiKey) { + t.Fatalf("redacted request error exposed API key: %v", err) + } + var requestErr *url.Error + if !errors.As(err, &requestErr) { + t.Fatalf("redacted error %T does not preserve the request error type", err) + } + if requestErr.URL != "" { + t.Fatalf("redacted request URL = %q, want empty URL", requestErr.URL) + } + if !errors.Is(err, cause) { + t.Fatalf("redacted error does not preserve the transport cause") + } +} + func TestCometAPIRerankReturnsNoSuchMethod(t *testing.T) { withSSRFBypass(t) ctx := t.Context() diff --git a/internal/entity/models/gitee.go b/internal/entity/models/gitee.go index 56b28117ed..e60fc2a696 100644 --- a/internal/entity/models/gitee.go +++ b/internal/entity/models/gitee.go @@ -24,6 +24,7 @@ import ( "io" "mime/multipart" "net/http" + "net/url" "ragflow/internal/common" "strings" "time" @@ -542,41 +543,47 @@ func (g *GiteeModel) ParseFile(ctx context.Context, modelName *string, content [ if err = json.Unmarshal(body, &giteeParseFileResp); err != nil { return nil, fmt.Errorf("failed to parse response: %w", err) } + if strings.TrimSpace(giteeParseFileResp.TaskID) == "" { + return nil, fmt.Errorf("Gitee parse API response is missing task_id") + } _, err = g.getParseFile(ctx, &baseURL, apiConfig.ApiKey, &giteeParseFileResp.TaskID, 5*time.Second, 10) if err != nil { return nil, err } - var parseFileResponse = ParseFileResponse{} - - return &parseFileResponse, nil + return &ParseFileResponse{TaskID: giteeParseFileResp.TaskID}, nil } type giteeGetParseFileResponse struct { + Status string `json:"status"` } -func (g *GiteeModel) getParseFile(ctx context.Context, baseURL *string, apiKey, taskID *string, timeOut time.Duration, count int) (*giteeGetParseFileResponse, error) { - url := fmt.Sprintf("%s/task/%s/status", strings.TrimSuffix(*baseURL, "/"), *taskID) - - reqBody := map[string]interface{}{} - - jsonData, err := json.Marshal(reqBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal request: %w", err) +func (g *GiteeModel) getParseFile(ctx context.Context, baseURL *string, apiKey, taskID *string, pollInterval time.Duration, maxPolls int) (*giteeGetParseFileResponse, error) { + if baseURL == nil || strings.TrimSpace(*baseURL) == "" { + return nil, fmt.Errorf("Gitee base URL is required") } + if apiKey == nil || strings.TrimSpace(*apiKey) == "" { + return nil, fmt.Errorf("Gitee API key is required") + } + if taskID == nil || strings.TrimSpace(*taskID) == "" { + return nil, fmt.Errorf("Gitee task ID is required") + } + if maxPolls <= 0 { + return nil, fmt.Errorf("Gitee parse task polling count must be positive") + } + statusURL := fmt.Sprintf("%s/task/%s/status", strings.TrimRight(strings.TrimSpace(*baseURL), "/"), url.PathEscape(strings.TrimSpace(*taskID))) ctx, cancel := context.WithTimeout(ctx, longOpCallTimeout) defer cancel() - for i := 0; i < count; i++ { - req, err := http.NewRequestWithContext(ctx, "GET", url, bytes.NewBuffer(jsonData)) + for i := 0; i < maxPolls; i++ { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, statusURL, nil) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", *apiKey)) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", strings.TrimSpace(*apiKey))) resp, err := g.baseModel.httpClient.Do(req) if err != nil { @@ -593,23 +600,32 @@ func (g *GiteeModel) getParseFile(ctx context.Context, baseURL *string, apiKey, return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body)) } - // Parse response - var result map[string]interface{} + var result giteeGetParseFileResponse if err = json.Unmarshal(body, &result); err != nil { return nil, fmt.Errorf("failed to parse response: %w", err) } + switch status := strings.ToLower(strings.TrimSpace(result.Status)); status { + case "success": + return &result, nil + case "failure", "cancelled": + return nil, fmt.Errorf("Gitee parse task %s: %s", strings.TrimSpace(*taskID), status) + case "waiting", "in_progress": + default: + return nil, fmt.Errorf("Gitee parse task %s returned unknown status %q", strings.TrimSpace(*taskID), result.Status) + } - // Wait before the next poll, but honor context cancellation / - // longOpCallTimeout instead of blocking uninterruptibly. - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(timeOut): + if i+1 < maxPolls && pollInterval > 0 { + timer := time.NewTimer(pollInterval) + select { + case <-ctx.Done(): + timer.Stop() + return nil, ctx.Err() + case <-timer.C: + } } } - // if resp show the file is ok, download it. otherwise, provide timeout info - return nil, nil + return nil, fmt.Errorf("timed out waiting for Gitee parse task %s after %d polls", strings.TrimSpace(*taskID), maxPolls) } func (g *GiteeModel) ListModels(ctx context.Context, apiConfig *APIConfig) ([]ListModelResponse, error) { @@ -776,15 +792,15 @@ type giteeTaskListResponse struct { } type giteeTaskItem struct { - TaskID string `json:"task_id"` - //Output giteeTaskOutput `json:"output"` - Status string `json:"status"` - CreatedAt int64 `json:"created_at"` - StartedAt int64 `json:"started_at,omitempty"` - CompletedAt int64 `json:"completed_at,omitempty"` - Price float64 `json:"price"` - Currency string `json:"currency"` - URLs giteeTaskURLs `json:"urls"` + TaskID string `json:"task_id"` + Output giteeTaskOutput `json:"output"` + Status string `json:"status"` + CreatedAt int64 `json:"created_at"` + StartedAt int64 `json:"started_at,omitempty"` + CompletedAt int64 `json:"completed_at,omitempty"` + Price float64 `json:"price"` + Currency string `json:"currency"` + URLs giteeTaskURLs `json:"urls"` } type giteeTaskOutput struct { @@ -864,25 +880,21 @@ func (g *GiteeModel) ShowTask(ctx context.Context, taskID string, apiConfig *API if err := g.baseModel.APIConfigCheck(apiConfig); err != nil { return nil, err } + taskID = strings.TrimSpace(taskID) + if taskID == "" { + return nil, fmt.Errorf("Gitee task ID is required") + } taskURL, err := g.baseModel.EndpointURL(apiConfig, g.baseModel.URLSuffix.Task) if err != nil { return nil, err } - url := taskURL + "/" + taskID + "/get" - - // Build request body - reqBody := map[string]interface{}{} - - jsonData, err := json.Marshal(reqBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal request: %w", err) - } + taskRecordURL := taskURL + "/" + url.PathEscape(taskID) ctx, cancel := context.WithTimeout(ctx, nonStreamCallTimeout) defer cancel() - req, err := http.NewRequestWithContext(ctx, "GET", url, bytes.NewBuffer(jsonData)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, taskRecordURL, nil) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } @@ -905,14 +917,17 @@ func (g *GiteeModel) ShowTask(ctx context.Context, taskID string, apiConfig *API return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body)) } - var taskOutput giteeTaskOutput - if err = json.Unmarshal(body, &taskOutput); err != nil { + var taskRecord giteeTaskItem + if err = json.Unmarshal(body, &taskRecord); err != nil { return nil, fmt.Errorf("failed to parse response: %w", err) } + if strings.ToLower(strings.TrimSpace(taskRecord.Status)) != "success" { + return nil, fmt.Errorf("Gitee parse task %s is not successful: status %q", taskID, taskRecord.Status) + } taskResp := &TaskResponse{} - for _, segment := range taskOutput.Segments { + for _, segment := range taskRecord.Output.Segments { taskResp.Segments = append(taskResp.Segments, TaskSegment{ Index: segment.Index, Content: segment.Content, diff --git a/internal/entity/models/gitee_test.go b/internal/entity/models/gitee_test.go index 2e801ec8ac..fc1e9a5cf6 100644 --- a/internal/entity/models/gitee_test.go +++ b/internal/entity/models/gitee_test.go @@ -50,6 +50,129 @@ func newGiteeForChatTest(baseURL string) *GiteeModel { return NewGiteeModel(map[string]string{"default": baseURL}, URLSuffix{Chat: "chat/completions"}) } +func TestGiteeGetParseFileReturnsSuccessfulTask(t *testing.T) { + withSSRFBypass(t) + polls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/task/task-123/status" { + t.Errorf("request = %s %s, want GET /task/task-123/status", r.Method, r.URL.Path) + } + polls++ + status := "in_progress" + if polls == 2 { + status = "success" + } + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "task_id": "task-123", + "status": status, + "output": map[string]interface{}{ + "segments": []map[string]interface{}{{"index": 0, "content": "parsed text"}}, + }, + }) + })) + defer server.Close() + + model := NewGiteeModel(map[string]string{"default": server.URL}, URLSuffix{}) + apiKey, taskID := "test-key", "task-123" + result, err := model.getParseFile(t.Context(), &server.URL, &apiKey, &taskID, 0, 2) + if err != nil { + t.Fatalf("getParseFile: %v", err) + } + if result == nil { + t.Fatal("getParseFile result = nil, want successful task") + } + if result.Status != "success" { + t.Fatalf("task status = %q, want success", result.Status) + } + if polls != 2 { + t.Fatalf("poll count = %d, want 2", polls) + } +} + +func TestGiteeGetParseFileRejectsFailedAndUnfinishedTasks(t *testing.T) { + withSSRFBypass(t) + for _, tc := range []struct { + name string + status string + wantError string + }{ + {name: "failure", status: "failure", wantError: "Gitee parse task task-123: failure"}, + {name: "cancelled", status: "cancelled", wantError: "Gitee parse task task-123: cancelled"}, + {name: "timeout", status: "in_progress", wantError: "timed out waiting for Gitee parse task task-123"}, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{"status": tc.status}) + })) + defer server.Close() + + model := NewGiteeModel(map[string]string{"default": server.URL}, URLSuffix{}) + apiKey, taskID := "test-key", "task-123" + _, err := model.getParseFile(t.Context(), &server.URL, &apiKey, &taskID, 0, 1) + if err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Fatalf("getParseFile error = %v, want %q", err, tc.wantError) + } + }) + } +} + +func TestGiteeParseFileReturnsTaskID(t *testing.T) { + withSSRFBypass(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/async/documents/parse": + _ = json.NewEncoder(w).Encode(map[string]interface{}{"task_id": "task-123", "status": "waiting"}) + case r.Method == http.MethodGet && r.URL.Path == "/task/task-123/status": + _ = json.NewEncoder(w).Encode(map[string]interface{}{"task_id": "task-123", "status": "success"}) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + } + })) + defer server.Close() + + model := NewGiteeModel(map[string]string{"default": server.URL}, URLSuffix{ + DocumentParse: "async/documents/parse", + }) + apiKey, modelName := "test-key", "MinerU2.5" + response, err := model.ParseFile(t.Context(), &modelName, []byte("file content"), nil, &APIConfig{ApiKey: &apiKey}, nil, nil) + if err != nil { + t.Fatalf("ParseFile: %v", err) + } + if response == nil || response.TaskID != "task-123" { + t.Fatalf("ParseFile response = %#v, want task_id task-123", response) + } +} + +func TestGiteeShowTaskReadsParsedSegments(t *testing.T) { + withSSRFBypass(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/task/task-123" { + t.Errorf("request = %s %s, want GET /task/task-123", r.Method, r.URL.Path) + http.NotFound(w, r) + return + } + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "task_id": "task-123", + "status": "success", + "output": map[string]interface{}{ + "segments": []map[string]interface{}{{"index": 1, "content": "parsed text"}}, + }, + }) + })) + defer server.Close() + + apiKey := "test-key" + model := NewGiteeModel(map[string]string{"default": server.URL}, URLSuffix{Task: "task"}) + response, err := model.ShowTask(t.Context(), "task-123", &APIConfig{ApiKey: &apiKey}) + if err != nil { + t.Fatalf("ShowTask: %v", err) + } + if len(response.Segments) != 1 || response.Segments[0].Content != "parsed text" { + t.Fatalf("ShowTask segments = %#v, want parsed text", response.Segments) + } +} + func TestGiteeBalanceRejectsMalformedBalanceWithoutPanicking(t *testing.T) { withSSRFBypass(t) cases := []struct { diff --git a/internal/service/connector.go b/internal/service/connector.go index 9eed9864ed..f4ba3ffd1c 100644 --- a/internal/service/connector.go +++ b/internal/service/connector.go @@ -797,13 +797,14 @@ func exchangeGoogleWebOAuthCode(clientConfig map[string]interface{}, scopes []st clientID := strings.TrimSpace(stringValue(webConfig["client_id"])) clientSecret := strings.TrimSpace(stringValue(webConfig["client_secret"])) - tokenURI := strings.TrimSpace(stringValue(webConfig["token_uri"])) - if tokenURI == "" { - tokenURI = googleOAuthTokenURL + configuredTokenURI := strings.TrimSpace(stringValue(webConfig["token_uri"])) + if configuredTokenURI != "" && configuredTokenURI != googleOAuthTokenURL { + return "", fmt.Errorf("invalid Google OAuth client configuration: token_uri must use Google's token endpoint") } - if clientID == "" || tokenURI == "" { + if clientID == "" { return "", fmt.Errorf("invalid Google OAuth client configuration") } + tokenURI := googleOAuthTokenURL form := url.Values{} form.Set("client_id", clientID) diff --git a/internal/service/connector_google_oauth_test.go b/internal/service/connector_google_oauth_test.go new file mode 100644 index 0000000000..63b0cefde7 --- /dev/null +++ b/internal/service/connector_google_oauth_test.go @@ -0,0 +1,48 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestExchangeGoogleWebOAuthCodeRejectsCustomTokenURI(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"unexpected"}`)) + })) + defer server.Close() + + clientConfig := map[string]interface{}{ + "web": map[string]interface{}{ + "client_id": "client-id", + "client_secret": "client-secret", + "token_uri": server.URL, + }, + } + _, err := exchangeGoogleWebOAuthCode(clientConfig, nil, "http://localhost/callback", "code", "verifier") + if err == nil { + t.Fatal("exchangeGoogleWebOAuthCode accepted a custom token URI") + } + if got := requests.Load(); got != 0 { + t.Fatalf("custom token URI received %d requests, want none", got) + } +}