From ad817bec2fa4039231dd8ea66dc11b32b22695ff Mon Sep 17 00:00:00 2001 From: yihuaz Date: Fri, 27 Aug 2021 13:36:14 -0700 Subject: [PATCH 1/4] change aud format in self-singed jwt --- credentials/oauth/oauth.go | 8 +++++++- internal/credentials/util.go | 16 +++++++++++++++- internal/credentials/util_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/credentials/oauth/oauth.go b/credentials/oauth/oauth.go index 852ae375cfc..038c884871f 100644 --- a/credentials/oauth/oauth.go +++ b/credentials/oauth/oauth.go @@ -29,6 +29,8 @@ import ( "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/grpc/credentials" + + credinternal "google.golang.org/grpc/internal/credentials" ) // TokenSource supplies PerRPCCredentials from an oauth2.TokenSource. @@ -75,9 +77,13 @@ func NewJWTAccessFromKey(jsonKey []byte) (credentials.PerRPCCredentials, error) } func (j jwtAccess) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { + aud, err := credinternal.RemoveServiceNameFromJwtUri(uri[0]) + if err != nil { + return nil, err + } // TODO: the returned TokenSource is reusable. Store it in a sync.Map, with // uri as the key, to avoid recreating for every RPC. - ts, err := google.JWTAccessTokenSourceFromJSON(j.jsonKey, uri[0]) + ts, err := google.JWTAccessTokenSourceFromJSON(j.jsonKey, aud) if err != nil { return nil, err } diff --git a/internal/credentials/util.go b/internal/credentials/util.go index 55664fa46b8..ae1b2a20fb2 100644 --- a/internal/credentials/util.go +++ b/internal/credentials/util.go @@ -18,7 +18,10 @@ package credentials -import "crypto/tls" +import ( + "crypto/tls" + "net/url" +) const alpnProtoStrH2 = "h2" @@ -48,3 +51,14 @@ func CloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } + +// RemoveServiceNameFromJwtUri removes RPC service name +// from URI that will be used as audience in a self-signed +// JWT token. It follows https://google.aip.dev/auth/4111. +func RemoveServiceNameFromJwtUri(uri string) (string, error) { + parsed, err := url.Parse(uri) + if err != nil { + return "", err + } + return parsed.Scheme + "://" + parsed.Host + "/", nil +} diff --git a/internal/credentials/util_test.go b/internal/credentials/util_test.go index 69e01eeefe9..91fb7ab42bf 100644 --- a/internal/credentials/util_test.go +++ b/internal/credentials/util_test.go @@ -58,3 +58,29 @@ func (s) TestAppendH2ToNextProtos(t *testing.T) { }) } } + +func (s) TestRemoveServiceNameFromJwtUri(t *testing.T) { + tests := []struct { + name string + uri string + want string + }{ + { + name: "invalid URI", + uri: "ht tp://foo.com", + want: "", + }, + { + name: "valid URI", + uri: "https://foo.com/go/", + want: "https://foo.com/", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got, _ := RemoveServiceNameFromJwtUri(tt.uri); got != tt.want { + t.Errorf("RemoveServiceNameFromJwtUri() = %v, want %v", got, tt.want) + } + }) + } +} From 8f02d80a639627d77f7649f6e2faa4dbe14fe4a5 Mon Sep 17 00:00:00 2001 From: yihuaz Date: Fri, 27 Aug 2021 13:49:23 -0700 Subject: [PATCH 2/4] fix lint error --- credentials/oauth/oauth.go | 2 +- internal/credentials/util.go | 4 ++-- internal/credentials/util_test.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/credentials/oauth/oauth.go b/credentials/oauth/oauth.go index 038c884871f..36bb5619c87 100644 --- a/credentials/oauth/oauth.go +++ b/credentials/oauth/oauth.go @@ -77,7 +77,7 @@ func NewJWTAccessFromKey(jsonKey []byte) (credentials.PerRPCCredentials, error) } func (j jwtAccess) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { - aud, err := credinternal.RemoveServiceNameFromJwtUri(uri[0]) + aud, err := credinternal.RemoveServiceNameFromJwtURI(uri[0]) if err != nil { return nil, err } diff --git a/internal/credentials/util.go b/internal/credentials/util.go index ae1b2a20fb2..e6a17051c35 100644 --- a/internal/credentials/util.go +++ b/internal/credentials/util.go @@ -52,10 +52,10 @@ func CloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } -// RemoveServiceNameFromJwtUri removes RPC service name +// RemoveServiceNameFromJwtURI removes RPC service name // from URI that will be used as audience in a self-signed // JWT token. It follows https://google.aip.dev/auth/4111. -func RemoveServiceNameFromJwtUri(uri string) (string, error) { +func RemoveServiceNameFromJwtURI(uri string) (string, error) { parsed, err := url.Parse(uri) if err != nil { return "", err diff --git a/internal/credentials/util_test.go b/internal/credentials/util_test.go index 91fb7ab42bf..c0cc725b89f 100644 --- a/internal/credentials/util_test.go +++ b/internal/credentials/util_test.go @@ -59,7 +59,7 @@ func (s) TestAppendH2ToNextProtos(t *testing.T) { } } -func (s) TestRemoveServiceNameFromJwtUri(t *testing.T) { +func (s) TestRemoveServiceNameFromJwtURI(t *testing.T) { tests := []struct { name string uri string @@ -78,8 +78,8 @@ func (s) TestRemoveServiceNameFromJwtUri(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got, _ := RemoveServiceNameFromJwtUri(tt.uri); got != tt.want { - t.Errorf("RemoveServiceNameFromJwtUri() = %v, want %v", got, tt.want) + if got, _ := RemoveServiceNameFromJwtURI(tt.uri); got != tt.want { + t.Errorf("RemoveServiceNameFromJwtURI() = %v, want %v", got, tt.want) } }) } From 4cc836ac75660bfd799ac358eb5cf7795a7885cf Mon Sep 17 00:00:00 2001 From: yihuaz Date: Mon, 30 Aug 2021 13:27:24 -0700 Subject: [PATCH 3/4] fix 1st round of comments --- credentials/oauth/oauth.go | 17 +++++++-- credentials/oauth/oauth_test.go | 62 +++++++++++++++++++++++++++++++ internal/credentials/util.go | 12 ------ internal/credentials/util_test.go | 26 ------------- 4 files changed, 76 insertions(+), 41 deletions(-) create mode 100644 credentials/oauth/oauth_test.go diff --git a/credentials/oauth/oauth.go b/credentials/oauth/oauth.go index 36bb5619c87..c748fd21ce2 100644 --- a/credentials/oauth/oauth.go +++ b/credentials/oauth/oauth.go @@ -23,14 +23,13 @@ import ( "context" "fmt" "io/ioutil" + "net/url" "sync" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "golang.org/x/oauth2/jwt" "google.golang.org/grpc/credentials" - - credinternal "google.golang.org/grpc/internal/credentials" ) // TokenSource supplies PerRPCCredentials from an oauth2.TokenSource. @@ -58,6 +57,16 @@ func (ts TokenSource) RequireTransportSecurity() bool { return true } +// removeServiceNameFromJWTURI removes RPC service name from URI. +func removeServiceNameFromJWTURI(uri string) (string, error) { + parsed, err := url.Parse(uri) + if err != nil { + return "", err + } + parsed.Path = "/" + return parsed.String(), nil +} + type jwtAccess struct { jsonKey []byte } @@ -77,7 +86,9 @@ func NewJWTAccessFromKey(jsonKey []byte) (credentials.PerRPCCredentials, error) } func (j jwtAccess) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { - aud, err := credinternal.RemoveServiceNameFromJwtURI(uri[0]) + // Remove RPC service name from URI that will be used as audience + // in a self-signed JWT token. It follows https://google.aip.dev/auth/4111. + aud, err := removeServiceNameFromJWTURI(uri[0]) if err != nil { return nil, err } diff --git a/credentials/oauth/oauth_test.go b/credentials/oauth/oauth_test.go new file mode 100644 index 00000000000..030f950b696 --- /dev/null +++ b/credentials/oauth/oauth_test.go @@ -0,0 +1,62 @@ +/* + * + * Copyright 2020 gRPC authors. + * + * 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 oauth + +import ( + "strings" + "testing" +) + +func checkErrorMsg(err error, msg string) bool { + if err == nil && msg == "" { + return true + } else if err != nil { + return strings.Contains(err.Error(), msg) + } + return false +} + +func TestRemoveServiceNameFromJwtURI(t *testing.T) { + tests := []struct { + name string + uri string + wantedURI string + wantedErrMsg string + }{ + { + name: "invalid URI", + uri: "ht tp://foo.com", + wantedURI: "", + wantedErrMsg: "first path segment in URL cannot contain colon", + }, + { + name: "valid URI", + uri: "https://foo.com/go/", + wantedURI: "https://foo.com/", + wantedErrMsg: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got, err := removeServiceNameFromJWTURI(tt.uri); got != tt.wantedURI || !checkErrorMsg(err, tt.wantedErrMsg) { + t.Errorf("RemoveServiceNameFromJWTURI() = %s, %v, want %s, %v", got, err, tt.wantedURI, tt.wantedErrMsg) + } + }) + } +} diff --git a/internal/credentials/util.go b/internal/credentials/util.go index e6a17051c35..f792fd22caf 100644 --- a/internal/credentials/util.go +++ b/internal/credentials/util.go @@ -20,7 +20,6 @@ package credentials import ( "crypto/tls" - "net/url" ) const alpnProtoStrH2 = "h2" @@ -51,14 +50,3 @@ func CloneTLSConfig(cfg *tls.Config) *tls.Config { return cfg.Clone() } - -// RemoveServiceNameFromJwtURI removes RPC service name -// from URI that will be used as audience in a self-signed -// JWT token. It follows https://google.aip.dev/auth/4111. -func RemoveServiceNameFromJwtURI(uri string) (string, error) { - parsed, err := url.Parse(uri) - if err != nil { - return "", err - } - return parsed.Scheme + "://" + parsed.Host + "/", nil -} diff --git a/internal/credentials/util_test.go b/internal/credentials/util_test.go index c0cc725b89f..69e01eeefe9 100644 --- a/internal/credentials/util_test.go +++ b/internal/credentials/util_test.go @@ -58,29 +58,3 @@ func (s) TestAppendH2ToNextProtos(t *testing.T) { }) } } - -func (s) TestRemoveServiceNameFromJwtURI(t *testing.T) { - tests := []struct { - name string - uri string - want string - }{ - { - name: "invalid URI", - uri: "ht tp://foo.com", - want: "", - }, - { - name: "valid URI", - uri: "https://foo.com/go/", - want: "https://foo.com/", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got, _ := RemoveServiceNameFromJwtURI(tt.uri); got != tt.want { - t.Errorf("RemoveServiceNameFromJwtURI() = %v, want %v", got, tt.want) - } - }) - } -} From 259e28920b9458fe6329e199d6a775ce1e9ca386 Mon Sep 17 00:00:00 2001 From: yihuaz Date: Thu, 2 Sep 2021 11:37:32 -0700 Subject: [PATCH 4/4] address 2nd round of comments --- credentials/oauth/oauth_test.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/credentials/oauth/oauth_test.go b/credentials/oauth/oauth_test.go index 030f950b696..7e62ecb36c1 100644 --- a/credentials/oauth/oauth_test.go +++ b/credentials/oauth/oauth_test.go @@ -1,6 +1,6 @@ /* * - * Copyright 2020 gRPC authors. + * Copyright 2021 gRPC authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,7 +32,7 @@ func checkErrorMsg(err error, msg string) bool { return false } -func TestRemoveServiceNameFromJwtURI(t *testing.T) { +func TestRemoveServiceNameFromJWTURI(t *testing.T) { tests := []struct { name string uri string @@ -42,14 +42,12 @@ func TestRemoveServiceNameFromJwtURI(t *testing.T) { { name: "invalid URI", uri: "ht tp://foo.com", - wantedURI: "", wantedErrMsg: "first path segment in URL cannot contain colon", }, { - name: "valid URI", - uri: "https://foo.com/go/", - wantedURI: "https://foo.com/", - wantedErrMsg: "", + name: "valid URI", + uri: "https://foo.com/go/", + wantedURI: "https://foo.com/", }, } for _, tt := range tests {