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.

Sign up for GitHub

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

Jump to bottom

force https in redirectUrl #438

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

force https in redirectUrl #438

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/DependencyInjection/ProviderFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public function createProvider($class, array $options, ?string $redirectUri = nu
if (null !== $redirectUri) {
$redirectUri = $this->generator
->generate($redirectUri, $redirectParams, UrlGeneratorInterface::ABSOLUTE_URL);
$redirectUri = str_replace('http:','https:', $redirectUri);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this sounds like you didn't pass the $redirectUri when this method is called. I think you should be explicit and specify that in your project, then there will be no need to hack this spot for you.

But even if you don't want to specify that $redirectUri method arg explicitly, the system works so that the Router generates the default schema without HTTPS for you. This means you're using HTTP globally on your website, ie. you're on the HTTP page instead of HTTPS. If you were on HTTPS - the router would generate HTTPS for you as well.

https://symfony.com/doc/2.x/routing/scheme.html or https://symfony.com/doc/2.x/security/force_https.html does not help in your case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None, none of those help.

I can't set the redirectUrl when creating because it happens in the DI and is cached without it, so the router is forced to create it.

I'm definitely on https: https://oauth-demo.survos.com/

second one comes from the $client->redirect call, first one created from the request scheme.

image

        $client = $this->clientRegistry
            ->getClient($service); // the name use in config/packages/knpu_oauth2_client.yaml
        $redirect = $client
            ->redirect( self::SCOPES[$service], [

            parse_str($queryString = parse_url($targetUrl = $redirect->getTargetUrl(), PHP_URL_QUERY), $array);
            $redirectUri = $array['redirect_uri'];
// $redirectUri is HTTP, not HTTPS!!

            $productionRedirctUri = str_replace($request->getSchemeAndHttpHost(),
                $this->getParameter('production_url'),
                $redirectUri
            );

// $productionRedirctUri is HTTPS!

yes, setting scheme

# services.yaml
# This file is the entry point to configure your own services.
# Files in the packages/ subdirectory configure your dependencies.

# Put parameters here that don't need to change on each machine where the app is deployed
# https://symfony.com/doc/current/best_practices.html#use-parameters-for-application-configuration
parameters:
    google_project_id: '%env(OAUTH_GOOGLE_PROJECT_ID)%'
    github_project_id: '%env(OAUTH_GITHUB_PROJECT_ID)%'
    production_url: '%env(PRODUCTION_URL)%'
    router.request_context.scheme: 'https'
    asset.request_context.secure: true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Google Client is created by the DI somewhere in the cache, I can't find it now, but there appears to be no way to explicitly set the redirect before that's called, or even after. If there's a way, please let me know, as I've spent hours on trying to get a simple demo to work in production.

Possibly helpful -- it only works with TRUSTED_PROXIES locally when using ngrok, but maybe that's okay because ngrok does the forwarding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness, here's my nginx.conf,perhaps there's something odd there?

add_header X-Frame-Options "SAMEORIGIN";
add_header X-XSS-Protection "1; mode=block";
add_header X-Content-Type-Options "nosniff";

index  index.php index.html index.htm;

charset utf-8;

gzip on;
gzip_comp_level 5;
gzip_min_length 256;
gzip_proxied any;
gzip_vary on;
gzip_types
  application/atom+xml
  application/javascript
  application/json
  application/ld+json
  application/manifest+json
  application/rss+xml
  application/vnd.geo+json
  application/vnd.ms-fontobject
  application/x-font-ttf
  application/x-web-app-manifest+json
  application/xhtml+xml
  application/xml
  font/opentype
  image/bmp
  image/svg+xml
  image/x-icon
  text/cache-manifest
  text/css
  text/plain
  text/vcard
  text/vnd.rim.location.xloc
  text/vtt
  text/x-component
  text/x-cross-domain-policy;

#
#if ($http_x_forwarded_proto != "https") {
#  return 301 https://$host$request_uri;
#}

# see https://medium.freecodecamp.org/how-you-can-host-multiple-domain-names-and-projects-in-one-vps-7aed4f56e7a1
client_max_body_size 100m;
location / {
    try_files $uri $uri/ /index.php?$query_string;

    fastcgi_param SCRIPT_FILENAME $realpath_root$fastcgi_script_name;
    fastcgi_param DOCUMENT_ROOT $realpath_root;
    client_max_body_size 100m;


    # if ($request_uri ~* ".(ico|css|js|gif|jpe?g|png)$") {
    #                                         expires 3d;
    #                                         access_log off;
    #                                         add_header Pragma public;
    #                                         add_header Cache-Control "public";
    #                                         break;
    #                                 }

}
location ~*  \.(jpg|jpeg|png|gif|ico)$ {
    expires 3d;
    log_not_found off;
    add_header Pragma public;
    add_header Cache-Control "public";
    access_log off;
    try_files $uri $uri/ /index.php?$query_string;
    client_max_body_size 100m;

}
location = /icons/favicon.ico { access_log off; log_not_found off; }
location = /robots.txt  { access_log off; log_not_found off; }

error_page 404 /index.php;


location ~ /\.(?!well-known).* {
    deny all;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! I think uncommenting this may help in your case:

if ($http_x_forwarded_proto != "https") {
  return 301 https://$host$request_uri;
}

In theory, it should redirect to HTTPS any requests that are not HTTPS.

It also has sense to me to work with TRUSTED_PROXIES because of the forwarding, but as I never tried it myself, not sure if that's required fairly speaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll try now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up with a too many redirects loop, I think because the DNS is running on Cloudflare, probably https://developers.cloudflare.com/ssl/troubleshooting/too-many-redirects/

But I don't think it has to do with nginx. The URL is wrong that we're sending to Google.

https://oauth-demo.survos.com/

image

The first URL is what we want, the second one is what we're sending to Google.

If there's a repository using the current version of this bundle that works to login via Google, I'll gladly study it. I know this PR is a hack and breaks too many tests, but I'm not sure how else to proceed. Thanks.


$options['redirectUri'] = $redirectUri;
}
Expand Down
Loading