ciscoheat / sveltekit-rate-limiter

A modular rate limiter for SvelteKit. Use in password resets, account registration, etc.
MIT License
216 stars 3 forks source link

Feature request: Allow additonal arguments to be passed to `limiter.isLimited` / `limiter.check` #6

Closed oscarhermoso closed 7 months ago

oscarhermoso commented 7 months ago

Feature request: Allow additonal arguments to be passed to limiter.isLimited / limiter.check

I want to implement a custom rate limiter for my app that takes into consideration some information that was not in the original request event, for example:

// include some info that is not in the RequestEvent
const status = await limiter.check(event, 'email@test.com');

I've skimmed the code and doesn't look too hard to implement, just need to pass arguments through a few points and update type definitions, assuming this is a feature that looks OK to implement.

Code

https://github.com/ciscoheat/sveltekit-rate-limiter/blob/0466070755d8458f3defb376df33b01888288df2/src/lib/server/index.ts#L256 https://github.com/ciscoheat/sveltekit-rate-limiter/blob/0466070755d8458f3defb376df33b01888288df2/src/lib/server/index.ts#L272-L274 https://github.com/ciscoheat/sveltekit-rate-limiter/blob/0466070755d8458f3defb376df33b01888288df2/src/lib/server/index.ts#L278 https://github.com/ciscoheat/sveltekit-rate-limiter/blob/0466070755d8458f3defb376df33b01888288df2/src/lib/server/index.ts#L409

Workaround

I was able to implement a work-around for now, by extending the type definition of the SvelteKit RequestEvent.

$lib/server/rate-limit.ts

import type { RequestEvent } from '@sveltejs/kit';
import {
    RetryAfterRateLimiter,
    type Rate,
    type RateLimiterPlugin,
} from 'sveltekit-rate-limiter/server';

declare module '@sveltejs/kit' {
    /**
     * The sveltekit-rate-limiter check function only takes a single argument, the RequestEvent.
     * This is a workaround to make additional arguments available to the RateLimiter.
     */
    export interface RequestEvent {
        _rate_limit_args?: { email: string };
    }
}

export class EmailAddressRateLimiter implements RateLimiterPlugin {
    readonly rate: Rate;

    constructor(rate: Rate) {
        this.rate = rate;
    }

    // eslint-disable-next-line @typescript-eslint/require-await
    async hash(event: RequestEvent) {
        return event._rate_limit_args?.email || false;
    }
}

$routes/+page.server.ts

// usage:
const limiter = new RetryAfterRateLimiter({
    IP: [10, 'h'],
    IPUA: [5, 'm'],
    plugins: [new EmailAddressRateLimiter([5, 'm'])],
});

export const actions = {
    default: async (event) => {

        // check auth, etc.

        const _rate_limit_args = { email: 'email@test.com' };

        const status = await limiter.check({ ...event, _rate_limit_args });

        if (status.limited) {
            // 429 response
        }

        // ...rest of the action
    },
};
ciscoheat commented 7 months ago

Added now in 0.5.0, check the readme for an example: https://github.com/ciscoheat/sveltekit-rate-limiter?tab=readme-ov-file#custom-data-for-the-limiter (and let me know if it works well!)

oscarhermoso commented 7 months ago

Woah, super awesome. Not every day you raise a GitHub issue and see a new release has been pushed the next morning

Looks like it's working great! (although I'm knee-deep in a Lucia auth migration from v2 to v3 - my app is not in a working state to test any more than type definitions right now)

It kind of sucks to use classes because of https://github.com/Microsoft/TypeScript/issues/1373, and I don't think generics are being inferred properly, but works great with regular functions.

import type { RequestEvent } from '@sveltejs/kit';
import { type Rate, type RateLimiterPlugin } from 'sveltekit-rate-limiter/server';

// pretty verbose, can't infer extraData type
export class EmailAddressRateLimiter implements RateLimiterPlugin<{ email: string }> {
    readonly rate: Rate;

    constructor(rate: Rate) {
        this.rate = rate;
    }

    // eslint-disable-next-line @typescript-eslint/require-await
    async hash(_: RequestEvent, extraData: { email: string }) {
        return extraData.email || false;
    }
}

// much cleaner
const email_rate_limiter = (rate: Rate): RateLimiterPlugin<{ email: string }> => {
    return {
        rate,
        // eslint-disable-next-line @typescript-eslint/require-await
        hash: async (_, extraData) => extraData.email || false,
    };
};

Side note - would be nice if the hash method returned a MaybePromise<...> instead of a Promise<...>, where MaybePromise<T> = T | Promise<T>.

Currently if I remove the async modifier I get a type error, and if I include the async modifier it I get a linter error.

ciscoheat commented 7 months ago

No problem, it was a useful feature and an update was overdue. :) I understand the problem with type inference in classes, so I'm glad it works with both versions. I think the classes are small enough to not be overly verbose though. Added MaybePromise to the interfaces in 0.5.1.

oscarhermoso commented 7 months ago

Thanks Andreas!