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

Support setting request headers for reverse-proxy command #5376

Closed
damnever opened this issue Feb 16, 2023 · 9 comments · Fixed by #5460
Closed

Support setting request headers for reverse-proxy command #5376

damnever opened this issue Feb 16, 2023 · 9 comments · Fixed by #5460
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers

Comments

@damnever
Copy link

The one-line reverse-proxy command could be even better if it supports basic authentication like this:

caddy reverse-proxy --from :10000 --to http://user:pass@:20000
@francislavoie
Copy link
Member

At that point, I'd argue you're better off using a Caddyfile and using header_up Authorization to set the credentials (base64 encode it ahead of time).

@damnever
Copy link
Author

In my case, the one-line command is for temporary use in a safe environment.

@francislavoie
Copy link
Member

francislavoie commented Feb 16, 2023

The problem is --to is an upstream address (host+port, with scheme just as a hint to the transport options), not a URL. So using URL semantics like authentication is not supported in general. I don't think the added complexity is warranted, because we'd have to change our upstream address parsing code. See https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#upstream-addresses for valid upstream addresses.

@damnever
Copy link
Author

damnever commented Feb 16, 2023

Another method is to add a command line option.

I am using caddy run -config - to work around this:

docker run --rm -p 10000:10000 library/caddy sh -c 'echo -e ":10000 {\n reverse_proxy :20000 {\n header_up Authorization \"Basic base64_encoded\"\n }\n}" | caddy run --adapter caddyfile --config -'

But this requires some extra effort to encode the credentials.

@mholt mholt added the feature ⚙️ New feature or request label Feb 16, 2023
@mholt
Copy link
Member

mholt commented Feb 16, 2023

I'm also inclined to suggest a config file is the best answer here; but would a command line flag to set upgoing headers work for you?

@francislavoie francislavoie changed the title Support basic authentication for reverse-proxy command Support setting request headers for reverse-proxy command Feb 17, 2023
@francislavoie francislavoie added the good first issue 🐤 Good for newcomers label Feb 17, 2023
@francislavoie
Copy link
Member

I'd say a --header-up "<field>: <value>" option would be the way to go here. Maybe simply --header because it's probably very rare to need to set response headers in this case, and --response-header could be used for that later on if necessary.

@damnever
Copy link
Author

An option to set headers sounds good to me.

@mholt
Copy link
Member

mholt commented Feb 21, 2023

--header-up is probably best (specific and consistent).

@francislavoie
Copy link
Member

This will sorta depend on the work from #5379 otherwise we'd waste a bit of time setting up the type to accept multiple header values. With the cobra changes, we can just use StringSliceP() which is simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants