provide a esm module compatible with bundlers

lovasoa commented 3 weeks ago

Hi !

There is an issue here:

https://github.com/CleverCloud/clever-client.js/blob/61fc4e691dee9345e61646e9869bf2f29e66ccee/esm/oauth.js#L3-L14

The code tries to be smart, but modern js bundlers will see this code, ignore the if, and try to resolve the import('node:crypto'), failing if it's not available. It would be great if we could import @clever-cloud/client/esm/oauth-web and @clever-cloud/client/esm/oauth-node separately, in order to avoid bundlers ever seeing node imports.

lovasoa commented 2 weeks ago

Here is a version that works in all modern js runtimes, including recent node versions, deno, browsers...

import OAuth from 'oauth-1.0a';

// Required because import { addOauthHeader } from './clevercloud-oauth'; does not work on cloudflare

// @ts-ignore
export function addOauthHeader(tokens: any) {
    return async function(requestParams: any) {
      const { method, url, headers, queryParams } = requestParams;

      const oauth = new OAuth({
        consumer: {
          key: tokens.OAUTH_CONSUMER_KEY,
          secret: tokens.OAUTH_CONSUMER_SECRET,
        },
        signature_method: 'HMAC-SHA512',
        // @ts-ignore
        hash_function: async (baseString, key) => { 
          const encoder = new TextEncoder();
          const encodedText = encoder.encode(baseString);
          const encodedKey = encoder.encode(key);

          const cryptoKey = await crypto.subtle.importKey(
            'raw',
            encodedKey,
            { name: 'HMAC', hash: 'SHA-512' },
            false,
            ['sign']
          );

          const result = await crypto.subtle.sign(
            { name: 'HMAC', hash: 'SHA-512' },
            cryptoKey,
            encodedText
          );

          // Use modern base64 encoding
          // @ts-ignore
          return btoa(String.fromCharCode.apply(null, new Uint8Array(result)));
        },
      });

      const requestData = { url, method, data: queryParams };
      const oauthData = oauth.authorize(requestData, {
        key: tokens.API_OAUTH_TOKEN,
        secret: tokens.API_OAUTH_TOKEN_SECRET,
      });

      oauthData.oauth_signature = await oauthData.oauth_signature;
      const oauthHeaders = oauth.toHeader(oauthData);

      return {
        ...requestParams,
        headers: {
          ...oauthHeaders,
          ...headers,
        },
      };
    };
  }

One could make a fallback version for old node versions, but honestly, it's probably better to just use the code that works in all modern environments.

hsablonniere commented 2 weeks ago

Hey @lovasoa,

Thanks for your message ;-)

We used to have two implementations (browser and Node.js) of the oauth file: https://github.com/CleverCloud/clever-client.js/commit/89cb501fabee530ece877e26f8028bf78558cc3f
It was a bit cumbersome and with new versions of Node.js, so we decided to "simplify" things and merge into one file.
The problem as you mentioned it is bundler usage.
- Technically, they don't ignore the if, they work on the AST level and will always try to handle the dynamic import.
- In our web console, we added a externals: { 'node:crypto': 'nodeCrypto' } in our Webpack config but I can understand the situation can/must be improved.
- In your CF worker, you could fix the situation with a custom build but it's way too much for such a simple situation.
Also, your comment made me realize that Node.js (like Deno, Bun and other runtimes like CF workers) expose a crypto object globally with the Web subtle implementation.

Anyway, now that I've explained the historical details, I'll take some time to check that your suggestion works in our console and CLI and prepare a release so we can fix and simplify your situation :wink:

hsablonniere commented 2 weeks ago

OK, I just checked on a Node.js 18 (which the version used by our CLI) and it's not available globally. I think that's why we went for an explicit dynamic import.

We're currently reworking lots of things in our CLI (to be able to migrate to v22 and many other nice things). Once we have that, we will be able to rely on crypto being global. This will happen in the next few weeks but I can imagine you'd like a solution before that.

Is it OK for you to have a patched copy of esm/oauth.js while we wait to settle things?

hsablonniere commented 2 weeks ago

Wait, thinking as I write... we could patch the global and add crypto ourselves in our Node.js 18 CLI.

hsablonniere commented 5 days ago

Fixed in 9.0.0 :wink:

CleverCloud / clever-client.js

provide a esm module compatible with bundlers #113