🔥 feat: Add cookie name authentication for EncryptCookie middleware#3788
🔥 feat: Add cookie name authentication for EncryptCookie middleware#3788ReneWerner87 merged 4 commits intomainfrom
Conversation
WalkthroughPublic Encryptor/Decryptor function signatures now include the cookie name as the first parameter. Middleware and utils were updated to pass and use the name (as AES‑GCM additional authenticated data). Tests and docs were adjusted to the new API and added a test rejecting swapped cookie names. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as Fiber App
participant EC as EncryptCookie Middleware
note right of EC #E6F4EA: Cookie name passed as first arg\nand used as AES‑GCM AAD
Client->>App: HTTP Request with Cookie(name, encryptedValue)
App->>EC: Run encryptcookie middleware
EC->>EC: Decryptor(name, encryptedValue, key)
alt Decryption OK
EC->>App: Set Cookie(name, plaintextValue)
else Decryption fails
EC->>App: Remove Cookie(name)
end
App-->>EC: Handler returns (may set response cookies)
loop For each response cookie
EC->>EC: Encryptor(name, plaintextValue, key)
alt Encryption OK
EC->>App: Set Cookie(name, encryptedValue)
else Encryption fails
EC->>App: Remove Cookie(name)
end
end
App-->>Client: HTTP Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)docs/**📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds cookie name authentication to the encryptcookie middleware to prevent cross-cookie reuse by binding the cookie name into AES-GCM additional authenticated data. This enhances security by ensuring encrypted cookies cannot be swapped between different cookie names.
- Updated function signatures to include cookie name parameter in encrypt/decrypt operations
- Modified AES-GCM encryption to use cookie name as additional authenticated data
- Added comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| middleware/encryptcookie/utils.go | Added cookie name parameter to EncryptCookie and DecryptCookie functions and bound it to AES-GCM AAD |
| middleware/encryptcookie/config.go | Updated Encryptor and Decryptor function signatures to include cookie name parameter |
| middleware/encryptcookie/encryptcookie.go | Modified middleware to pass cookie names to encrypt/decrypt functions |
| middleware/encryptcookie/encryptcookie_test.go | Updated all test calls and added new test for cross-cookie reuse prevention |
| docs/middleware/encryptcookie.md | Updated documentation to reflect new function signatures |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3788 +/- ##
==========================================
- Coverage 91.67% 91.65% -0.03%
==========================================
Files 113 113
Lines 11959 11959
==========================================
- Hits 10964 10961 -3
- Misses 731 733 +2
- Partials 264 265 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/middleware/encryptcookie.md(1 hunks)middleware/encryptcookie/config.go(1 hunks)middleware/encryptcookie/encryptcookie.go(2 hunks)middleware/encryptcookie/encryptcookie_test.go(9 hunks)middleware/encryptcookie/utils.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
middleware/encryptcookie/config.gomiddleware/encryptcookie/encryptcookie_test.gomiddleware/encryptcookie/encryptcookie.gomiddleware/encryptcookie/utils.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/middleware/encryptcookie.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-07-01T03:33:22.283Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Applied to files:
middleware/encryptcookie/encryptcookie_test.gomiddleware/encryptcookie/encryptcookie.go
📚 Learning: 2024-07-01T03:44:03.672Z
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
middleware/encryptcookie/encryptcookie_test.go
📚 Learning: 2024-07-01T03:33:22.283Z
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-07-01T03:33:22.283Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Applied to files:
middleware/encryptcookie/encryptcookie_test.go
🧬 Code graph analysis (1)
middleware/encryptcookie/encryptcookie_test.go (1)
middleware/encryptcookie/utils.go (3)
EncryptCookie(39-62)DecryptCookie(65-99)GenerateKey(104-116)
🪛 GitHub Actions: golangci-lint
middleware/encryptcookie/encryptcookie_test.go
[error] 252-252: golangci-lint: File is not properly formatted (gofumpt). Run 'gofumpt -w' to fix formatting.
🪛 GitHub Check: lint
middleware/encryptcookie/encryptcookie_test.go
[failure] 252-252:
File is not properly formatted (gofumpt)
[failure] 447-447:
File is not properly formatted (gofumpt)
[failure] 630-630:
File is not properly formatted (gofumpt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (14)
middleware/encryptcookie/utils.go (2)
39-61: LGTM! Cookie name correctly bound to ciphertext.The implementation correctly uses the cookie name as additional authenticated data (AAD) in AES-GCM. This prevents cross-cookie reuse attacks by cryptographically binding the ciphertext to its intended cookie name.
65-98: LGTM! Cookie name validation during decryption.The implementation correctly validates the cookie name during decryption using GCM's AAD verification. Any attempt to use a ciphertext with a different cookie name will fail authentication.
middleware/encryptcookie/config.go (1)
17-22: LGTM! Public API updated to expose cookie name.The function signatures for
EncryptorandDecryptornow correctly include the cookie name as the first parameter, enabling custom implementations to perform their own validation.docs/middleware/encryptcookie.md (1)
70-71: LGTM! Documentation accurately reflects API changes.The function signatures are correctly documented with the cookie name as the first parameter for both
EncryptorandDecryptor.middleware/encryptcookie/encryptcookie.go (2)
24-24: LGTM! Decryptor correctly invoked with cookie name.The middleware correctly passes the cookie name as the first parameter to the
Decryptorfunction.
43-43: LGTM! Encryptor correctly invoked with cookie name.The middleware correctly passes the cookie name as the first parameter to the
Encryptorfunction.middleware/encryptcookie/encryptcookie_test.go (8)
54-54: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
EncryptCookie.
65-65: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
DecryptCookie.
77-77: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
EncryptCookie.
84-84: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
DecryptCookie.
91-91: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
DecryptCookie.
147-147: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
DecryptCookie.
159-173: Excellent security test! Validates cross-cookie reuse prevention.This test correctly verifies that an encrypted cookie value cannot be decrypted using a different cookie name, confirming the security property introduced by this PR.
240-240: LGTM! Test updated with cookie name parameter.The test correctly passes the cookie name as the first parameter to
DecryptCookie.
|
@gaby pls extend the what´s new.md |
@ReneWerner87 Docs updated |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/middleware/encryptcookie.md (1)
77-79: Clarify the payload parameter name in the new bullet list.The second bullet labels the argument as
string, which doesn’t match any parameter name and reads like a type placeholder. Naming it (e.g.payload stringor explicitly mentioningdecryptedString/encryptedString) will keep the docs aligned with the actual signature. A quick tweak like the diff below would remove the ambiguity:-- `string`: The cookie payload. `EncryptCookie` accepts the decrypted value and returns ciphertext, while `DecryptCookie` receives ciphertext and must return the decrypted value. +- `payload string`: The cookie payload. `EncryptCookie` accepts the decrypted value and returns ciphertext, while `DecryptCookie` receives ciphertext and must return the decrypted value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/middleware/encryptcookie.md(1 hunks)docs/whats_new.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/whats_new.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/middleware/encryptcookie.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-07-01T03:33:22.283Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
|
@gaby can you create the migrator |
Summary
Fixes #3777