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

Cookie path is hardcoded to / limiting to only use the -cookie-domain field. #121

Closed
costelmoraru opened this issue Apr 9, 2019 · 2 comments

Comments

@costelmoraru
Copy link
Contributor

When installed multiple instances of oauth-proxy in the same k8s cluster, for example one instance for dev and a second one for qa purposes, the cookies generated for the two instances will have the same Domain yourcompany.com. The browser will submit both cookies with different names, given by the -cookie-name parameter.
Scale this with multiple instances used for various purposes, the cookie size increases unnecessary breaking the nginx proxy default configuration.

Expected Behavior

The configuration of the cookie being generated by one oauth-proxy instance should allow enough flexibility for the cookie to take into consideration the domain and path, standard attributes of the cookie.
In the scenario described above, the cookie generated by the dev instance of the proxy should have Domain yourcompany.com and Path /dev while the one for the qa instance should have Domain yourcompany.com and Path /qa. Thus, only the correct cookie generated by the oauth-proxy instance is being sent by the browser.

Current Behavior

Multiple cookies are being sent to the oauth-proxy instance increasing the header size, breaking the nginx proxy configuration.
The current implementation is hardcoding the Path to / thus removing the Cookie's flexibility to be specific for one path.

    return &http.Cookie{
        Name:     name,
        Value:    value,
        Path:     "/",
        Domain:   p.CookieDomain,
        HttpOnly: p.CookieHTTPOnly,
        Secure:   p.CookieSecure,
        Expires:  now.Add(expiration),
    }

Possible Solution

Expose the standard Path attribute as oauth-proxy configuration parameter under -cookie-path to allow customisation of the Cookie's standard attribute. If left empty, then the Path should be defaulted to / as it is now.

Steps to Reproduce (for bugs)

  1. Install oauth-proxy instance to k8s cluster for the dev environment under -proxy-prefix=/dev/oauth,
  2. Install oauth-proxy instance to k8s cluster for the qa environment under -proxy-prefix=/qa/oauth,
  3. Invoke an application secured by the qa proxy. Check for the cookies being sent by the browser,
  4. Both cookies are being sent not only the qa one. There is no way to configure the cookie with the current implementation.

Context

Your Environment

  • Version used: 3.1.0
@costelmoraru
Copy link
Contributor Author

I will create a PR with the solution proposed in the description, creating a new config optional parameter called -cookie-path , defaulted value being set to / , allowing the Path attribute of the Cookie to be configured.

@costelmoraru costelmoraru changed the title Cookie path is hardcoded to / limiting to one use the -cookie-domain field. Cookie path is hardcoded to / limiting to only use the -cookie-domain field. Apr 9, 2019
@JoelSpeed
Copy link
Member

This was fixed in #122

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

No branches or pull requests

2 participants