-
-
Notifications
You must be signed in to change notification settings - Fork 342
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 #1363
base: main
Are you sure you want to change the base?
OIDC issuer behind a proxy cannot be accessed #1363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patch @tristanrobert!
Some tests are failling. Some are just because of instabilities like Importer2
, we can ignore them.
But some of them are due to the enhancement you introduced (these tests). Are you comfortable with adapting them? Or do you want me to do that for you?
The tests to adapt are here:
https://github.com/gristlabs/grist-core/blob/main/test/server/lib/OIDCConfig.ts
To setup the environment and to run the tests: https://github.com/gristlabs/grist-core/blob/main/documentation/develop.md
I have fixed the tests @fflorent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds almost good.
I left you some remarks. Also I need to setup an environment to test your work. I am doing that ASAP.
Co-authored-by: Florent <[email protected]>
Co-authored-by: Florent <[email protected]>
Co-authored-by: Florent <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @tristanrobert!
The code here looks fine for doing what it intends, but I am not sure we want
@paulfitz , what do you think? |
That's a good point @dsagal. There are two distinct usages here, and they could conflict. A proxy set up specifically for untrusted requests might not even be able to reach some internal OIDC-related server. Not sure I have a good idea how to deal with this. Perhaps the proxying of untrusted traffic should have distinct configuration? The standard environment variable for this in other tools is |
Fixes #942
Context
Self hosted instance behind corporate proxy with OIDC issuer outside corporate LAN.
Proposed solution
Add ./ProxyAgent agent in openid-client custom http_options if issuer is on Internet
Related issues
Has this been tested?
Screenshots / Screencasts