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

chore!: deprecate reason for ArcjetAllowDecision #2715

Draft
wants to merge 10 commits into
base: main
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
27 changes: 19 additions & 8 deletions decorate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,16 @@ function isRateLimitReason(
return reason.isRateLimit();
}

function getRateLimitReason(
decision: ArcjetDecision,
): ArcjetRateLimitReason | undefined {
if (decision.isDenied() || decision.isChallenged()) {
if (decision.reason.isRateLimit()) {
return decision.reason;
}
}
}

function nearestLimit(
current: ArcjetRateLimitReason,
next: ArcjetRateLimitReason,
Expand Down Expand Up @@ -211,21 +221,22 @@ export function setRateLimitHeaders(
} else {
// For cached decisions, we may not have rule results, but we'd still have
// the top-level reason.
if (isRateLimitReason(decision.reason)) {
const rateLimitReason = getRateLimitReason(decision);
if (rateLimitReason) {
if (
typeof decision.reason.max !== "number" ||
typeof decision.reason.window !== "number" ||
typeof decision.reason.remaining !== "number" ||
typeof decision.reason.reset !== "number"
typeof rateLimitReason.max !== "number" ||
typeof rateLimitReason.window !== "number" ||
typeof rateLimitReason.remaining !== "number" ||
typeof rateLimitReason.reset !== "number"
) {
console.error(
format("Invalid rate limit encountered: %o", decision.reason),
format("Invalid rate limit encountered: %o", rateLimitReason),
);
return;
}

limit = toLimitString(decision.reason);
policy = toPolicyString([decision.reason.max, decision.reason.window]);
limit = toLimitString(rateLimitReason);
policy = toPolicyString([rateLimitReason.max, rateLimitReason.window]);
} else {
return;
}
Expand Down
31 changes: 16 additions & 15 deletions decorate/test/decorate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { expect } from "expect";
import { setRateLimitHeaders } from "../index";
import {
ArcjetAllowDecision,
ArcjetDenyDecision,
ArcjetRateLimitReason,
ArcjetReason,
ArcjetRuleResult,
Expand Down Expand Up @@ -307,7 +308,7 @@ describe("setRateLimitHeaders", () => {
const headers = new Headers();
setRateLimitHeaders(
headers,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -332,7 +333,7 @@ describe("setRateLimitHeaders", () => {
const headers = new Headers();
setRateLimitHeaders(
headers,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -356,7 +357,7 @@ describe("setRateLimitHeaders", () => {
const headers = new Headers();
setRateLimitHeaders(
headers,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -380,7 +381,7 @@ describe("setRateLimitHeaders", () => {
const headers = new Headers();
setRateLimitHeaders(
headers,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -404,7 +405,7 @@ describe("setRateLimitHeaders", () => {
const headers = new Headers();
setRateLimitHeaders(
headers,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand Down Expand Up @@ -1023,7 +1024,7 @@ describe("setRateLimitHeaders", () => {
const resp = new Response();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1048,7 +1049,7 @@ describe("setRateLimitHeaders", () => {
const resp = new Response();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1072,7 +1073,7 @@ describe("setRateLimitHeaders", () => {
const resp = new Response();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1096,7 +1097,7 @@ describe("setRateLimitHeaders", () => {
const resp = new Response();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1120,7 +1121,7 @@ describe("setRateLimitHeaders", () => {
const resp = new Response();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand Down Expand Up @@ -1834,7 +1835,7 @@ describe("setRateLimitHeaders", () => {
const resp = new OutgoingMessage();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1859,7 +1860,7 @@ describe("setRateLimitHeaders", () => {
const resp = new OutgoingMessage();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1883,7 +1884,7 @@ describe("setRateLimitHeaders", () => {
const resp = new OutgoingMessage();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1907,7 +1908,7 @@ describe("setRateLimitHeaders", () => {
const resp = new OutgoingMessage();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand All @@ -1931,7 +1932,7 @@ describe("setRateLimitHeaders", () => {
const resp = new OutgoingMessage();
setRateLimitHeaders(
resp,
new ArcjetAllowDecision({
new ArcjetDenyDecision({
results: [],
ttl: 0,
reason: new ArcjetRateLimitReason({
Expand Down
70 changes: 53 additions & 17 deletions examples/nextjs-bot-categories/app/api/arcjet/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import arcjet, { botCategories, detectBot } from "@arcjet/next";
import arcjet, { ArcjetRuleResult, botCategories, detectBot } from "@arcjet/next";
import { NextResponse } from "next/server";

const aj = arcjet({
Expand All @@ -23,30 +23,66 @@ const aj = arcjet({
],
});

function reduceAllowedBots(bots: string[], rule: ArcjetRuleResult) {
if (rule.reason.isBot()) {
return [...bots, ...rule.reason.allowed]
} else {
return bots;
}
}

function reduceDeniedBots(bots: string[], rule: ArcjetRuleResult) {
if (rule.reason.isBot()) {
return [...bots, ...rule.reason.denied]
} else {
return bots;
}
}

function checkSpoofed(rule: ArcjetRuleResult) {
return rule.reason.isBot() && rule.reason.isSpoofed()
}

function collectErrors(errors: string[], rule: ArcjetRuleResult) {
if (rule.reason.isError()) {
return [...errors, rule.reason.message];
} else {
return errors;
}
}

export async function GET(req: Request) {
const decision = await aj.protect(req);

if (decision.isErrored()) {
const errors = decision.results.reduce(collectErrors, []);
if (errors.length > 0) {
return NextResponse.json(
{ error: decision.reason.message },
{ errors },
{ status: 500, statusText: "Internal Server Error" },
)
}

const headers = new Headers();
if (decision.reason.isBot()) {
// WARNING: This is illustrative! Don't share this metadata with users;
// otherwise they may use it to subvert bot detection!
headers.set("X-Arcjet-Bot-Allowed", decision.reason.allowed.join(", "))
headers.set("X-Arcjet-Bot-Denied", decision.reason.denied.join(", "))

// We need to check that the bot is who they say they are.
if (decision.reason.isSpoofed()) {
return NextResponse.json(
{ error: "You are pretending to be a good bot!" },
{ status: 403, headers },
);
}
const allowedBots = decision.results.reduce<string[]>(reduceAllowedBots, []);

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed you are still using map/filter in the other examples (for rate limit). We should keep it uniform. The reason to use a reduce is to avoid creating extra arrays, but the implementation of the reduction function is creating a new array on each iteration which is worse than a single filter then a map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed this to a reduce. This removes checking that a limit isn't duplicated which is discussed in another comment though.

const deniedBots = decision.results.reduce<string[]>(reduceDeniedBots, []);

const isSpoofed = decision.results.some(checkSpoofed);

// WARNING: This is illustrative! Don't share this metadata with users;
// otherwise they may use it to subvert bot detection!
const headers = new Headers({
"X-Arcjet-Bot-Allowed": allowedBots.join(", "),
"X-Arcjet-Bot-Denied": deniedBots.join(", "),
});
headers.set("X-Arcjet-Bot-Allowed", allowedBots.join(", "))
headers.set("X-Arcjet-Bot-Denied", deniedBots.join(", "))

// We need to check that the bot is who they say they are.
if (isSpoofed) {
return NextResponse.json(
{ error: "You are pretending to be a good bot!" },
{ status: 403, headers },
);
}

if (decision.isDenied()) {
Expand Down
60 changes: 53 additions & 7 deletions examples/nextjs-clerk-rate-limit/app/api/arcjet/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import arcjet, { ArcjetDecision, tokenBucket, shield } from "@arcjet/next";
import arcjet, { ArcjetDecision, tokenBucket, shield, ArcjetRateLimitReason, ArcjetRuleResult } from "@arcjet/next";
import { NextRequest, NextResponse } from "next/server";
import { currentUser } from "@clerk/nextjs/server";
import ip from "@arcjet/ip";
Expand All @@ -16,6 +16,49 @@ const aj = arcjet({
],
});

function nearestLimit(
current: ArcjetRateLimitReason,
next: ArcjetRateLimitReason,
) {
if (current.remaining < next.remaining) {
return current;
}

if (current.remaining > next.remaining) {
return next;
}

// Reaching here means `remaining` is equal so prioritize closest reset
if (current.reset < next.reset) {
return current;
}

if (current.reset > next.reset) {
return next;
}

// Reaching here means that `remaining` and `reset` are equal, so prioritize
// the smallest `max`
if (current.max < next.max) {
return current;
}

// All else equal, just return the next item in the list
return next;
}

function reduceNearestLimit(currentNearest: ArcjetRateLimitReason | undefined, rule: ArcjetRuleResult) {
if (rule.reason.isRateLimit()) {
if (typeof currentNearest !== "undefined") {
return nearestLimit(currentNearest, rule.reason);
} else {
return rule.reason;
}
} else {
return currentNearest;
}
}

export async function GET(req: NextRequest) {
// Get the current user from Clerk
// See https://clerk.com/docs/references/nextjs/current-user
Expand Down Expand Up @@ -69,13 +112,16 @@ export async function GET(req: NextRequest) {
);
}
}


// We need to find the nearest rate limit, because multiple rate limit rules could be defined
const nearestRateLimit = decision.results.reduce<ArcjetRateLimitReason | undefined>(reduceNearestLimit, undefined)

let reset: Date | undefined;
let remaining: number | undefined;

if (decision.reason.isRateLimit()) {
reset = decision.reason.resetTime;
remaining = decision.reason.remaining;
if (typeof nearestRateLimit !== "undefined") {
reset = nearestRateLimit.resetTime;
remaining = nearestRateLimit.remaining;
}

return NextResponse.json({ message: "Hello World", reset, remaining });}
return NextResponse.json({ message: "Hello World", reset, remaining });
}
18 changes: 18 additions & 0 deletions examples/nextjs-clerk-rate-limit/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion examples/nextjs-clerk-rate-limit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@
"tailwindcss": "^3.4.17",
"typescript": "^5"
}
}
}
Loading
Loading