Skip to content

Commit

Permalink
fix(idtoken): configure validator constructor to use no authentication (
Browse files Browse the repository at this point in the history 
#1789)

Fixes: #1682
  • Loading branch information
@adrianajg
adrianajg committed Jan 3, 2023
1 parent ca86833 commit b35900a
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion idtoken/validate.go
View file
Expand Up @@ -19,6 +19,7 @@ import (
"strings"
"time"

"google.golang.org/api/option"
"google.golang.org/api/option/internaloption"
htransport "google.golang.org/api/transport/http"
)
Expand All @@ -36,7 +37,10 @@ var (
)

func defaultValidatorOpts() []ClientOption {
return []ClientOption{internaloption.WithDefaultScopes("https://www.googleapis.com/auth/cloud-platform")}
return []ClientOption{
internaloption.WithDefaultScopes("https://www.googleapis.com/auth/cloud-platform"),
option.WithoutAuthentication(),
}
}

// Payload represents a decoded payload of an ID Token.
Expand Down

4 comments on commit b35900a

@henrisson
Copy link

@henrisson henrisson commented on b35900a Feb 5, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[EDIT: see https://github.com//issues/1842#issuecomment-1421897504 this is working as intended]

This seems to break use cases using an API key.

Consider

	tokenValidator, err := idtoken.NewValidator(context.Background(),
		option.WithAPIKey(myGoogleAPIKey))

NewValidator will append option.WithAPIKey(myGoogleAPIKey), which will trigger the error in settings.go:

google-api-go-client/internal/settings.go

Line 85 in 4b4c9d4

if ds.NoAuth && hasCreds { 

	if ds.NoAuth && hasCreds {
		return errors.New("options.WithoutAuthentication is incompatible with any option that provides credentials")
	}

@taco-wang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, it's a big mistake. when upgrade to this version, you cannot use the idtoken with credentail

@henrisson
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, it's a big mistake. when upgrade to this version, you cannot use the idtoken with credentail

Actually, check #1842 (comment)

You should not pass an API key for this API.

@taco-wang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, it's a big mistake. when upgrade to this version, you cannot use the idtoken with credentail

Actually, check #1842 (comment)

You should not pass an API key for this API.

ok, in general, I think maybe need to pass an api key

Please sign in to comment.