Support multiple accounts by samtstern · Pull Request #3192 · firebase/firebase-tools

samtstern · 2021-03-05T17:38:57Z

Description

Implement multiple authentication (see Internal API proposal)

Fixes #1906
Fixes #181

Scenarios Tested

login:add

$ firebase login:add

Visit this URL on this device to log in:
https://accounts.google.com/o/oauth2/auth?client_id=563584335869-fgrhgmd47bqnekij5i8b5pr03ho849e6.apps.googleusercontent.com&scope=email%20openid%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloudplatformprojects.readonly%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Ffirebase%20https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fcloud-platform&response_type=code&state=75254219&redirect_uri=http%3A%2F%2Flocalhost%3A9005

Waiting for authentication...

✔  Success! Added account samsternota@gmail.com

login:list

$ firebase login:list
Logged in as samsterntest@gmail.com

Other available accounts (switch with login:use)
 - samstern@google.com
 - samsternota@gmail.com

login:use

$ firebase login:use samsternota@gmail.com
✔  Set default account samsternota@gmail.com for current project directory.

# ...

$ firebase login:list
Logged in as samsternota@gmail.com

Other available accounts (switch with login:use)
 - samstern@google.com
 - samsterntest@gmail.com

logout

$ firebase logout
✔  Logged out from samstern@google.com
✔  Logged out from samsterntest@gmail.com
✔  Logged out from samsternota@gmail.com

