Skip to content

Commit 59f7b6e

Browse files
authored
feat(auth/google): require audience or clientId for mcpEnabled (#3450)
Make audience validation mandatory. Reported by: [HE WEI(ギカク)](https://buganizer.corp.google.com/issues/525094792)
1 parent 2c3ca5d commit 59f7b6e

3 files changed

Lines changed: 31 additions & 28 deletions

File tree

internal/auth/google/google.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,11 @@ func (cfg Config) AuthServiceConfigType() string {
5050

5151
// Initialize a Google auth service
5252
func (cfg Config) Initialize() (auth.AuthService, error) {
53-
if !cfg.McpEnabled {
53+
if cfg.McpEnabled {
54+
if cfg.Audience == "" && cfg.ClientID == "" {
55+
return nil, fmt.Errorf("`audience` or `clientId` is required when `mcpEnabled` is true")
56+
}
57+
} else {
5458
if cfg.Audience != "" {
5559
return nil, fmt.Errorf("`audience` is not allowed when `mcpEnabled` is false")
5660
}

internal/auth/google/google_test.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,26 @@ func TestInitialize_Validation(t *testing.T) {
7070
wantError: true,
7171
},
7272
{
73-
name: "scopesRequired, mcpEnabled true (allowed)",
73+
name: "scopesRequired, mcpEnabled true, with audience (allowed)",
7474
config: Config{
7575
Name: "google-auth",
7676
Type: "google",
7777
ScopesRequired: []string{"scope"},
78+
Audience: "my-audience",
7879
McpEnabled: true,
7980
},
8081
wantError: false,
8182
},
83+
{
84+
name: "scopesRequired, mcpEnabled true, without audience or clientID (disallowed)",
85+
config: Config{
86+
Name: "google-auth",
87+
Type: "google",
88+
ScopesRequired: []string{"scope"},
89+
McpEnabled: true,
90+
},
91+
wantError: true,
92+
},
8293
{
8394
name: "both clientID and audience, mcpEnabled true",
8495
config: Config{
@@ -99,6 +110,15 @@ func TestInitialize_Validation(t *testing.T) {
99110
},
100111
wantError: false,
101112
},
113+
{
114+
name: "neither clientID nor audience, mcpEnabled true (disallowed)",
115+
config: Config{
116+
Name: "google-auth",
117+
Type: "google",
118+
McpEnabled: true,
119+
},
120+
wantError: true,
121+
},
102122
}
103123

104124
for _, tc := range tests {

tests/auth/auth_integration_test.go

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -399,17 +399,11 @@ func TestGoogleOAuthValidation(t *testing.T) {
399399
}
400400
}
401401

402-
// TestGoogleOAuthValidationNoClientID tests validation of Google access token using type: google without clientId
403-
func TestGoogleOAuthValidationNoClientID(t *testing.T) {
402+
// TestGoogleOAuthValidationNoClientIDOrAudienceFails tests that initialization fails when neither clientId nor audience is configured with mcpEnabled: true.
403+
func TestGoogleOAuthValidationNoClientIDOrAudienceFails(t *testing.T) {
404404
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
405405
defer cancel()
406406

407-
// Get access token
408-
accessToken, err := sources.GetIAMAccessToken(ctx)
409-
if err != nil {
410-
t.Errorf("error getting access token from ADC: %s", err)
411-
}
412-
413407
toolsFile := map[string]any{
414408
"sources": map[string]any{},
415409
"authServices": map[string]any{
@@ -430,25 +424,10 @@ func TestGoogleOAuthValidationNoClientID(t *testing.T) {
430424

431425
waitCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
432426
defer cancel()
433-
out, err := testutils.WaitForString(waitCtx, regexp.MustCompile(`Server ready to serve`), cmd.Out)
427+
expectedErrPattern := regexp.MustCompile(`audience.*or.*clientId.*is required when.*mcpEnabled.*is true`)
428+
out, err := testutils.WaitForString(waitCtx, expectedErrPattern, cmd.Out)
434429
if err != nil {
435430
t.Logf("toolbox command logs: \n%s", out)
436-
t.Fatalf("toolbox didn't start successfully: %s", err)
437-
}
438-
439-
api := "http://127.0.0.1:5007/mcp/sse"
440-
441-
req, _ := http.NewRequest(http.MethodGet, api, nil)
442-
req.Header.Add("Authorization", "Bearer "+accessToken)
443-
444-
resp, err := http.DefaultClient.Do(req)
445-
if err != nil {
446-
t.Fatalf("unable to send request: %s", err)
447-
}
448-
defer resp.Body.Close()
449-
450-
if resp.StatusCode != http.StatusOK {
451-
bodyBytes, _ := io.ReadAll(resp.Body)
452-
t.Fatalf("expected 200, got %d: %s", resp.StatusCode, string(bodyBytes))
431+
t.Fatalf("expected toolbox to fail with validation error, but it did not: %s", err)
453432
}
454433
}

0 commit comments

Comments
 (0)