* Strip the port from the request host when matching cookie domains
GetCookieDomain suffix-matches the request host against each configured
cookie domain. When a reverse proxy rewrites the Host header to the
upstream's address, that host arrives as "host:port" and never matches,
so the request falls through to the "did not match any of the specific
cookie domains" warning and the shortest configured domain is used.
One way to hit this is Traefik's Errors middleware: it fetches a page
from a Service on port 443 with passHostHeader disabled, and forwards
the Host as "<host>:443". The request reaches oauth2-proxy correctly and
the resulting cookie is still usable, but every such request logs an
error that suggests a misconfiguration when none exists.
warnInvalidDomain, a few lines below in the same file, already calls
net.SplitHostPort on the host before comparing it to the cookie domain.
This makes GetCookieDomain consistent with it.
Tests cover a Host header with a port, an X-Forwarded-Host header with a
port, a non-standard port, and a host with a port that matches no
configured domain.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* chore: add changelog entry for #3546
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* test: keep the new port entries at the end of the table
The port cases were inserted between the Host and X-Forwarded-Host
variants of the existing suffix-match case, which split a pair that
reads as one. Move them after the last existing entry instead.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
* test: pin that a port part can no longer match a cookie domain
SplitHostPort does not require the port to be numeric, so a host such as
"evil.com:.cookies.test" used to suffix-match a configured cookie domain
through its port part, and the cookie was then set for that domain. Matching
now runs against the host alone, so it does not.
Covers both the Host header and X-Forwarded-Host, since GetRequestHost
returns one or the other into the same comparison.
A Host of this shape cannot be expressed through the request URL, because
http.NewRequest rejects it as an invalid port, so the table gains a rawHost
field that assigns req.Host after the request is built. That is how a server
populates it from the wire, and pkg/upstream and pkg/middleware already set
req.Host the same way in their tests.
Thanks to @Lrifton92 for spotting this.
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
---------
Signed-off-by: Kiril Jurbinsky <kiril@instaffo.com>
Drop use of k8s.io/apimachinery/pkg/util/errors.NewAggregate and replace
to with errors.Join which is available in stdlib since Go 1.20.
This allows to drop one dependency (which itself brings its own
contraints in its go.mod) and use more standard behavior from stdlib.
Co-authored-by: Olivier Mengué <dolmen@cpan.org>
* fix: invalidate session on fatal OAuth2 refresh errors
When a token refresh fails with a fatal OAuth2 error (invalid_grant,
invalid_client), the session is now cleared from the session store
and the cookie is removed, forcing re-authentication.
Previously, fatal refresh errors were logged but the stale session
continued to be served, leaving users logged in indefinitely after
their session was revoked at the provider level.
Transient errors (network timeouts, server errors) continue to
preserve the existing session as before.
Fixes#1945
Signed-off-by: Francesco Pasqualini <frapas@gmail.com>
* fix: apply review nits and add CHANGELOG entry
Signed-off-by: Francesco Pasqualini <frapas@gmail.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: Francesco Pasqualini <frapas@gmail.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
* Propagate errors during route building
This fixes cases such as invalid paths being silently discarded after
creation by throwing a visible error in such cases.
Due to the way gorilla/mux's fluent API is designed, it is necessary to
manually call `.GetError()` to check for errors while building routes.
Signed-off-by: Simon Engmann <simon.engmann@sovity.de>
* Add test for route building error propagation
Signed-off-by: Simon Engmann <simon.engmann@sovity.de>
* Add route building error propagation to changelog
Signed-off-by: Simon Engmann <simon.engmann@sovity.de>
---------
Signed-off-by: Simon Engmann <simon.engmann@sovity.de>
Co-authored-by: Simon Engmann <simon.engmann@sovity.de>
* Improve logging for session refresh token status
Signed-off-by: Yosri Barhoumi <med.yosri.brh@gmail.com>
* doc: add changelog entry for #3327
Signed-off-by: Jan Larwig <jan@larwig.com>
* test: fix existing test cases for new behaviour
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: Yosri Barhoumi <med.yosri.brh@gmail.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
* fix: handle Unix socket RemoteAddr in IP resolution
When oauth2-proxy listens on a Unix socket, Go sets RemoteAddr to "@"
instead of the usual "host:port" format. This caused net.SplitHostPort
to fail on every request, flooding logs with errors:
Error obtaining real IP for trusted IP list: unable to get ip and
port from http.RemoteAddr (@)
Fix by handling the "@" RemoteAddr at the source in getRemoteIP,
returning nil without error since Unix sockets have no meaningful
client IP. Also simplify the isTrustedIP guard and add a nil check
in GetClientString to prevent calling String() on nil net.IP.
Fixes#3373
Signed-off-by: h1net <ben@freshdevs.com>
* docs: add changelog entry and Unix socket trusted IPs documentation
Add changelog entry for #3374. Document that trusted IPs cannot match
against RemoteAddr for Unix socket listeners since Go sets it to "@",
and that IP-based trust still works via X-Forwarded-For with reverse-proxy.
Signed-off-by: Ben Newbery <ben.newbery@gmail.com>
Signed-off-by: h1net <ben@freshdevs.com>
* doc: fix changelog entry for #3374
Signed-off-by: Jan Larwig <jan@larwig.com>
* doc: add trusted ip a section to versioned docs as well
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: h1net <ben@freshdevs.com>
Signed-off-by: Ben Newbery <ben.newbery@gmail.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
fix: linter issues and set default unix socket permissions to 0660
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Tristan <tristan@mangadex.org>
* feat: add support for specifying allowed OIDC JWT signing algorithms (#2753)
TODO:
- [X] update docs
- [X] add support in yaml (modern) config
- [X] add more test(s)?
Add (legacy for now) configuration flag "oidc-enabled-signing-alg" (cfg:
oidc_enabled_signing_algs) that allows setting what signing algorithms
are specified by provider in JWT header ("alg" header claim).
In particular useful when skip_oidc_discovery = true, as verifier
defaults to only accept "RS256" in alg field in such circumstances.
Signed-off-by: Jan Larwig <jan@larwig.com>
* doc: update changelog and alpha config
Signed-off-by: Jan Larwig <jan@larwig.com>
* feat: add signing algorithm intersection handling with oidc discovery and additional tests
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
* feat: support additional claims
Signed-off-by: afsu <suaf2020@163.com>
Signed-off-by: af su <saf@zjuici.com>
* docs: clarify that AdditionalClaims may come from id_token or userinfo endpoint
Signed-off-by: afsu <suaf2020@163.com>
Signed-off-by: af su <saf@zjuici.com>
* feat: include AdditionalClaims in /oauth2/userinfo response (#834)
Signed-off-by: afsu <suaf2020@163.com>
Signed-off-by: af su <saf@zjuici.com>
* refactor: extract coerceClaim logic into util
Signed-off-by: afsu <suaf2020@163.com>
Signed-off-by: af su <saf@zjuici.com>
* doc: add changelog entry for #2685
Signed-off-by: Jan Larwig <jan@larwig.com>
* refactor: added more verbose comments to some struct fields and minor code cleanup
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: afsu <suaf2020@163.com>
Signed-off-by: af su <saf@zjuici.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: af su <saf@zjuici.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
* Fix WebSocket proxy to respect PassHostHeader setting
When PassHostHeader is set to false, the regular HTTP proxy correctly
sets the Host header to the upstream backend URL. However, the WebSocket
proxy was not respecting this setting, causing WebSocket connections to
fail when backend services validate the Host header.
This commit:
- Adds passHostHeader parameter to newWebSocketReverseProxy()
- Applies setProxyUpstreamHostHeader() when PassHostHeader=false
- Ensures consistent behavior between HTTP and WebSocket proxies
Fixes#3288
Signed-off-by: Pascal Schmiel <pascal.schmiel@gmail.com>
* chore(): add tests, update changelog
Signed-off-by: Pascal Schmiel <pascal.schmiel@gmail.com>
---------
Signed-off-by: Pascal Schmiel <pascal.schmiel@gmail.com>
remove color output in tests for better readability in github actions
bugfix: remove google as default provider for alpha options
fix conversion flow for toml to yaml
revert ginkgo color deactivation
revert claim- and secret source back to pointers
regenerate alpha config
Signed-off-by: Jan Larwig <jan@larwig.com>
* fix: NewRemoteKeySet is not using DefaultHTTPClient
Signed-off-by: Jan Larwig <jan@larwig.com>
* doc: add changelog entry
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
* fix: upstream licensing issue by adopting hmacauth library and changing asserting library for its test cases
Signed-off-by: Jan Larwig <jan@larwig.com>
* fix: golang code quality and linting issues for hmacauth
Signed-off-by: Jan Larwig <jan@larwig.com>
---------
Signed-off-by: Jan Larwig <jan@larwig.com>
kirilju