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

Could not resolve "crypto" #1

Closed ccremer closed 1 year ago

ccremer commented 1 year ago

When using the adapter-cloudflare, the build fails with an error like this:

> Using @sveltejs/adapter-cloudflare
✘ [ERROR] Could not resolve "crypto"

    .svelte-kit/output/server/chunks/ratelimit.js:3:19:
      3 │ import crypto from "crypto";
        ╵                    ~~~~~~~~

  The package "crypto" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

error during build:
Error: Build failed with 1 error:
.svelte-kit/output/server/chunks/ratelimit.js:3:19: ERROR: Could not resolve "crypto"

I've looked around for solutions, for but I didn't find any, so I opened this issue.

It seems the only occurrence of crypto is 1 line for hashing, so maybe this could be solved differently, e.g. with a custom function? https://github.com/ciscoheat/sveltekit-rate-limiter/blob/eb88637b4eab570746726c1797a447569f89d2b3/src/lib/server/index.ts#L155

Cloudflare recommends their Web-Crypto API (https://developers.cloudflare.com/workers/runtime-apis/web-crypto) for doing stuff crypto-related.

The Web Crypto API differs significantly from Node’s Crypto API. If you want to port JavaScript code that relies on Node’s Crypto API, you will need to adapt it to use Web Crypto primitives.

Possible solutions

Personally, I'd prefer the 2nd option, unless you have a better proposal :)

ciscoheat commented 1 year ago

Hi, I was waiting for this issue to appear. :) Adding it as an function would work, though I'd like to keep the current as the default however. I wonder if that could be done with a dynamic import?

ccremer commented 1 year ago

+1 for keeping the default. Haven't used dynamic import myself yet, but it sounds like it could be used for this case. We could conditionally import crypto when a custom function is undefined somewhere in a constructor or so.

ciscoheat commented 1 year ago

I'll make some experimentation with that. I can't foresee that the hash function will be anything else than (string) => string, so adding it as an option should be enough.

ciscoheat commented 1 year ago

I've released v0.4.0 now, which should handle this automatically in both cases now. A hashFunction option has been added as well for completeness. Can you check that it's working for you?

Just a note for v0.4, the cookie limiter's preflight method is now async, so you need to await for it.

ccremer commented 1 year ago

Hm, unfortunately it still doesn't work:


export const rateLimiter = new RateLimiter({
  // A rate is defined as [number, unit]
  rates: {
    IP: [10, 'h'], // IP address limiter
    IPUA: [4, 'm'], // IP + User Agent limiter
    cookie: {
      name: 'limiterid',
      secret: env.RATELIMIT_SECRET_KEY ?? 'DEV',
      rate: [30, 'm'],
      preflight: true,
    },

  },
  hashFunction: async (v) => {
    return `hash`
  }
});

yields error when building:

> Using @sveltejs/adapter-cloudflare
✘ [ERROR] Could not resolve "crypto"

    .svelte-kit/output/server/chunks/ratelimit.js:81:9:
      81 │   import("crypto").then((crypto2) => {
         ╵          ~~~~~~~~

  The package "crypto" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

error during build:
Error: Build failed with 1 error:
.svelte-kit/output/server/chunks/ratelimit.js:81:9: ERROR: Could not resolve "crypto"
ccremer commented 1 year ago

You could actually reproduce it locally, without having a cloudflare account. Just switch to adapter-cloudflare in svelte.config.js and run vite build, the error should pop up

ciscoheat commented 1 year ago

Ok, should be no problem to fix it then, if I can reproduce it locally!

ciscoheat commented 1 year ago

Let's try again in 0.4.1 then!

ccremer commented 1 year ago

:tada: This works, and also deployed on cloudflare I'm able to trigger the limiter :) Many thanks!

From my PoV you can close this issue, unless you want to keep it open for adding documentation or so.

ciscoheat commented 1 year ago

I've added a bit about to the docs, so I'll close it. Thanks for helping out!