providers/proxy: Fix incorrect comparison of redirect URL and CookieDomain#15686
providers/proxy: Fix incorrect comparison of redirect URL and CookieDomain#15686dewi-tik merged 4 commits intogoauthentik:mainfrom
Conversation
✅ Deploy Preview for authentik-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for authentik-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for authentik-integrations ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
you’re right that u.Host includes both the hostname and port, while u.Hostname() strips the port, per the Go documentation That said, I’m trying to connect the dots here: the current logic checks if u.Host ends with the cookie domain, which might fail if there’s a port in u.Host (?) However, that alone should only affect domain matching cookie wise, not cause a complete redirect to the “My Applications” page. |
|
During your testing/finding the bug, did this issue occur 100% of the time? cc @BeryJu |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15686 +/- ##
===========================================
+ Coverage 71.47% 93.27% +21.79%
===========================================
Files 949 949
Lines 52187 52187
===========================================
+ Hits 37303 48678 +11375
+ Misses 14884 3509 -11375
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
It does cause the redirect to the “My Applications” page, because this check against the cookie domain is used in the function "checkRedirectParam". And yes, it happened every time the user went through the Authentik interface. |
…hentik#15685 According to docs, URL.Host contains the host and port, while Hostname returns only the host without the port. CookieDomain obviously does not contain the port. string.HasSuffix function is used, so if a port is set in the redirect URL, this check always fails.
* main: (115 commits) internal: fix incorrect metric calculation (#19701) core, web: update translations (#19684) core: bump goauthentik.io/api/v3 from 3.2026020.12 to 3.2026020.14 (#19686) lifecycle/aws: bump aws-cdk from 2.1101.0 to 2.1102.0 in /lifecycle/aws (#19687) core: bump goauthentik/selenium from 143.0-ak-0.35.3 to 144.0-ak-0.35.7 in /tests/e2e (#19688) core: bump msgraph-sdk from 1.52.0 to 1.53.0 (#19689) core: bump ruff from 0.14.13 to 0.14.14 (#19690) core: bump twilio from 9.9.1 to 9.10.0 (#19691) core: bump gunicorn from 23.0.0 to 24.0.0 (#19692) web: bump the bundler group across 1 directory with 3 updates (#19693) web: bump unist-util-visit from 5.0.0 to 5.1.0 in /web (#19694) web: bump globals from 17.0.0 to 17.1.0 in /web (#19695) ci: bump actions/checkout from 6.0.1 to 6.0.2 (#19696) web: Form Modal Independence: Part 1 (#19395) web/common: add dev middleware to show warnings for consecutive identical requests (#19671) web/admin: fix file upload not preserving extension for custom names with dots (#19548) web/admin: fix brand form sending "undefined" string for blank default application (#19658) providers/proxy: Fix incorrect comparison of redirect URL and CookieDomain (#15686) core: add bulk session revocation (#18564) website/docs: endpoint devices: add serial number note (#19677) ...
|
Thank you for merging it ❤️ |
This is my suggested fix for #15685
According to docs, URL.Host contains the host and port, while URL.Hostname returns only the host without the port. CookieDomain obviously does not contain the port. string.HasSuffix function is used, so if a port is set in the redirect URL, this check always fails.
There might be more occurrences of this mistake in the Authentik code.