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

OIDC issuer behind a proxy cannot be accessed #942

Open
gautierrog opened this issue Apr 18, 2024 · 4 comments · May be fixed by #1363
Open

OIDC issuer behind a proxy cannot be accessed #942

gautierrog opened this issue Apr 18, 2024 · 4 comments · May be fixed by #1363

Comments

@gautierrog
Copy link

Hello,

Self hosting grist with docker here. I have an OIDC issuer which can only be accessed through a proxy.

I pass the following environment variables on my container:

  • HTTP_PROXY
  • HTTPS_PROXY
  • http_proxy
  • https_proxy
  • NO_PROXY
  • no_proxy

However, issuer is still unreachable:

RPError: outgoing request timed out after 3500ms
    at /grist/node_modules/openid-client/lib/helpers/request.js:137:13
    at async Issuer.discover (/grist/node_modules/openid-client/lib/issuer.js:171:22)
    at async OIDCConfig.initOIDC (/grist/_build/app/server/lib/OIDCConfig.js:100:24)
    at async Object.getMiddleware (/grist/_build/app/server/lib/OIDCConfig.js:229:13)
    at async FlexServer.loadConfig (/grist/_build/app/server/lib/FlexServer.js:1093:33)
    at async main (/grist/_build/app/server/mergedServerMain.js:83:5)
    at async main (/grist/_build/stubs/app/server/server.js:144:20)

I did not find any documentation telling how to setup a proxy for grist server. What would be the solution ?

@plimptm
Copy link

plimptm commented May 30, 2024

I just had the exact same error reported here in my instance which is also behind a (corporate) forward proxy.

I was suspicious that the environment variables were not being utilized so I set NODE_DEBUG=https and confirmed that the outgoing connection to my OIDC IDP issuer is not using the proxy which has been set.

I think this node-openid-client functionality might be relevant to getting this enabled in grist-core.

Perhaps adding something like the following into OIDCConfig.ts would do the trick:

import { ProxyAgent } from 'proxy-agent';
// The correct proxy `Agent` implementation to use will be determined
// via the `http_proxy` / `https_proxy` / `no_proxy` / etc. env vars
const agent = new ProxyAgent();

import { custom } from 'openid-client';
client[custom.http_options] = function (url, options) {
  const result = {};
  // use HTTP(S)_PROXY
  // https://nodejs.org/api/http.html#httprequesturl-options-callback
  // e.g. using https://www.npmjs.com/package/proxy-agent
  result.agent = agent;

  return result;
}

@paulfitz
Copy link
Member

Node has been a bit of an outlier in terms of support for proxy variables. Since nodejs/undici#2994 (undici is what node uses for its native fetch implementation) that may be changing. In the meantime, there is a proxyAgent(url) method in app/server/lib/ProxyAgent.ts that can give an agent consistent with other parts of Grist. @fflorent what do you think of telling openid-client about that in the way @plimptm suggests?

@fflorent
Copy link
Collaborator

Since nodejs/undici#2994 (undici is what node uses for its native fetch implementation) that may be changing.

Unfortunately, node-openid-client uses directly the http / https native module: https://nodejs.org/api/http.html

@fflorent what do you think of telling openid-client about that in the way @plimptm suggests?

That should be pretty much doable, and not very difficult. The harder would be to setup an environment for testing manually (or even better but even harder: setting up an integration test in the CI), that's the reason why I would not flag it good-first-issue.

@plimptm
Copy link

plimptm commented May 30, 2024

@fflorent I'm more than willing to help with testing code changes, both using my existing environment and hopefully coming up with a similar test environment.

I took a stab at implementing my own suggestion earlier and built a fresh docker image. It built and started up fine, but the timeout issue did not go away. I probably just didn't do it properly...admittedly my js skills are not the best.

Here is my diff, maybe something obvious will stand out:

diff --git a/app/server/lib/OIDCConfig.ts b/app/server/lib/OIDCConfig.ts
index 86f78bce..9f9e78ee 100644
--- a/app/server/lib/OIDCConfig.ts
+++ b/app/server/lib/OIDCConfig.ts
@@ -52,12 +52,13 @@
 
 import * as express from 'express';
 import { GristLoginSystem, GristServer } from './GristServer';
-import { Client, generators, Issuer, UserinfoResponse } from 'openid-client';
+import { Client, generators, Issuer, UserinfoResponse, custom } from 'openid-client';
 import { Sessions } from './Sessions';
 import log from 'app/server/lib/log';
 import { appSettings } from './AppSettings';
 import { RequestWithLogin } from './Authorizer';
 import { UserProfile } from 'app/common/LoginSessionAPI';
+import { ProxyAgent } from 'proxy-agent';
 
 const CALLBACK_URL = '/oauth2/callback';
 
@@ -121,6 +122,12 @@ export class OIDCConfig {
       redirect_uris: [ this._redirectUrl ],
       response_types: [ 'code' ],
     });
+    this._client[custom.http_options] = function(url, options) {
+           // use HTTP(S)_PROXY env vars
+           // https://nodejs.org/api/http.html#httprequesturl-options-callback
+           const agent = new ProxyAgent();
+           return { agent };
+    }
     if (this._client.issuer.metadata.end_session_endpoint === undefined &&
         !this._endSessionEndpoint && !this._skipEndSessionEndpoint) {
       throw new Error('The Identity provider does not propose end_session_endpoint. ' +
diff --git a/package.json b/package.json
index bbe1c5e5..c6d83c88 100644
--- a/package.json
+++ b/package.json
@@ -182,6 +182,7 @@
     "popper-max-size-modifier": "0.2.0",
     "popweasel": "0.1.20",
     "prom-client": "14.2.0",
+    "proxy-agent": "6.4.0",

tristanrobert added a commit to tristanrobert/grist-core that referenced this issue Jan 6, 2025
@tristanrobert tristanrobert linked a pull request Jan 6, 2025 that will close this issue
4 tasks
tristanrobert added a commit to tristanrobert/grist-core that referenced this issue Jan 8, 2025
tristanrobert pushed a commit to tristanrobert/grist-core that referenced this issue Jan 10, 2025
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 a pull request may close this issue.

4 participants