Improve token.ts

[flevi29]

Pull Request

What does this PR do?

Important

Because this code can now be run in a browser as well, an additional, enabled-by-default, environment check is added to assert that the code runs server-side.

• fixes Avoid using Node.js specific API so that the project may conform to the WinterCG specification #1746
• switches from Node.js Crypto to Web Crypto, which makes this packages compatible with WinterCG specs
• removes some unnecessary checks
  • remove parameter type validations that are already validated by TypeScript
  • expiredAt is no longer checked for being in the past, because maybe the system time is wrong or cannot be determined accurately locally, let meilisearch determine it after using the token

Caution

BREAKING

• reworks generateTenantToken function, which now only accepts one object as a parameter, TenantTokenGeneratorOptions, all other parameters are now properties of this object
• new parameters/extended functionality
  • force - use to disable safety environment check for when it's necessary to do so
  • algorithm - use to select encryption algorithm
  • searchRules - can now be omitted, default value is ["*"]
  • expiresAt - can now be a number, a UNIX timestamp number
• removes type TokenOptions

• adds detailed documentation for types and functions, especially exported ones

TODO: Add option of MeiliSearch instead of apiKeyUid? Or rather raise issue about it for now.

[codecov]

Codecov Report

Attention: Patch coverage is 89.47368% with 10 lines in your changes missing coverage. Please review.

Project coverage is 98.20%. Comparing base (47fcb09) to head (4d305ed).
Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/token.ts	89.47%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1661      +/-   ##
==========================================
+ Coverage   97.93%   98.20%   +0.27%     
==========================================
  Files          18       18              
  Lines        1546     1562      +16     
  Branches      326      333       +7     
==========================================
+ Hits         1514     1534      +20     
+ Misses         32       28       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[flevi29]

Oh well, crypto global is only available since Node.js 19, bummer, will try to fix this later.
https://developer.mozilla.org/en-US/docs/Web/API/Crypto#browser_compatibility

[flevi29]

Hmm, maybe I should do something so @types/node only applies to webcrypto import on fix, I'll convert it to draft for now.

[flevi29]

I tried to fix the typing issue for the node:crypto import properly, but that opened up another can of worms, found some more bugs/issues, too many changes for this PR, so I just @ts-expect-error it for now.

[flevi29]

This PR would make #1595 more or less obsolete, but there might be more things to consider before we can truly say that.
EDIT: I'm not sure what I was thinking when I wrote the last part of my comment, but yeah, it does make it obsolete, now there's only one client, JS client.

[brunoocasali]

Hi @flevi29 so the idea of not allowing the customer to use the generateTenantToken in the web version is a business decision, not a technical one. Let me give you the context:

In order to generate the token, the user will have to expose a real key (usually a key with more permissions like the masterKey). If they do that using the front-end integration, our users will be exposing that info incorrectly. To prevent the users from making those mistakes, we only allowed them to generate the keys using this integration in the backend (node).

[flevi29]

I see, Web Crypto can only be used in secure contexts, not sure if that changes anything, but I see why we need to separate this part of the code.

I still think it's the right move to use Web Crypto, because of WinterCG and the Minimum Common API that Node.js and Deno for example subscribe to otherwise.

I will try and separate tokens into a separate export, related to #1611.

[brunoocasali]

I see, Web Crypto can only be used in secure contexts, not sure if that changes anything, but I see why we need to separate this part of the code.

I still think it's the right move to use Web Crypto, because of WinterCG and the Minimum Common API that Node.js and Deno for example subscribe to otherwise.

I will try and separate tokens into a separate export, related to #1611.

but how can we protect the user to make those mistakes?

[flevi29]

It's going to be a separate export. They're going to have to import from "meilisearch/tokens", or whatever we name it, otherwise it won't be bundled into their code in any way.

We will warn people to use it only on backend and any other way only if they really know what they're doing, use it at their own risk, and explain the risks involved. Maybe even console log some big warnings when they use it, although I can see this becoming a little annoying.

If they really wanted to they could implement this on their own on the front end via Web Crypto, as you can see it's not a complex change, there's only so much we can do for users to be "protected" from their own mistakes, and even so there are protections in place, for example they can only use Web Crypto in a browser in a secure context (HTTPS), although I know this might not be entirely foolproof.

Anyhow not having to worry about two separate JS clients is a worthwhile change in my opinion.

[flevi29]

This is not valid, looking at the rust source code I couldn't find such a thing.

[flevi29]

@brunoocasali Hey, this PR is finally ready for another review. Now users will be protected from using this on the frontend.

[brunoocasali]

LGTM!!!

Can you just create a nice upgrade path for the users when they face the changes in this PR?

[curquiza]

⚠️ We will merge this AFTER 23rd of December please 😊

[flevi29]

Migration

Old

await generateTenantToken("74c9c733-3368-4738-bbe5-1d18a5fecb37", [], {
  apiKey: "0a6e572506c52ab0bd6195921575d23092b7f0c284ab4ac86d12346c33057f99",
  expiresAt: new Date("December 17, 4000 03:24:00"),
});

New

await generateTenantToken({
  apiKey: "0a6e572506c52ab0bd6195921575d23092b7f0c284ab4ac86d12346c33057f99",
  apiKeyUid: "74c9c733-3368-4738-bbe5-1d18a5fecb37",
  searchRules: [],
  expiresAt: new Date("December 17, 4000 03:24:00"),
});

• TokenOptions -> TenantTokenGeneratorOptions
  • now contains all the parameters generateTenantToken accepts as properties

[flevi29]

bors merge

[meili-bors]

Build succeeded:

• types-check
• integration-tests (Node.js 20)
• integration-tests (Node.js 18)
• style-check