Commit Graph
6 Commits
Author SHA1 Message Date
lukaszraczyloandGitHub f320d0a316 fix(settings): repair #149 + AST-based header template validation + allowedClaims (#150)
* fix(settings): allow documented range + static header templates (#149)

Upgrading past the release where NewWithContext began calling
config.Validate() (546ceb9) broke `headers` configs that worked in 0.8.25:
New() returned a nil handler and Traefik logged the opaque "invalid handler
type: <nil>". Two header-validation checks were stricter than the documented,
legitimate usage:

- validateTemplateSecure rejected the documented multi-valued-claim join
  "{{range $i,$e := .Claims.groups}}...{{end}}" as a dangerous {{range}},
  even though README/CONFIGURATION show exactly that for groups/roles.
- Config.Validate required every header value to contain {{ }}, rejecting
  static/literal values such as a shared proxy secret.

Fixes:
- Allow static/literal header values (reject only CR/LF/NUL header-injection
  chars); flag unbalanced {{ }} explicitly.
- Allow {{range}} only when its clause targets .Claims.<field>; a range over
  the bare .Claims map stays blocked and a decoy {{.Claims.x}} elsewhere no
  longer launders it. Generalize the claims whitelist scan to .Claims. so a
  claim reached via range/with/default is whitelisted identically to a direct
  {{.Claims.x}} reference.
- normalizeTemplateDelimiters now collapses ALL whitespace inside {{ }}
  (newline/CR/FF too), closing a newline-obfuscation bypass ({{\nrange,
  {{\nindex) found in review.
- Log the concrete validation reason at construction so operators see WHY the
  middleware failed to build instead of only "invalid handler type: <nil>".

Adds issue149_regression_test.go: pins the exact issue config, the documented
groups template rendering "admin,users" past the runtime sanitizer, static and
delimiter edge cases, and the security boundary (whole-Claims-map dump via
range, non-whitelisted claims, call/define/template/printf, and
newline-obfuscated actions all rejected).

* fix(settings): AST-based header template validation, close whitelist bypasses

Follow-up hardening to the #149 work. An adversarial review showed the
substring-based validateTemplateSecure could not actually bound what a header
template reads: get/index with arbitrary keys, {{with .Claims}} field access,
and a whitelisted-decoy + bare {{.}}/{{$}}/{{.Claims}} render all leaked the
whole root (every raw token) or the whole Claims map past validation. The core
problem is structural — the meaning of "." depends on scope, which substrings
cannot see.

Replace the substring validator with an AST walk (text/template/parse):
- Parse with only get/default known; every other function (printf, call, index,
  js, urlquery, ...) then fails to parse and is rejected — no blocklist.
- Reject {{define}}/{{block}} (they produce extra trees).
- Walk the tree tracking whether "." is the root. Permit ONLY: literal text;
  the four root token fields (AccessToken, IdToken, IDToken, RefreshToken);
  whitelisted .Claims.<field>; range/with over a whitelisted .Claims.<field>
  ("." inside the body is the already-whitelisted element); get on .Claims with
  a whitelisted literal key; default over safe values; {{if}} control flow and
  $-scoped range/with/decl variables. Everything else — bare {{.}} / {{$}}
  (whole-root dump), bare {{.Claims}} (whole-map dump), non-whitelisted claims,
  $.Claims root reach — is rejected.

Also closes the get/default exfil and the with-over-bare-map vectors, and fixes
ID-token forwarding: templateData now carries both IDToken and IdToken and the
validator accepts {{.IdToken}} (documented) and {{.IDToken}}; previously the
documented spelling rendered empty and the working one was rejected.

Runtime `get` additionally enforces the claims whitelist on its key, and
safeClaimsFields is promoted to a package-level single source of truth shared by
the validator and the get helper. The header template funcMap is extracted to
headerTemplateFuncMap() so tests exercise the exact production map. Removes the
now-dead normalizeTemplateDelimiters/regex (the parser handles whitespace).

Extends issue149_regression_test.go: renders through the real funcMap to prove
get cannot leak; asserts the whole-root/whole-map/decoy/var-launder/index
vectors are rejected while every documented pattern (groups/roles join,
with .Claims.email, get .Claims "internal_role", default, both IdToken
spellings) still validates and renders.

* feat(settings): allowedClaims to extend the header-template claim whitelist

The AST validator and the runtime `get` only permit a fixed set of claim fields
in header value templates. Providers issue custom claims (e.g. employee_id) that
aren't built in, and there was no supported way to forward them — the previous
`get` escape hatch was closed by the security hardening.

Add `allowedClaims []string` to Config. Listed names extend the effective
whitelist for BOTH static validation and the runtime `get` (a single
claimsWhitelist() = built-in safeClaimsFields + AllowedClaims, so the two never
drift). Built-in claims need not be listed; unlisted claims stay rejected.

  allowedClaims:
    - employee_id
  headers:
    - name: X-Employee-Id
      value: "{{`{{.Claims.employee_id}}`}}"

Implementation: the whitelist is threaded through the validator (now a small
templateValidator struct carrying the allowed set) and headerTemplateFuncMap
instead of the package global, so it is per-config with no shared mutable state.

Docs: README and docs/CONFIGURATION document the whitelist, the validation
behaviour, and allowedClaims (grounded in safeClaimsFields).

Tests: TestIssue149_AllowedClaimsExtendsWhitelist pins that a custom claim is
rejected by default and validates + renders once listed, via both direct access
and get, while an unlisted claim stays rejected. A regression assertion that
coupled to the old validator's "dangerous" wording now checks the stable
"failed security validation" wrapper (the templates are still rejected).
2026-07-21 13:04:39 +01:00
lukaszraczylo 48a19e04f8 fix(settings): tolerate whitespace in header template delimiters (#148)
validateTemplateSecure matched safe functions with literal strings.Contains
("{{get "), so a space after {{ ("{{ get") failed the whitelist and
Config.Validate() rejected the config, surfacing as Traefik's opaque
"invalid handler type: <nil>". Go's text/template treats {{ get and {{get
identically, so the validator was stricter than the engine.

Normalize inner-delimiter whitespace before matching. This also closes a
security gap where a leading space let dangerous patterns ("{{ range",
"{{ call") evade detection.

Adds unit + regression coverage.
2026-06-26 11:49:31 +01:00
lukaszraczylo 9cbca4c4fb fix(refresh): honor userIdentifierClaim in token refresh path (#132)
patch-release

The refresh path in token_manager.go hardcoded the "email" claim when
extracting the user identifier from a refreshed ID token, ignoring the
configured userIdentifierClaim. Keycloak users without an email claim
(using sub or another identifier) were kicked out on refresh even
though their initial login worked.

The callback path (auth_flow.go:226-239) already honored
userIdentifierClaim with "sub" fallback; PR #100 (commit a316a98)
added that support but missed the refresh path.

Mirror the callback logic in refreshToken so both paths behave the same.

Cleanup: rename Get/SetEmail to Get/SetUserIdentifier on SessionData
to match the actual semantics. The slot already stored the configured
identifier (email, sub, oid, upn, preferred_username), only the API
name was misleading. Storage key "email" → "user_identifier" and
combinedSessionPayload field E (json:"e") → Ui (json:"ui").

Compat note: existing user sessions invalidate on upgrade — every active
user re-authenticates once after deploying this change.
2026-05-07 09:21:41 +01:00
lukaszraczyloandGitHub 6efb78b7a8 Smarter approach to the cookies (#103)
* Smarter approach to the cookies

  - Single maxCookieSize = 1400 constant with clear documentation
  - Combined cookie storage for ~40-45% size reduction
  - Backward compatible migration from legacy cookies

* Tuneup the code.
2025-12-12 18:35:06 +00:00
lukaszraczyloGitHubCopilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
e64fc7f730 Add redis support for distributed caching (#83)
* Add redis support for distributed caching

* Move towards the self-provided Redis connection pool and RESP protocol implementation.
Official redis client library won't work with yaegi.

* fixup! Move towards the self-provided Redis connection pool and RESP protocol implementation. Official redis client library won't work with yaegi.

* fixup! fixup! Move towards the self-provided Redis connection pool and RESP protocol implementation. Official redis client library won't work with yaegi.

* fixup! fixup! fixup! Move towards the self-provided Redis connection pool and RESP protocol implementation. Official redis client library won't work with yaegi.

* fixup! fixup! fixup! fixup! Move towards the self-provided Redis connection pool and RESP protocol implementation. Official redis client library won't work with yaegi.

* fixup! fixup! fixup! fixup! fixup! Move towards the self-provided Redis connection pool and RESP protocol implementation. Official redis client library won't work with yaegi.

* ... and another all nighter.

* fixup! ... and another all nighter.

* fixup! fixup! ... and another all nighter.

* fixup! fixup! fixup! ... and another all nighter.

* Resolve issue #85 by adding ability to set custom claims in JWT tokens

* Remove redundant validation in auth middleware ( issue #89 )

* Add ability to set cookie prefix for session cookies ( #87 )

* fixup! Add ability to set cookie prefix for session cookies ( #87 )

* Add ability to set cookie max age - issue #91

* Potential fix for code scanning alert no. 10: Size computation for allocation may overflow

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* fixup! Merge main into 0.8.0-redis: resolve conflicts

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
2025-11-30 02:18:46 +00:00
lukaszraczyloandGitHub 1b49e133da Complete rebuild of the plugin
* Fix bug affecting Azure OIDC authentication ( and most likely others )

* Fixes issue #51

* Ensure that appended roles are unique. Update the documentation.

* Improvements targetting possible memory usage spikes.

* Additional fixes and cleanup

* Refactoring code to fix the issues identified by the users.

* Modernize run

* Fieldalignment

* Multiple changes to improve performance and reduce complexity.
- Optimise the errors and recovery.
- Deduplicate code in metadata cache.
- Remove unused performance monitoring code.
- Simplify session management and settings handling.

* Fix claims issue.

* Add ability to overwrite the default scopes in the settings file

* Well.. that escalated quickly.

Completely forgot that Traefik uses outdated Yaegi and requires compatibility with 1.20 ( pre-generic Go code ).

* Bugfix #51: Ensures that user provided scopes overrides work.

* fixup! Bugfix #51: Ensures that user provided scopes overrides work.

* fixup! fixup! Bugfix #51: Ensures that user provided scopes overrides work.

* Abstract the provider logic into a separate package.

* Additional micro fixes and cleanups.

* Simplify all the things.

* fixup! Simplify all the things.

* fixup! fixup! Simplify all the things.

* fixup! fixup! fixup! Simplify all the things.

* fixup! fixup! fixup! fixup! Simplify all the things.

* ...

* Cleanup tests.

* fixup! Cleanup tests.

* fixup! fixup! fixup! Cleanup tests.

* fixup! fixup! fixup! fixup! Cleanup tests.

* fixup! fixup! fixup! fixup! fixup! Cleanup tests.

* Issue #53: Fix CSRF token handling in reverse proxy

1. ✅ HTTPS Detection Fixed (session.go:723)
- Now uses X-Forwarded-Proto header instead of r.URL.Scheme
- Properly detects HTTPS in reverse proxy environments
2. ✅ SameSite Cookie Attribute Fixed
- Removed automatic SameSiteStrictMode for HTTPS (would break OAuth)
- Keeps SameSiteLaxMode to allow OAuth callbacks from external domains
- Only uses Strict for AJAX requests which don't involve OAuth redirects
3. ✅ Cookie Domain Handling Fixed
- Now respects X-Forwarded-Host header for cookie domain
- Ensures cookies are set for the public domain, not internal proxy domain
4. ✅ EnhanceSessionSecurity Properly Integrated
- Function is now actually called during session save
- Applies security enhancements without breaking OAuth flow

Why Issue #53 Failed Before:

1. Cookies were not marked Secure in HTTPS environments (browser wouldn't send them back)
2. If they had been Secure with SameSite=Strict, Azure callbacks would still fail
3. Cookie domain might have been wrong (internal vs public domain)

Why It Works Now:

1. Cookies are properly marked Secure for HTTPS
2. Uses SameSite=Lax to allow OAuth provider callbacks
3. Cookie domain uses public domain from X-Forwarded-Host
4. CSRF token persists through the entire OAuth flow

* Next set of enhancements together with memory usage improvements.

* Memory leak fixes and optimisations.

* CSRF and Cookie Domain fixes

* fixup! CSRF and Cookie Domain fixes

* Metadata cache leak fix + profiling

* fixup! Metadata cache leak fix + profiling

* Memory leaks hunting, part 1337.

* Further pursue of perfection.

* fixup! Further pursue of perfection.

* fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Further pursue of perfection.

* fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Further pursue of perfection.

* Clear race conditions

* fixup! Clear race conditions

* Weekend fun with memory leaks

* Splitting code into multiple files with reasonable testing coverage.

```
ok      github.com/lukaszraczylo/traefikoidc    117.017s        coverage: 72.6% of statements
ok      github.com/lukaszraczylo/traefikoidc/auth       0.505s  coverage: 87.1% of statements
ok      github.com/lukaszraczylo/traefikoidc/circuit_breaker    0.283s  coverage: 99.0% of statements
        github.com/lukaszraczylo/traefikoidc/config             coverage: 0.0% of statements
ok      github.com/lukaszraczylo/traefikoidc/handlers   0.349s  coverage: 98.2% of statements
ok      github.com/lukaszraczylo/traefikoidc/internal/providers (cached)        coverage: 94.3% of statements
ok      github.com/lukaszraczylo/traefikoidc/middleware 0.808s  coverage: 78.0% of statements
ok      github.com/lukaszraczylo/traefikoidc/recovery   0.653s  coverage: 100.0% of statements
ok      github.com/lukaszraczylo/traefikoidc/session/chunking   (cached)        coverage: 87.8% of statements
ok      github.com/lukaszraczylo/traefikoidc/session/core       (cached)        coverage: 85.6% of statements
ok      github.com/lukaszraczylo/traefikoidc/session/crypto     (cached)        coverage: 81.8% of statements
ok      github.com/lukaszraczylo/traefikoidc/session/storage    (cached)        coverage: 93.5% of statements
ok      github.com/lukaszraczylo/traefikoidc/session/validators (cached)        coverage: 98.8% of statements
````

* fixup! Splitting code into multiple files with reasonable testing coverage.

* fixup! fixup! Splitting code into multiple files with reasonable testing coverage.

* Weekend fun with further optimisations.

* fixup! Weekend fun with further optimisations.

* fixup! fixup! Weekend fun with further optimisations.

* fixup! fixup! fixup! Weekend fun with further optimisations.

* fixup! fixup! fixup! fixup! Weekend fun with further optimisations.

* fixup! fixup! fixup! fixup! fixup! Weekend fun with further optimisations.

* Pre-release cleanup.

* Enhance test coverage.

* fixup! Enhance test coverage.

* fixup! fixup! Enhance test coverage.

* fixup! fixup! fixup! Enhance test coverage.
2025-09-18 11:01:30 +01:00