Skip to content

Prototype of login via prototyper in Studio #8938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@
return getGlobalDefaultAccount();
}

const activeAccounts = configstore.get("activeAccounts") || {};

Check warning on line 74 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
const email: string | undefined = activeAccounts[projectDir];

Check warning on line 75 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access [projectDir] on an `any` value

Check warning on line 75 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value

if (!email) {
return getGlobalDefaultAccount();
Expand All @@ -86,7 +86,7 @@
* Get all authenticated accounts _besides_ the default account.
*/
export function getAdditionalAccounts(): Account[] {
return configstore.get("additionalAccounts") || [];

Check warning on line 89 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe return of an `any` typed value
}

/**
Expand All @@ -108,7 +108,7 @@
/**
* Throw an error if the provided email is not a signed-in user.
*/
export function assertAccount(email: string, options?: { mcp?: boolean }) {

Check warning on line 111 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
const allAccounts = getAllAccounts();
const accountExists = allAccounts.some((a) => a.user.email === email);
if (!accountExists) {
Expand All @@ -128,20 +128,20 @@
* @param options options object.
* @param account account to make active.
*/
export function setActiveAccount(options: any, account: Account) {

Check warning on line 131 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type

Check warning on line 131 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
if (account.tokens.refresh_token) {
setRefreshToken(account.tokens.refresh_token);
}

options.user = account.user;

Check warning on line 136 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .user on an `any` value
options.tokens = account.tokens;

Check warning on line 137 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .tokens on an `any` value
}

/**
* Set the global refresh token in both api and apiv2.
* @param token refresh token string
*/
export function setRefreshToken(token: string) {

Check warning on line 144 in src/auth.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
apiv2.setRefreshToken(token);
}

Expand Down Expand Up @@ -425,6 +425,67 @@
return base64string.replace(/\+/g, "-").replace(/=+$/, "").replace(/\//g, "_");
}

interface PrototyperRes {
uri: string;
sessionId: string;
authorize: (authorizationCode: string) => Promise<UserCredentials>;
}

export async function loginPrototyper(): Promise<PrototyperRes> {
const authProxyClient = new apiv2.Client({
urlPrefix: authProxyOrigin(),
auth: false,
});

const sessionId = uuidv4();
const codeVerifier = randomBytes(32).toString("hex");
// urlsafe base64 is required for code_challenge in OAuth PKCE
const codeChallenge = urlsafeBase64(createHash("sha256").update(codeVerifier).digest("base64"));

const attestToken = (
await authProxyClient.post<{ session_id: string }, { token: string }>("/attest", {
session_id: sessionId,
})
).body?.token;
Comment on lines +445 to +449
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The current implementation doesn't handle the case where authProxyClient.post returns a response without a body or a body without a token. This would result in attestToken being undefined, which would then be embedded in the loginUrl, likely breaking the authentication flow.

Please add a check for the token and throw an error if it's missing.

Suggested change
const attestToken = (
await authProxyClient.post<{ session_id: string }, { token: string }>("/attest", {
session_id: sessionId,
})
).body?.token;
const attestRes = await authProxyClient.post<{ session_id: string }, { token: string }>("/attest", {
session_id: sessionId,
});
const attestToken = attestRes.body?.token;
if (!attestToken) {
throw new FirebaseError("Failed to get attestation token, please try again.");
}


const loginUrl = `${authProxyOrigin()}/login?code_challenge=${codeChallenge}&session=${sessionId}&attest=${attestToken}`;
return {
uri: loginUrl,
sessionId: sessionId.substring(0, 5).toUpperCase(),
authorize: async (code: string) => {
try {
const tokens = await getTokensFromAuthorizationCode(
code,
`${authProxyOrigin()}/complete`,
codeVerifier,
);

const creds = {
user: jwt.decode(tokens.id_token!, { json: true }) as any as User,
tokens: tokens,
scopes: SCOPES,
};
recordCredentials(creds);
return creds;
} catch (e) {
throw new FirebaseError(
"Unable to authenticate using the provided code. Please try again.",
);
}
Comment on lines +470 to +474
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The catch block currently swallows the original error, which can make debugging difficult if an unexpected issue occurs during authorization. It would be helpful to log the original error for better diagnostics.

      } catch (e: unknown) {
        logger.debug("Prototyper login authorization failed:", e);
        throw new FirebaseError(
          "Unable to authenticate using the provided code. Please try again.",
        );
      }

},
};
}

// recordCredentials saves credentials to configstore to be used in future command runs.
export function recordCredentials(creds: UserCredentials) {
configstore.set("user", creds.user);
configstore.set("tokens", creds.tokens);
// store login scopes in case mandatory scopes grow over time
configstore.set("loginScopes", creds.scopes);
// remove old session token, if it exists
configstore.delete("session");
}

async function loginRemotely(): Promise<UserCredentials> {
const authProxyClient = new apiv2.Client({
urlPrefix: authProxyOrigin(),
Expand Down
27 changes: 16 additions & 11 deletions src/commands/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ import * as auth from "../auth";
import { isCloudEnvironment } from "../utils";
import { User, Tokens } from "../types/auth";

import { Options } from "../options";

export interface LoginOptions extends Options {
prototyperLogin?: boolean;
}

export const command = new Command("login")
.description("log the CLI into Firebase")
.option("--no-localhost", "login from a device without an accessible localhost")
.option("--reauth", "force reauthentication even if already logged in")
.action(async (options: any) => {
if (options.nonInteractive) {
.action(async (options: LoginOptions) => {
if (options.nonInteractive && !options.prototyperLogin) {
throw new FirebaseError(
"Cannot run login in non-interactive mode. See " +
clc.bold("login:ci") +
Expand All @@ -33,7 +39,7 @@ export const command = new Command("login")
return user;
}

if (!options.reauth) {
if (!options.reauth && !options.prototyperLogin) {
utils.logBullet(
"The Firebase CLI’s MCP server feature can optionally make use of Gemini in Firebase. " +
"Learn more about Gemini in Firebase and how it uses your data: https://firebase.google.com/docs/gemini-in-firebase#how-gemini-in-firebase-uses-your-data",
Expand All @@ -58,18 +64,17 @@ export const command = new Command("login")
}
}

// Special escape hatch for logging in when using firebase-tools as a module.
if (options.prototyperLogin) {
return await auth.loginPrototyper();
}

// Default to using the authorization code flow when the end
// user is within a cloud-based environment, and therefore,
// the authorization callback couldn't redirect to localhost.
const useLocalhost = isCloudEnvironment() ? false : options.localhost;

const useLocalhost = isCloudEnvironment() ? false : !!options.localhost;
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The logic for determining useLocalhost appears to be incorrect. The --no-localhost option implies that using localhost is the default behavior (true). If options.localhost is undefined when the flag is not provided, !!options.localhost evaluates to false, which is the opposite of the intended default. This would break the standard login flow for users not in a cloud environment.

Suggested change
const useLocalhost = isCloudEnvironment() ? false : !!options.localhost;
const useLocalhost = isCloudEnvironment() ? false : options.localhost !== false;

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
configstore.set("loginScopes", result.scopes);
// remove old session token, if it exists
configstore.delete("session");
auth.recordCredentials(result);

logger.info();
if (typeof result.user !== "string") {
Expand Down
Loading