Skip to content

add chunking for cookies#497

Open
jollyekb wants to merge 2 commits intoskyhook-io:mainfrom
jollyekb:coocie-chunk
Open

add chunking for cookies#497
jollyekb wants to merge 2 commits intoskyhook-io:mainfrom
jollyekb:coocie-chunk

Conversation

@jollyekb
Copy link
Copy Markdown

Description

Add chunked cookie support for large session cookies to prevent browser rejection when OIDC ID tokens exceed the 4KB cookie size limit.

Problem

  • OIDC ID tokens (especially from Keycloak) can exceed the 4KB browser cookie limit
  • Large cookies cause authentication failures after OIDC callback
  • Session cookies without proper Secure flag handling in TLS-terminated environments

Solution

  • Implement automatic cookie chunking when payload exceeds 3800 bytes
  • Properly handle Secure flag based on X-Forwarded-Proto header
  • Preserve existing cookie API compatibility (still returns *http.Cookie)
  • Fall back to truncation if chunking fails (with warning log)

Technical details

  • Split large cookie values into multiple cookies with _chunk_N suffix
  • Add _chunks meta-cookie to track number of chunks
  • Reassemble chunks transparently in ParseSessionCookie
  • Prioritize dropping ID token before chunking when possible

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

How has this been tested?

  • Tested locally with minikube/kind
  • Tested against a remote cluster
  • Added/updated unit tests

Test cases:

  1. TestCreateSessionCookie_NormalSize - verifies normal cookies (<4KB) work unchanged
  2. TestCreateSessionCookie_LargeToken - verifies ID token dropping when too large
  3. TestCreateSessionCookie_Chunking - verifies chunking for extremely large payloads
  4. TestParseSessionCookie_Chunked - verifies chunk reassembly works correctly
  5. TestParseSessionCookie_ChunkedWithSecurePrefix - tests Secure cookie name handling
  6. TestOIDCCallback_SetsCookie - end-to-end test with actual OIDC flow

Manual testing:

  • Deployed to cluster with Keycloak OIDC
  • Verified cookies are set correctly in browser dev tools
  • Confirmed authentication persists after page reload
  • Tested with both chunked and non-chunked scenarios

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my code
  • I have added comments where necessary
  • My changes generate no new warnings
  • Any dependent changes have been merged

Related issues

Fixes #(issue number if exists)

Additional Notes

Breaking changes

  • CreateSessionCookie now returns *http.Cookie (unchanged) but may set multiple cookies via the same *http.Cookie instance with internal chunking metadata
  • Callers must use http.SetCookie for each chunk (handled automatically by the returned cookie's internal state)

Migration

No changes required for existing callers. The API remains backward compatible.

Performance impact

  • Minimal overhead: chunking only triggers for cookies > 3.8KB
  • JWT signing/verification unchanged
  • Additional cookie parsing overhead only for chunked cookies

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.

1 participant