google: fix nomenclature feedback and changed time.Now implementation to allow for testing for time in unit tests. The unit tests will not pass at the moment; awaiting feedback on whether to use a test credential source or rely on a separate one.

Change-Id: I20605fa161911f325ab41fc345436d08ed17ed5e

google/internal/externalaccount/basecredentials.go

@@ -12,6 +12,9 @@ import (
 	"time"
 )
 
+// now aliases time.Now for testing
+var now = time.Now
+
 // Config stores the configuration for fetching tokens with external credentials.
 type Config struct {
 	Audience                       string
@@ -64,15 +67,9 @@ type CredentialSource struct {
 }
 
 // instance determines the type of CredentialSource needed
-func (cs CredentialSource) instance() baseCredentialSource {
+func (c *Config) parse() baseCredentialSource {
-	if cs.EnvironmentID == "awsX" {
+	if c.CredentialSource.File != "" {
-		return nil
+		return fileCredentialSource{File: c.CredentialSource.File}
-	} else if cs.File == "internalTestingFile" {
-		return testCredentialSource{}
-	} else if cs.File != "" {
-		return fileCredentialSource{File: cs.File}
-	} else if cs.URL != "" {
-		return nil
 	}
 	return nil
 }
@@ -91,7 +88,7 @@ type tokenSource struct {
 func (ts tokenSource) Token() (*oauth2.Token, error) {
 	conf := ts.conf
 
-	subjectToken, err := conf.CredentialSource.instance().retrieveSubjectToken(conf)
+	subjectToken, err := conf.parse().retrieveSubjectToken(conf)
 	if err != nil {
 		return nil, err
 	}
@@ -122,8 +119,9 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
 	if stsResp.ExpiresIn < 0 {
 		return nil, fmt.Errorf("google/oauth2: got invalid expiry from security token service")
 	} else if stsResp.ExpiresIn > 0 {
-		accessToken.Expiry = time.Now().Add(time.Duration(stsResp.ExpiresIn) * time.Second)
+		accessToken.Expiry = now().Add(time.Duration(stsResp.ExpiresIn) * time.Second)
 	}
+	// TODO: For reviewers- what should I do if ExpiresIn is equal to 0?  Would that correspond to a one-use token, or something else?
 
 	if stsResp.RefreshToken != "" {
 		accessToken.RefreshToken = stsResp.RefreshToken
@@ -134,12 +132,3 @@ func (ts tokenSource) Token() (*oauth2.Token, error) {
 
 // NOTE: this method doesn't exist yet. It is being investigated to add this method to oauth2.TokenSource.
 //func (ts tokenSource) TokenInfo() (*oauth2.TokenInfo, error)
-
-// testCredentialSource is only used for testing, but must be defined here in order to avoid undefined errors when testing.
-type testCredentialSource struct {
-	File string
-}
-
-func (cs testCredentialSource) retrieveSubjectToken(c *Config) (string, error) {
-	return "Sample.Subject.Token", nil
-}

google/internal/externalaccount/basecredentials_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 )
 
 var testBaseCredSource = CredentialSource{
@@ -24,10 +25,15 @@ var testConfig = Config{
 	Scopes:                         []string{"https://www.googleapis.com/auth/devstorage.full_control"},
 }
 
-var baseCredsRequestBody = "audience=32555940559.apps.googleusercontent.com&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&options=null&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=Sample.Subject.Token&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt"
+var (
-var baseCredsResponseBody = `{"access_token":"Sample.Access.Token","issued_token_type":"urn:ietf:params:oauth:token-type:access_token","token_type":"Bearer","expires_in":3600,"scope":"https://www.googleapis.com/auth/cloud-platform"}`
+	baseCredsRequestBody        = "audience=32555940559.apps.googleusercontent.com&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&options=null&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=Sample.Subject.Token&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt"
-
+	baseCredsResponseBody       = `{"access_token":"Sample.Access.Token","issued_token_type":"urn:ietf:params:oauth:token-type:access_token","token_type":"Bearer","expires_in":3600,"scope":"https://www.googleapis.com/auth/cloud-platform"}`
-var correctAT = "Sample.Access.Token"
+	correctAT                   = "Sample.Access.Token"
+	expiry                int64 = 234852
+)
+var (
+	testNow = func() time.Time { return time.Unix(expiry, 0) }
+)
 
 func TestToken_Func(t *testing.T) {
 
@@ -63,16 +69,25 @@ func TestToken_Func(t *testing.T) {
 		conf: &testConfig,
 	}
 
+	oldNow := now
+	defer func() { now = oldNow }()
+	now = testNow
+
 	tok, err := ourTS.Token()
 	if err != nil {
 		t.Errorf("Unexpected error: %e", err)
 	}
-	if tok.AccessToken != correctAT {
+	if got, want := tok.AccessToken, correctAT; got != want {
-		t.Errorf("Unexpected access token: got %v, but wanted %v", tok.AccessToken, correctAT)
+		t.Errorf("Unexpected access token: got %v, but wanted %v", got, want)
 	}
-	if tok.TokenType != "Bearer" {
+	if got, want := tok.TokenType, "Bearer"; got != want {
-		t.Errorf("Unexpected TokenType: got %v, but wanted \"Bearer\"", tok.TokenType)
+		t.Errorf("Unexpected TokenType: got %v, but wanted %v", got, want)
 	}
+
+	if got, want := tok.Expiry, now().Add(time.Duration(3600)*time.Second); got != want {
+		t.Errorf("Unexpected Expiry: got %v, but wanted %v", got, want)
+	}
+
 	//We don't check the correct expiry here because that's dependent on the current time.
 
 }

google/internal/externalaccount/filecredsource_test.go

@@ -50,7 +50,7 @@ func TestRetrieveFileSubjectToken(t *testing.T) {
 		tfc := testFileConfig
 		tfc.CredentialSource = test.cs
 
-		out, err := test.cs.instance().retrieveSubjectToken(&tfc)
+		out, err := tfc.parse().retrieveSubjectToken(&tfc)
 		if err != nil {
 			t.Errorf("Method retrieveSubjectToken for type fileCredentialSource in test %v failed; %e", test.name, err)
 		}