Refactor CSRF middleware and enhance documentation#3598
Merged
ReneWerner87 merged 19 commits intomainfrom Jul 19, 2025
Merged
Conversation
…tractor usage docs: simplify and improve
…nd improve formatting
Contributor
WalkthroughThe CSRF middleware was refactored to remove the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FiberApp
participant CSRF_Middleware
participant TokenStorage
Client->>FiberApp: Sends HTTP request (with CSRF token in header/form/query/param)
FiberApp->>CSRF_Middleware: Passes request to middleware
CSRF_Middleware->>CSRF_Middleware: Extract token using configured extractor(s)
CSRF_Middleware->>TokenStorage: Validate token (compare with cookie if Double Submit)
TokenStorage-->>CSRF_Middleware: Validation result
CSRF_Middleware-->>FiberApp: Allow or reject request (403 on failure)
FiberApp-->>Client: Responds to client
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
There was a problem hiding this comment.
Summary of Changes
Hello @sixcolors, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly refactors the CSRF middleware, simplifying its configuration and enhancing its token extraction capabilities. It includes a comprehensive rewrite of the documentation to provide clearer guidance and best practices for securing applications against CSRF attacks, alongside internal code clean-up and improved testing.
Highlights
- Configuration Simplification: The
KeyLookupoption has been removed from the CSRF middleware configuration, streamlining how token extractors are defined. Users now explicitly set theExtractorfunction, making the configuration more direct and less ambiguous. - Enhanced Token Extractors: Introduced a new
Chainextractor, allowing the middleware to attempt token extraction from multiple sources (e.g., header then form) in a defined order. The documentation now clearly categorizes and explains various built-in extractors. - Documentation Overhaul: The
csrf.mddocumentation has been completely rewritten for improved clarity, conciseness, and practical applicability. It now includes quick start guides, configuration examples tailored for Server-Side Rendered (SSR) and Single Page Applications (SPAs), and detailed security best practices. - Security Enhancements & Guidance: The middleware's internal validation logic has been refined to reinforce the Double Submit Cookie pattern. Crucially, the documentation now includes explicit warnings against insecure token extraction methods (like extracting from the same cookie used for validation) and provides comprehensive best practices for robust CSRF protection.
- Code Clean-up and Testing: Removed deprecated
KeyLookuprelated logic and theFromCookieextractor function from the core middleware. Comprehensive new and updated tests have been added to ensure the reliability and correctness of the refactored extractor functionality.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the CSRF middleware to improve security by removing cookie-based token extraction, simplifies the configuration API by replacing the KeyLookup pattern with direct Extractor functions, and significantly enhances the documentation. The changes include adding a new Chain extractor for handling multiple token sources, comprehensive test coverage for all extractors, and detailed security guidance.
- Removes cookie-based CSRF token extraction for improved security
- Replaces KeyLookup string parsing with direct Extractor function configuration
- Adds Chain extractor for multiple token source fallbacks with comprehensive test coverage
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| middleware/csrf/token.go | Adds documentation comment for Token struct |
| middleware/csrf/extractors_test.go | Removes cookie extractor tests and adds comprehensive Chain extractor testing |
| middleware/csrf/extractors.go | Removes FromCookie function and adds Chain extractor with security documentation |
| middleware/csrf/csrf_test.go | Updates all tests to use new Extractor config and adds extensive Chain/extractor testing |
| middleware/csrf/csrf.go | Removes cookie detection logic and simplifies double submit cookie validation |
| middleware/csrf/config.go | Removes KeyLookup configuration in favor of direct Extractor specification |
| docs/middleware/csrf.md | Complete documentation rewrite with security best practices and usage examples |
| app := fiber.New() | ||
| ctx := app.AcquireCtx(&fasthttp.RequestCtx{}) | ||
| defer app.ReleaseCtx(ctx) | ||
| t.Cleanup(func() { app.ReleaseCtx(ctx) }) |
There was a problem hiding this comment.
[nitpick] While using t.Cleanup is good practice, this change is inconsistent with the rest of the test file which uses defer app.ReleaseCtx(ctx). For consistency, either update all tests to use t.Cleanup or keep using defer.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3598 +/- ##
==========================================
- Coverage 90.97% 90.96% -0.02%
==========================================
Files 111 111
Lines 11208 11195 -13
==========================================
- Hits 10197 10184 -13
Misses 759 759
Partials 252 252
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:
|
Contributor
There was a problem hiding this comment.
Code Review
The CSRF middleware refactoring simplifies configuration by using a more explicit Extractor function, enhancing type safety. Documentation improvements clarify usage, while the Chain extractor adds flexibility. Tests have been updated to cover new functionality.
Comment on lines
+104
to
+107
| function getCsrfToken() { | ||
| const value = `; ${document.cookie}`; | ||
| const parts = value.split(`; __Host-csrf_=`); | ||
| if (parts.length === 2) return parts.pop().split(';').shift(); |
Contributor
There was a problem hiding this comment.
To avoid confusion, clarify that the cookie name __Host-csrf_ in the JavaScript example must match the CookieName configured on the server-side. This ensures proper token retrieval and validation.
Suggested change
| function getCsrfToken() { | |
| const value = `; ${document.cookie}`; | |
| const parts = value.split(`; __Host-csrf_=`); | |
| if (parts.length === 2) return parts.pop().split(';').shift(); | |
| function getCsrfToken() { | |
| // Ensure the cookie name `__Host-csrf_` matches the server-side CookieName configuration. | |
| const value = `; ${document.cookie}`; | |
| const parts = value.split(`; __Host-csrf_=`); | |
| if (parts.length === 2) return parts.pop().split(';').shift(); | |
| } |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/middleware/csrf.md (1)
171-174: Constant name casingThe code defines
HeaderName = "X-Csrf-Token"yet all examples use"X-Csrf-Token".
If the canonical header is intended to follow common casing (X-CSRF-Token), align the constant and examples to avoid subtle bugs with case-sensitive clients.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Makefile(1 hunks)docs/middleware/csrf.md(3 hunks)middleware/csrf/config.go(5 hunks)middleware/csrf/csrf.go(3 hunks)middleware/csrf/csrf_test.go(6 hunks)middleware/csrf/extractors.go(2 hunks)middleware/csrf/extractors_test.go(0 hunks)middleware/csrf/token.go(1 hunks)
💤 Files with no reviewable changes (1)
- middleware/csrf/extractors_test.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
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.
Learnt from: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-06-09T18:50:02.075Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-06-15T19:26:06.401Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-10-08T19:06:06.583Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Makefile (3)
Learnt from: CR
PR: gofiber/fiber#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-16T14:08:00.084Z
Learning: Run `make lint` for static analysis after modifying code
Learnt from: CR
PR: gofiber/fiber#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-16T14:08:00.084Z
Learning: Run `make markdown` to lint all Markdown files after modifying code
Learnt from: CR
PR: gofiber/fiber#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-16T14:08:00.084Z
Learning: Run `make format` to format the project after modifying code
middleware/csrf/extractors.go (4)
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-09-25T16:15:39.392Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
middleware/csrf/config.go (10)
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-09-25T16:15:39.392Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:26-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware, the `newStore`, `New`, and `NewWithStore` functions ensure that a `Store` is present even if it is not initialized in `ConfigDefault`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:26-26
Timestamp: 2024-09-25T16:18:46.641Z
Learning: In the session middleware, the `newStore`, `New`, and `NewWithStore` functions ensure that a `Store` is present even if it is not initialized in `ConfigDefault`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
middleware/csrf/csrf.go (2)
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
middleware/csrf/csrf_test.go (23)
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.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-07-02T13:29:56.992Z
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.
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: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: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Learnt from: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-06-30T00:38:06.580Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-10-12T10:01:44.206Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-09-25T17:05:06.991Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
docs/middleware/csrf.md (7)
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
🧬 Code Graph Analysis (2)
middleware/csrf/extractors.go (2)
ctx_interface_gen.go (1)
Ctx(17-395)middleware/csrf/csrf.go (1)
ErrTokenNotFound(15-15)
middleware/csrf/config.go (5)
middleware/session/store.go (2)
Store(31-33)Store(74-76)app.go (1)
ErrorHandler(57-57)ctx_interface_gen.go (1)
Ctx(17-395)constants.go (1)
ErrForbidden(120-120)middleware/session/config.go (1)
ConfigDefault(109-115)
🪛 GitHub Check: lint
middleware/csrf/csrf_test.go
[failure] 1911-1911:
fieldalignment: struct with 48 pointer bytes could be 40 (govet)
[failure] 1818-1818:
fieldalignment: struct with 24 pointer bytes could be 16 (govet)
[failure] 1814-1814:
File is not properly formatted (gofumpt)
[failure] 1717-1717:
fieldalignment: struct with 32 pointer bytes could be 24 (govet)
🪛 GitHub Actions: golangci-lint
middleware/csrf/csrf_test.go
[error] 1717-1717: golangci-lint: struct with 32 pointer bytes could be 24 (fieldalignment, govet)
⏰ 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). (4)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (18)
Makefile (1)
37-38: Indentation fix makes thelinttarget runnable againSwitching the space indentation to a tab is the correct fix—without it,
make lintwould trigger a “missing separator” error. Looks good.middleware/csrf/token.go (1)
7-8: Documentation improvement looks good!The added comment clearly describes the purpose of the Token struct and its intended usage.
middleware/csrf/extractors.go (2)
17-19: Good security decision to omit FromCookie!The comment clearly explains why cookie-based extraction is omitted, which helps prevent future developers from inadvertently reintroducing this security vulnerability.
63-96: Well-implemented Chain extractor!The Chain function provides a clean way to combine multiple extractors with proper error handling:
- Returns early on successful extraction
- Preserves the last error for better debugging
- Handles edge cases (empty extractors list, nil errors)
middleware/csrf/csrf.go (3)
23-23: Helpful clarification for dummyValue!The comment explains that the actual validation relies on the key rather than this placeholder value.
133-133: Comment accurately reflects the removal of cookie extraction.
143-147: Excellent documentation of the Double Submit Cookie pattern!The comment clearly explains:
- The security mechanism (requiring both cookie AND another channel)
- How it prevents CSRF attacks
- An important warning about custom extractors
middleware/csrf/config.go (3)
11-109: Clean refactoring of the Config struct!The removal of
KeyLookupand the shift to explicit extractor functions improves the API clarity and type safety.
46-47: Critical security warning - well placed!This warning is essential to prevent developers from inadvertently defeating CSRF protection by creating custom extractors that read from the same cookie.
121-121: Proper default configuration.Setting the default extractor to
FromHeader(HeaderName)maintains backward compatibility while using the new extractor pattern.Also applies to: 156-156
middleware/csrf/csrf_test.go (5)
355-355: Test updates correctly reflect the new Extractor API.All tests have been properly updated to use the new
Extractorconfiguration instead of the deprecatedKeyLookup.Also applies to: 458-458, 495-495, 533-533
546-546: Good use of t.Cleanup!Using
t.Cleanupis the preferred approach in tests as it ensures cleanup happens even if the test panics.
1567-1640: Comprehensive test coverage for the Chain extractor!The test thoroughly validates:
- Header extraction (first in chain)
- Form extraction (fallback)
- Precedence when both are present
- Error cases
1714-1815: Excellent test coverage for all extractors!The
Test_CSRF_All_Extractorsfunction provides thorough coverage of all built-in extractors with both success and failure cases.
1907-1966: Good error type validation tests!Testing that each extractor returns the correct error type helps ensure proper error handling throughout the middleware.
docs/middleware/csrf.md (3)
12-15: Import path points to a non-existent v3 module
github.com/gofiber/fiber/v3(and the matching middleware path) does not exist on the public repo – current releases are still onv2. Using a versioned import that has not been tagged will breakgo getfor every reader who copies this snippet.
Change the snippet back togithub.com/gofiber/fiber/v2(or wait until v3 is actually tagged) to prevent confusion and build failures.
52-56: SameSite constant issue is repeated in SPA exampleThe same type-safety concern applies here – use the constant rather than the raw string.
Also double-check thatCookieSameSite:"Lax"is correct for an SPA that needs cross-originfetch; many deployments requireSameSite=Noneinstead.
141-145:IdleTimeoutname may not match the actual struct fieldIn the current CSRF middleware implementation the expiration field is still called
Expiration.
Renaming it in the docs without renaming the struct will confuse users and cause compile errors.Confirm the field was really renamed; if not, revert the table entry:
-| IdleTimeout | `time.Duration` | Token expiration time | `30 * time.Minute` | +| Expiration | `time.Duration` | Token expiration time | `30 * time.Minute` |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
middleware/csrf/csrf_test.go (2)
1546-1546: Consider consistency with rest of test file.While using
t.Cleanupis good practice, this change is inconsistent with the rest of the test file which usesdefer app.ReleaseCtx(ctx). For consistency, either update all tests to uset.Cleanupor keep usingdefer.
1717-1722: Struct field alignment optimization already identified.The static analysis correctly identifies field alignment issues in these test case structs that could improve memory efficiency. However, this same concern was already raised in past review comments for similar structs in this file.
Also applies to: 1819-1823, 1912-1916
🧹 Nitpick comments (2)
docs/whats_new.md (2)
1888-1902: Header name inconsistency (X-CSRF-TokenvsX-Csrf-Token)The “After” example uses
X-Csrf-Token, whereas the text and the “Before” block useX-CSRF-Token. Mixed-case header names make copy-paste migrations error-prone.- Extractor: csrf.FromHeader("X-Csrf-Token"), + Extractor: csrf.FromHeader("X-CSRF-Token"),Please pick one canonical spelling (ideally the fully-uppercase form already used elsewhere) and use it consistently across all snippets.
1904-1920: Doc misses the new “route-specific extractor” feature introduced in this PRThe paragraph correctly documents the removal of
FromCookie, but the PR also adds the ability to supply extractors per route (e.g.csrf.New().WithExtractor(...)orcsrf.Chain(...), depending on the final API). Mentioning this here would prevent readers from overlooking the new, more granular option while they migrate away fromKeyLookup.Suggest adding a short bullet such as:
• Route-Specific Extractors: You can now override the global extractor on a per-route basis using
csrf.Chain(...)or the handler’sWithExtractorhelper.This keeps the What's New section in sync with the actual capabilities exposed by the refactor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/whats_new.md(1 hunks)middleware/csrf/csrf_test.go(6 hunks)middleware/csrf/extractors.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- middleware/csrf/extractors.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
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.
Learnt from: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-06-09T18:50:02.075Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-06-15T19:26:06.401Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-10-08T19:06:06.583Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
docs/whats_new.md (9)
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/session_manager.go:30-43
Timestamp: 2024-09-25T16:15:39.392Z
Learning: In the session middleware, `session.FromContext(c)` returns `*session.Middleware`, whereas `m.session.Get(c)` returns `*session.Store`, so they are not directly interchangeable.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
middleware/csrf/csrf_test.go (28)
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: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: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.
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-07-02T13:29:56.992Z
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.
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.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Learnt from: sixcolors
PR: gofiber/fiber#3051
File: middleware/session/session.go:215-216
Timestamp: 2024-06-30T00:38:06.580Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Learnt from: efectn
PR: gofiber/fiber#3162
File: app_test.go:893-895
Timestamp: 2024-11-29T12:37:27.581Z
Learning: In the `Test_App_ShutdownWithContext` function in `app_test.go`, the `clientDone` channel is used to synchronize the client's request completion before proceeding, eliminating the need for additional `time.Sleep` calls.
Learnt from: efectn
PR: gofiber/fiber#3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the `require` methods from the `testify` package for assertions instead of manual comparisons and calls to `t.Fatal` or `t.Fatalf`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-10-12T10:01:44.206Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-09-25T17:05:06.991Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
Learnt from: gaby
PR: gofiber/fiber#3170
File: ctx_test.go:1721-1724
Timestamp: 2024-10-16T12:12:30.506Z
Learning: In the Go unit tests in `ctx_test.go`, it is acceptable to use invalid CIDR notation such as `"0.0.0.1/31junk"` for testing purposes.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Learnt from: sixcolors
PR: gofiber/fiber#2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-09-25T16:18:34.719Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:122-122
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In `DefaultErrorHandler(c *fiber.Ctx, err error)`, since `c` is a pointer to an interface, we need to dereference `*c` when calling interface methods like `SendStatus`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
🪛 GitHub Check: lint
middleware/csrf/csrf_test.go
[failure] 1912-1912:
fieldalignment: struct with 48 pointer bytes could be 40 (govet)
[failure] 1819-1819:
fieldalignment: struct with 24 pointer bytes could be 16 (govet)
[failure] 1717-1717:
fieldalignment: struct with 32 pointer bytes could be 24 (govet)
🪛 GitHub Actions: golangci-lint
middleware/csrf/csrf_test.go
[error] 1717-1717: golangci-lint: fieldalignment: struct with 32 pointer bytes could be 24 (govet)
🔇 Additional comments (5)
middleware/csrf/csrf_test.go (5)
355-355: LGTM: Proper migration from deprecated KeyLookup to Extractor API.This change correctly migrates from the deprecated
KeyLookupconfiguration to the newExtractorfunction-based API, maintaining the same functionality while adopting the updated design.
458-458: LGTM: Consistent migration to extractor functions.These changes properly migrate from the deprecated
KeyLookupconfiguration to the new extractor function API (FromForm,FromQuery,FromParam), maintaining test functionality while adopting the updated design pattern.Also applies to: 495-495, 533-533
1567-1640: Excellent test coverage for extractor chaining functionality.This test comprehensively validates the new extractor chaining feature, covering:
- Primary extractor success (header)
- Fallback behavior (form)
- Precedence rules (header over form)
- Failure scenarios (missing/invalid tokens)
The test design aligns well with the PR objective of providing more granular control over CSRF token extraction.
1642-1712: Good edge case coverage for extractor chaining.These tests properly validate edge cases for the chaining functionality:
- Empty chain behavior (proper failure)
- Single extractor chain (behaves like individual extractor)
This ensures robustness of the chaining implementation across boundary conditions.
1714-1967: Comprehensive test coverage for the new extractor API.These test functions provide excellent coverage of the extractor functionality:
- Parameterized testing of all extractor types (
Test_CSRF_All_Extractors)- Detailed param extractor testing with routing considerations
- Proper error type validation for different extraction failures
- Missing token scenarios across all extractors
The test design is thorough and ensures the new extractor API is robust across various use cases, aligning well with the PR's goal of enhancing CSRF middleware functionality.
Member
ReneWerner87
approved these changes
Jul 19, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/middleware/csrf.md (2)
138-144: Add clarification that cookie name must match server-side configThis was noted in an earlier review but not yet reflected in the snippet. A brief comment avoids subtle bugs when the cookie name is customised.
38-42: Align SameSite values with Fiber v3 constants (lower-case)Fiber v3 defines
const ( CookieSameSiteLaxMode = "lax" CookieSameSiteStrictMode = "strict" )Using the capitalized strings
"Lax"/"Strict"deviates from those constants and from the examples elsewhere in the docs. Update every occurrence to either the constants themselves or the lower-case literals to avoid confusing users and code-search/grep mismatches.Diff (sample for one snippet – apply similarly to all):
- CookieSameSite: "Lax", + CookieSameSite: fiber.CookieSameSiteLaxMode,Bullet list & config table:
-- `CookieSameSite: "Lax"` or `"Strict"` +- `CookieSameSite: "lax"` or `"strict"` (or the typed constants)-| CookieSameSite | `string` | SameSite attribute (**use "Lax" or "Strict"**) | `"Lax"` | +| CookieSameSite | `string` | SameSite attribute (**use "lax" or "strict"**) | `"lax"` |Also applies to: 50-53, 74-78, 88-92, 392-395
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/middleware/csrf.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .github/copilot-instructions.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
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: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: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.
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: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-06-09T18:50:02.075Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: dave-gray101
PR: gofiber/fiber#3027
File: docs/api/middleware/keyauth.md:222-222
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `KeyLookup` field in the `keyauth` middleware does not support pipe-delimited keys. Instead, it specifies a single source and name for key extraction, with additional sources specified in the `FallbackKeyLookups` field.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-10-08T19:06:06.583Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-06-15T19:26:06.401Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
docs/middleware/csrf.md (16)
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Learnt from: sixcolors
PR: gofiber/fiber#3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.844Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
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: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: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Learnt from: gaby
PR: gofiber/fiber#3193
File: middleware/cache/cache_test.go:897-897
Timestamp: 2024-11-08T04:10:42.990Z
Learning: In the Fiber framework, `Context()` is being renamed to `RequestCtx()`, and `UserContext()` to `Context()` to improve clarity and align with Go's context conventions.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-10-08T19:06:06.583Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Learnt from: hcancelik
PR: gofiber/fiber#3036
File: docs/middleware/cache.md:103-103
Timestamp: 2024-06-15T19:26:06.401Z
Learning: There are no hard tabs in the lines 100 to 105 of the `docs/middleware/cache.md` file. Future comments about formatting should accurately reflect the actual content.
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Fiber currently supports square bracket notation in form data with 'application/x-www-form-urlencoded' content type, but this feature is not available for multipart form data. The implementation for form-urlencoded is in ctx.go (BodyParser method), while multipart form data handling needs enhancement to support this feature.
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.
⏰ 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). (4)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
- GitHub Check: Analyse
Member
|
This was referenced Jul 20, 2025
This was referenced Jul 28, 2025
This was referenced Aug 22, 2025
16 tasks
This was referenced Nov 30, 2025
20 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Simplify CSRF configuration by removing unnecessary components and improving extractor functionality. Add support for route-specific extractors and enhance documentation for clarity and accuracy. Improve error handling and validation within the CSRF middleware.
Supersedes #3589