logout (primary account

$ firebase logout samstern@google.com
? You are logging out of your default account, which account should become the 
new default? samsternota@gmail.com
✔  Logged out from samstern@google.com
✔  Setting default account to "samsternota@gmail.com"

# ...

$ firebase login:list
Logged in as samsternota@gmail.com

Other available accounts (switch with "firebase login:use")
 - samsterntest@gmail.com

init

$ firebase init database

     ######## #### ########  ######## ########     ###     ######  ########
     ##        ##  ##     ## ##       ##     ##  ##   ##  ##       ##
     ######    ##  ########  ######   ########  #########  ######  ######
     ##        ##  ##    ##  ##       ##     ## ##     ##       ## ##
     ##       #### ##     ## ######## ########  ##     ##  ######  ########

You're about to initialize a Firebase project in this directory:

  /private/var/folders/xl/6lkrzp7j07581mw8_4dlt3b000643s/T/tmp.2OgG9jvU

Before we get started, keep in mind:

  * You are currently outside your home directory


=== Account Setup

Which account do you want to use for this project? Choose an account or add a new one now

? Please select an option: samsternota@gmail.com

✔  Using account: samsternota@gmail.com

=== Project Setup

First, let's associate this project directory with a Firebase project.
You can create multiple project aliases by running firebase use --add, 
but for now we'll just set up a default project.

? Please select an option: Use an existing project
? Select a default Firebase project for this directory: fir-dumpster-ota (fir-du
mpster-ota)
i  Using project fir-dumpster-ota (fir-dumpster-ota)

=== Database Setup
i  database: ensuring required API firebasedatabase.googleapis.com is enabled...
⚠  database: missing required API firebasedatabase.googleapis.com. Enabling now...
✔  database: required API firebasedatabase.googleapis.com is enabled

? It seems like you haven’t initialized Realtime Database in your project yet. D
o you want to set it up? No

Firebase Realtime Database Security Rules allow you to define how your data should be
structured and when your data can be read from and written to.

? What file should be used for Realtime Database Security Rules? database.rules.
json
✔  Default rules have been written to database.rules.json.
Future modifications to database.rules.json will update Realtime Database Security Rules when you run
firebase deploy.

i  Writing configuration info to firebase.json...
i  Writing project information to .firebaserc...
i  Writing gitignore file to .gitignore...

✔  Firebase initialization complete!

--account flag (denied)

$ firebase --account=samstern@google.com --project=fir-dumpster-ota functions:config:get

Error: HTTP Error: 403, The caller does not have permission

--account flag (succeed)

$ firebase --account=samsternota@gmail.com --project=fir-dumpster-ota functions:config:get
{
  "foo": {
    "baz": "qux"
  }
}

Sample Commands

Internal API proposal

samtstern · 2021-03-05T17:40:04Z

@@ -1,50 +0,0 @@
-"use strict";


Re-written in TS

joehan · 2021-03-09T17:17:28Z

Did some testing, and noticed that firebase init still prompts me to choose an account if I use the --account flag.
Based on the API proposal, I think this should just use the account specified in the flag

samtstern · 2021-03-09T17:23:40Z

Did some testing, and noticed that firebase init still prompts me to choose an account if I use the --account flag.
Based on the API proposal, I think this should just use the account specified in the flag

@joehan thanks! I just pushed a fix for this, I always forget about "inherited options"

joehan

Some cleanup, but most of the core logic seems good to me

joehan · 2021-03-15T16:30:02Z

+
+  // Clear any matching project defaults
+  const activeAccounts = configstore.get("activeAccounts") || {};
+  for (const projectDir of Object.keys(activeAccounts)) {


Prefer using for.. in here instead of Object.keys, unless theres something reason not to

I changed it but then eslint yelled at me about guarding it, which I think would be uglier:
https://eslint.org/docs/rules/guard-for-in

Ahhh, yeah, I ran into that same rule recently. Maybe we should get rid of the rule, but I agree this is less ugly than adding an eslint-ignore or guarding it

joehan · 2021-03-15T16:36:21Z

-      configstore.set("tokens", lastAccessToken);
+    const account = findAccountByRefreshToken(refreshToken);
+    if (account && lastAccessToken) {
+      account.tokens = lastAccessToken;


Nit: Why is this account.tokens if it only has one token?

Trying to keep terminology similar where I can. The "tokens" key has been used in configstore to refer to the object of type Tokens which contains access and refresh tokens. This is the same type.

joehan

LGTM once you fix the typo in commands/index.js

Probably deserves a second set of eyes though given the scope.

Co-authored-by: joehan <joehanley@google.com>

bkendall · 2021-03-17T20:43:07Z

@@ -0,0 +1 @@
+- Adds support for multiple accounts via new commands `login:use`, `login:add` and `login:list`.


This feels like a feature that could do with (a) documentation in the README and (b) documentation in FireSite

cc @rachelsaunders I added a README section, can you take a look?

bkendall · 2021-03-17T20:45:32Z

+ * Get the default account associated with a project directory, or the global default.
+ * @param projectDir the Firebase project directory.
+ */
+export function getProjectDefaultAccount(projectDir?: string | null): Account | undefined {


Can projectDir really be null? What does that mean rather than just an empty string?

This comes from detectProjectRoot which sets options.projectRoot to string | null ... it's a nasty type but I want the code to be honest about what options.projectRoot (which is typed as any) can be and because detectProjectRoot is not always called both undefined and null are possible.

bkendall · 2021-03-17T20:47:24Z

+/**
+ * Get all authenticated accounts _besides_ the default account.
+ */
+export function getAdditionalAccounts(): Account[] {


I'm noticing that all these new functions are exported - is that necessary? Some of these seem like helpers...

They're all used in at least one other file.

bkendall · 2021-03-17T20:56:58Z

+      );
+    }
+
+    const projectDir = options.projectRoot as string | null;


Why not just options.projectRoot as string || ""? (I'm not a fan of using null if I can help it, as you may have guessed by now)

This is the actual type based on detectProjectRoot, the as is just because I don't want to use any.

…to ss-multi-auth

bkendall

I didn't hammer every corner, I'm sure, but I pulled this down and played with it a bit, init-ing and deploying things. Looks good to me!

Get basic commands in place

98faa1e

google-cla Bot added the cla: yes Manual indication that this has passed CLA. label Mar 5, 2021

samtstern commented Mar 5, 2021

View reviewed changes

Comment thread src/commands/logout.js

@@ -1,50 +0,0 @@

"use strict";

Copy link
Copy Markdown

Contributor Author

samtstern Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-written in TS

samtstern added 9 commits March 8, 2021 15:40

Functions emulator args

75dcfdf

Logout

4f7aa73

Fix compile

1b203e3

Handle token refreshing

a83bf46

Lint

575fb0c

Fix tests

25d4347

Fix inherited option bug

8a06b92

Make firebase init work

e3d2e78

Add some docstrings, clean up some global state

8c9493a

samtstern changed the title ~~[WIP] Support multiple accounts~~ Support multiple accounts Mar 9, 2021

samtstern added 3 commits March 9, 2021 16:04

Unit tests

a8aef0a

Merge branch 'master' into ss-multi-auth

209e209

Changelog

65a9bf3

samtstern marked this pull request as ready for review March 9, 2021 16:05

samtstern requested review from bkendall, mbleigh and rachelsaunders March 9, 2021 16:06

Fix unit tests

b9f5379

Fix inherited option issue with account

0397763

joehan requested changes Mar 15, 2021

View reviewed changes

samtstern added 2 commits March 16, 2021 10:45

Review comments

c01acf5

Merge remote-tracking branch 'github/master' into ss-multi-auth

22c9cdd

samtstern requested a review from joehan March 16, 2021 10:49

Fix imports

851323f

joehan reviewed Mar 17, 2021

View reviewed changes

Comment thread src/commands/index.js Outdated

joehan approved these changes Mar 17, 2021

View reviewed changes

Comment thread src/defaultCredentials.ts Outdated

Comment thread src/management/projects.ts Outdated

samtstern and others added 2 commits March 17, 2021 14:20

Apply suggestions from code review

17e3dde

Co-authored-by: joehan <joehanley@google.com>

Merge branch 'master' into ss-multi-auth

0d409f8

bkendall requested changes Mar 17, 2021

View reviewed changes

samtstern added 3 commits March 23, 2021 11:57

Merge remote-tracking branch 'github/master' into ss-multi-auth

d8754a4

Review comments

12fa104

Merge branch 'ss-multi-auth' of github.com:firebase/firebase-tools in…

bfd9211

…to ss-multi-auth

samtstern requested a review from bkendall March 23, 2021 12:20

bkendall approved these changes Mar 24, 2021

View reviewed changes

samtstern added 4 commits April 6, 2021 13:38

Prompt for new account if logging out of global default

a6a5cd7

Fix duplicate accounts

b2bf7ca

Fix requirePermissions issue

faea732

Merge branch 'master' into ss-multi-auth

f3ec559

samtstern added cla: yes Manual indication that this has passed CLA. and removed cla: yes Manual indication that this has passed CLA. labels Apr 6, 2021

samtstern merged commit 0dd344d into master Apr 6, 2021

gavinsharp mentioned this pull request Apr 7, 2021

fix typo in README #3264

Merged

mikehardy mentioned this pull request Apr 24, 2021

deployFile.js stops working on 9.9.0+ 'FirebaseError: Failed to authenticate, have you run firebase login?' #3308

Closed

bkendall deleted the ss-multi-auth branch August 4, 2021 19:27

devpeerapong pushed a commit to devpeerapong/firebase-tools that referenced this pull request Dec 14, 2021

Support multiple accounts (firebase#3192)

14089cf

		@@ -0,0 +1 @@
		- Adds support for multiple accounts via new commands `login:use`, `login:add` and `login:list`.

Conversation

samtstern commented Mar 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Scenarios Tested

Sample Commands

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joehan commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samtstern commented Mar 9, 2021

Uh oh!

joehan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joehan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samtstern Mar 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bkendall left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samtstern commented Mar 5, 2021 •

edited

Loading

joehan commented Mar 9, 2021 •

edited

Loading

joehan left a comment •

edited

Loading

samtstern Mar 23, 2021 •

edited

Loading