Dont try to use expired refresh tokens (#7445)

Author: joehan

CHANGELOG.md

@@ -0,0 +1 @@
+- Fix an issue where refresh_tokens would be used before checking whether they were expired. (#7442)

firebase-vscode/src/cli.ts

@@ -12,7 +12,7 @@ import { Options } from "../../src/options";
 import { currentOptions, getCommandOptions } from "./options";
 import { EmulatorUiSelections } from "../common/messaging/types";
 import { pluginLogger } from "./logger-wrapper";
-import { setAccessToken } from "../../src/apiv2";
+import { getAccessToken, setAccessToken } from "../../src/apiv2";
 import {
   startAll as startAllEmulators,
   cleanShutdown as stopAllEmulators,
@@ -56,7 +56,7 @@ export async function requireAuthWrapper(
     return (
       account &&
       account.user.email === optionsUser.email &&
-      account.tokens.access_token === optionsTokens.access_token // TODO: check if this is necessary
+      account.tokens.refresh_token === optionsTokens.refresh_token // Should check refresh token which is consistent, not access_token which is short lived. 
     );
   }
 
@@ -80,8 +80,12 @@ export async function requireAuthWrapper(
   // over Google login tokens and must be removed if a Google
   // account is the current user.
   try {
-    setAccessToken(); // clears the current access token
     const userEmail = await requireAuth(currentOptions.value); // client email
+    // SetAccessToken is necessary here to ensure that access_tokens are available when:
+    // - we are using tokens from configstore (aka those set by firebase login), AND
+    // - we are calling CLI code that skips Command (where we normally call this)
+
+    setAccessToken(await getAccessToken()); 
     if (userEmail) {
       pluginLogger.debug("User found: ", userEmail);
 

src/auth.ts

@@ -106,14 +106,13 @@ export function getAllAccounts(): Account[] {
 
 /**
  * Set the globally active account. Modifies the options object
- * and sets global refresh and access token state.
+ * and sets global refresh token state.
  * @param options options object.
  * @param account account to make active.
  */
-export async function setActiveAccount(options: any, account: Account) {
+export function setActiveAccount(options: any, account: Account) {
   if (account.tokens.refresh_token) {
     setRefreshToken(account.tokens.refresh_token);
-    setAccessToken(await apiv2.getAccessToken());
   }
 
   options.user = account.user;
@@ -128,14 +127,6 @@ export function setRefreshToken(token: string) {
   apiv2.setRefreshToken(token);
 }
 
-/**
- * Set the global access token in both api and apiv2.
- * @param token access token string
- */
-export function setAccessToken(token: string) {
-  apiv2.setAccessToken(token);
-}
-
 /**
  * Select the right account to use based on the --account flag and the
  * project defaults.

src/command.ts

@@ -319,7 +319,7 @@ export class Command {
     const activeAccount = selectAccount(account, projectRoot);
 
     if (activeAccount) {
-      await setActiveAccount(options, activeAccount);
+      setActiveAccount(options, activeAccount);
     }
 
     this.applyRC(options);

src/commands/login.ts

@@ -1,4 +1,3 @@
-import * as _ from "lodash";
 import * as clc from "colorette";
 
 import { Command } from "../command";
@@ -57,7 +56,7 @@ export const command = new Command("login")
     // the authorization callback couldn't redirect to localhost.
     const useLocalhost = isCloudEnvironment() ? false : options.localhost;
 
-    const result = await auth.loginGoogle(useLocalhost, _.get(user, "email"));
+    const result = await auth.loginGoogle(useLocalhost, user?.email);
     configstore.set("user", result.user);
     configstore.set("tokens", result.tokens);
     // store login scopes in case mandatory scopes grow over time

src/init/features/account.ts

@@ -73,7 +73,7 @@ export async function doSetup(setup: any, config: any, options: any): Promise<vo
   }
 
   // Set the global auth state
-  await setActiveAccount(options, account);
+  setActiveAccount(options, account);
 
   // Set the project default user
   if (config.projectDir) {

src/requireAuth.ts

@@ -51,7 +51,7 @@ async function autoAuth(options: Options, authScopes: string[]): Promise<void |
   }
   if (process.env.MONOSPACE_ENV && token && clientEmail) {
     // Within monospace, this a OAuth token for the user, so we make it the active user.
-    await setActiveAccount(options, {
+    setActiveAccount(options, {
       user: { email: clientEmail },
       tokens: { access_token: token },
     });
@@ -110,6 +110,8 @@ export async function requireAuth(options: any): Promise<string | void> {
     throw new FirebaseError(AUTH_ERROR_MESSAGE);
   }
 
-  await setActiveAccount(options, { user, tokens });
+  // TODO: 90 percent sure this is redundant, as the only time we hit this is if options.user/options.token is set, and
+  // setActiveAccount is the only code that sets those.
+  setActiveAccount(options, { user, tokens });
   return user.email;
 }