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

HTTP_UPGRADE_INSECURE_REQUESTS should be optional #174

Closed
mvdobrinin opened this issue Jul 25, 2019 · 3 comments
Closed

HTTP_UPGRADE_INSECURE_REQUESTS should be optional #174

mvdobrinin opened this issue Jul 25, 2019 · 3 comments

Comments

@mvdobrinin
Copy link

I am running into an issue with a test environment where the protocol is upgraded to https when I don't want it to be.

  1. The call is made to http://localhost/app.
  2. Chrome automatically adds the Upgrade-Insecure-Requests: 1 header.
  3. The library fetches the well-known config for the authorization endpoint and sees it is http://localhost:9100/authorize.
  4. In this library the getRedirectURL() function upgrades the protocol automatically.
  5. When redirecting to the authorize endpoint, the url is now http://localhost:9100/authorize?response_type=code&redirect_uri=https%3A%2F%2Flocalhost%2Fapp%2Findex.php.
  6. User fills in their credentials and presses enter.
  7. The IDP does not redirect back to the app because the URL does not match a known redirect URL.
    Invalid redirect: https://localhost/app/index.php does not match one of the registered values: [http://localhost:4500/ws/test_oidc/home2, http://localhost/app/index.php, http://localhost:4500/ws/test_oidc/home]

Should this step be made optional?

@markembling
Copy link

markembling commented Nov 2, 2019

I've just run into this issue as well, and agree. I know it's trying to help (and, frankly, sticking to the standards which say HTTPS must be used), but it makes development in a local environment without HTTPS - like the PHP built-in server - more difficult than it need be.

I see two workarounds right now:

  • Call $_SERVER['HTTP_UPGRADE_INSECURE_REQUESTS'] = 0; just before making the call to $oidc->authenticate();. Optionally you could temporarily hold and then restore the proper value after, just in case it matters to anything coming later.
  • Re-implement most of the same logic in your own code and then call $oidc->setRedirectUrl(<your_callback_url>);

Both of these seem a bit wrong, but I think both would work. The first one in particular should definitely only be done if it's a development environment, so would really only be safe if your code is able to make that distinction at runtime.

JuliusPC added a commit to JuliusPC/OpenID-Connect-PHP that referenced this issue Apr 18, 2021
@JuliusPC
Copy link
Contributor

JuliusPC commented Apr 20, 2021

I forked this library and added a new method setHttpUpgradeInsecureRequests(false) to be able to suppress this behavior in development environments. I will add some other improvements in the future.

@d-javu
Copy link

d-javu commented Jun 12, 2021

I think the correct fix is to revert the commit that added this feature in the first place. (c3ba743)

The header should be handled by the server that accepts the HTTP request, and it should respond with an appropriate Location header if a secure channel is available, and configured to honor the header.

The Upgrade-Insecure-Requests HTTP header is not about upgrading insecure links inside an application.

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

No branches or pull requests

4 participants