Skip to content

Commit d02b9f8

Browse files
committed
Improve auth policy configuration for callable functions.
1 parent bcac255 commit d02b9f8

File tree

3 files changed

+172
-48
lines changed

3 files changed

+172
-48
lines changed

spec/v2/providers/https.spec.ts

Lines changed: 105 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@ import { expectedResponseHeaders, MockRequest } from "../../fixtures/mockrequest
3030
import { runHandler } from "../../helper";
3131
import { FULL_ENDPOINT, MINIMAL_V2_ENDPOINT, FULL_OPTIONS, FULL_TRIGGER } from "./fixtures";
3232
import { onInit } from "../../../src/v2/core";
33-
import { Handler } from "express";
3433
import { genkit } from "genkit";
3534

3635
function request(args: {
3736
data?: any;
38-
auth?: Record<string, string>;
37+
auth?: Record<string, any>;
3938
headers?: Record<string, string>;
4039
method?: MockRequest["method"];
4140
}): any {
@@ -526,34 +525,92 @@ describe("onCall", () => {
526525
expect(anonResp.status).to.equal(403);
527526
});
528527

529-
it("should check hasClaim", async () => {
530-
const anyValue = https.onCall(
531-
{
532-
authPolicy: https.hasClaim("meaning"),
533-
},
534-
() => "HHGTTG"
535-
);
536-
const specificValue = https.onCall(
537-
{
538-
authPolicy: https.hasClaim("meaning", "42"),
539-
},
540-
() => "HHGTG"
541-
);
542-
543-
const cases: Array<{ fn: Handler; auth?: Record<string, string>; status: number }> = [
544-
{ fn: anyValue, auth: { meaning: "42" }, status: 200 },
545-
{ fn: anyValue, auth: { meaning: "43" }, status: 200 },
546-
{ fn: anyValue, auth: { order: "66" }, status: 403 },
547-
{ fn: anyValue, status: 403 },
548-
{ fn: specificValue, auth: { meaning: "42" }, status: 200 },
549-
{ fn: specificValue, auth: { meaning: "43" }, status: 403 },
550-
{ fn: specificValue, auth: { order: "66" }, status: 403 },
551-
{ fn: specificValue, status: 403 },
552-
];
553-
for (const test of cases) {
554-
const resp = await runHandler(test.fn, request({ auth: test.auth }));
555-
expect(resp.status).to.equal(test.status);
556-
}
528+
describe("hasClaim", () => {
529+
it("should check single claim with specific value", async () => {
530+
const func = https.onCall(
531+
{
532+
authPolicy: https.hasClaim("meaning", "42"),
533+
},
534+
() => true
535+
);
536+
const validResp = await runHandler(func, request({ auth: { meaning: "42" } }));
537+
expect(validResp.status).to.equal(200);
538+
539+
const noClaimResp = await runHandler(func, request({ auth: {} }));
540+
expect(noClaimResp.status).to.equal(403);
541+
});
542+
543+
it("should check single claim with default value (true)", async () => {
544+
const func = https.onCall(
545+
{
546+
authPolicy: https.hasClaim("admin"),
547+
},
548+
() => true
549+
);
550+
const validResp = await runHandler(func, request({ auth: { admin: true } }));
551+
expect(validResp.status).to.equal(200);
552+
553+
const wrongTypeResp = await runHandler(func, request({ auth: { admin: "true" } }));
554+
expect(wrongTypeResp.status).to.equal(403);
555+
556+
const falseResp = await runHandler(func, request({ auth: { admin: false } }));
557+
expect(falseResp.status).to.equal(403);
558+
559+
const noClaimResp = await runHandler(func, request({ auth: {} }));
560+
expect(noClaimResp.status).to.equal(403);
561+
});
562+
563+
it("should check multiple claims with default value (true)", async () => {
564+
const func = https.onCall(
565+
{
566+
authPolicy: https.hasClaim(["pro", "eap"]),
567+
},
568+
() => true
569+
);
570+
const validResp = await runHandler(func, request({ auth: { pro: true, eap: true } }));
571+
expect(validResp.status).to.equal(200);
572+
573+
const missingResp = await runHandler(func, request({ auth: { pro: true } }));
574+
expect(missingResp.status).to.equal(403);
575+
576+
const wrongTypeResp = await runHandler(
577+
func,
578+
request({ auth: { pro: "true", eap: "true" } })
579+
);
580+
expect(wrongTypeResp.status).to.equal(403);
581+
582+
const noClaimResp = await runHandler(func, request({ auth: {} }));
583+
expect(noClaimResp.status).to.equal(403);
584+
});
585+
586+
it("should check multiple claims with specific values", async () => {
587+
const func = https.onCall(
588+
{
589+
authPolicy: https.hasClaim({
590+
meaning: 42,
591+
animal: "dolphin",
592+
}),
593+
},
594+
() => true
595+
);
596+
const validResp = await runHandler(
597+
func,
598+
request({ auth: { meaning: 42, animal: "dolphin" } })
599+
);
600+
expect(validResp.status).to.equal(200);
601+
602+
const wrongTypeResp = await runHandler(
603+
func,
604+
request({ auth: { meaning: "42", animal: "dolphin" } })
605+
);
606+
expect(wrongTypeResp.status).to.equal(403);
607+
608+
const missingResp = await runHandler(func, request({ auth: { meaing: 42 } }));
609+
expect(missingResp.status).to.equal(403);
610+
611+
const noClaimResp = await runHandler(func, request({ auth: {} }));
612+
expect(noClaimResp.status).to.equal(403);
613+
});
557614
});
558615

559616
it("can be any callback", async () => {
@@ -569,6 +626,24 @@ describe("onCall", () => {
569626
const accessDenied = await runHandler(divTwo, request({ data: 1 }));
570627
expect(accessDenied.status).to.equal(403);
571628
});
629+
630+
it("should check emailVerified", async () => {
631+
const func = https.onCall(
632+
{
633+
authPolicy: https.emailVerified(),
634+
},
635+
() => 42
636+
);
637+
638+
const verifiedResp = await runHandler(func, request({ auth: { email_verified: true } }));
639+
expect(verifiedResp.status).to.equal(200);
640+
641+
const unverifiedResp = await runHandler(func, request({ auth: { email_verified: false } }));
642+
expect(unverifiedResp.status).to.equal(403);
643+
644+
const noAuthResp = await runHandler(func, request({ auth: {} }));
645+
expect(noAuthResp.status).to.equal(403);
646+
});
572647
});
573648
});
574649

src/common/providers/https.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -846,9 +846,13 @@ function wrapOnCallHandler<Req = any, Res = any, Stream = unknown>(
846846

847847
const data: Req = decode(req.body.data);
848848
if (options.authPolicy) {
849-
const authorized = await options.authPolicy(context.auth ?? null, data);
850-
if (!authorized) {
851-
throw new HttpsError("permission-denied", "Permission Denied");
849+
try {
850+
const authorized = await options.authPolicy(context.auth ?? null, data);
851+
if (!authorized) {
852+
throw new HttpsError("permission-denied", "Permission Denied");
853+
}
854+
} catch (e: unknown) {
855+
throw new HttpsError("permission-denied", (e as any).message || "Permission Denied");
852856
}
853857
}
854858
let result: Res;

src/v2/providers/https.ts

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ export interface HttpsOptions extends Omit<GlobalOptions, "region" | "enforceApp
165165
invoker?: "public" | "private" | string | string[];
166166
}
167167

168+
/**
169+
*
170+
Callback function to run before processing a request for callable functions to authorize the request.
171+
*/
172+
export type AuthPolicy<T = any> = (auth: AuthData | null, data: T) => boolean | Promise<boolean>;
173+
168174
/**
169175
* Options that can be set on a callable HTTPS function.
170176
*/
@@ -212,34 +218,73 @@ export interface CallableOptions<T = any> extends HttpsOptions {
212218
/**
213219
* Callback for whether a request is authorized.
214220
*
215-
* Designed to allow reusable auth policies to be passed as an options object. Two built-in reusable policies exist:
216-
* isSignedIn and hasClaim.
221+
* Designed to allow reusable auth policies to be passed as an options object.
217222
*/
218-
authPolicy?: (auth: AuthData | null, data: T) => boolean | Promise<boolean>;
223+
authPolicy?: AuthPolicy<T>;
219224
}
220225

221226
/**
222227
* An auth policy that requires a user to be signed in.
223228
*/
224-
export const isSignedIn =
225-
() =>
226-
(auth: AuthData | null): boolean =>
227-
!!auth;
229+
export function isSignedIn(): AuthPolicy {
230+
return (auth: AuthData | null): boolean => {
231+
if (!auth) {
232+
throw new Error("Must be signed in");
233+
}
234+
return true;
235+
};
236+
}
228237

238+
export function hasClaim(claim: string, value?: string): AuthPolicy;
239+
export function hasClaim(claims: string[]): AuthPolicy;
240+
export function hasClaim(claims: Record<string, unknown>): AuthPolicy;
229241
/**
230-
* An auth policy that requires a user to be both signed in and have a specific claim (optionally with a specific value)
242+
* An auth policy that requires a user to be both signed in and have a specific claims (optionally with a specific value)
231243
*/
232-
export const hasClaim =
233-
(claim: string, value?: string) =>
234-
(auth: AuthData | null): boolean => {
244+
export function hasClaim(
245+
claimOrClaims: string | string[] | Record<string, unknown>,
246+
value?: string
247+
): AuthPolicy {
248+
let claimsToCheck: Record<string, unknown> = {};
249+
250+
if (typeof claimOrClaims === "string") {
251+
claimsToCheck[claimOrClaims] = value ?? true;
252+
} else if (Array.isArray(claimOrClaims)) {
253+
for (const claim of claimOrClaims) {
254+
claimsToCheck[claim] = true;
255+
}
256+
} else {
257+
claimsToCheck = claimOrClaims;
258+
}
259+
260+
return (auth: AuthData | null): boolean => {
235261
if (!auth) {
236-
return false;
262+
throw new Error("Must be signed in");
237263
}
238-
if (!(claim in auth.token)) {
239-
return false;
264+
for (const claim of Object.keys(claimsToCheck)) {
265+
if (!(claim in auth.token)) {
266+
throw new Error(`Missing claim '${claim}'`);
267+
}
268+
if (auth.token[claim] !== claimsToCheck[claim]) {
269+
throw new Error(`Missing claim '${claim}' with value '${value}'`);
270+
}
271+
}
272+
return true;
273+
};
274+
}
275+
276+
/**
277+
* An auth policy that requires a user to be both signed in and have email verified
278+
*/
279+
export function emailVerified(): AuthPolicy {
280+
return (auth: AuthData | null): boolean | Promise<boolean> => {
281+
if (!auth) {
282+
throw new Error("Must be signed in");
240283
}
241-
return !value || auth.token[claim] === value;
284+
console.log(auth)
285+
return hasClaim("email_verified")(auth, undefined);
242286
};
287+
}
243288

244289
/**
245290
* Handles HTTPS requests.

0 commit comments

Comments
 (0)