From b06bcb535a72d9b5c58dd6c33f1f9dfe542824d4 Mon Sep 17 00:00:00 2001 From: Billy Bolton Date: Fri, 26 Mar 2021 13:27:47 -0400 Subject: [PATCH 1/3] set google access token expiration threshold --- .gitignore | 1 + pkg/google/client.go | 11 +- pkg/google/googlefakes/fake_client.go | 40 ++---- pkg/http/token.go | 30 +++-- pkg/http/token_test.go | 14 +- pkg/rancher/rancherfakes/fake_client.go | 172 +++++++++--------------- 6 files changed, 114 insertions(+), 154 deletions(-) diff --git a/.gitignore b/.gitignore index e1d92f2..dc29c32 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /arcade +*.coverprofile diff --git a/pkg/google/client.go b/pkg/google/client.go index 37920f2..5a72810 100644 --- a/pkg/google/client.go +++ b/pkg/google/client.go @@ -4,6 +4,7 @@ import ( "context" "github.com/gin-gonic/gin" + "golang.org/x/oauth2" "golang.org/x/oauth2/google" ) @@ -18,7 +19,7 @@ var clientScopes = []string{ //go:generate counterfeiter . Client type Client interface { - NewToken() (string, error) + NewToken() (*oauth2.Token, error) } func NewClient() Client { @@ -27,18 +28,18 @@ func NewClient() Client { type client struct{} -func (client) NewToken() (string, error) { +func (client) NewToken() (*oauth2.Token, error) { tokenSource, err := google.DefaultTokenSource(context.Background(), clientScopes...) if err != nil { - return "", err + return nil, err } token, err := tokenSource.Token() if err != nil { - return "", err + return nil, err } - return token.AccessToken, nil + return token, nil } func Instance(c *gin.Context) Client { diff --git a/pkg/google/googlefakes/fake_client.go b/pkg/google/googlefakes/fake_client.go index 172cbe9..15087ae 100644 --- a/pkg/google/googlefakes/fake_client.go +++ b/pkg/google/googlefakes/fake_client.go @@ -5,30 +5,29 @@ import ( "sync" "github.com/homedepot/arcade/pkg/google" + "golang.org/x/oauth2" ) type FakeClient struct { - NewTokenStub func() (string, error) + NewTokenStub func() (*oauth2.Token, error) newTokenMutex sync.RWMutex - newTokenArgsForCall []struct { - } - newTokenReturns struct { - result1 string + newTokenArgsForCall []struct{} + newTokenReturns struct { + result1 *oauth2.Token result2 error } newTokenReturnsOnCall map[int]struct { - result1 string + result1 *oauth2.Token result2 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeClient) NewToken() (string, error) { +func (fake *FakeClient) NewToken() (*oauth2.Token, error) { fake.newTokenMutex.Lock() ret, specificReturn := fake.newTokenReturnsOnCall[len(fake.newTokenArgsForCall)] - fake.newTokenArgsForCall = append(fake.newTokenArgsForCall, struct { - }{}) + fake.newTokenArgsForCall = append(fake.newTokenArgsForCall, struct{}{}) fake.recordInvocation("NewToken", []interface{}{}) fake.newTokenMutex.Unlock() if fake.NewTokenStub != nil { @@ -37,8 +36,7 @@ func (fake *FakeClient) NewToken() (string, error) { if specificReturn { return ret.result1, ret.result2 } - fakeReturns := fake.newTokenReturns - return fakeReturns.result1, fakeReturns.result2 + return fake.newTokenReturns.result1, fake.newTokenReturns.result2 } func (fake *FakeClient) NewTokenCallCount() int { @@ -47,34 +45,24 @@ func (fake *FakeClient) NewTokenCallCount() int { return len(fake.newTokenArgsForCall) } -func (fake *FakeClient) NewTokenCalls(stub func() (string, error)) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() - fake.NewTokenStub = stub -} - -func (fake *FakeClient) NewTokenReturns(result1 string, result2 error) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() +func (fake *FakeClient) NewTokenReturns(result1 *oauth2.Token, result2 error) { fake.NewTokenStub = nil fake.newTokenReturns = struct { - result1 string + result1 *oauth2.Token result2 error }{result1, result2} } -func (fake *FakeClient) NewTokenReturnsOnCall(i int, result1 string, result2 error) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() +func (fake *FakeClient) NewTokenReturnsOnCall(i int, result1 *oauth2.Token, result2 error) { fake.NewTokenStub = nil if fake.newTokenReturnsOnCall == nil { fake.newTokenReturnsOnCall = make(map[int]struct { - result1 string + result1 *oauth2.Token result2 error }) } fake.newTokenReturnsOnCall[i] = struct { - result1 string + result1 *oauth2.Token result2 error }{result1, result2} } diff --git a/pkg/http/token.go b/pkg/http/token.go index 031b8b6..0b5508b 100644 --- a/pkg/http/token.go +++ b/pkg/http/token.go @@ -9,18 +9,19 @@ import ( "github.com/gin-gonic/gin" "github.com/homedepot/arcade/pkg/google" "github.com/homedepot/arcade/pkg/rancher" + "golang.org/x/oauth2" ) var ( - err error - googleMux sync.Mutex - t time.Time - token string - expiration = 1 * time.Minute - rancherMux sync.Mutex - kubeconfigToken rancher.KubeconfigToken + err error + googleMux sync.Mutex + googleExpiration time.Time + googleToken *oauth2.Token + rancherMux sync.Mutex + kubeconfigToken rancher.KubeconfigToken ) +// GetToken returns a new access token for a given provider. func GetToken(c *gin.Context) { provider := c.Query("provider") @@ -39,26 +40,27 @@ func getGoogleToken(c *gin.Context) { googleMux.Lock() defer googleMux.Unlock() - if time.Since(t) > expiration || token == "" { + if time.Now().UTC().After(googleExpiration) || googleToken == nil { googleClient := google.Instance(c) - token, err = googleClient.NewToken() + googleToken, err = googleClient.NewToken() if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - t = time.Now().In(time.UTC) + // Set the expiration for the Google token to be 90% expiry-threshold. + googleExpiration = time.Now().UTC().Add((time.Until(googleToken.Expiry) / 10) * 9) } - c.JSON(http.StatusOK, gin.H{"token": token}) + c.JSON(http.StatusOK, gin.H{"token": googleToken.AccessToken}) } func getRancherToken(c *gin.Context) { - if time.Now().In(time.UTC).After(kubeconfigToken.ExpiresAt) || kubeconfigToken.Token == "" { - rancherMux.Lock() - defer rancherMux.Unlock() + rancherMux.Lock() + defer rancherMux.Unlock() + if time.Now().In(time.UTC).After(kubeconfigToken.ExpiresAt) || kubeconfigToken.Token == "" { rancherClient := rancher.Instance(c) if rancherClient == nil { c.JSON(http.StatusBadRequest, gin.H{"error": "token provider not configured: rancher"}) diff --git a/pkg/http/token_test.go b/pkg/http/token_test.go index 4f58d01..ed9a20e 100644 --- a/pkg/http/token_test.go +++ b/pkg/http/token_test.go @@ -17,6 +17,7 @@ import ( "github.com/homedepot/arcade/pkg/rancher/rancherfakes" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "golang.org/x/oauth2" ) type Tokens struct { @@ -33,9 +34,11 @@ var ( res *http.Response tokens Tokens fakeGoogleClient *googlefakes.FakeClient - fakeGoogleToken = "fake-google-token" fakeRancherClient *rancherfakes.FakeClient - fakeRancherToken = rancher.KubeconfigToken{ + fakeGoogleToken = &oauth2.Token{ + AccessToken: "fake-google-token", + } + fakeRancherToken = rancher.KubeconfigToken{ Token: "fake-rancher-token", } expiredRancherToken = rancher.KubeconfigToken{ @@ -49,6 +52,11 @@ var ( ) var _ = Describe("Token", func() { + BeforeEach(func() { + // Disable debug logging. + gin.SetMode(gin.ReleaseMode) + }) + Describe("#GetToken", func() { When("provider is not supported", func() { BeforeEach(func() { @@ -109,7 +117,7 @@ var _ = Describe("Token", func() { When("getting a new token from google fails", func() { BeforeEach(func() { - fakeGoogleClient.NewTokenReturns("", errors.New("error getting token from google")) + fakeGoogleClient.NewTokenReturns(nil, errors.New("error getting token from google")) }) It("returns an internal server error", func() { diff --git a/pkg/rancher/rancherfakes/fake_client.go b/pkg/rancher/rancherfakes/fake_client.go index e51c30a..452dfd4 100644 --- a/pkg/rancher/rancherfakes/fake_client.go +++ b/pkg/rancher/rancherfakes/fake_client.go @@ -23,16 +23,6 @@ type FakeClient struct { result1 rancher.KubeconfigToken result2 error } - WithPasswordStub func(string) - withPasswordMutex sync.RWMutex - withPasswordArgsForCall []struct { - arg1 string - } - WithTransportStub func(*http.Transport) - withTransportMutex sync.RWMutex - withTransportArgsForCall []struct { - arg1 *http.Transport - } WithURLStub func(string) withURLMutex sync.RWMutex withURLArgsForCall []struct { @@ -43,6 +33,16 @@ type FakeClient struct { withUsernameArgsForCall []struct { arg1 string } + WithPasswordStub func(string) + withPasswordMutex sync.RWMutex + withPasswordArgsForCall []struct { + arg1 string + } + WithTransportStub func(*http.Transport) + withTransportMutex sync.RWMutex + withTransportArgsForCall []struct { + arg1 *http.Transport + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -61,8 +61,7 @@ func (fake *FakeClient) NewToken(arg1 context.Context) (rancher.KubeconfigToken, if specificReturn { return ret.result1, ret.result2 } - fakeReturns := fake.newTokenReturns - return fakeReturns.result1, fakeReturns.result2 + return fake.newTokenReturns.result1, fake.newTokenReturns.result2 } func (fake *FakeClient) NewTokenCallCount() int { @@ -71,22 +70,13 @@ func (fake *FakeClient) NewTokenCallCount() int { return len(fake.newTokenArgsForCall) } -func (fake *FakeClient) NewTokenCalls(stub func(context.Context) (rancher.KubeconfigToken, error)) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() - fake.NewTokenStub = stub -} - func (fake *FakeClient) NewTokenArgsForCall(i int) context.Context { fake.newTokenMutex.RLock() defer fake.newTokenMutex.RUnlock() - argsForCall := fake.newTokenArgsForCall[i] - return argsForCall.arg1 + return fake.newTokenArgsForCall[i].arg1 } func (fake *FakeClient) NewTokenReturns(result1 rancher.KubeconfigToken, result2 error) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() fake.NewTokenStub = nil fake.newTokenReturns = struct { result1 rancher.KubeconfigToken @@ -95,8 +85,6 @@ func (fake *FakeClient) NewTokenReturns(result1 rancher.KubeconfigToken, result2 } func (fake *FakeClient) NewTokenReturnsOnCall(i int, result1 rancher.KubeconfigToken, result2 error) { - fake.newTokenMutex.Lock() - defer fake.newTokenMutex.Unlock() fake.NewTokenStub = nil if fake.newTokenReturnsOnCall == nil { fake.newTokenReturnsOnCall = make(map[int]struct { @@ -110,68 +98,6 @@ func (fake *FakeClient) NewTokenReturnsOnCall(i int, result1 rancher.KubeconfigT }{result1, result2} } -func (fake *FakeClient) WithPassword(arg1 string) { - fake.withPasswordMutex.Lock() - fake.withPasswordArgsForCall = append(fake.withPasswordArgsForCall, struct { - arg1 string - }{arg1}) - fake.recordInvocation("WithPassword", []interface{}{arg1}) - fake.withPasswordMutex.Unlock() - if fake.WithPasswordStub != nil { - fake.WithPasswordStub(arg1) - } -} - -func (fake *FakeClient) WithPasswordCallCount() int { - fake.withPasswordMutex.RLock() - defer fake.withPasswordMutex.RUnlock() - return len(fake.withPasswordArgsForCall) -} - -func (fake *FakeClient) WithPasswordCalls(stub func(string)) { - fake.withPasswordMutex.Lock() - defer fake.withPasswordMutex.Unlock() - fake.WithPasswordStub = stub -} - -func (fake *FakeClient) WithPasswordArgsForCall(i int) string { - fake.withPasswordMutex.RLock() - defer fake.withPasswordMutex.RUnlock() - argsForCall := fake.withPasswordArgsForCall[i] - return argsForCall.arg1 -} - -func (fake *FakeClient) WithTransport(arg1 *http.Transport) { - fake.withTransportMutex.Lock() - fake.withTransportArgsForCall = append(fake.withTransportArgsForCall, struct { - arg1 *http.Transport - }{arg1}) - fake.recordInvocation("WithTransport", []interface{}{arg1}) - fake.withTransportMutex.Unlock() - if fake.WithTransportStub != nil { - fake.WithTransportStub(arg1) - } -} - -func (fake *FakeClient) WithTransportCallCount() int { - fake.withTransportMutex.RLock() - defer fake.withTransportMutex.RUnlock() - return len(fake.withTransportArgsForCall) -} - -func (fake *FakeClient) WithTransportCalls(stub func(*http.Transport)) { - fake.withTransportMutex.Lock() - defer fake.withTransportMutex.Unlock() - fake.WithTransportStub = stub -} - -func (fake *FakeClient) WithTransportArgsForCall(i int) *http.Transport { - fake.withTransportMutex.RLock() - defer fake.withTransportMutex.RUnlock() - argsForCall := fake.withTransportArgsForCall[i] - return argsForCall.arg1 -} - func (fake *FakeClient) WithURL(arg1 string) { fake.withURLMutex.Lock() fake.withURLArgsForCall = append(fake.withURLArgsForCall, struct { @@ -190,17 +116,10 @@ func (fake *FakeClient) WithURLCallCount() int { return len(fake.withURLArgsForCall) } -func (fake *FakeClient) WithURLCalls(stub func(string)) { - fake.withURLMutex.Lock() - defer fake.withURLMutex.Unlock() - fake.WithURLStub = stub -} - func (fake *FakeClient) WithURLArgsForCall(i int) string { fake.withURLMutex.RLock() defer fake.withURLMutex.RUnlock() - argsForCall := fake.withURLArgsForCall[i] - return argsForCall.arg1 + return fake.withURLArgsForCall[i].arg1 } func (fake *FakeClient) WithUsername(arg1 string) { @@ -221,17 +140,58 @@ func (fake *FakeClient) WithUsernameCallCount() int { return len(fake.withUsernameArgsForCall) } -func (fake *FakeClient) WithUsernameCalls(stub func(string)) { - fake.withUsernameMutex.Lock() - defer fake.withUsernameMutex.Unlock() - fake.WithUsernameStub = stub -} - func (fake *FakeClient) WithUsernameArgsForCall(i int) string { fake.withUsernameMutex.RLock() defer fake.withUsernameMutex.RUnlock() - argsForCall := fake.withUsernameArgsForCall[i] - return argsForCall.arg1 + return fake.withUsernameArgsForCall[i].arg1 +} + +func (fake *FakeClient) WithPassword(arg1 string) { + fake.withPasswordMutex.Lock() + fake.withPasswordArgsForCall = append(fake.withPasswordArgsForCall, struct { + arg1 string + }{arg1}) + fake.recordInvocation("WithPassword", []interface{}{arg1}) + fake.withPasswordMutex.Unlock() + if fake.WithPasswordStub != nil { + fake.WithPasswordStub(arg1) + } +} + +func (fake *FakeClient) WithPasswordCallCount() int { + fake.withPasswordMutex.RLock() + defer fake.withPasswordMutex.RUnlock() + return len(fake.withPasswordArgsForCall) +} + +func (fake *FakeClient) WithPasswordArgsForCall(i int) string { + fake.withPasswordMutex.RLock() + defer fake.withPasswordMutex.RUnlock() + return fake.withPasswordArgsForCall[i].arg1 +} + +func (fake *FakeClient) WithTransport(arg1 *http.Transport) { + fake.withTransportMutex.Lock() + fake.withTransportArgsForCall = append(fake.withTransportArgsForCall, struct { + arg1 *http.Transport + }{arg1}) + fake.recordInvocation("WithTransport", []interface{}{arg1}) + fake.withTransportMutex.Unlock() + if fake.WithTransportStub != nil { + fake.WithTransportStub(arg1) + } +} + +func (fake *FakeClient) WithTransportCallCount() int { + fake.withTransportMutex.RLock() + defer fake.withTransportMutex.RUnlock() + return len(fake.withTransportArgsForCall) +} + +func (fake *FakeClient) WithTransportArgsForCall(i int) *http.Transport { + fake.withTransportMutex.RLock() + defer fake.withTransportMutex.RUnlock() + return fake.withTransportArgsForCall[i].arg1 } func (fake *FakeClient) Invocations() map[string][][]interface{} { @@ -239,14 +199,14 @@ func (fake *FakeClient) Invocations() map[string][][]interface{} { defer fake.invocationsMutex.RUnlock() fake.newTokenMutex.RLock() defer fake.newTokenMutex.RUnlock() - fake.withPasswordMutex.RLock() - defer fake.withPasswordMutex.RUnlock() - fake.withTransportMutex.RLock() - defer fake.withTransportMutex.RUnlock() fake.withURLMutex.RLock() defer fake.withURLMutex.RUnlock() fake.withUsernameMutex.RLock() defer fake.withUsernameMutex.RUnlock() + fake.withPasswordMutex.RLock() + defer fake.withPasswordMutex.RUnlock() + fake.withTransportMutex.RLock() + defer fake.withTransportMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value From 0c6ba6de0e8a393051d50f811e4751ba493f2c19 Mon Sep 17 00:00:00 2001 From: Billy Bolton Date: Fri, 26 Mar 2021 14:43:22 -0400 Subject: [PATCH 2/3] move rancher mux --- pkg/http/token.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/http/token.go b/pkg/http/token.go index 0b5508b..42f2437 100644 --- a/pkg/http/token.go +++ b/pkg/http/token.go @@ -57,10 +57,10 @@ func getGoogleToken(c *gin.Context) { } func getRancherToken(c *gin.Context) { - rancherMux.Lock() - defer rancherMux.Unlock() - if time.Now().In(time.UTC).After(kubeconfigToken.ExpiresAt) || kubeconfigToken.Token == "" { + rancherMux.Lock() + defer rancherMux.Unlock() + rancherClient := rancher.Instance(c) if rancherClient == nil { c.JSON(http.StatusBadRequest, gin.H{"error": "token provider not configured: rancher"}) From 608c541d10d5872eb25efed18859ec50aff0f205 Mon Sep 17 00:00:00 2001 From: Billy Bolton Date: Fri, 26 Mar 2021 14:54:18 -0400 Subject: [PATCH 3/3] add comment around expiry --- pkg/http/token.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/http/token.go b/pkg/http/token.go index 42f2437..6e2a366 100644 --- a/pkg/http/token.go +++ b/pkg/http/token.go @@ -50,6 +50,7 @@ func getGoogleToken(c *gin.Context) { } // Set the expiration for the Google token to be 90% expiry-threshold. + // Expiry looks something like '2021-03-26 15:53:24.513497 -0400 EDT m=+3599.302993422' googleExpiration = time.Now().UTC().Add((time.Until(googleToken.Expiry) / 10) * 9) }