-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(csrf): Enhance extractor functionality with metadata and security validation #3630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…y validation - Introduced Extractor struct for CSRF token extraction with metadata (source type, key). - Updated extractor methods (FromHeader, FromForm, FromQuery, FromParam) to return Extractor instances. - Added Chain method to allow multiple extractors with fallback behavior. - Implemented security validation for extractors to prevent insecure configurations. - Added tests for extractor security, metadata, and error handling.
WalkthroughThis update refactors the CSRF middleware's token extractor interface from a simple function to a structured Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FiberCtx as fiber.Ctx
participant CSRF as CSRF Middleware
participant Extractor as Extractor Struct
Client->>CSRF: Sends request with CSRF token
CSRF->>Extractor: Extract.Extract(fiber.Ctx)
Extractor->>FiberCtx: Reads token from source (header, form, etc.)
Extractor-->>CSRF: Returns token (or error)
CSRF-->>Client: Proceeds or returns error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)docs/**📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (3)📓 Common learningsmiddleware/csrf/config.go (10)Learnt from: sixcolors Learnt from: gaby Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors docs/middleware/csrf.md (9)Learnt from: sixcolors Learnt from: hcancelik Learnt from: hcancelik Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors Learnt from: sixcolors 🧬 Code Graph Analysis (1)middleware/csrf/config.go (2)
🔇 Additional comments (10)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 enhances the CSRF middleware by introducing a more structured and secure approach to token extraction. It refactors the existing extractor functions into a new Extractor struct, which now carries important metadata about the token's source. This change not only improves the clarity and flexibility of token extraction, including the ability to chain multiple extractors, but also integrates crucial security validations to prevent common misconfigurations, thereby strengthening the overall CSRF protection.
Highlights
- Enhanced CSRF Extractor Structure: Introduced a new
Extractorstruct that encapsulates the token extraction logic along with metadata such asSource(e.g., Header, Form, Query, Param, Cookie, Custom) andKey(the parameter/header name). This provides more context and control over how tokens are extracted. - Standardized Extractor Functions: All built-in extractor functions (
FromHeader,FromForm,FromQuery,FromParam) now return instances of the newExtractorstruct, ensuring consistency and allowing for metadata inspection. - Flexible Extractor Chaining: The
Chainfunction has been updated to accept and returnExtractorstructs, enabling developers to define a sequence of fallback extraction methods with clear metadata about the primary source and the chain itself. - Robust Security Validation: Implemented compile-time and runtime checks to prevent insecure CSRF configurations, specifically panicking if an extractor attempts to read the token from the same cookie used for CSRF token storage. Warnings are also logged for less secure extraction methods like URL query or path parameters.
- Comprehensive Documentation Updates: The
docs/middleware/csrf.mdfile has been significantly updated to explain the newExtractorstruct,Sourcetypes, security implications of different extraction methods (especially cookie-based), and provides new examples for custom and chained extractors. - New Test Coverage: Added a dedicated test file (
middleware/csrf/config_test.go) to thoroughly validate the new security checks, extractor metadata, and error handling for various extraction scenarios.
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 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 or fill out our survey 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. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3630 +/- ##
==========================================
+ Coverage 91.14% 91.17% +0.02%
==========================================
Files 113 113
Lines 11388 11422 +34
==========================================
+ Hits 10380 10414 +34
Misses 750 750
Partials 258 258
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a significant improvement to the CSRF middleware. The introduction of the Extractor struct with metadata and the associated security validations make the middleware more robust and user-friendly. The documentation has also been greatly enhanced. My review includes a few suggestions to further refine the implementation, mainly focusing on improving the new security validation logic for chained extractors and fixing some minor formatting issues in the documentation. Overall, this is an excellent contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
middleware/csrf/config.go (1)
173-211: Security validation looks good, but consider custom extractors.The validation logic effectively prevents the most common CSRF misconfigurations. The panic for same-cookie extraction and warnings for less secure methods are appropriate.
However, custom extractors (SourceCustom) could potentially read from cookies without being detected by this validation. Consider documenting this limitation or implementing runtime checks if feasible.
middleware/csrf/config_test.go (1)
220-221: Improve cookie token extraction for better test reliability.The current cookie parsing logic is fragile and may fail if the cookie format changes slightly.
Consider using a more robust cookie parsing approach:
-token := string(ctx.Response.Header.Peek(fiber.HeaderSetCookie)) -token = strings.Split(strings.Split(token, ";")[0], "=")[1] +setCookieHeader := string(ctx.Response.Header.Peek(fiber.HeaderSetCookie)) +// Parse "csrf_=tokenvalue; Path=/; ..." format +parts := strings.Split(setCookieHeader, ";") +if len(parts) > 0 { + cookiePart := strings.TrimSpace(parts[0]) + if equalIndex := strings.Index(cookiePart, "="); equalIndex > 0 { + token = cookiePart[equalIndex+1:] + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/middleware/csrf.md(5 hunks)middleware/csrf/config.go(3 hunks)middleware/csrf/config_test.go(1 hunks)middleware/csrf/csrf.go(1 hunks)middleware/csrf/csrf_test.go(5 hunks)middleware/csrf/extractors.go(2 hunks)middleware/csrf/extractors_test.go(6 hunks)
🧰 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/csrf.md
🧠 Learnings (8)
📓 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: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
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.
middleware/csrf/csrf.go (1)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
middleware/csrf/csrf_test.go (15)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Learnt from: ReneWerner87
PR: #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: #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: #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: #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: #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: #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: #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: gaby
PR: #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: #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: #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: #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: #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: #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: #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.
middleware/csrf/extractors_test.go (16)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Learnt from: sixcolors
PR: #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: #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: gaby
PR: #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: #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: #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: ReneWerner87
PR: #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: #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: #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: #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: #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: #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: #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: #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: #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: 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.
middleware/csrf/config_test.go (12)
Learnt from: gaby
PR: #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: #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: sixcolors
PR: #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: sixcolors
PR: #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: gaby
PR: #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: #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: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Learnt from: gaby
PR: #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: #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: sixcolors
PR: #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: #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: #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.
middleware/csrf/config.go (10)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Learnt from: gaby
PR: #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: #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: #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: #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: #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: #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: #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: #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: #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.
middleware/csrf/extractors.go (1)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
docs/middleware/csrf.md (7)
Learnt from: sixcolors
PR: #3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware Config struct, the Extractor field uses function closures (like FromCookie(key)), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Learnt from: sixcolors
PR: #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: #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: #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: #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: #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: #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.
🧬 Code Graph Analysis (1)
middleware/csrf/extractors.go (1)
ctx_interface_gen.go (1)
Ctx(17-395)
🪛 GitHub Check: markdownlint
docs/middleware/csrf.md
[failure] 238-238: Lists should be surrounded by blank lines
docs/middleware/csrf.md:238 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- You read from a different co..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
🪛 GitHub Actions: markdownlint
docs/middleware/csrf.md
[error] 238-238: markdownlint MD032/blanks-around-lists: Lists should be surrounded by blank lines.
⏰ 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). (6)
- GitHub Check: unit (1.24.x, macos-13)
- GitHub Check: unit (1.24.x, windows-latest)
- GitHub Check: unit (1.24.x, macos-latest)
- GitHub Check: repeated
- GitHub Check: Compare
- GitHub Check: lint
🔇 Additional comments (27)
middleware/csrf/csrf.go (1)
134-134: LGTM!The extractor invocation correctly uses the new
Extractorstruct'sExtractmethod, maintaining compatibility with the enhanced extractor interface.middleware/csrf/extractors_test.go (3)
20-44: LGTM!All extractor invocations correctly updated to use the
.Extract()method of the newExtractorstruct.
64-95: LGTM!Extractor method calls properly migrated to the new interface.
107-148: LGTM!Chain extractor calls and the dummy extractor implementation correctly updated to the new
Extractorstruct interface, including proper metadata fields.middleware/csrf/csrf_test.go (3)
571-584: LGTM!Custom extractor correctly converted to the new
Extractorstruct with appropriate metadata (SourceCustom and Key).
623-629: LGTM!Empty string extractor properly implemented with the new struct interface.
1728-1728: LGTM!Test case struct fields correctly updated to use the
Extractortype.Also applies to: 1924-1924, 1970-1970
middleware/csrf/config.go (3)
4-10: LGTM!New imports appropriately added to support security validation and logging functionality.
76-89: Excellent documentation!The comprehensive security guidance and clear warning about cookie-based extractors significantly improve the API's security posture.
163-168: LGTM!Proper nil checking for the struct field and security validation correctly placed after defaults are set.
middleware/csrf/extractors.go (5)
9-42: Excellent enum design with comprehensive security documentation!The
Sourcetype and constants provide clear guidance on security implications for each extraction method.
44-50: LGTM!Well-structured
Extractortype with appropriate fields for both functionality and metadata.
63-87: LGTM!
FromParamcorrectly implemented with comprehensive documentation and appropriate security warnings.
90-168: LGTM!All extractors (
FromForm,FromHeader,FromQuery) correctly implemented with consistent structure and appropriate security documentation.
171-228: LGTM!The
Chainfunction is well-implemented with proper error handling and metadata preservation. The security warning about increased attack surface is appropriate.middleware/csrf/config_test.go (6)
1-11: LGTM! Well-structured test file with proper imports.The imports are appropriate for the comprehensive CSRF testing, including proper use of testify for assertions and fasthttp for low-level request simulation.
14-35: Excellent security validation test coverage.The test properly validates that secure extractor configurations (header, form, query, param, and chained) don't cause panics during configuration setup. This is crucial for ensuring the security validation logic works correctly.
37-84: Comprehensive insecure extractor detection tests.These tests properly validate that:
- Cookie extractors reading from the same cookie name as the CSRF cookie cause panics
- Chained extractors containing insecure cookie extractors also cause panics
The custom extractor construction using the new
Extractorstruct pattern is correct and demonstrates proper security enforcement.
189-239: Well-implemented custom extractor test.The test demonstrates the new
Extractorstruct pattern effectively:
- Custom extraction logic from a specific header
- Proper error handling with
ErrMissingHeader- Correct metadata assignment (
Source: SourceCustom,Key: "X-Custom-CSRF")- Functional validation with both success and failure scenarios
330-384: Thorough unit test for the security helper function.This test provides excellent coverage of the
isInsecureCookieExtractorfunction with various scenarios:
- Secure extractors (different sources, different cookie names)
- Insecure extractors (same cookie name)
- Edge cases with custom extractors
The test cases are well-structured and comprehensive.
386-407: Important case-insensitive security test.This test ensures that case-insensitive cookie name matches are handled correctly (should warn but not panic), which is an important security consideration.
docs/middleware/csrf.md (6)
219-247: Excellent security-focused extractor categorization.The reorganization of extractors by security level is very helpful:
- "Most Secure" for headers and forms
- "Less Secure" for query/param extractors
- Clear warnings about cookie-based extraction
The detailed explanation of why cookies are problematic for CSRF tokens is comprehensive and educational.
248-269: Great addition of extractor metadata documentation.The new section explaining the
Extractorstruct's metadata fields (Source,Key) with code examples is very helpful for developers. The source type constants are well-documented with security implications.
321-377: Comprehensive custom extractor examples.The Bearer Token and JSON Body extractor examples are excellent:
- Demonstrate proper
Extractorstruct construction- Include realistic use cases
- Show proper error handling with appropriate error types
- Include metadata assignment (
Source: SourceCustom)These examples will be very helpful for developers implementing custom extractors.
379-399: Well-documented Chain extractor functionality.The Chain extractor documentation properly explains:
- Usage patterns with fallback behavior
- Metadata inspection capabilities
- Security warnings about increased complexity
The code examples are clear and practical.
495-495: Correct API reference update.The Config table properly reflects the change from function type to
csrf.Extractorstruct type, maintaining documentation accuracy.
522-531: Excellent addition of source type constants.The new constants section documenting the
Sourceenum values with security annotations is very helpful for developers working with extractor metadata.
Uh oh!
There was an error while loading. Please reload this page.