feat: Implement cross-domain cookie sharing #15849
Conversation
Demo
Jenkins 
demos.haus There was a problem hiding this comment.
Pull request overview
This PR implements cross-domain cookie sharing by integrating a cookie service Flask extension with cookies.canonical.com. The implementation includes upgrading the cookie-policy npm package, adding Flask-Caching for health status monitoring, and configuring the necessary session and cookie consent settings.
- Updates cookie-policy package from version 3.7.x to 3.8.0
- Adds canonicalwebteam.cookie-service and Flask-Caching Python dependencies
- Configures Flask session and cookie service with caching support
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updates @canonical/cookie-policy dependency to 3.8.0 with exact version pinning |
| yarn.lock | Reflects the updated cookie-policy package version and integrity hash |
| requirements.txt | Adds canonicalwebteam.cookie-service (1.0.0) and Flask-Caching (2.1.0) dependencies |
| webapp/app.py | Implements cookie service initialization with Flask-Caching, session configuration, and cache helper functions |
| konf/site.yaml | Adds COOKIE_SERVICE_API_KEY environment variable from Kubernetes secrets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| app.config["PREFERENCES_COOKIE_EXPIRY_DAYS"] = 365 | ||
|
|
||
| # Initialize Flask-Caching | ||
| app.config["CACHE_TYPE"] = "SimpleCache" |
There was a problem hiding this comment.
SimpleCache is not thread-safe and should not be used in production environments. According to Flask-Caching documentation, SimpleCache is only suitable for development. For production, consider using RedisCache, MemcachedCache, or FileSystemCache. This is particularly important since the cookie service health check runs in the background and could cause race conditions.
| app.config["CACHE_TYPE"] = "SimpleCache" | |
| app.config["CACHE_TYPE"] = "FileSystemCache" | |
| app.config["CACHE_DIR"] = "/tmp/flask_cache" |
| "dependencies": { | ||
| "@axe-core/playwright": "^4.8.5", | ||
| "@canonical/cookie-policy": "^3.7.4", | ||
| "@canonical/cookie-policy": "3.8.0", |
There was a problem hiding this comment.
The version specifier has changed from a flexible semantic version range (^3.7.4) to an exact version pin (3.8.0). This inconsistency with other dependencies (which use flexible ranges like ^) may make future updates more difficult. Consider using ^3.8.0 or ~3.8.0 to allow automatic patch updates while maintaining the current minor version, consistent with other dependencies like @canonical/global-nav.
| "@canonical/cookie-policy": "3.8.0", | |
| "@canonical/cookie-policy": "^3.8.0", |
| "dependencies": { | ||
| "@axe-core/playwright": "^4.8.5", | ||
| "@canonical/cookie-policy": "^3.7.4", | ||
| "@canonical/cookie-policy": "3.8.0", |
There was a problem hiding this comment.
The PR description states that the cookie-policy package should be bumped to version 3.8.1, but the actual version in the code is 3.8.0. Please update to 3.8.1 to match the PR description, or update the description if 3.8.0 is the intended version.
| "@canonical/cookie-policy": "3.8.0", | |
| "@canonical/cookie-policy": "3.8.1", |
| # Configuration for shared cookie service | ||
|
|
||
| # Configure Flask session | ||
| app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=365) | ||
| app.config["SESSION_COOKIE_SAMESITE"] = "Lax" | ||
| app.config["SESSION_COOKIE_HTTPONLY"] = True | ||
| app.config["SESSION_COOKIE_SECURE"] = True | ||
|
|
||
| # Number of days before preference cookies expire (default: 365) | ||
| app.config["PREFERENCES_COOKIE_EXPIRY_DAYS"] = 365 | ||
|
|
||
| # Initialize Flask-Caching | ||
| app.config["CACHE_TYPE"] = "SimpleCache" | ||
| cache = Cache(app) | ||
|
|
||
|
|
||
| # Set up cache functions for cookie consent service | ||
| def get_cache(key): | ||
| return cache.get(key) | ||
|
|
||
|
|
||
| def set_cache(key, value, timeout): | ||
| cache.set(key, value, timeout) | ||
|
|
||
|
|
||
| cookie_service = CookieConsent().init_app( | ||
| app, | ||
| get_cache_func=get_cache, | ||
| set_cache_func=set_cache, | ||
| start_health_check=True, | ||
| ) |
There was a problem hiding this comment.
Missing documentation for the cookie service configuration. Consider adding comments explaining:
- What the COOKIE_SERVICE_API_KEY environment variable is used for (defined in konf/site.yaml but not explicitly referenced here)
- The purpose of each configuration option
- Why SimpleCache is being used and its limitations for production deployments
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15849 +/- ##
=======================================
Coverage 48.24% 48.24%
=======================================
Files 37 37
Lines 5615 5615
=======================================
Hits 2709 2709
Misses 2906 2906
🚀 New features to boost your workflow:
|
Done
QA
./run serveordotrunIssue / Card
Fixes #
Screenshots
[If relevant, please include a screenshot.]
Help
QA steps - Commit guidelines