Skip to content

Commit

Permalink
add manual check for turtle service accounts (#7041)
Browse files Browse the repository at this point in the history
* add manual check for turtle service accounts

* ensure the IAM API is enabled

* add missing expectations
  • Loading branch information
bkendall authored Apr 24, 2024
1 parent 9425e31 commit 95eb94a
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 95 deletions.
57 changes: 40 additions & 17 deletions src/apphosting/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
cloudRunApiOrigin,
cloudbuildOrigin,
developerConnectOrigin,
iamOrigin,
secretManagerOrigin,
} from "../api";
import { Backend, BackendOutputOnlyFields, API_VERSION, Build, Rollout } from "../gcp/apphosting";
Expand Down Expand Up @@ -48,6 +49,7 @@ export async function doSetup(
ensure(projectId, secretManagerOrigin(), "apphosting", true),
ensure(projectId, cloudRunApiOrigin(), "apphosting", true),
ensure(projectId, artifactRegistryDomain(), "apphosting", true),
ensure(projectId, iamOrigin(), "apphosting", true),
]);

const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
Expand Down Expand Up @@ -90,6 +92,8 @@ export async function doSetup(
message: "Specify your app's root directory relative to your repository",
});

await ensureAppHostingComputeServiceAccount(projectId, serviceAccount);

const backend = await createBackend(
projectId,
location,
Expand Down Expand Up @@ -137,6 +141,41 @@ export async function doSetup(
logSuccess(`Your backend is now deployed at:\n\thttps://${backend.uri}`);
}

/**
* Ensures the service account is present the user has permissions to use it by
* checking the `iam.serviceAccounts.actAs` permission. If the permissions
* check fails, this returns an error. If the permission check fails with a
* "not found" error, this attempts to provision the service account.
*/
export async function ensureAppHostingComputeServiceAccount(
projectId: string,
serviceAccount: string | null,
): Promise<void> {
const sa = serviceAccount || defaultComputeServiceAccountEmail(projectId);
const name = `projects/${projectId}/serviceAccounts/${sa}`;
try {
await iam.testResourceIamPermissions(
iamOrigin(),
"v1",
name,
["iam.serviceAccounts.actAs"],
`projects/${projectId}`,
);
} catch (err: unknown) {
if (!(err instanceof FirebaseError)) {
throw err;
}
if (err.status === 404) {
await provisionDefaultComputeServiceAccount(projectId);
} else if (err.status === 403) {
throw new FirebaseError(
`Failed to create backend due to missing delegation permissions for ${sa}. Make sure you have the iam.serviceAccounts.actAs permission.`,
{ original: err },
);
}
}
}

/**
* Prompts the user for a backend id and verifies that it doesn't match a pre-existing backend.
*/
Expand Down Expand Up @@ -200,23 +239,7 @@ export async function createBackend(
});
}

try {
return await createBackendAndPoll();
} catch (err: any) {
if (err.status === 403) {
if (err.message.includes(defaultServiceAccount)) {
// Create the default service account if it doesn't exist and try again.
await provisionDefaultComputeServiceAccount(projectId);
return await createBackendAndPoll();
} else if (serviceAccount && err.message.includes(serviceAccount)) {
throw new FirebaseError(
`Failed to create backend due to missing delegation permissions for ${serviceAccount}. Make sure you have the iam.serviceAccounts.actAs permission.`,
{ children: [err] },
);
}
}
throw err;
}
return await createBackendAndPoll();
}

