Juriy / swapi

SWAPI - Star Wars API service
https://swapi.dev
BSD 3-Clause "New" or "Revised" License
286 stars 91 forks source link

Switch URL protocol for resource links to HTTPS #1

Closed dpilafian closed 3 years ago

dpilafian commented 4 years ago

The parent project has an issue filed to make the resource links the same protocol as the request.

Change URL protocol in response data based on whether the request was made over HTTPS: https://github.com/phalt/swapi/issues/66

Since the swapi.dev server uses HTTPS (it even redirects HTTP to HTTPS) and the world has pretty much switched over to HTTPS, it probably makes sense to just do a simple straight switch to HTTPS. There's little value in making the response intelligent based on the protocol of the request.

Currently the next field in responses uses the HTTP protocol:

GET https://swapi.dev/api/vehicles/?format=json

{
   "count": 39,
   "next": "http://swapi.dev/api/vehicles/?page=2&format=json",
   "previous": null,
   "results": [
      {
         "name": "Sand Crawler",
         "model": "Digger Crawler",
         "manufacturer": "Corellia Mining Corporation",
         "cost_in_credits": "150000",
         "length": "36.8 ",
         "max_atmosphering_speed": "30",

Following the next link can result in errors like:

Fetch API cannot load http://swapi.dev/api/vehicles/?page=2&format=json due to access control 

This problem would be resolved by using HTTPS in the next field:

   "next": "https://swapi.dev/api/vehicles/?page=2&format=json",
neoreddog commented 4 years ago

I think this is the case for any url provided through the API.

Tested with the following requests: https://swapi.dev/api/people/ https://swapi.dev/api/people/?page=1 https://swapi.dev/api/planets/ https://swapi.dev/api/films/ https://swapi.dev/api/starships/

EmmaDawsonDev commented 4 years ago

I am running into the same problem. I first make a fetch request to retrieve page 1 data which works fine but then when I try to use the urls contained within that data to make additional fetch requests I'm getting failures because they are all http instead of https. Would be great if this could be updated.

Juriy commented 3 years ago

Help would be highly appreciated with this issue. In my current setup I have nginx in front of SWAPI application. NginX is terminating SSL, so Django that runs SWAPI thinks that it is requested via HTTP. That's why (I think) the links appear broken. The problem is - I have very little knowledge of Django and I am not sure if I can configure protocol for the links in JSON responses explicitly.

leonidpodriz commented 3 years ago

Hello, @Juriy. I am really glad to see your activity on this project. :)

To make Django use HTTPS instead of HTTP, set HTTPS=on in environment variables.

Juriy commented 3 years ago

@leonidpodriz thanks for the hint! The thing is that I want Django to stay on HTTP, since it is NGINX that is dealing with HTTPS and SSL termination. Then it acts as a reverse proxy, and passes the request to Django app over plain HTTP. So it is fine for Django to be on HTTP ( it is not exposed externally ) but somehow we need to let it know that it is behind a reverse proxy, and the real protocol that clients use is HTTPS.

image

This StackOverflow question seems to be relevant. I will experiment with USE_X_FORWARDED_HOST later tonight. https://stackoverflow.com/questions/32631903/django-rest-framework-reverse-relationship-breaks-when-behind-proxy

leonidpodriz commented 3 years ago

@Juriy Well, you're right. But after HTTPS=on Django will stay on plain HTTP and reverse proxy will works correct.

Proofs from local testing:

Screenshot 2021-06-17 at 17 28 12

This environment variable is uses only to build absolute URLs. I had search where HTTPS variable is using and not found more places despite this: https://github.com/django/django/blob/b626c289ccf9cc487f97d91c2a45cac096d9d0c7/django/http/request.py#L135

Django has a few more ways to make it return HTTPS URLs, but this way is the easiest and impact only one function (see above), not the whole application.

Juriy commented 3 years ago

Thank you for the hint, let me test it on prod-like first and I'll update prod right after.

leonidpodriz commented 3 years ago

Hey, don't spend your time for my "hint". :) I got wrong, because this won't be work when Nginx connects to Django thorough WSGI.

In my environment, rewritten "SWAPI" handle HTTPS well. I think it is related to this: https://docs.djangoproject.com/en/3.2/releases/1.11.22/

I propose the next solution: To the Nginx configuration add proxy_set_header 'X_FORWARDED_PROTO' 'https'; That's works. :)

Juriy commented 3 years ago

Fixed.

Basically I was overthinking it a little 🙂 For the historical purposes: original configuration already had a config line that would make Django realize that request was originally secure and it is behind the reverse proxy:

SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')

https://docs.djangoproject.com/en/1.8/ref/settings/#secure-proxy-ssl-header

All I had to do is make Nginx send this header. Now this was very close to what @leonidpodriz suggested, except for a minor difference in syntax for config.

location /api {
    proxy_set_header X-Forwarded-Proto $scheme;
    proxy_pass "http://localhost:8000/api";
}

Thanks for help!