From 462f53d16fb78a9509a31c918145cbbef18332cd Mon Sep 17 00:00:00 2001 From: Cody Oss Date: Tue, 3 Nov 2020 13:53:54 -0700 Subject: [PATCH 1/2] feat: generate a default audience for clients This option will be used to generate self-signed JWTs when the client is authenticated with a service account and the user does not provide additional scopes. Also, now marking the generated scopes as default so we can distinguish between generated and user provided scopes. Related: googleapis/google-api-go-client#738 --- internal/gengapic/client_init.go | 22 ++++++++++++++++++- internal/gengapic/client_init_test.go | 22 +++++++++++++++++++ internal/gengapic/testdata/empty_opt.want | 3 ++- internal/gengapic/testdata/foo_opt.want | 3 ++- internal/gengapic/testdata/host_port_opt.want | 3 ++- 5 files changed, 49 insertions(+), 4 deletions(-) diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go index 9bc31a8e..e22e6087 100644 --- a/internal/gengapic/client_init.go +++ b/internal/gengapic/client_init.go @@ -67,8 +67,9 @@ func (g *generator) clientOptions(serv *descriptor.ServiceDescriptorProto, servN p(" return []option.ClientOption{") p(" internaloption.WithDefaultEndpoint(%q),", host) p(" internaloption.WithDefaultMTLSEndpoint(%q),", generateDefaultMTLSEndpoint(host)) + p(" internaloption.WithDefaultAudience(%q),", generateDefaultAudience(host)) p(" option.WithGRPCDialOption(grpc.WithDisableServiceConfig()),") - p(" option.WithScopes(DefaultAuthScopes()...),") + p(" internaloption.WithDefaultScopes(DefaultAuthScopes()...),") p(" option.WithGRPCDialOption(grpc.WithDefaultCallOptions(") p(" grpc.MaxCallRecvMsgSize(math.MaxInt32))),") p(" }") @@ -310,3 +311,22 @@ func generateDefaultMTLSEndpoint(defaultEndpoint string) string { } return defaultEndpoint } + +// generateDefaultAudience transforms a host into a an audience that can be used +// as the `aud` claim in a JWT. +func generateDefaultAudience(host string) string { + aud := host + // Add a scheme if not present. + if !strings.Contains(aud, "://") { + aud = "https://" + aud + } + // Remove port if present. + if strings.Count(aud, ":") > 1 { + aud = aud[:strings.LastIndex(aud, ":")] + } + // Add trailing slash if not present. + if !strings.HasSuffix(aud, "/") { + aud = aud + "/" + } + return aud +} diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index b826303f..f1317916 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -208,3 +208,25 @@ func TestClientInit(t *testing.T) { txtdiff.Diff(t, tst.tstName, g.pt.String(), filepath.Join("testdata", tst.tstName+".want")) } } + +func TestGenerateDefaultAudience(t *testing.T) { + want := "https://foo.googleapis.com/" + tests := []struct { + name string + host string + }{ + {name: "plain host", host: "foo.googleapis.com"}, + {name: "host with port", host: "foo.googleapis.com:443"}, + {name: "host with scheme", host: "https://foo.googleapis.com"}, + {name: "host with scheme and port", host: "https://foo.googleapis.com:1234"}, + {name: "host is a proper audience", host: want}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := generateDefaultAudience(tc.host); got != want { + t.Errorf("generateDefaultAudience(%q) = %q, want %q", tc.host, got, want) + } + }) + } +} diff --git a/internal/gengapic/testdata/empty_opt.want b/internal/gengapic/testdata/empty_opt.want index 59ae8507..c72611b1 100644 --- a/internal/gengapic/testdata/empty_opt.want +++ b/internal/gengapic/testdata/empty_opt.want @@ -9,8 +9,9 @@ func defaultClientOptions() []option.ClientOption { return []option.ClientOption{ internaloption.WithDefaultEndpoint("foo.googleapis.com:443"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:443"), + internaloption.WithDefaultAudience("https://foo.googleapis.com/"), option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), - option.WithScopes(DefaultAuthScopes()...), + internaloption.WithDefaultScopes(DefaultAuthScopes()...), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), } diff --git a/internal/gengapic/testdata/foo_opt.want b/internal/gengapic/testdata/foo_opt.want index 676566cd..65f5f3ea 100644 --- a/internal/gengapic/testdata/foo_opt.want +++ b/internal/gengapic/testdata/foo_opt.want @@ -9,8 +9,9 @@ func defaultFooClientOptions() []option.ClientOption { return []option.ClientOption{ internaloption.WithDefaultEndpoint("foo.googleapis.com:443"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:443"), + internaloption.WithDefaultAudience("https://foo.googleapis.com/"), option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), - option.WithScopes(DefaultAuthScopes()...), + internaloption.WithDefaultScopes(DefaultAuthScopes()...), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), } diff --git a/internal/gengapic/testdata/host_port_opt.want b/internal/gengapic/testdata/host_port_opt.want index 0331feeb..125b05ae 100644 --- a/internal/gengapic/testdata/host_port_opt.want +++ b/internal/gengapic/testdata/host_port_opt.want @@ -7,8 +7,9 @@ func defaultBarClientOptions() []option.ClientOption { return []option.ClientOption{ internaloption.WithDefaultEndpoint("foo.googleapis.com:1234"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:1234"), + internaloption.WithDefaultAudience("https://foo.googleapis.com/"), option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), - option.WithScopes(DefaultAuthScopes()...), + internaloption.WithDefaultScopes(DefaultAuthScopes()...), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), } From e67ea5099d70ea1b19df19a3311ccff73981a38a Mon Sep 17 00:00:00 2001 From: Cody Oss Date: Wed, 4 Nov 2020 10:36:01 -0700 Subject: [PATCH 2/2] codereview comments --- internal/gengapic/client_init.go | 8 +++++--- internal/gengapic/client_init_test.go | 18 ++++++++++-------- internal/gengapic/testdata/empty_opt.want | 2 +- internal/gengapic/testdata/foo_opt.want | 2 +- internal/gengapic/testdata/host_port_opt.want | 2 +- 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go index e22e6087..52ffd3e3 100644 --- a/internal/gengapic/client_init.go +++ b/internal/gengapic/client_init.go @@ -68,8 +68,8 @@ func (g *generator) clientOptions(serv *descriptor.ServiceDescriptorProto, servN p(" internaloption.WithDefaultEndpoint(%q),", host) p(" internaloption.WithDefaultMTLSEndpoint(%q),", generateDefaultMTLSEndpoint(host)) p(" internaloption.WithDefaultAudience(%q),", generateDefaultAudience(host)) - p(" option.WithGRPCDialOption(grpc.WithDisableServiceConfig()),") p(" internaloption.WithDefaultScopes(DefaultAuthScopes()...),") + p(" option.WithGRPCDialOption(grpc.WithDisableServiceConfig()),") p(" option.WithGRPCDialOption(grpc.WithDefaultCallOptions(") p(" grpc.MaxCallRecvMsgSize(math.MaxInt32))),") p(" }") @@ -320,9 +320,11 @@ func generateDefaultAudience(host string) string { if !strings.Contains(aud, "://") { aud = "https://" + aud } - // Remove port if present. + // Remove port, and everything after, if present. if strings.Count(aud, ":") > 1 { - aud = aud[:strings.LastIndex(aud, ":")] + firstIndex := strings.Index(aud, ":") + secondIndex := strings.Index(aud[firstIndex+1:], ":") + firstIndex + 1 + aud = aud[:secondIndex] } // Add trailing slash if not present. if !strings.HasSuffix(aud, "/") { diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go index f1317916..fc866c68 100644 --- a/internal/gengapic/client_init_test.go +++ b/internal/gengapic/client_init_test.go @@ -210,22 +210,24 @@ func TestClientInit(t *testing.T) { } func TestGenerateDefaultAudience(t *testing.T) { - want := "https://foo.googleapis.com/" tests := []struct { name string host string + want string }{ - {name: "plain host", host: "foo.googleapis.com"}, - {name: "host with port", host: "foo.googleapis.com:443"}, - {name: "host with scheme", host: "https://foo.googleapis.com"}, - {name: "host with scheme and port", host: "https://foo.googleapis.com:1234"}, - {name: "host is a proper audience", host: want}, + {name: "plain host", host: "foo.googleapis.com", want: "https://foo.googleapis.com/"}, + {name: "host with port", host: "foo.googleapis.com:443", want: "https://foo.googleapis.com/"}, + {name: "host with scheme", host: "https://foo.googleapis.com", want: "https://foo.googleapis.com/"}, + {name: "host with scheme and port", host: "https://foo.googleapis.com:1234", want: "https://foo.googleapis.com/"}, + {name: "host is a proper audience", host: "https://foo.googleapis.com/", want: "https://foo.googleapis.com/"}, + {name: "host with non-http scheme", host: "ftp://foo.googleapis.com", want: "ftp://foo.googleapis.com/"}, + {name: "host with path", host: "foo.googleapis.com:443/extra/path", want: "https://foo.googleapis.com/"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if got := generateDefaultAudience(tc.host); got != want { - t.Errorf("generateDefaultAudience(%q) = %q, want %q", tc.host, got, want) + if got := generateDefaultAudience(tc.host); got != tc.want { + t.Errorf("generateDefaultAudience(%q) = %q, want %q", tc.host, got, tc.want) } }) } diff --git a/internal/gengapic/testdata/empty_opt.want b/internal/gengapic/testdata/empty_opt.want index c72611b1..0a04244c 100644 --- a/internal/gengapic/testdata/empty_opt.want +++ b/internal/gengapic/testdata/empty_opt.want @@ -10,8 +10,8 @@ func defaultClientOptions() []option.ClientOption { internaloption.WithDefaultEndpoint("foo.googleapis.com:443"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:443"), internaloption.WithDefaultAudience("https://foo.googleapis.com/"), - option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), internaloption.WithDefaultScopes(DefaultAuthScopes()...), + option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), } diff --git a/internal/gengapic/testdata/foo_opt.want b/internal/gengapic/testdata/foo_opt.want index 65f5f3ea..ef96d4d2 100644 --- a/internal/gengapic/testdata/foo_opt.want +++ b/internal/gengapic/testdata/foo_opt.want @@ -10,8 +10,8 @@ func defaultFooClientOptions() []option.ClientOption { internaloption.WithDefaultEndpoint("foo.googleapis.com:443"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:443"), internaloption.WithDefaultAudience("https://foo.googleapis.com/"), - option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), internaloption.WithDefaultScopes(DefaultAuthScopes()...), + option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), } diff --git a/internal/gengapic/testdata/host_port_opt.want b/internal/gengapic/testdata/host_port_opt.want index 125b05ae..cdf279a9 100644 --- a/internal/gengapic/testdata/host_port_opt.want +++ b/internal/gengapic/testdata/host_port_opt.want @@ -8,8 +8,8 @@ func defaultBarClientOptions() []option.ClientOption { internaloption.WithDefaultEndpoint("foo.googleapis.com:1234"), internaloption.WithDefaultMTLSEndpoint("foo.mtls.googleapis.com:1234"), internaloption.WithDefaultAudience("https://foo.googleapis.com/"), - option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), internaloption.WithDefaultScopes(DefaultAuthScopes()...), + option.WithGRPCDialOption(grpc.WithDisableServiceConfig()), option.WithGRPCDialOption(grpc.WithDefaultCallOptions( grpc.MaxCallRecvMsgSize(math.MaxInt32))), }