async function provisionDefaultComputeServiceAccount(projectId: string): Promise<void> {
Expand Down
155 changes: 77 additions & 78 deletions src/test/apphosting/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import {
deleteBackendAndPoll,
promptLocation,
setDefaultTrafficPolicy,
ensureAppHostingComputeServiceAccount,
} from "../../apphosting/index";
import * as deploymentTool from "../../deploymentTool";
import { FirebaseError } from "../../error";

describe("operationsConverter", () => {
const sandbox: sinon.SinonSandbox = sinon.createSandbox();
describe("apphosting setup functions", () => {
const projectId = "projectId";
const location = "us-central1";
const backendId = "backendId";
Expand All @@ -29,37 +29,39 @@ describe("operationsConverter", () => {
let listLocationsStub: sinon.SinonStub;
let createServiceAccountStub: sinon.SinonStub;
let addServiceAccountToRolesStub: sinon.SinonStub;
let testResourceIamPermissionsStub: sinon.SinonStub;

beforeEach(() => {
promptOnceStub = sandbox.stub(prompt, "promptOnce").throws("Unexpected promptOnce call");
pollOperationStub = sandbox
.stub(poller, "pollOperation")
.throws("Unexpected pollOperation call");
createBackendStub = sandbox
promptOnceStub = sinon.stub(prompt, "promptOnce").throws("Unexpected promptOnce call");
pollOperationStub = sinon.stub(poller, "pollOperation").throws("Unexpected pollOperation call");
createBackendStub = sinon
.stub(apphosting, "createBackend")
.throws("Unexpected createBackend call");
deleteBackendStub = sandbox
deleteBackendStub = sinon
.stub(apphosting, "deleteBackend")
.throws("Unexpected deleteBackend call");
updateTrafficStub = sandbox
updateTrafficStub = sinon
.stub(apphosting, "updateTraffic")
.throws("Unexpected updateTraffic call");
listLocationsStub = sandbox
listLocationsStub = sinon
.stub(apphosting, "listLocations")
.throws("Unexpected listLocations call");
createServiceAccountStub = sandbox
createServiceAccountStub = sinon
.stub(iam, "createServiceAccount")
.throws("Unexpected createServiceAccount call");
addServiceAccountToRolesStub = sandbox
addServiceAccountToRolesStub = sinon
.stub(resourceManager, "addServiceAccountToRoles")
.throws("Unexpected addServiceAccountToRoles call");
testResourceIamPermissionsStub = sinon
.stub(iam, "testResourceIamPermissions")
.throws("Unexpected testResourceIamPermissions call");
});

afterEach(() => {
sandbox.verifyAndRestore();
sinon.verifyAndRestore();
});

describe("onboardBackend", () => {
describe("createBackend", () => {
const webAppId = "webAppId";

const op = {
Expand Down Expand Up @@ -108,68 +110,6 @@ describe("operationsConverter", () => {
expect(createBackendStub).to.be.calledWith(projectId, location, backendInput);
});

it("should provision the default compute service account", async () => {
createBackendStub.resolves(op);
pollOperationStub
// Initial CreateBackend operation should throw a permission denied to trigger service account creation.
.onFirstCall()
.throws(
new FirebaseError(
`missing actAs permission on firebase-app-hosting-compute@${projectId}.iam.gserviceaccount.com`,
{ status: 403 },
),
)
.onSecondCall()
.resolves(completeBackend);
createServiceAccountStub.resolves({});
addServiceAccountToRolesStub.resolves({});

await createBackend(
projectId,
location,
backendId,
cloudBuildConnRepo,
/* serviceAccount= */ null,
webAppId,
);

// CreateBackend should be called twice; once initially and once after the service account was created
expect(createBackendStub).to.be.calledTwice;
expect(createServiceAccountStub).to.be.calledOnce;
expect(addServiceAccountToRolesStub).to.be.calledOnce;
});

it("does not try to provision a custom service account", () => {
createBackendStub.resolves(op);
pollOperationStub
// Initial CreateBackend operation should throw a permission denied to
// potentially trigger service account creation.
.onFirstCall()
.throws(
new FirebaseError("missing actAs permission on my-service-account", { status: 403 }),
)
.onSecondCall()
.resolves(completeBackend);

expect(
createBackend(
projectId,
location,
backendId,
cloudBuildConnRepo,
/* serviceAccount= */ "my-service-account",
webAppId,
),
).to.be.rejectedWith(
FirebaseError,
"Failed to create backend due to missing delegation permissions for my-service-account. Make sure you have the iam.serviceAccounts.actAs permission.",
);

expect(createBackendStub).to.be.calledOnce;
expect(createServiceAccountStub).to.not.have.been.called;
expect(addServiceAccountToRolesStub).to.not.have.been.called;
});

it("should set default rollout policy to 100% all at once", async () => {
const completeTraffic: apphosting.Traffic = {
name: `projects/${projectId}/locations/${location}/backends/${backendId}/traffic`,
Expand Down Expand Up @@ -199,7 +139,66 @@ describe("operationsConverter", () => {
});
});

describe("delete backend", () => {
describe("ensureAppHostingComputeServiceAccount", () => {
const serviceAccount = "[email protected]";

it("should succeed if the user has permissions for the service account", async () => {
testResourceIamPermissionsStub.resolves();

await expect(ensureAppHostingComputeServiceAccount(projectId, serviceAccount)).to.be
.fulfilled;

expect(testResourceIamPermissionsStub).to.be.calledOnce;
expect(createServiceAccountStub).to.not.be.called;
expect(addServiceAccountToRolesStub).to.not.be.called;
});

it("should succeed if the user can create the service account when it does not exist", async () => {
testResourceIamPermissionsStub.rejects(
new FirebaseError("Permission denied", { status: 404 }),
);
createServiceAccountStub.resolves();
addServiceAccountToRolesStub.resolves();

await expect(ensureAppHostingComputeServiceAccount(projectId, serviceAccount)).to.be
.fulfilled;

expect(testResourceIamPermissionsStub).to.be.calledOnce;
expect(createServiceAccountStub).to.be.calledOnce;
expect(addServiceAccountToRolesStub).to.be.calledOnce;
});

it("should throw an error if the user does not have permissions", async () => {
testResourceIamPermissionsStub.rejects(
new FirebaseError("Permission denied", { status: 403 }),
);

await expect(
ensureAppHostingComputeServiceAccount(projectId, serviceAccount),
).to.be.rejectedWith(/Failed to create backend due to missing delegation permissions/);

expect(testResourceIamPermissionsStub).to.be.calledOnce;
expect(createServiceAccountStub).to.not.be.called;
expect(addServiceAccountToRolesStub).to.not.be.called;
});

it("should throw the error if the user cannot create the service account", async () => {
testResourceIamPermissionsStub.rejects(
new FirebaseError("Permission denied", { status: 404 }),
);
createServiceAccountStub.rejects(new FirebaseError("failed to create SA"));

await expect(
ensureAppHostingComputeServiceAccount(projectId, serviceAccount),
).to.be.rejectedWith("failed to create SA");

expect(testResourceIamPermissionsStub).to.be.calledOnce;
expect(createServiceAccountStub).to.be.calledOnce;
expect(addServiceAccountToRolesStub).to.not.be.called;
});
});

describe("deleteBackendAndPoll", () => {
it("should delete a backend", async () => {
const op = {
name: `projects/${projectId}/locations/${location}/backends/${backendId}`,
Expand All @@ -214,7 +213,7 @@ describe("operationsConverter", () => {
});
});

describe("prompt location", () => {
describe("promptLocation", () => {
const supportedLocations = [{ name: "us-central1", locationId: "us-central1" }];

beforeEach(() => {
Expand Down

0 comments on commit 95eb94a

Please sign in to comment.