Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@uppy/companion: add oauthOrigin option #5297

Merged
merged 4 commits into from
Jul 2, 2024
Merged

@uppy/companion: add oauthOrigin option #5297

merged 4 commits into from
Jul 2, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 2, 2024

No description provided.

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Diff output files
diff --git a/packages/@uppy/companion/lib/server/controllers/connect.js b/packages/@uppy/companion/lib/server/controllers/connect.js
index ceac203..49b0c5c 100644
--- a/packages/@uppy/companion/lib/server/controllers/connect.js
+++ b/packages/@uppy/companion/lib/server/controllers/connect.js
@@ -14,11 +14,24 @@ const queryString = (params, prefix = "?") => {
  */
 module.exports = function connect(req, res) {
   var _a;
-  const { secret } = req.companion.options;
+  const { secret, oauthOrigin } = req.companion.options;
   const stateObj = oAuthState.generateState();
-  if (req.query.state) {
+  // not sure if we need to store origin in the session state (e.g. we could've just gotten it directly inside send-token)
+  // but we're afraid to change the logic there
+  if (oauthOrigin && !Array.isArray(oauthOrigin)) {
+    // If the server only allows a single origin, we ignore the client-supplied
+    // origin from query because we don't need it.
+    stateObj.origin = oauthOrigin;
+  } else if (oauthOrigin && oauthOrigin.length < 2) {
+    // eslint-disable-next-line prefer-destructuring
+    stateObj.origin = oauthOrigin[0];
+  } else {
+    // If we have multiple allowed origins, we need to check the client-supplied origin from query.
+    // If the client provides an untrusted origin,
+    // we want to send `undefined`. `undefined` means `/`, which is the same origin when passed to `postMessage`.
+    // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-window-postmessage-options-dev
     const { origin } = JSON.parse(atob(req.query.state));
-    stateObj.origin = origin;
+    stateObj.origin = oauthOrigin ? oauthOrigin.find(o => o === origin) : origin;
   }
   if (req.companion.options.server.oauthDomain) {
     stateObj.companionInstance = req.companion.buildURL("", true);
diff --git a/packages/@uppy/companion/lib/standalone/helper.js b/packages/@uppy/companion/lib/standalone/helper.js
index 6a83a86..d42c91c 100644
--- a/packages/@uppy/companion/lib/standalone/helper.js
+++ b/packages/@uppy/companion/lib/standalone/helper.js
@@ -62,6 +62,7 @@ const defaultStandaloneGetKey = (...args) => `${s3Prefix}${utils.defaultGetKey(.
  * @returns {object}
  */
 const getConfigFromEnv = () => {
+  var _a;
   const uploadUrls = process.env.COMPANION_UPLOAD_URLS;
   const domains = process.env.COMPANION_DOMAINS || process.env.COMPANION_DOMAIN || null;
   const validHosts = domains ? domains.split(",") : [];
@@ -180,6 +181,9 @@ const getConfigFromEnv = () => {
     corsOrigins: getCorsOrigins(),
     testDynamicOauthCredentials: process.env.COMPANION_TEST_DYNAMIC_OAUTH_CREDENTIALS === "true",
     testDynamicOauthCredentialsSecret: process.env.COMPANION_TEST_DYNAMIC_OAUTH_CREDENTIALS_SECRET,
+    oauthOrigin: ((_a = process.env.COMPANION_OAUTH_ORIGIN) === null || _a === void 0 ? void 0 : _a.includes(","))
+      ? process.env.COMPANION_OAUTH_ORIGIN.split(",")
+      : process.env.COMPANION_OAUTH_ORIGIN,
   };
 };
 /**

Co-authored-by: Merlijn Vos <merlijn@soverin.net>
docs/companion.md Outdated Show resolved Hide resolved
docs/companion.md Outdated Show resolved Hide resolved
@aduh95 aduh95 merged commit e849827 into main Jul 2, 2024
20 checks passed
@aduh95 aduh95 deleted the oauth-origin branch July 2, 2024 13:33
This was referenced Jul 2, 2024
github-actions bot added a commit that referenced this pull request Jul 2, 2024
| Package                | Version | Package                | Version |
| ---------------------- | ------- | ---------------------- | ------- |
| @uppy/companion        |  4.15.0 | @uppy/drag-drop        |   3.1.1 |
| @uppy/companion-client |   3.8.2 | @uppy/form             |   3.2.2 |
| @uppy/core             |  3.13.1 | uppy                   |  3.27.2 |

- @uppy/form: do not emit `'submit'` event more than once (Merlijn Vos / #5299)
- @uppy/companion: add `s3.forcePathStyle` option (Nadeem Reinhardt / #5066)
- meta: fix broken workflow status badges in README (Alexander Zaytsev / #5298)
- @uppy/core: add `clearUploadedFiles` to type definition (Augustine Smith / #5295)
- @uppy/companion: add `oauthOrigin` option (Antoine du Hamel / #5297)
- meta: add dark-mode Transloadit logo in README (Alexander Zaytsev / #5291)
- docs,@uppy/drag-drop: `uppy.io/docs` - fix typos/broken links (Evgenia Karunus / #5296)
- meta: Bump docker/build-push-action from 6.1.0 to 6.2.0 (dependabot[bot] / #5290)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants