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

forwardauth: Support renaming copied headers, block support #4783

Merged
merged 1 commit into from Jun 16, 2022

Conversation

francislavoie
Copy link
Member

This twitter thread https://twitter.com/coldmacchiato_/status/1524683154237407232 made me realize there were some shortcomings to copy_headers; it wasn't possible to rename response headers when copied to the request, and if the header names are long, it can get unwieldy to have it all on one line.

So to fix this, I'm adding a > syntax where if a token has a >, the left side is the response header name, and the right side is the "destination" request header name.

	copy_headers {
		Tailscale-User>X-Webauth-User
		Tailscale-Name>X-Webauth-Name
		Tailscale-Login>X-Webauth-Login
		Tailscale-Tailnet>X-Webauth-Tailnet
		Tailscale-Profile-Picture>X-Webauth-Profile-Picture
	}

@francislavoie francislavoie requested a review from mholt May 12, 2022 23:00
@francislavoie francislavoie added the feature ⚙️ New feature or request label May 12, 2022
@francislavoie francislavoie added this to the v2.5.2 milestone May 12, 2022
@mholt
Copy link
Member

mholt commented May 13, 2022

Probably a good improvement; could this work with a space around the > though? For readability sake.

@francislavoie
Copy link
Member Author

francislavoie commented May 13, 2022

I think that would be way harder to parse cause then we need to deal with counting tokens and merging them if the next token is a > etc. And we need to delete tokens here so it's even harder to do correctly. Making each replacement a single token is way easier to handle, much less code.

There's precedent for this kind of syntax, see logger fields. Obviously it has different meaning there, but still.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Alrighty, I guess this is good. Thanks very much for the enhancement, as usual! :)

@mholt mholt merged commit 0b6f764 into master Jun 16, 2022
@mholt mholt deleted the forward-auth-rename-copied branch June 16, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants