Skip to content

Commit 0428afd

Browse files
pushkar0108drstrangelookerYuan325
authored
fix(source/looker): dynamically resolve public host URL (googleapis#3603)
fix(source/looker): dynamically resolve public host URL to prevent internal address leakage and permission errors Update Looker tools to dynamically fetch the public host URL using the Looker Versions API instead of querying the admin-restricted `host_url` setting or falling back to the connection BaseUrl. 1. Fixes permission errors (403/404) for standard (non-admin) users who lack privileges to call `sdk.GetSetting("host_url")`. 2. Prevents leakage of internal service addresses in isolated network environments by resolving the correct public custom domain from the WebServerUrl returned by the Versions API. 🛠️ Fixes googleapis#3602 --------- Co-authored-by: Dr. Strangelove <drstrangelove@google.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
1 parent b574b07 commit 0428afd

7 files changed

Lines changed: 345 additions & 11 deletions

File tree

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ require (
7575
go.opentelemetry.io/otel/sdk/metric v1.44.0
7676
go.opentelemetry.io/otel/trace v1.44.0
7777
golang.org/x/oauth2 v0.36.0
78+
golang.org/x/sync v0.21.0
7879
google.golang.org/api v0.285.0
7980
google.golang.org/genai v1.61.0
8081
google.golang.org/genproto v0.0.0-20260519071638-aa98bba5eb94
@@ -271,7 +272,6 @@ require (
271272
golang.org/x/crypto v0.53.0 // indirect
272273
golang.org/x/mod v0.36.0 // indirect
273274
golang.org/x/net v0.56.0 // indirect
274-
golang.org/x/sync v0.21.0 // indirect
275275
golang.org/x/sys v0.46.0 // indirect
276276
golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 // indirect
277277
golang.org/x/term v0.44.0 // indirect

‎internal/sources/looker/looker.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import (
1818
"crypto/tls"
1919
"fmt"
2020
"net/http"
21+
"net/url"
2122
"strings"
23+
"sync"
2224
"time"
2325

2426
geminidataanalytics "cloud.google.com/go/geminidataanalytics/apiv1"
@@ -28,6 +30,7 @@ import (
2830
"go.opentelemetry.io/otel/trace"
2931
"golang.org/x/oauth2"
3032
"golang.org/x/oauth2/google"
33+
"golang.org/x/sync/singleflight"
3134

3235
"github.com/looker-open-source/sdk-codegen/go/rtl"
3336
v4 "github.com/looker-open-source/sdk-codegen/go/sdk/v4"
@@ -153,6 +156,13 @@ type Source struct {
153156
ApiSettings *rtl.ApiSettings
154157
TokenSource oauth2.TokenSource
155158
AuthTokenHeaderName string
159+
160+
hostURLMu sync.RWMutex
161+
cachedHostURL string
162+
lastFetchTime time.Time
163+
lastFetchFailed bool
164+
lastFetchErr error
165+
hostURLGroup singleflight.Group
156166
}
157167

158168
func (s *Source) SourceType() string {
@@ -274,3 +284,115 @@ func initGoogleCloudConnection(ctx context.Context) (oauth2.TokenSource, error)
274284

275285
return cred.TokenSource, nil
276286
}
287+
288+
func (s *Source) GetHostURL(ctx context.Context, sdk *v4.LookerSDK) (string, error) {
289+
defaultURL := strings.TrimSuffix(s.ApiSettings.BaseUrl, "/")
290+
291+
if sdk == nil {
292+
return defaultURL, nil
293+
}
294+
295+
// 1. Fast path: Read lock to check cache TTL
296+
s.hostURLMu.RLock()
297+
if !s.lastFetchFailed && !s.lastFetchTime.IsZero() && time.Since(s.lastFetchTime) < 10*time.Minute {
298+
urlStr := s.cachedHostURL
299+
s.hostURLMu.RUnlock()
300+
return urlStr, nil
301+
}
302+
if s.lastFetchFailed && time.Since(s.lastFetchTime) < time.Minute {
303+
urlStr := s.cachedHostURL
304+
if urlStr == "" {
305+
urlStr = defaultURL
306+
}
307+
err := s.lastFetchErr
308+
s.hostURLMu.RUnlock()
309+
return urlStr, err
310+
}
311+
s.hostURLMu.RUnlock()
312+
313+
// 2. Slow path: Use singleflight to deduplicate concurrent network calls
314+
res, err, _ := s.hostURLGroup.Do("get_host_url", func() (any, error) {
315+
// Double check within singleflight callback if another thread updated cache just before us
316+
s.hostURLMu.RLock()
317+
if !s.lastFetchFailed && !s.lastFetchTime.IsZero() && time.Since(s.lastFetchTime) < 10*time.Minute {
318+
urlStr := s.cachedHostURL
319+
s.hostURLMu.RUnlock()
320+
return urlStr, nil
321+
}
322+
if s.lastFetchFailed && time.Since(s.lastFetchTime) < time.Minute {
323+
urlStr := s.cachedHostURL
324+
if urlStr == "" {
325+
urlStr = defaultURL
326+
}
327+
err := s.lastFetchErr
328+
s.hostURLMu.RUnlock()
329+
return urlStr, err
330+
}
331+
s.hostURLMu.RUnlock()
332+
333+
logger, err := util.LoggerFromContext(ctx)
334+
if err != nil {
335+
return defaultURL, err
336+
}
337+
338+
// Perform network call outside of locks to prevent blocking concurrent readers
339+
versionInfo, err := sdk.Versions("", s.ApiSettings)
340+
if err != nil || versionInfo.WebServerUrl == nil || *versionInfo.WebServerUrl == "" {
341+
fetchErr := err
342+
if fetchErr == nil {
343+
fetchErr = fmt.Errorf("empty web_server_url in versions payload")
344+
}
345+
s.hostURLMu.Lock()
346+
s.lastFetchFailed = true
347+
s.lastFetchTime = time.Now()
348+
s.lastFetchErr = fetchErr
349+
// DO NOT overwrite s.cachedHostURL on transient errors; keep the stale valid URL for stale fallbacks
350+
s.hostURLMu.Unlock()
351+
352+
if err != nil {
353+
logger.WarnContext(ctx, fmt.Sprintf("unable to retrieve host_url via versions, using stale/default fallback: %v", err))
354+
} else {
355+
logger.DebugContext(ctx, "versions web_server_url is empty, using stale/default fallback")
356+
}
357+
return "", fetchErr
358+
}
359+
360+
// Validate the retrieved host_url is a valid absolute HTTP/HTTPS URL
361+
valURL := *versionInfo.WebServerUrl
362+
u, err := url.Parse(valURL)
363+
if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
364+
parseErr := fmt.Errorf("invalid web_server_url format: %q", valURL)
365+
s.hostURLMu.Lock()
366+
s.lastFetchFailed = true
367+
s.lastFetchTime = time.Now()
368+
s.lastFetchErr = parseErr
369+
s.hostURLMu.Unlock()
370+
logger.WarnContext(ctx, fmt.Sprintf("invalid host_url setting retrieved %q, using stale/default fallback: %v", valURL, err))
371+
return "", parseErr
372+
}
373+
374+
resolvedURL := strings.TrimSuffix(valURL, "/")
375+
376+
s.hostURLMu.Lock()
377+
s.cachedHostURL = resolvedURL
378+
s.lastFetchTime = time.Now()
379+
s.lastFetchFailed = false
380+
s.lastFetchErr = nil
381+
s.hostURLMu.Unlock()
382+
383+
logger.DebugContext(ctx, "successfully fetched and cached host_url: "+resolvedURL)
384+
return resolvedURL, nil
385+
})
386+
387+
if err != nil {
388+
// If singleflight failed, return last known good cache, else defaultURL
389+
s.hostURLMu.RLock()
390+
defer s.hostURLMu.RUnlock()
391+
if s.cachedHostURL != "" {
392+
return s.cachedHostURL, err
393+
}
394+
return defaultURL, err
395+
}
396+
397+
return res.(string), nil
398+
}

‎internal/sources/looker/looker_test.go‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ import (
1919
"io"
2020
"net/http"
2121
"net/http/httptest"
22+
"strings"
23+
"sync"
2224
"testing"
25+
"time"
2326

2427
"github.com/google/go-cmp/cmp"
2528
toolboxlog "github.com/googleapis/mcp-toolbox/internal/log"
@@ -236,3 +239,186 @@ func TestGetLookerSDK_ClientIPPropagation(t *testing.T) {
236239
t.Errorf("expected Authorization header to be %q, got %q", "mock-token-123", gotAuth)
237240
}
238241
}
242+
243+
func TestGetHostURL(t *testing.T) {
244+
// 1. Setup mock server that handles /api/4.0/versions
245+
var requestCount int
246+
var responseBody []byte
247+
var responseStatus int
248+
249+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
250+
if !strings.HasSuffix(r.URL.Path, "/api/4.0/versions") {
251+
w.WriteHeader(http.StatusNotFound)
252+
return
253+
}
254+
requestCount++
255+
w.WriteHeader(responseStatus)
256+
if _, err := w.Write(responseBody); err != nil {
257+
t.Errorf("failed to write mock response: %v", err)
258+
}
259+
}))
260+
defer ts.Close()
261+
262+
// 2. Configure Looker config pointing to mock server
263+
cfg := looker.Config{
264+
Name: "test-looker",
265+
Type: "looker",
266+
BaseURL: ts.URL,
267+
Timeout: "5s",
268+
SslVerification: false,
269+
}
270+
271+
ctx := context.Background()
272+
logger, _ := toolboxlog.NewStdLogger(io.Discard, io.Discard, "DEBUG")
273+
ctx = util.WithLogger(ctx, logger)
274+
ctx = util.WithUserAgent(ctx, "test-agent")
275+
276+
srcVal, err := cfg.Initialize(ctx, nil)
277+
if err != nil {
278+
t.Fatalf("failed to initialize source: %v", err)
279+
}
280+
src := srcVal.(*looker.Source)
281+
282+
sdk, err := src.GetLookerSDK(ctx, "mock-token-123")
283+
if err != nil {
284+
t.Fatalf("failed to get sdk: %v", err)
285+
}
286+
287+
// Scenario 1: Success Path & Success Cache TTL
288+
responseStatus = http.StatusOK
289+
responseBody = []byte(`{"web_server_url": "https://public.customdomain.com"}`)
290+
requestCount = 0
291+
292+
// First call - should perform request
293+
url1, err := src.GetHostURL(ctx, sdk)
294+
if err != nil {
295+
t.Fatalf("GetHostURL failed: %v", err)
296+
}
297+
if url1 != "https://public.customdomain.com" {
298+
t.Errorf("expected URL to be %q, got %q", "https://public.customdomain.com", url1)
299+
}
300+
if requestCount != 1 {
301+
t.Errorf("expected exactly 1 request to mock server, got %d", requestCount)
302+
}
303+
304+
// Second call - should hit success cache, no extra request
305+
url2, err := src.GetHostURL(ctx, sdk)
306+
if err != nil {
307+
t.Fatalf("GetHostURL failed: %v", err)
308+
}
309+
if url2 != "https://public.customdomain.com" {
310+
t.Errorf("expected URL to be %q, got %q", "https://public.customdomain.com", url2)
311+
}
312+
if requestCount != 1 {
313+
t.Errorf("expected request count to remain 1 (cached), got %d", requestCount)
314+
}
315+
316+
// Scenario 2: Failure Path & Fallback TTL
317+
// Reinitialize Source to clear the success cache
318+
srcVal, _ = cfg.Initialize(ctx, nil)
319+
src = srcVal.(*looker.Source)
320+
sdk, _ = src.GetLookerSDK(ctx, "mock-token-123")
321+
322+
responseStatus = http.StatusInternalServerError
323+
responseBody = []byte(`Internal Server Error`)
324+
requestCount = 0
325+
326+
// First failure call - should hit server and fallback to BaseUrl, returning error
327+
expectedFallback := strings.TrimSuffix(ts.URL, "/")
328+
urlFail1, err := src.GetHostURL(ctx, sdk)
329+
if err == nil {
330+
t.Fatal("expected GetHostURL to return an error on failure")
331+
}
332+
if urlFail1 != expectedFallback {
333+
t.Errorf("expected fallback URL to be %q, got %q", expectedFallback, urlFail1)
334+
}
335+
if requestCount != 1 {
336+
t.Errorf("expected exactly 1 request for failed fetch, got %d", requestCount)
337+
}
338+
339+
// Second call within 1 minute - should immediately return fallback without hitting server, returning cached error
340+
urlFail2, err := src.GetHostURL(ctx, sdk)
341+
if err == nil {
342+
t.Fatal("expected GetHostURL to return an error on cached failure")
343+
}
344+
if urlFail2 != expectedFallback {
345+
t.Errorf("expected fallback URL to be %q, got %q", expectedFallback, urlFail2)
346+
}
347+
if requestCount != 1 {
348+
t.Errorf("expected request count to remain 1 (cached failure), got %d", requestCount)
349+
}
350+
}
351+
352+
func TestGetHostURL_Concurrent(t *testing.T) {
353+
// Setup mock server
354+
var requestCount int
355+
var mu sync.Mutex
356+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
357+
mu.Lock()
358+
requestCount++
359+
mu.Unlock()
360+
// simulate slow API latency
361+
time.Sleep(100 * time.Millisecond)
362+
w.WriteHeader(http.StatusOK)
363+
_, _ = w.Write([]byte(`{"web_server_url": "https://public-concurrent.looker.com"}`))
364+
}))
365+
defer ts.Close()
366+
367+
cfg := looker.Config{
368+
Name: "test-looker-concurrent",
369+
Type: "looker",
370+
BaseURL: ts.URL,
371+
Timeout: "5s",
372+
SslVerification: false,
373+
}
374+
375+
ctx := context.Background()
376+
logger, _ := toolboxlog.NewStdLogger(io.Discard, io.Discard, "DEBUG")
377+
ctx = util.WithLogger(ctx, logger)
378+
ctx = util.WithUserAgent(ctx, "test-agent")
379+
380+
srcVal, _ := cfg.Initialize(ctx, nil)
381+
src := srcVal.(*looker.Source)
382+
sdk, _ := src.GetLookerSDK(ctx, "mock-token-123")
383+
384+
// Spawn 50 goroutines to concurrently call GetHostURL
385+
concurrency := 50
386+
errChan := make(chan error, concurrency)
387+
urlChan := make(chan string, concurrency)
388+
389+
var wg sync.WaitGroup
390+
for i := 0; i < concurrency; i++ {
391+
wg.Add(1)
392+
go func() {
393+
defer wg.Done()
394+
resolved, err := src.GetHostURL(ctx, sdk)
395+
if err != nil {
396+
errChan <- err
397+
return
398+
}
399+
urlChan <- resolved
400+
}()
401+
}
402+
403+
wg.Wait()
404+
close(errChan)
405+
close(urlChan)
406+
407+
for err := range errChan {
408+
t.Fatalf("concurrent GetHostURL failed: %v", err)
409+
}
410+
411+
for resolved := range urlChan {
412+
if resolved != "https://public-concurrent.looker.com" {
413+
t.Errorf("expected resolved URL to be %q, got %q", "https://public-concurrent.looker.com", resolved)
414+
}
415+
}
416+
417+
// Verify singleflight deduplicated the requests to exactly 1
418+
mu.Lock()
419+
finalCount := requestCount
420+
mu.Unlock()
421+
if finalCount != 1 {
422+
t.Errorf("expected exactly 1 network request due to singleflight deduplication, got %d", finalCount)
423+
}
424+
}

0 commit comments

Comments
 (0)