Skip to content

fix(oauth2-proxy): update cookie domains handling and set SameSite attribute to lax - #3595

Open
CasLubbers wants to merge 4 commits into
mainfrom
APL-2099
Open

fix(oauth2-proxy): update cookie domains handling and set SameSite attribute to lax#3595
CasLubbers wants to merge 4 commits into
mainfrom
APL-2099

Conversation

@CasLubbers

@CasLubbers CasLubbers commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

📌 Summary

Together with: linode/apl-api#1074

🔍 Reviewer Notes

🧹 Checklist

  • Code is readable, maintainable, and robust.
  • Unit tests added/updated

Copilot AI lite review requested due to automatic review settings August 26, 2026 08:44
@svcAPLBot

svcAPLBot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Comparison of Helm chart templating output:

# oauth2-proxy/templates/configmap.yaml

@@ data.oauth2_proxy.cfg @@
! ± value change in multiline text (one insert, one deletion)
  
  # Defaults
  email_domains = [ "*" ]
  # Upon successful token verification, oauth2-proxy returns 200 and K8s gateway passes the request to the app route
  
  [five lines unchanged)]
  
  scope = "openid email profile"
  oidc_issuer_url = "https://keycloak.dev.linode-apl.net/realms/otomi"
  code_challenge_method = "S256"
  insecure_oidc_allow_unverified_email = true
- cookie_domains = [".dev.linode-apl.net","oauth2-proxy.istio-system.svc.cluster.local"]
- cookie_samesite = "none"
+ cookie_domains = ["console.dev.linode-apl.net","auth.dev.linode-apl.net","api.dev.linode-apl.net","tty.dev.linode-apl.net","gitea.dev.linode-apl.net","keycloak.dev.linode-apl.net","harbor.dev.linode-apl.net","argocd.dev.linode-apl.net","tekton.dev.linode-apl.net","kubeflow-pipelines.dev.linode-apl.net","alertmanager.dev.linode-apl.net","grafana.dev.linode-apl.net","prometheus.dev.linode-apl.net","oauth2-proxy.istio-system.svc.cluster.local","alertmanager-admin.dev.linode-apl.net","alertmanager-demo.dev.linode-apl.net","alertmanager-dev.dev.linode-apl.net","grafana-admin.dev.linode-apl.net","grafana-demo.dev.linode-apl.net","grafana-dev.dev.linode-apl.net","tekton-admin.dev.linode-apl.net","tekton-demo.dev.linode-apl.net","tekton-dev.dev.linode-apl.net"]
+ # lax is enough for the redirect-based OIDC login and blocks cross-site delivery
+ cookie_samesite = "lax"
  # set to the same as keycloak realm's accessCodeLifespan
  cookie_refresh = "1m"
  # set to the same as keycloak client idle timeout
  cookie_expire = "30m"
  
  [one line unchanged)]
  
  pass_authorization_header = true
  skip_auth_routes = "/manifest.json"
  silence_ping_logging = true
  custom_templates_dir = "/etc/oauth2-proxy"

# oauth2-proxy/templates/deployment.yaml

# otomi-api/templates/configmap.yaml

@@ data @@
! + one map entry added:
+ RESERVED_SERVICE_NAMES: "alertmanager,grafana,tekton"

# otomi-api/templates/deployment.yaml

# rabbitmq-cluster-operator/templates/messaging-topology-operator/validating-webhook-configuration.yaml

# values-repo.yaml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates oauth2-proxy session cookie behavior to better scope cookies to specific hostnames and changes SameSite policy to lax, while adding a Helm template guard to prevent team service routes from colliding with reserved default team hostnames.

Changes:

  • Replace oauth2-proxy cookie_domains from a wildcard-oriented list to an explicit hostname list (plus per-team hostnames for select team endpoints).
  • Set oauth2-proxy cookie_samesite from "none" to "lax".
  • Fail Helm rendering for team-ns routes when a team service name would claim a reserved default team hostname.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
values/oauth2-proxy/oauth2-proxy.gotmpl Refines cookie domain selection logic and changes SameSite to lax.
charts/team-ns/templates/routes.yaml Adds a render-time guard against reserved team service names/hostnames.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +24 to +27
{{- /* core.yaml teamApps with ownHost already claim these hostnames. */}}
{{- if has $serviceName (list "grafana" "alertmanager" "tekton") }}
{{- fail (printf "team service name %q is reserved: it would claim the platform hostname %s" $serviceName $defaultHostname) }}
{{- end }}
(print "grafana." $domainSuffix)
(print "prometheus." $domainSuffix)
"oauth2-proxy.istio-system.svc.cluster.local" }}
{{- /* core.yaml teamApps with ownHost. Last, so team count does not slow the lookup. */}}
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

Successfully merging this pull request may close these issues.

4 participants