Skip to content

Commit

Permalink
ENG-2931 Fix error when no identity file is present (#134)
Browse files Browse the repository at this point in the history
Fixes the error that is shown when no identity file is present.

The CLI now again shows a message asking the user to login:

./p0 ls gcloud role oauth
Please run `p0 login <organization>` to use the P0 CLI.

Added unit test.
  • Loading branch information
fabgo authored Oct 28, 2024
1 parent dfe8f33 commit 605bf37
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 28 deletions.
56 changes: 41 additions & 15 deletions src/commands/__tests__/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jest.mock("../../drivers/auth", () => ({
jest.mock("../../drivers/config", () => ({
...jest.requireActual("../../drivers/config"),
saveConfig: jest.fn(),
getTenantConfig: jest.fn(() => bootstrapConfig),
loadConfig: jest.fn(() => bootstrapConfig),
}));
jest.mock("../../drivers/stdio");
jest.mock("../../plugins/login");
Expand Down Expand Up @@ -54,25 +54,26 @@ describe("login", () => {

describe("organization exists", () => {
let credentialData: string = "";
mockReadFile.mockImplementation(async () =>
Buffer.from(credentialData, "utf-8")
);
mockWriteFile.mockImplementation(async (_path, data) => {
credentialData = data;
});
mockSignInWithCredential.mockImplementation(
async (_auth, _firebaseCredential) =>
Promise.resolve({
user: {
email: "user@p0.dev",
},
})
);

beforeEach(() => {
credentialData = "";
jest.clearAllMocks();

mockReadFile.mockImplementation(async () =>
Buffer.from(credentialData, "utf-8")
);
mockWriteFile.mockImplementation(async (_path, data) => {
credentialData = data;
});
mockSignInWithCredential.mockImplementation(
async (_auth, _firebaseCredential) =>
Promise.resolve({
user: {
email: "user@p0.dev",
},
})
);

mockGetDoc({
slug: "test-org",
tenantId: "test-tenant",
Expand Down Expand Up @@ -105,4 +106,29 @@ Please contact support@p0.dev to enable this user."
`);
});
});

describe("identity file does not exist", () => {
beforeEach(() => {
jest.clearAllMocks();

// Mock `readFile` to throw an "ENOENT" error
mockReadFile.mockImplementation(() => {
const error = new Error("File not found");
(error as any).code = "ENOENT";
return Promise.reject(error);
});

mockGetDoc({
slug: "test-org",
tenantId: "test-tenant",
ssoProvider: "google",
});
});

it("it should ask user to log in", async () => {
await expect(login({ org: "test-org" })).rejects.toMatchInlineSnapshot(
`"Please run \`p0 login <organization>\` to use the P0 CLI."`
);
});
});
});
12 changes: 4 additions & 8 deletions src/drivers/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ You should have received a copy of the GNU General Public License along with @p0
import { login } from "../commands/login";
import { Authn, Identity } from "../types/identity";
import { P0_PATH } from "../util";
import { loadConfig } from "./config";
import { authenticateToFirebase, initializeFirebase } from "./firestore";
import { authenticateToFirebase } from "./firestore";
import { print2 } from "./stdio";
import * as fs from "fs/promises";
import * as path from "path";
Expand Down Expand Up @@ -66,7 +65,7 @@ export const cached = async <T>(
}
};

export const loadCredentials = async (options?: {
const loadCredentialsWithAutoLogin = async (options?: {
noRefresh?: boolean;
}): Promise<Identity> => {
try {
Expand All @@ -78,7 +77,7 @@ export const loadCredentials = async (options?: {
) {
await login({ org: identity.org.slug }, { skipAuthenticate: true });
print2("\u200B"); // Force a new line
return loadCredentials({ noRefresh: true });
return loadCredentialsWithAutoLogin({ noRefresh: true });
}
return identity;
} catch (error: any) {
Expand All @@ -92,10 +91,7 @@ export const loadCredentials = async (options?: {
export const authenticate = async (options?: {
noRefresh?: boolean;
}): Promise<Authn> => {
await loadConfig();
initializeFirebase();

const identity = await loadCredentials(options);
const identity = await loadCredentialsWithAutoLogin(options);
const userCredential = await authenticateToFirebase(identity);

return { userCredential, identity };
Expand Down
3 changes: 2 additions & 1 deletion src/drivers/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export async function saveConfig(config: Config) {
tenantConfig = config;
}

export async function loadConfig() {
export async function loadConfig(): Promise<Config> {
const buffer = await fs.readFile(CONFIG_FILE_PATH);
tenantConfig = JSON.parse(buffer.toString());
return tenantConfig;
}
13 changes: 9 additions & 4 deletions src/drivers/firestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ This file is part of @p0security/cli
You should have received a copy of the GNU General Public License along with @p0security/cli. If not, see <https://www.gnu.org/licenses/>.
**/
import { Identity } from "../types/identity";
import { getTenantConfig } from "./config";
import { loadConfig } from "./config";
import { bootstrapConfig } from "./env";
import { FirebaseApp, initializeApp } from "firebase/app";
import {
getAuth,
OAuthProvider,
SignInMethod,
signInWithCredential,
UserCredential,
} from "firebase/auth";
import {
collection as fsCollection,
Expand All @@ -34,16 +35,20 @@ const bootstrapFirestore = getFirestore(bootstrapApp);
let app: FirebaseApp;
let firestore: Firestore;

export function initializeFirebase() {
const tenantConfig = getTenantConfig();
async function initializeFirebase() {
const tenantConfig = await loadConfig();
app = initializeApp(tenantConfig.fs, "authFirebase");
firestore = getFirestore(app);
}

export async function authenticateToFirebase(identity: Identity) {
export async function authenticateToFirebase(
identity: Identity
): Promise<UserCredential> {
const { credential } = identity;
const tenantId = identity.org.tenantId;

await initializeFirebase();

// TODO: Move to map lookup
const provider = new OAuthProvider(
identity.org.ssoProvider === "google"
Expand Down

0 comments on commit 605bf37

Please sign in to comment.