From 85982932aab8bf3ef7d91ec48c61e33041c55776 Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 6 Sep 2017 10:43:31 -0700 Subject: [PATCH 1/3] =?UTF-8?q?Using=20the=20ClientOptions=20provided=20at?= =?UTF-8?q?=20App=20initialization=20to=20create=20the=20=E2=80=A6=20(#12)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Using the ClientOptions provided at App initialization to create the HTTPClient in auth package. * Fixed context import * Updated test case * Fixing a test failure; Calling transport.NewHTTPClient() only when ctx and opts are available to avoid an unnecessary default credentials lookup. --- auth/auth.go | 10 +++-- auth/auth_test.go | 5 ++- auth/crypto.go | 30 ++++++++++---- auth/crypto_test.go | 93 ++++++++++++++++++++++++++++++++------------ firebase.go | 2 + internal/internal.go | 5 +++ 6 files changed, 108 insertions(+), 37 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index c7f1b316..f2e237f5 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -71,8 +71,13 @@ type Client struct { // This function can only be invoked from within the SDK. Client applications should access the // the Auth service through firebase.App. func NewClient(c *internal.AuthConfig) (*Client, error) { + ks, err := newHTTPKeySource(c.Ctx, googleCertURL, c.Opts...) + if err != nil { + return nil, err + } + client := &Client{ - ks: newHTTPKeySource(googleCertURL), + ks: ks, projectID: c.ProjectID, } if c.Creds == nil || len(c.Creds.JSON) == 0 { @@ -83,8 +88,7 @@ func NewClient(c *internal.AuthConfig) (*Client, error) { ClientEmail string `json:"client_email"` PrivateKey string `json:"private_key"` } - err := json.Unmarshal(c.Creds.JSON, &svcAcct) - if err != nil { + if err := json.Unmarshal(c.Creds.JSON, &svcAcct); err != nil { return nil, err } diff --git a/auth/auth_test.go b/auth/auth_test.go index 4bf8b32d..90057e50 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -16,6 +16,7 @@ package auth import ( "errors" + "log" "os" "strings" "testing" @@ -98,7 +99,7 @@ func TestMain(m *testing.M) { opt := option.WithCredentialsFile("../testdata/service_account.json") creds, err = transport.Creds(context.Background(), opt) if err != nil { - os.Exit(1) + log.Fatalln(err) } client, err = NewClient(&internal.AuthConfig{ @@ -106,7 +107,7 @@ func TestMain(m *testing.M) { ProjectID: "mock-project-id", }) if err != nil { - os.Exit(1) + log.Fatalln(err) } client.ks = &fileKeySource{FilePath: "../testdata/public_certs.json"} diff --git a/auth/crypto.go b/auth/crypto.go index c1b00895..97fc39d1 100644 --- a/auth/crypto.go +++ b/auth/crypto.go @@ -29,6 +29,11 @@ import ( "strings" "sync" "time" + + "golang.org/x/net/context" + + "google.golang.org/api/option" + "google.golang.org/api/transport" ) type publicKey struct { @@ -70,12 +75,24 @@ type httpKeySource struct { Mutex *sync.Mutex } -func newHTTPKeySource(uri string) *httpKeySource { - return &httpKeySource{ - KeyURI: uri, - Clock: systemClock{}, - Mutex: &sync.Mutex{}, +func newHTTPKeySource(ctx context.Context, uri string, opts ...option.ClientOption) (*httpKeySource, error) { + var hc *http.Client + if ctx != nil && len(opts) > 0 { + var err error + hc, _, err = transport.NewHTTPClient(ctx, opts...) + if err != nil { + return nil, err + } + } else { + hc = http.DefaultClient } + + return &httpKeySource{ + KeyURI: uri, + HTTPClient: hc, + Clock: systemClock{}, + Mutex: &sync.Mutex{}, + }, nil } // Keys returns the RSA Public Keys hosted at this key source's URI. Refreshes the data if @@ -99,9 +116,6 @@ func (k *httpKeySource) hasExpired() bool { func (k *httpKeySource) refreshKeys() error { k.CachedKeys = nil - if k.HTTPClient == nil { - k.HTTPClient = http.DefaultClient - } resp, err := k.HTTPClient.Get(k.KeyURI) if err != nil { return err diff --git a/auth/crypto_test.go b/auth/crypto_test.go index 5dcb3c59..2616bde9 100644 --- a/auth/crypto_test.go +++ b/auth/crypto_test.go @@ -15,11 +15,16 @@ package auth import ( + "fmt" "io" "io/ioutil" "net/http" "testing" "time" + + "golang.org/x/net/context" + + "google.golang.org/api/option" ) type mockHTTPResponse struct { @@ -37,6 +42,27 @@ type mockReadCloser struct { closeCount int } +func newHTTPClient(data []byte) (*http.Client, *mockReadCloser) { + rc := &mockReadCloser{ + data: string(data), + closeCount: 0, + } + client := &http.Client{ + Transport: &mockHTTPResponse{ + Response: http.Response{ + Status: "200 OK", + StatusCode: 200, + Header: http.Header{ + "Cache-Control": {"public, max-age=100"}, + }, + Body: rc, + }, + Err: nil, + }, + } + return client, rc +} + func (r *mockReadCloser) Read(p []byte) (n int, err error) { if len(p) == 0 { return 0, nil @@ -61,39 +87,57 @@ func TestHTTPKeySource(t *testing.T) { t.Fatal(err) } - mc := &mockClock{now: time.Unix(0, 0)} - rc := &mockReadCloser{ - data: string(data), - closeCount: 0, + ks, err := newHTTPKeySource(context.Background(), "http://mock.url") + if err != nil { + t.Fatal(err) } - ks := newHTTPKeySource("http://mock.url") - ks.HTTPClient = &http.Client{ - Transport: &mockHTTPResponse{ - Response: http.Response{ - Status: "200 OK", - StatusCode: 200, - Header: http.Header{ - "Cache-Control": {"public, max-age=100"}, - }, - Body: rc, - }, - Err: nil, - }, + + if ks.HTTPClient == nil { + t.Errorf("HTTPClient = nil; want non-nil") } + hc, rc := newHTTPClient(data) + ks.HTTPClient = hc + if err := verifyHTTPKeySource(ks, rc); err != nil { + t.Fatal(err) + } +} + +func TestHTTPKeySourceWithClient(t *testing.T) { + data, err := ioutil.ReadFile("../testdata/public_certs.json") + if err != nil { + t.Fatal(err) + } + + hc, rc := newHTTPClient(data) + ks, err := newHTTPKeySource(context.Background(), "http://mock.url", option.WithHTTPClient(hc)) + if err != nil { + t.Fatal(err) + } + + if ks.HTTPClient != hc { + t.Errorf("HTTPClient = %v; want %v", ks.HTTPClient, hc) + } + if err := verifyHTTPKeySource(ks, rc); err != nil { + t.Fatal(err) + } +} + +func verifyHTTPKeySource(ks *httpKeySource, rc *mockReadCloser) error { + mc := &mockClock{now: time.Unix(0, 0)} ks.Clock = mc exp := time.Unix(100, 0) for i := 0; i <= 100; i++ { keys, err := ks.Keys() if err != nil { - t.Fatal(err) + return err } if len(keys) != 3 { - t.Errorf("Keys: %d; want: 3", len(keys)) + return fmt.Errorf("Keys: %d; want: 3", len(keys)) } else if rc.closeCount != 1 { - t.Errorf("HTTP calls: %d; want: 1", rc.closeCount) + return fmt.Errorf("HTTP calls: %d; want: 1", rc.closeCount) } else if ks.ExpiryTime != exp { - t.Errorf("Expiry: %v; want: %v", ks.ExpiryTime, exp) + return fmt.Errorf("Expiry: %v; want: %v", ks.ExpiryTime, exp) } mc.now = mc.now.Add(time.Second) } @@ -101,11 +145,12 @@ func TestHTTPKeySource(t *testing.T) { mc.now = time.Unix(101, 0) keys, err := ks.Keys() if err != nil { - t.Fatal(err) + return err } if len(keys) != 3 { - t.Errorf("Keys: %d; want: 3", len(keys)) + return fmt.Errorf("Keys: %d; want: 3", len(keys)) } else if rc.closeCount != 2 { - t.Errorf("HTTP calls: %d; want: 2", rc.closeCount) + return fmt.Errorf("HTTP calls: %d; want: 2", rc.closeCount) } + return nil } diff --git a/firebase.go b/firebase.go index 00b4d7fc..66a4a9c8 100644 --- a/firebase.go +++ b/firebase.go @@ -53,8 +53,10 @@ type Config struct { // Auth returns an instance of auth.Client. func (a *App) Auth() (*auth.Client, error) { conf := &internal.AuthConfig{ + Ctx: a.ctx, Creds: a.creds, ProjectID: a.projectID, + Opts: a.opts, } return auth.NewClient(conf) } diff --git a/internal/internal.go b/internal/internal.go index 43e25fc4..0c388b78 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -16,11 +16,16 @@ package internal import ( + "golang.org/x/net/context" + "google.golang.org/api/option" + "golang.org/x/oauth2/google" ) // AuthConfig represents the configuration of Firebase Auth service. type AuthConfig struct { + Ctx context.Context + Opts []option.ClientOption Creds *google.DefaultCredentials ProjectID string } From 4b922f626c29067b1d72a4f15c18011bd538abde Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Wed, 6 Sep 2017 11:06:01 -0700 Subject: [PATCH 2/3] Passing a non-nil context to AuthConfig during testing; Replacing Print+Exit calls with log.Fatal() (#13) --- auth/auth_test.go | 5 +++-- integration/auth_test.go | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/auth/auth_test.go b/auth/auth_test.go index 90057e50..e29bb7f5 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -103,6 +103,7 @@ func TestMain(m *testing.M) { } client, err = NewClient(&internal.AuthConfig{ + Ctx: context.Background(), Creds: creds, ProjectID: "mock-project-id", }) @@ -164,7 +165,7 @@ func TestCustomTokenError(t *testing.T) { } func TestCustomTokenInvalidCredential(t *testing.T) { - s, err := NewClient(&internal.AuthConfig{}) + s, err := NewClient(&internal.AuthConfig{Ctx: context.Background()}) if err != nil { t.Fatal(err) } @@ -227,7 +228,7 @@ func TestVerifyIDTokenError(t *testing.T) { } func TestNoProjectID(t *testing.T) { - c, err := NewClient(&internal.AuthConfig{Creds: creds}) + c, err := NewClient(&internal.AuthConfig{Ctx: context.Background(), Creds: creds}) if err != nil { t.Fatal(err) } diff --git a/integration/auth_test.go b/integration/auth_test.go index cbf0d92e..37bf07c6 100644 --- a/integration/auth_test.go +++ b/integration/auth_test.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "io/ioutil" + "log" "net/http" "os" "testing" @@ -37,18 +38,18 @@ var client *auth.Client func TestMain(m *testing.M) { flag.Parse() if testing.Short() { - fmt.Println("skipping auth integration tests in short mode.") + log.Println("skipping auth integration tests in short mode.") os.Exit(0) } app, err := internal.NewTestApp(context.Background()) if err != nil { - os.Exit(1) + log.Fatalln(err) } client, err = app.Auth() if err != nil { - os.Exit(1) + log.Fatalln(err) } os.Exit(m.Run()) From 7c502c9db3c81b9107395c49f10bb2bafbee2ded Mon Sep 17 00:00:00 2001 From: Hiranya Jayathilaka Date: Thu, 7 Sep 2017 11:20:35 -0700 Subject: [PATCH 3/3] Bumped version to 1.0.1 (#15) --- firebase.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase.go b/firebase.go index 66a4a9c8..44c675f0 100644 --- a/firebase.go +++ b/firebase.go @@ -35,7 +35,7 @@ var firebaseScopes = []string{ } // Version of the Firebase Go Admin SDK. -const Version = "1.0.0" +const Version = "1.0.1" // An App holds configuration and state common to all Firebase services that are exposed from the SDK. type App struct {