Skip to content
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

Improve auth policy configuration for callable functions. #1673

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 1 addition & 3 deletions spec/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ export function runHandler(

const toSend = typeof sendBody === "object" ? JSON.stringify(sendBody) : sendBody;
const body =
typeof this.sentBody === "undefined"
? toSend
: this.sentBody + String(toSend || "");
typeof this.sentBody === "undefined" ? toSend : this.sentBody + String(toSend || "");
this.end(body);
}

Expand Down
139 changes: 109 additions & 30 deletions spec/v2/providers/https.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@ import { expectedResponseHeaders, MockRequest } from "../../fixtures/mockrequest
import { runHandler } from "../../helper";
import { FULL_ENDPOINT, MINIMAL_V2_ENDPOINT, FULL_OPTIONS, FULL_TRIGGER } from "./fixtures";
import { onInit } from "../../../src/v2/core";
import { Handler } from "express";
import { genkit } from "genkit";

function request(args: {
data?: any;
auth?: Record<string, string>;
auth?: Record<string, any>;
headers?: Record<string, string>;
method?: MockRequest["method"];
}): any {
Expand Down Expand Up @@ -526,34 +525,96 @@ describe("onCall", () => {
expect(anonResp.status).to.equal(403);
});

it("should check hasClaim", async () => {
const anyValue = https.onCall(
{
authPolicy: https.hasClaim("meaning"),
},
() => "HHGTTG"
);
const specificValue = https.onCall(
{
authPolicy: https.hasClaim("meaning", "42"),
},
() => "HHGTG"
);

const cases: Array<{ fn: Handler; auth?: Record<string, string>; status: number }> = [
{ fn: anyValue, auth: { meaning: "42" }, status: 200 },
{ fn: anyValue, auth: { meaning: "43" }, status: 200 },
{ fn: anyValue, auth: { order: "66" }, status: 403 },
{ fn: anyValue, status: 403 },
{ fn: specificValue, auth: { meaning: "42" }, status: 200 },
{ fn: specificValue, auth: { meaning: "43" }, status: 403 },
{ fn: specificValue, auth: { order: "66" }, status: 403 },
{ fn: specificValue, status: 403 },
];
for (const test of cases) {
const resp = await runHandler(test.fn, request({ auth: test.auth }));
expect(resp.status).to.equal(test.status);
}
describe("hasClaim", () => {
it("should check single claim with specific value", async () => {
const func = https.onCall(
{
authPolicy: https.hasClaim("meaning", "42"),
},
() => true
);
const validResp = await runHandler(func, request({ auth: { meaning: "42" } }));
expect(validResp.status).to.equal(200);

const wrongValResp = await runHandler(func, request({ auth: { meaning: "43" } }));
expect(wrongValResp.status).to.equal(403);

const noClaimResp = await runHandler(func, request({ auth: {} }));
expect(noClaimResp.status).to.equal(403);
});

it("should check single claim with default value (truthy)", async () => {
const func = https.onCall(
{
authPolicy: https.hasClaim("admin"),
},
() => true
);
const validResp = await runHandler(func, request({ auth: { admin: true } }));
expect(validResp.status).to.equal(200);

const truthyResp = await runHandler(func, request({ auth: { admin: "true" } }));
expect(truthyResp.status).to.equal(200);

const falseResp = await runHandler(func, request({ auth: { admin: false } }));
expect(falseResp.status).to.equal(403);

const falseStrResp = await runHandler(func, request({ auth: { admin: "false" } }));
expect(falseStrResp.status).to.equal(403);

const noClaimResp = await runHandler(func, request({ auth: {} }));
expect(noClaimResp.status).to.equal(403);
});

it("should check multiple claims with default value (true)", async () => {
const func = https.onCall(
{
authPolicy: https.hasClaim(["pro", "eap"]),
},
() => true
);

const validResp = await runHandler(func, request({ auth: { pro: true, eap: true } }));
expect(validResp.status).to.equal(200);

const truthyResp = await runHandler(func, request({ auth: { pro: "true", eap: "abc" } }));
expect(truthyResp.status).to.equal(200);

const missingResp = await runHandler(func, request({ auth: { pro: true } }));
expect(missingResp.status).to.equal(403);

const noClaimResp = await runHandler(func, request({ auth: {} }));
expect(noClaimResp.status).to.equal(403);
});

it("should check multiple claims with specific values", async () => {
const func = https.onCall(
{
authPolicy: https.hasClaim({
meaning: 42,
animal: "dolphin",
}),
},
() => true
);
const validResp = await runHandler(
func,
request({ auth: { meaning: 42, animal: "dolphin" } })
);
expect(validResp.status).to.equal(200);

const wrongTypeResp = await runHandler(
func,
request({ auth: { meaning: "42", animal: "dolphin" } })
);
expect(wrongTypeResp.status).to.equal(403);

const missingResp = await runHandler(func, request({ auth: { meaing: 42 } }));
expect(missingResp.status).to.equal(403);

const noClaimResp = await runHandler(func, request({ auth: {} }));
expect(noClaimResp.status).to.equal(403);
});
});

it("can be any callback", async () => {
Expand All @@ -569,6 +630,24 @@ describe("onCall", () => {
const accessDenied = await runHandler(divTwo, request({ data: 1 }));
expect(accessDenied.status).to.equal(403);
});

it("should check emailVerified", async () => {
const func = https.onCall(
{
authPolicy: https.emailVerified(),
},
() => 42
);

const verifiedResp = await runHandler(func, request({ auth: { email_verified: true } }));
expect(verifiedResp.status).to.equal(200);

const unverifiedResp = await runHandler(func, request({ auth: { email_verified: false } }));
expect(unverifiedResp.status).to.equal(403);

const noAuthResp = await runHandler(func, request({ auth: {} }));
expect(noAuthResp.status).to.equal(403);
});
});
});

Expand Down
10 changes: 7 additions & 3 deletions src/common/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,13 @@ function wrapOnCallHandler<Req = any, Res = any, Stream = unknown>(

const data: Req = decode(req.body.data);
if (options.authPolicy) {
const authorized = await options.authPolicy(context.auth ?? null, data);
if (!authorized) {
throw new HttpsError("permission-denied", "Permission Denied");
try {
const authorized = await options.authPolicy(context.auth ?? null, data);
if (!authorized) {
throw new HttpsError("permission-denied", "Permission Denied");
}
} catch (e: unknown) {
throw new HttpsError("permission-denied", (e as any).message || "Permission Denied");
}
}
let result: Res;
Expand Down
91 changes: 71 additions & 20 deletions src/v2/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@

/** HTTP functions can override global options and can specify multiple regions to deploy to. */
region?:
| SupportedRegion
| string
| Array<SupportedRegion | string>
| Expression<string>
| ResetValue;
| SupportedRegion

Check failure on line 64 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Insert `··`
| string

Check failure on line 65 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Insert `··`
| Array<SupportedRegion | string>

Check failure on line 66 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Replace `··` with `····`
| Expression<string>

Check failure on line 67 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Insert `··`
| ResetValue;

Check failure on line 68 in src/v2/providers/https.ts

View workflow job for this annotation

GitHub Actions / lint (18.x)

Insert `··`

/** If true, allows CORS on requests to this function.
* If this is a `string` or `RegExp`, allows requests from domains that match the provided value.
Expand Down Expand Up @@ -165,6 +165,12 @@
invoker?: "public" | "private" | string | string[];
}

/**
*
Callback function to run before processing a request for callable functions to authorize the request.
*/
export type AuthPolicy<T = any> = (auth: AuthData | null, data: T) => boolean | Promise<boolean>;

/**
* Options that can be set on a callable HTTPS function.
*/
Expand Down Expand Up @@ -212,34 +218,79 @@
/**
* Callback for whether a request is authorized.
*
* Designed to allow reusable auth policies to be passed as an options object. Two built-in reusable policies exist:
* isSignedIn and hasClaim.
* Designed to allow reusable auth policies to be passed as an options object.
*/
authPolicy?: (auth: AuthData | null, data: T) => boolean | Promise<boolean>;
authPolicy?: AuthPolicy<T>;
}

/**
* An auth policy that requires a user to be signed in.
*/
export const isSignedIn =
() =>
(auth: AuthData | null): boolean =>
!!auth;
export function isSignedIn(): AuthPolicy {
return (auth: AuthData | null): boolean => {
if (!auth) {
throw new Error("Must be signed in");
}
return true;
};
}

export function hasClaim(claim: string, value?: string): AuthPolicy;
export function hasClaim(claims: string[]): AuthPolicy;
export function hasClaim(claims: Record<string, unknown>): AuthPolicy;
/**
* An auth policy that requires a user to be both signed in and have a specific claim (optionally with a specific value)
* An auth policy that requires a user to be both signed in and have a specific claims (optionally with a specific value)
*/
export const hasClaim =
(claim: string, value?: string) =>
(auth: AuthData | null): boolean => {
export function hasClaim(
claimOrClaims: string | string[] | Record<string, unknown>,
value?: string
): AuthPolicy {
let claimsToCheck: Record<string, unknown> = {};

if (typeof claimOrClaims === "string") {
claimsToCheck[claimOrClaims] = value;
} else if (Array.isArray(claimOrClaims)) {
for (const claim of claimOrClaims) {
claimsToCheck[claim] = undefined;
}
} else {
claimsToCheck = claimOrClaims;
}

return (auth: AuthData | null): boolean => {
if (!auth) {
return false;
throw new Error("Must be signed in");
}
if (!(claim in auth.token)) {
return false;
for (const claim of Object.keys(claimsToCheck)) {
if (!(claim in auth.token)) {
throw new Error(`Missing claim '${claim}'`);
}
const expectedValue = claimsToCheck[claim];
const actualValue = auth.token[claim];

if (expectedValue === undefined) {
if (!actualValue || actualValue === "false") {
return false;
}
} else if (actualValue !== expectedValue) {
return false;
}
}
return true;
};
}

/**
* An auth policy that requires a user to be both signed in and have email verified
*/
export function emailVerified(): AuthPolicy {
return (auth: AuthData | null): boolean | Promise<boolean> => {
if (!auth) {
throw new Error("Must be signed in");
}
return !value || auth.token[claim] === value;
return hasClaim("email_verified")(auth, undefined);
};
}

/**
* Handles HTTPS requests.
Expand Down
Loading