From 5b9eb6ec84df0ad9da70e32d890adaecf0f2f35a Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 10 Apr 2020 15:17:25 -0700 Subject: [PATCH] fix: Deferring credential loading until required --- auth/auth.go | 8 ++++-- auth/auth_test.go | 63 +++++++++++++++++++++++++++++++++++++++----- firebase.go | 39 +++++++++++++-------------- firebase_test.go | 41 +++++++++------------------- internal/internal.go | 2 -- 5 files changed, 94 insertions(+), 59 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index c12a82ed..b5088806 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -57,14 +57,18 @@ func NewClient(ctx context.Context, conf *internal.AuthConfig) (*Client, error) signer cryptoSigner err error ) + + creds, _ := transport.Creds(ctx, conf.Opts...) + // Initialize a signer by following the go/firebase-admin-sign protocol. - if conf.Creds != nil && len(conf.Creds.JSON) > 0 { + if creds != nil && len(creds.JSON) > 0 { // If the SDK was initialized with a service account, use it to sign bytes. - signer, err = signerFromCreds(conf.Creds.JSON) + signer, err = signerFromCreds(creds.JSON) if err != nil && err != errNotAServiceAcct { return nil, err } } + if signer == nil { if conf.ServiceAccountID != "" { // If the SDK was initialized with a service account email, use it with the IAM service diff --git a/auth/auth_test.go b/auth/auth_test.go index 72ad19d9..1e8e1098 100644 --- a/auth/auth_test.go +++ b/auth/auth_test.go @@ -34,6 +34,7 @@ import ( ) const ( + credEnvVar = "GOOGLE_APPLICATION_CREDENTIALS" testProjectID = "mock-project-id" testVersion = "test-version" ) @@ -82,7 +83,6 @@ func TestNewClientWithServiceAccountCredentials(t *testing.T) { t.Fatal(err) } client, err := NewClient(context.Background(), &internal.AuthConfig{ - Creds: creds, Opts: optsWithServiceAcct, ProjectID: creds.ProjectID, Version: testVersion, @@ -176,7 +176,6 @@ func TestNewClientWithUserCredentials(t *testing.T) { }`), } conf := &internal.AuthConfig{ - Creds: creds, Opts: []option.ClientOption{option.WithCredentials(creds)}, Version: testVersion, } @@ -206,7 +205,11 @@ func TestNewClientWithMalformedCredentials(t *testing.T) { creds := &google.DefaultCredentials{ JSON: []byte("not json"), } - conf := &internal.AuthConfig{Creds: creds} + conf := &internal.AuthConfig{ + Opts: []option.ClientOption{ + option.WithCredentials(creds), + }, + } if c, err := NewClient(context.Background(), conf); c != nil || err == nil { t.Errorf("NewClient() = (%v,%v); want = (nil, error)", c, err) } @@ -222,12 +225,61 @@ func TestNewClientWithInvalidPrivateKey(t *testing.T) { t.Fatal(err) } creds := &google.DefaultCredentials{JSON: b} - conf := &internal.AuthConfig{Creds: creds} + conf := &internal.AuthConfig{ + Opts: []option.ClientOption{ + option.WithCredentials(creds), + }, + } if c, err := NewClient(context.Background(), conf); c != nil || err == nil { t.Errorf("NewClient() = (%v,%v); want = (nil, error)", c, err) } } +func TestNewClientAppDefaultCredentialsWithInvalidFile(t *testing.T) { + current := os.Getenv(credEnvVar) + + if err := os.Setenv(credEnvVar, "../testdata/non_existing.json"); err != nil { + t.Fatal(err) + } + defer os.Setenv(credEnvVar, current) + + conf := &internal.AuthConfig{} + if c, err := NewClient(context.Background(), conf); c != nil || err == nil { + t.Errorf("Auth() = (%v, %v); want (nil, error)", c, err) + } +} + +func TestNewClientInvalidCredentialFile(t *testing.T) { + invalidFiles := []string{ + "testdata", + "testdata/plain_text.txt", + } + + ctx := context.Background() + for _, tc := range invalidFiles { + conf := &internal.AuthConfig{ + Opts: []option.ClientOption{ + option.WithCredentialsFile(tc), + }, + } + if c, err := NewClient(ctx, conf); c != nil || err == nil { + t.Errorf("Auth() = (%v, %v); want (nil, error)", c, err) + } + } +} + +func TestNewClientExplicitNoAuth(t *testing.T) { + ctx := context.Background() + conf := &internal.AuthConfig{ + Opts: []option.ClientOption{ + option.WithoutAuthentication(), + }, + } + if c, err := NewClient(ctx, conf); c == nil || err != nil { + t.Errorf("Auth() = (%v, %v); want (auth, nil)", c, err) + } +} + func TestCustomToken(t *testing.T) { client := &Client{ signer: testSigner, @@ -298,8 +350,7 @@ func TestCustomTokenError(t *testing.T) { func TestCustomTokenInvalidCredential(t *testing.T) { ctx := context.Background() conf := &internal.AuthConfig{ - Creds: nil, - Opts: optsWithTokenSource, + Opts: optsWithTokenSource, } s, err := NewClient(ctx, conf) if err != nil { diff --git a/firebase.go b/firebase.go index d66b1fb0..62426ed1 100644 --- a/firebase.go +++ b/firebase.go @@ -31,7 +31,6 @@ import ( "firebase.google.com/go/internal" "firebase.google.com/go/messaging" "firebase.google.com/go/storage" - "golang.org/x/oauth2/google" "google.golang.org/api/option" "google.golang.org/api/transport" ) @@ -47,7 +46,6 @@ const firebaseEnvName = "FIREBASE_CONFIG" // An App holds configuration and state common to all Firebase services that are exposed from the SDK. type App struct { authOverride map[string]interface{} - creds *google.DefaultCredentials dbURL string projectID string serviceAccountID string @@ -67,7 +65,6 @@ type Config struct { // Auth returns an instance of auth.Client. func (a *App) Auth(ctx context.Context) (*auth.Client, error) { conf := &internal.AuthConfig{ - Creds: a.creds, ProjectID: a.projectID, Opts: a.opts, ServiceAccountID: a.serviceAccountID, @@ -142,28 +139,14 @@ func (a *App) Messaging(ctx context.Context) (*messaging.Client, error) { func NewApp(ctx context.Context, config *Config, opts ...option.ClientOption) (*App, error) { o := []option.ClientOption{option.WithScopes(internal.FirebaseScopes...)} o = append(o, opts...) - creds, err := transport.Creds(ctx, o...) - if err != nil { - return nil, err - } if config == nil { + var err error if config, err = getConfigDefaults(); err != nil { return nil, err } } - var pid string - if config.ProjectID != "" { - pid = config.ProjectID - } else if creds.ProjectID != "" { - pid = creds.ProjectID - } else { - pid = os.Getenv("GOOGLE_CLOUD_PROJECT") - if pid == "" { - pid = os.Getenv("GCLOUD_PROJECT") - } - } - + pid := getProjectID(ctx, config, o...) ao := defaultAuthOverrides if config.AuthOverride != nil { ao = *config.AuthOverride @@ -171,7 +154,6 @@ func NewApp(ctx context.Context, config *Config, opts ...option.ClientOption) (* return &App{ authOverride: ao, - creds: creds, dbURL: config.DatabaseURL, projectID: pid, serviceAccountID: config.ServiceAccountID, @@ -213,3 +195,20 @@ func getConfigDefaults() (*Config, error) { } return fbc, nil } + +func getProjectID(ctx context.Context, config *Config, opts ...option.ClientOption) string { + if config.ProjectID != "" { + return config.ProjectID + } + + creds, _ := transport.Creds(ctx, opts...) + if creds != nil && creds.ProjectID != "" { + return creds.ProjectID + } + + if pid := os.Getenv("GOOGLE_CLOUD_PROJECT"); pid != "" { + return pid + } + + return os.Getenv("GCLOUD_PROJECT") +} diff --git a/firebase_test.go b/firebase_test.go index 831712f4..1b367f1c 100644 --- a/firebase_test.go +++ b/firebase_test.go @@ -58,11 +58,6 @@ func TestServiceAcctFile(t *testing.T) { if len(app.opts) != 2 { t.Errorf("Client opts: %d; want: 2", len(app.opts)) } - if app.creds == nil { - t.Error("Credentials: nil; want creds") - } else if len(app.creds.JSON) == 0 { - t.Error("JSON: empty; want; non-empty") - } } func TestClientOptions(t *testing.T) { @@ -116,11 +111,6 @@ func TestRefreshTokenFile(t *testing.T) { if len(app.opts) != 2 { t.Errorf("Client opts: %d; want: 2", len(app.opts)) } - if app.creds == nil { - t.Error("Credentials: nil; want creds") - } else if len(app.creds.JSON) == 0 { - t.Error("JSON: empty; want; non-empty") - } } func TestRefreshTokenFileWithConfig(t *testing.T) { @@ -135,11 +125,6 @@ func TestRefreshTokenFileWithConfig(t *testing.T) { if len(app.opts) != 2 { t.Errorf("Client opts: %d; want: 2", len(app.opts)) } - if app.creds == nil { - t.Error("Credentials: nil; want creds") - } else if len(app.creds.JSON) == 0 { - t.Error("JSON: empty; want; non-empty") - } } func TestRefreshTokenWithEnvVar(t *testing.T) { @@ -158,11 +143,6 @@ func TestRefreshTokenWithEnvVar(t *testing.T) { if app.projectID != "mock-project-id" { t.Errorf("[env=%s] Project ID: %q; want: mock-project-id", varName, app.projectID) } - if app.creds == nil { - t.Errorf("[env=%s] Credentials: nil; want creds", varName) - } else if len(app.creds.JSON) == 0 { - t.Errorf("[env=%s] JSON: empty; want; non-empty", varName) - } } for _, varName := range []string{"GCLOUD_PROJECT", "GOOGLE_CLOUD_PROJECT"} { verify(varName) @@ -185,11 +165,6 @@ func TestAppDefault(t *testing.T) { if len(app.opts) != 1 { t.Errorf("Client opts: %d; want: 1", len(app.opts)) } - if app.creds == nil { - t.Error("Credentials: nil; want creds") - } else if len(app.creds.JSON) == 0 { - t.Error("JSON: empty; want; non-empty") - } } func TestAppDefaultWithInvalidFile(t *testing.T) { @@ -201,8 +176,8 @@ func TestAppDefaultWithInvalidFile(t *testing.T) { defer os.Setenv(credEnvVar, current) app, err := NewApp(context.Background(), nil) - if app != nil || err == nil { - t.Errorf("NewApp() = (%v, %v); want: (nil, error)", app, err) + if app == nil || err != nil { + t.Fatalf("NewApp() = (%v, %v); want = (app, nil)", app, err) } } @@ -215,12 +190,20 @@ func TestInvalidCredentialFile(t *testing.T) { ctx := context.Background() for _, tc := range invalidFiles { app, err := NewApp(ctx, nil, option.WithCredentialsFile(tc)) - if app != nil || err == nil { - t.Errorf("NewApp(%q) = (%v, %v); want: (nil, error)", tc, app, err) + if app == nil || err != nil { + t.Fatalf("NewApp() = (%v, %v); want = (app, nil)", app, err) } } } +func TestExplicitNoAuth(t *testing.T) { + ctx := context.Background() + app, err := NewApp(ctx, nil, option.WithoutAuthentication()) + if app == nil || err != nil { + t.Fatalf("NewApp() = (%v, %v); want = (app, nil)", app, err) + } +} + func TestAuth(t *testing.T) { ctx := context.Background() app, err := NewApp(ctx, nil, option.WithCredentialsFile("testdata/service_account.json")) diff --git a/internal/internal.go b/internal/internal.go index d5164e74..8edc7e4d 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -20,7 +20,6 @@ import ( "time" "golang.org/x/oauth2" - "golang.org/x/oauth2/google" "google.golang.org/api/option" ) @@ -40,7 +39,6 @@ var SystemClock = &systemClock{} // AuthConfig represents the configuration of Firebase Auth service. type AuthConfig struct { Opts []option.ClientOption - Creds *google.DefaultCredentials ProjectID string ServiceAccountID string Version string