Skip to content

Derive schemes from endpoint :url#222

Open
slissner wants to merge 2 commits intoxerions:masterfrom
slissner:master
Open

Derive schemes from endpoint :url#222
slissner wants to merge 2 commits intoxerions:masterfrom
slissner:master

Conversation

@slissner
Copy link
Copy Markdown

@slissner slissner commented Feb 9, 2019

Summary

When handling url schemes (http/https), phoenix swagger currently does only take into account if there is set up a :https server in the endpoint config. Instead, it should first lookup if under the endpoint :url config a custom scheme is defined.

Problem

Currently, phoenix swagger is generating the base url with scheme “https” only if in the phoenix endpoint_config it is set as distinct :https server. (see https://github.com/xerions/phoenix_swagger/blob/master/lib/mix/tasks/swagger.generate.ex#L178) The collect_host_from_endpoint. It does not consider the endpoint :url config, in particular it ignores :scheme (see below).

On the other side, it is a common scenario to run one's application behind a (https) reverse proxy. It is therefore in the general interest, to allow generating https base paths for the client communication – although the phoenix server is running only in https.

The phoenix docs are clear on the issue. It says that :url is for “generating URLs throughout the app”. https://hexdocs.pm/phoenix/Phoenix.Endpoint.html Moreover, the docs explicitly mention the reverse proxy issue:

The :scheme option accepts "http" and "https" values. Default value is infered from top level :http or :https option. It is useful when hosting Phoenix behind a load balancer or reverse proxy and terminating SSL there.

This PR alters the behavior of phoenix swagger to that extent, that first the configuration in :url is taken into account.

Copy link
Copy Markdown
Contributor

@mbuhot mbuhot left a comment

Choose a reason for hiding this comment

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

Code looks good to me 👍

Comment thread .gitignore
/deps
erl_crash.dump
*.ez
.idea/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest ignoring IDE generated files globally in your git config so they don’t need to be ignored in each repo.


swagger_map =
if scheme == "https" do
Map.put_new(swagger_map, :schemes, ["https", "http"])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is plain http included for local development, even if the production endpoint is always https ?

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 this pull request may close these issues.

3 participants