Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Add support for returning authenticated user #173

Closed
wants to merge 3 commits into from

Conversation

lsiudut
Copy link
Contributor

@lsiudut lsiudut commented Nov 18, 2015

As for now, if oauth2_proxy is used only as authenticator, it doesn't return information about authenticated user. This patch introduces header X-Authenticated-User which is returned from proxy upstream. This way further user-based authorization is possible.

As for now, if oauth2_proxy is used only as authenticator, it
doesn't return information about authenticated user. This patch
introduces header X-Authenticated-User which is returned from
proxy upstream. This way further user-based authorization
is possible.
@jehiah
Copy link
Member

jehiah commented Nov 18, 2015

@lsiudut I generally think oauth2_proxy should continue to be agnostic of the actual application that is being proxied/authenticated. Having oauth2_proxy set response headers with authentication seems to break that assumption and change that dynamic. I think responsibility for response headers should rest entirely on the proxied application.

I suggest that a better way to implement this would be to enable --pass-basic-auth and have your application behind oauth2_proxy copy X-Forwarded-Email request into X-Authenticted-Email response header.

@lsiudut
Copy link
Contributor Author

lsiudut commented Nov 18, 2015

I understand you completely and I was trying to make my solution this way. I thought that returning name of authenticated user is similar action as forwarding basic auth to application with difference, that we may perform some actions on http server level (nginx and lua authorization in this case). Unfortunately we have setup which makes inserting another proxy very difficult.

If you're finding this solution too invasive just close this PR.

@pawelgocek
Copy link

What would be good way to pass authentication data in use case of nginx auth_request directive?

@jehiah
Copy link
Member

jehiah commented Nov 19, 2015

@pawelgocek can you expand upon your question? What are you trying to accomplish?

@lsiudut
Copy link
Contributor Author

lsiudut commented Nov 19, 2015

To pass information about authenticated user name/email to upstream. Right now it is possible when oauth2_proxy works as proxy, but when it's used as authenticator (for auth_request directive) we don't receive any information except authentication status (HTTP 202 or 401).

In our use case nginx is proxying directly to microservices and there's no easy way to inject oauth2_proxy between them. Also we can't use it before nginx, as not all endpoints are suppose to be covered by authenticator. auth_request directive is perfect for us, but we lack info about authenticated user name. We could try to make some hacky solution and proxy authenticated request back to nginx, but we'd like to avoid it.

@mbland
Copy link
Contributor

mbland commented Nov 19, 2015

Actually, this should be possible with the auth_request_set Nginx directive, something like:

auth_request_set $auth_user $upstream_http_x_authenticated_email;
proxy_set_header x-authenticated-email $auth_user;

See also: http://stackoverflow.com/questions/19366215/setting-headers-with-nginx-auth-request-proxy

@lsiudut
Copy link
Contributor Author

lsiudut commented Nov 19, 2015

Yes, but only if oauth2_proxy return X-Authenticated-Email in response for /auth request :). For now it's not returning such header. My patch adds exactly this functionality.

@jehiah
Copy link
Member

jehiah commented Nov 19, 2015

@lsiudut @mbland ahh. thanks this context is helpful. When using nginx auth is a good argument for setting return parameters so that nginx can pass them upstream. Let me think this a little more.

If email is present user name is forged from it, what may duplicate
user names when multiple domains are allowed.
@szibis
Copy link

szibis commented Feb 2, 2016

+1

@rcoup
Copy link

rcoup commented Mar 14, 2016

This looks super-helpful for auth_request users, is there anything else that needs doing to get it landed? :)

@tamsky
Copy link
Contributor

tamsky commented Aug 22, 2016

This still looks super-helpful for auth_request users (&& required for some grafana users).
Is there anything else that needs doing to get it landed?

@tamsky
Copy link
Contributor

tamsky commented Aug 25, 2016

I noticed there were merge conflicts.
After resolving those, I noticed travis was busted.
Travis is now fixed in #299.
Travis build and tests now pass.

New pull request is #298

@caffeineshock
Copy link

We are currently using the auth_request directive as well which saves us a lot of nginx instances and configuration details for oauth2_proxy. So is there anything still blocking #298 or can we expect it to be merged soon? Otherwise we will have to bite the bullet and either switch to the branch containing the change or start using oauth2_proxy as an actual proxy.

@cndbain
Copy link

cndbain commented Nov 10, 2016

We are also using the auth_request directive and would like to use #298 if it were merged.

fschaefer pushed a commit to Securepoint/oauth2_proxy-legacy that referenced this pull request Nov 25, 2016
This is enhancement of bitly#173 to use "Auth Request" consistently in
the command-line option, configuration file and response headers.
It always sets the X-Auth-Request-User response header and if the
email is available, sets X-Auth-Request-Email as well.
AaronKalair pushed a commit to conversocial/oauth2_proxy that referenced this pull request Feb 1, 2017
This is enhancement of bitly#173 to use "Auth Request" consistently in
the command-line option, configuration file and response headers.
It always sets the X-Auth-Request-User response header and if the
email is available, sets X-Auth-Request-Email as well.
@ashkulz
Copy link
Contributor

ashkulz commented Mar 29, 2017

#319 has been merged, so this can be closed.

@jehiah jehiah closed this Mar 29, 2017
madmod pushed a commit to daffinity/oauth2_proxy that referenced this pull request Dec 1, 2017
This is enhancement of bitly#173 to use "Auth Request" consistently in
the command-line option, configuration file and response headers.
It always sets the X-Auth-Request-User response header and if the
email is available, sets X-Auth-Request-Email as well.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

10 participants