Merge pull request #23 from apostrophecms/pro-6133-esm

enablePassportStrategies is now async
apostrophecms · Oct 25, 2024 · f5fd3d8 · f5fd3d8
2 parents 6baf478 + b67f844
commit f5fd3d8
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 39 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -5,9 +5,9 @@ name: tests
 # Controls when the action will run.
 on:
   push:
-    branches: [ '*' ]
+    branches: ['main']
   pull_request:
-    branches: [ '*' ]
+    branches: ['*']
 
   # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
@@ -20,26 +20,26 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node-version: [14, 16, 18]
-        mongodb-version: [4.2, 4.4, 5.0]
+        node-version: [18, 20, 22]
+        mongodb-version: [6.0, 7.0, 8.0]
 
     # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-    - name: Git checkout
-      uses: actions/checkout@v2
+      - name: Git checkout
+        uses: actions/checkout@v4
 
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v1
-      with:
-        node-version: ${{ matrix.node-version }}
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
 
-    - name: Start MongoDB
-      uses: supercharge/mongodb-github-action@1.3.0
-      with:
-        mongodb-version: ${{ matrix.mongodb-version }}
+      - name: Start MongoDB
+        uses: supercharge/mongodb-github-action@1.11.0
+        with:
+          mongodb-version: ${{ matrix.mongodb-version }}
 
-    - run: npm install
+      - run: npm install
 
-    - run: npm test
-      env:
-        CI: true
+      - run: npm test
+        env:
+          CI: true
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## UNRELEASED
+
+* Use `self.apos.root.import` instead of `self.apos.root.require`.
+* `enablePassportStrategies` is now async.
+
 ## 1.2.1 (2024-10-03)
 
 * Adds translation strings.
@@ -12,7 +17,7 @@ their account to a github account when the appropriate features are active as de
 pass it on to the strategy in both ways, as well as to both the login and callback routes. This allows `passport-github2`
 to capture the user's private email address correctly, and should help with other differences between strategies as well.
 * Back to using upstream `passport-oauth2-refresh` now that our PR has been accepted (thanks).
- 
+
 ## 1.2.0-alpha.4 - 2023-04-07
 
 * More dependency games.
@@ -48,4 +53,3 @@ Declared stable. No code changes.
 ## 1.0.0-beta - 2022-01-06
 
 Initial release for A3. Tested and working with Google and Okta. Other standard passport modules should also work, especially those based on OpenAuth.
-
diff --git a/index.js b/index.js
@@ -8,8 +8,8 @@ module.exports = {
     directory: 'modules',
     modules: getBundleModuleNames()
   },
-  init(self) {
-    self.enablePassportStrategies();
+  async init(self) {
+    await self.enablePassportStrategies();
   },
   options: {
     i18n: {
@@ -18,7 +18,7 @@ module.exports = {
   },
   methods(self) {
     return {
-      enablePassportStrategies() {
+      async enablePassportStrategies() {
         self.refresh = new AuthTokenRefresh();
         self.strategies = {};
         if (!self.apos.baseUrl) {
@@ -28,12 +28,14 @@ module.exports = {
           throw new Error('@apostrophecms/passport-bridge: you must configure the "strategies" option');
         }
 
-        self.options.strategies.forEach(spec => {
+        for (const spec of self.options.strategies) {
           // Works with npm modules that export the strategy directly, npm modules
           // that export a Strategy property, and directly passing in a strategy property
           // in the spec
-          const strategyModule = spec.module && self.apos.root.require(spec.module);
-          const Strategy = strategyModule ? (strategyModule.Strategy || strategyModule) : spec.Strategy;
+          const strategyModule = spec.module && await self.apos.root.import(spec.module);
+          const Strategy = strategyModule
+            ? (strategyModule.Strategy || strategyModule)
+            : spec.Strategy;
           if (!Strategy) {
             throw new Error('@apostrophecms/passport-bridge: each strategy must have a "module" setting\n' +
               'giving the name of an npm module installed in your project that\n' +
@@ -67,7 +69,7 @@ module.exports = {
           }, self.findOrCreateUser(spec));
           self.apos.login.passport.use(self.strategies[spec.name]);
           self.refresh.use(self.strategies[spec.name]);
-        });
+        };
       },
 
       // Returns the oauth2 callback URL, which must match the route
@@ -111,10 +113,12 @@ module.exports = {
         return (absolute ? (self.apos.baseUrl + self.apos.prefix) : '') + '/auth/' + strategyName + '/connect/' + token;
       },
 
-      // Adds the login route, which will be `/auth/strategyname/login`, where the strategy name
+      // Adds the login route
+      // which will be `/auth/strategyname/login`, where the strategy name
       // depends on the passport module being used.
       //
-      // Redirect users to this URL to start the process of logging them in via each strategy
+      // Redirect users to this URL
+      // to start the process of logging them in via each strategy
       addLoginRoute(spec) {
         self.apos.app.get(self.getLoginUrl(spec), (req, res, next) => {
           if (req.query.newLocale) {
@@ -208,7 +212,7 @@ module.exports = {
         self.apos.app.get(self.getFailureUrl(spec), function (req, res) {
           // Gets i18n'd in the template
           return self.sendPage(req, 'error', {
-            spec: spec,
+            spec,
             message: 'aposPassportBridge:rejected'
           });
         });
@@ -289,7 +293,12 @@ module.exports = {
             return callback(null, false);
           }
           try {
-            const user = await self.apos.user.find(adminReq, criteria).toObject() || (self.options.create && !connectingUserId && await self.createUser(spec, profile));
+            const user = await self.apos.user.find(adminReq, criteria).toObject() ||
+              (
+                self.options.create &&
+                !connectingUserId &&
+                await self.createUser(spec, profile)
+              );
             // Legacy, incompatible with Passport 0.6
             if (self.options.retainAccessTokenInSession && user && req) {
               req.session.accessToken = accessToken;
@@ -473,7 +482,9 @@ module.exports = {
           }
           let route;
           if (doc) {
-            const action = self.apos.page.isPage(doc) ? self.apos.page.action : self.apos.doc.getManager(doc).action;
+            const action = self.apos.page.isPage(doc)
+              ? self.apos.page.action
+              : self.apos.doc.getManager(doc).action;
             route = `${action}/${doc._id}/locale/${newLocale}`;
           } else {
             // Fall back to home page, with appropriate prefix

diff --git a/modules/@apostrophecms/user-passport-bridge/index.js b/modules/@apostrophecms/user-passport-bridge/index.js
@@ -75,7 +75,9 @@ module.exports = {
           return await fn(accessToken);
         } catch (e) {
           if (e.status && e.status === 401) {
-            const { accessToken } = await self.refreshTokens(user, strategy, refreshToken);
+            const {
+              accessToken
+            } = await self.refreshTokens(user, strategy, refreshToken);
             // On the second try, failure is failure
             // We don't need "await" because we are already returning
             // a promise

diff --git a/package.json b/package.json
@@ -16,15 +16,15 @@
   "author": "Apostrophe Technologies",
   "license": "MIT",
   "devDependencies": {
-    "eslint": "^7.9.0",
-    "eslint-config-apostrophe": "^3.4.0",
-    "eslint-config-standard": "^14.1.1",
+    "eslint": "^8.57.1",
+    "eslint-config-apostrophe": "^4.3.0",
+    "eslint-config-standard": "^17.1.0",
     "eslint-plugin-import": "^2.22.0",
     "eslint-plugin-node": "^11.1.0",
-    "eslint-plugin-promise": "^4.2.1",
+    "eslint-plugin-promise": "^6.6.0",
     "eslint-plugin-standard": "^4.0.1",
-    "stylelint": "^13.7.1",
-    "stylelint-config-apostrophe": "^1.0.0"
+    "stylelint": "^16.9.0",
+    "stylelint-config-apostrophe": "^4.1.0"
   },
   "dependencies": {
     "humanname": "^0.2.2",