Switch KeyAuth to extractor pattern#3651
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughKeyAuth middleware was refactored from string-based KeyLookup to a function-based Extractor API with constructors and chaining; defaults, error handling, and WWW-Authenticate behavior were updated. EncryptCookie docs clarified required key byte lengths. Tests and docs were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Fiber
participant KeyAuth
participant Extractor
participant Validator
participant Handler
Client->>Fiber: HTTP Request
Fiber->>KeyAuth: Invoke middleware
KeyAuth->>Extractor: Extract(c)
alt key extracted
KeyAuth->>Validator: Validate(c, key)
alt valid
KeyAuth->>Fiber: store key in context
KeyAuth->>Handler: SuccessHandler / next()
Handler-->>Client: Response
else invalid
KeyAuth->>Fiber: set WWW-Authenticate (if scheme)
KeyAuth-->>Client: ErrorHandler -> 401
end
else missing
KeyAuth->>Fiber: set WWW-Authenticate (if scheme)
KeyAuth-->>Client: ErrorHandler -> 401
end
sequenceDiagram
participant KeyAuth
participant Chain
participant Ext1 as Extractor A
participant Ext2 as Extractor B
participant Ext3 as Extractor C
KeyAuth->>Chain: Extract(c)
Chain->>Ext1: Extract(c)
alt A succeeds
Ext1-->>Chain: token
Chain-->>KeyAuth: token
else A fails
Chain->>Ext2: Extract(c)
alt B succeeds
Ext2-->>Chain: token
Chain-->>KeyAuth: token
else B fails
Chain->>Ext3: Extract(c)
alt C succeeds
Ext3-->>Chain: token
Chain-->>KeyAuth: token
else
Ext3-->>Chain: last error
Chain-->>KeyAuth: error
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ReneWerner87, 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!
I've refactored the KeyAuth middleware to adopt a new, more flexible extractor pattern. This change introduces a dedicated extractor package that provides reusable utilities for retrieving values from incoming requests, which other middleware like CSRF and Session also now leverage internally. This improves consistency and modularity in how request data is accessed across the framework.
Highlights
- New
extractorPackage: A new publicextractorpackage has been introduced, providing standardized helper functions (e.g.,FromHeader,FromCookie,Chain) for efficiently extracting values fromfiber.Ctx. KeyAuthMiddleware Refactor: TheKeyAuthmiddleware has been refactored to utilize the newextractor.Extractortype, replacing the previous string-basedKeyLookupandCustomKeyLookupconfigurations for improved flexibility and type safety.- Middleware Integration: Existing middleware such as
CSRFandSessionhave been updated to internally leverage the newextractorpackage, promoting code reuse and consistency across the framework's value extraction mechanisms. - Documentation & Migration: Comprehensive documentation for the new
extractorutilities has been added, alongside updates toEncryptCookiedocumentation and a detailed migration guide forKeyAuthusers to facilitate a smooth transition.
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. ↩
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the KeyAuth middleware to use a new, reusable extractor pattern, which is a great improvement for modularity and code reuse. The changes are well-implemented across the middleware, documentation, and tests. I've identified a few minor issues, including a redundant code block, some outdated comments, an obsolete test case, and a documentation clarification that would improve the changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.golangci.ymlis excluded by!**/*.yml
📒 Files selected for processing (11)
docs/guide/extractor.md(1 hunks)docs/middleware/encryptcookie.md(2 hunks)docs/middleware/keyauth.md(4 hunks)docs/whats_new.md(3 hunks)extractor/extractor.go(1 hunks)middleware/csrf/extractors.go(7 hunks)middleware/encryptcookie/encryptcookie.go(1 hunks)middleware/keyauth/config.go(4 hunks)middleware/keyauth/keyauth.go(4 hunks)middleware/keyauth/keyauth_test.go(10 hunks)middleware/session/extractors.go(8 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/encryptcookie.mddocs/guide/extractor.mddocs/whats_new.mddocs/middleware/keyauth.md
🧠 Learnings (20)
📓 Common learnings
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#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: ksw2000
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2025-07-08T13:41:20.627Z
Learning: ksw2000 suggests implementing a pluggable encoder/decoder registry system in Fiber instead of hardcoding all encoders in app.config. This would solve dependency management issues by allowing users to opt-in only to the formats they need, keeping JSON and XML as defaults while making CBOR and MsgPack explicit registrations.
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#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: sixcolors
PR: gofiber/fiber#3446
File: docs/middleware/logger.md:44-44
Timestamp: 2025-05-13T00:19:16.407Z
Learning: In documentation files for the Fiber framework, code examples are often partial and don't repeat import statements that were shown in earlier examples, focusing instead on demonstrating specific usage patterns.
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`.
📚 Learning: in the session middleware `config` struct, the `extractor` field uses function closures (like `fromc...
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.
Applied to files:
middleware/keyauth/config.gomiddleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mdmiddleware/encryptcookie/encryptcookie.godocs/guide/extractor.mdextractor/extractor.godocs/whats_new.mddocs/middleware/keyauth.mdmiddleware/csrf/extractors.gomiddleware/session/extractors.gomiddleware/keyauth/keyauth.go
📚 Learning: the `keylookup` field in the `keyauth` middleware does not support pipe-delimited keys. instead, it ...
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.
Applied to files:
middleware/keyauth/config.gomiddleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mddocs/whats_new.mddocs/middleware/keyauth.mdmiddleware/keyauth/keyauth.go
📚 Learning: in the `middleware/adaptor/adaptor.go` file of the fiber framework, when updating context handling, ...
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`.
Applied to files:
middleware/keyauth/config.gomiddleware/keyauth/keyauth_test.goextractor/extractor.godocs/middleware/keyauth.mdmiddleware/session/extractors.gomiddleware/keyauth/keyauth.go
📚 Learning: in the fiber framework tests, using `ctx.response.header.cookie` may not be suitable for parsing coo...
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.
Applied to files:
middleware/keyauth/config.gomiddleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mdmiddleware/encryptcookie/encryptcookie.godocs/guide/extractor.mdextractor/extractor.godocs/middleware/keyauth.mdmiddleware/session/extractors.gomiddleware/keyauth/keyauth.go
📚 Learning: the `encryptcookie_test.go` file contains unit tests that validate key lengths for both `encryptcook...
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Applied to files:
middleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mdmiddleware/encryptcookie/encryptcookie.godocs/whats_new.md
📚 Learning: unit tests for key length enforcement in both `encryptcookie` and `decryptcookie` functions have bee...
Learnt from: gaby
PR: gofiber/fiber#3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
middleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mdmiddleware/encryptcookie/encryptcookie.godocs/whats_new.md
📚 Learning: unit tests for key length enforcement in `decryptcookie` have been added to ensure consistency and s...
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.
Applied to files:
middleware/keyauth/keyauth_test.godocs/middleware/encryptcookie.mdmiddleware/encryptcookie/encryptcookie.godocs/whats_new.md
📚 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-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 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-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: in go test files, prefer using the `require` methods from the `testify` package for assertions inste...
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`.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: in the fiber framework, breaking changes are acceptable when moving from version 2 to version 3, inc...
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`.
Applied to files:
middleware/keyauth/keyauth_test.godocs/guide/extractor.mddocs/whats_new.mdmiddleware/session/extractors.go
📚 Learning: in the session middleware `config` struct, `store` is backed by `fiber.storage`; they are different ...
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.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: in the fiber framework, `context()` is being renamed to `requestctx()`, and `usercontext()` to `cont...
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.
Applied to files:
middleware/keyauth/keyauth_test.goextractor/extractor.gomiddleware/session/extractors.go
📚 Learning: in the `test_app_shutdownwithcontext` function in `app_test.go`, the `clientdone` channel is used to...
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.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: in fiber v3, the cookiesamesite constants use lowercase values: cookiesamesitelaxmode = "lax", cooki...
Learnt from: sixcolors
PR: gofiber/fiber#3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
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".
Applied to files:
docs/middleware/encryptcookie.mddocs/whats_new.md
📚 Learning: in the fasthttp library, the `cookie.copyto(src *cookie)` method copies the source cookie (parameter...
Learnt from: ReneWerner87
PR: gofiber/fiber#3564
File: client/cookiejar.go:128-130
Timestamp: 2025-07-17T11:39:38.265Z
Learning: In the fasthttp library, the `Cookie.CopyTo(src *Cookie)` method copies the source cookie (parameter) to the receiver cookie, so `nc.CopyTo(c)` copies cookie `c` into cookie `nc`.
Applied to files:
middleware/encryptcookie/encryptcookie.go
📚 Learning: detailed usage examples and explanations for new methods like `removeroute` and `removeroutebyname` ...
Learnt from: ckoch786
PR: gofiber/fiber#3230
File: docs/whats_new.md:944-951
Timestamp: 2024-12-15T19:56:45.935Z
Learning: Detailed usage examples and explanations for new methods like `RemoveRoute` and `RemoveRouteByName` are documented in `docs/api/app.md`, so it's unnecessary to duplicate them in `docs/whats_new.md`.
Applied to files:
docs/whats_new.md
📚 Learning: in documentation files for the fiber framework, code examples are often partial and don't repeat imp...
Learnt from: sixcolors
PR: gofiber/fiber#3446
File: docs/middleware/logger.md:44-44
Timestamp: 2025-05-13T00:19:16.407Z
Learning: In documentation files for the Fiber framework, code examples are often partial and don't repeat import statements that were shown in earlier examples, focusing instead on demonstrating specific usage patterns.
Applied to files:
docs/whats_new.md
📚 Learning: in the `middleware/session/session.go` file, the `savesession()` method returns either `nil` or an e...
Learnt from: sixcolors
PR: gofiber/fiber#3016
File: middleware/session/session.go:272-293
Timestamp: 2024-10-02T15:16:39.868Z
Learning: In the `middleware/session/session.go` file, the `saveSession()` method returns either `nil` or an error, so additional error wrapping in the `Save()` method is unnecessary.
Applied to files:
middleware/session/extractors.go
🧬 Code Graph Analysis (1)
middleware/keyauth/config.go (3)
middleware/session/extractors.go (2)
Extractor(37-42)FromHeader(119-126)middleware/keyauth/keyauth.go (1)
FromHeader(115-146)constants.go (1)
HeaderAuthorization(163-163)
🪛 GitHub Check: markdownlint
docs/whats_new.md
[failure] 1958-1958: Multiple consecutive blank lines
docs/whats_new.md:1958 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md012.md
🪛 GitHub Actions: markdownlint
docs/whats_new.md
[error] 1958-1958: markdownlint MD012/no-multiple-blanks: Multiple consecutive blank lines [Expected: 1; Actual: 2]
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: repeated
- GitHub Check: Compare
- GitHub Check: Analyse
🔇 Additional comments (42)
middleware/encryptcookie/encryptcookie.go (2)
21-33: LGTM! Improved cookie processing with better error handling.The refactoring to use
VisitAllCookiecallback method improves code clarity and consistency. The early return pattern for disabled cookies and immediate cookie deletion on decryption failure enhances both readability and security.
39-58: LGTM! Proper handling of response cookie encryption.The response cookie encryption logic correctly handles the complexity of preserving cookie attributes during encryption. Using a temporary
Cookieobject to retrieve full cookie data before encryption ensures all cookie properties (domain, path, expires, etc.) are maintained.The panic on encryption error is appropriate as it indicates a configuration issue that should be caught during development.
extractor/extractor.go (5)
1-7: LGTM! Clean package structure.The package declaration and imports are minimal and appropriate for the functionality provided.
9-14: Excellent abstraction design.The
Extractorstruct provides a clean interface for value extraction with proper metadata support. The function signaturefunc(fiber.Ctx) (string, error)is well-suited for HTTP request processing, and theChainfield enables composition patterns.
16-23: Comprehensive error handling.The error definitions provide good granularity for different extraction sources, enabling precise error handling and debugging. The naming follows Go conventions consistently.
25-93: Well-implemented helper functions with consistent patterns.All helper functions follow a consistent implementation pattern:
- Use appropriate Fiber context methods for each source
- Consistent empty string validation treating
""as missing values- Return source-specific errors for better debugging
- Properly set Key metadata
The use of
fiber.Query[string](c, param)on Line 85 is appropriate for type-safe query parameter extraction.
95-126: Robust Chain implementation with proper edge case handling.The Chain function correctly handles:
- Empty extractor list edge case (lines 97-104)
- Sequential extraction with early success return (lines 111-112)
- Error tracking and fallback logic (lines 114-121)
- Proper metadata preservation (lines 123-124)
The logic requiring both no error AND non-empty value for success is appropriate for this use case.
docs/middleware/encryptcookie.md (2)
70-70: Good clarification of key requirements.The updated description clearly specifies the required key lengths (16, 24, or 32 bytes) and their correspondence to AES encryption standards. This helps users choose appropriate key lengths for their security needs.
98-98: Correct migration to extractor pattern.The update from
KeyLookuptoExtractor: csrf.FromHeader(csrf.HeaderName)properly demonstrates the new extractor-based configuration approach, maintaining consistency with the framework's migration to function-based extraction.middleware/keyauth/config.go (4)
8-8: Good import alias choice.Using
intextractoras an alias avoids potential naming conflicts with local extractor functions while clearly indicating the source package.
29-31: Clean migration from string-based to function-based extraction.The replacement of
KeyLookupandCustomKeyLookupwith a singleExtractorfield simplifies the configuration while providing more flexibility through the function-based approach.
47-51: Maintains backward compatibility with new extractor pattern.The default configuration preserves the same behavior (
Authorizationheader withBearerscheme) while leveraging the new extractor abstraction. The use offiber.HeaderAuthorizationfollows Fiber's constants pattern.
64-69: Proper default configuration logic.The sequence correctly:
- Sets AuthScheme default first (lines 64-66)
- Creates default extractor using the configured AuthScheme (lines 67-69)
- Uses
Extract == nilas the appropriate test for uninitialized extractorThis maintains the relationship between
AuthSchemeand extractor behavior.docs/middleware/keyauth.md (4)
47-47: Consistent migration to extractor pattern in examples.The examples properly demonstrate the new
Extractorfield usingkeyauth.FromCookie("access_token"), replacing the deprecated string-basedKeyLookupapproach. This provides a clearer, more type-safe API.Also applies to: 125-125
221-221: Accurate configuration documentation.The config table correctly documents the new
Extractorfield with proper type information and describes the helper functions (keyauth.FromHeader,keyauth.FromCookie) that users should utilize.
234-234: Consistent with implementation.The default configuration documentation accurately reflects the implementation in
config.go, showingkeyauth.FromHeader(fiber.HeaderAuthorization, "Bearer")as the default extractor.
240-240: Appropriate section restructure and helper documentation.The rename from "CustomKeyLookup" to "Extractor Helpers" better reflects the new functionality. The helper function descriptions accurately document the new API while maintaining the same utility concepts (parsing string syntax, chaining multiple sources).
Also applies to: 244-245
docs/guide/extractor.md (3)
1-8: Clear and informative introduction.The documentation properly introduces the extractor package purpose and scope, noting its use both by middleware and for direct use by developers.
9-31: Accurate documentation of the API.The struct definition and helper function list accurately reflect the implementation in
extractor/extractor.go. The description of Chain behavior ("tries each extractor in order") correctly summarizes the implementation logic.
33-57: Excellent practical example.The example code effectively demonstrates:
- Creating individual extractors for different sources
- Combining extractors using
Chainfor fallback behavior- Integration in route handlers with proper error handling
- Realistic usage patterns that users can adapt
This provides clear guidance for practical implementation.
docs/whats_new.md (2)
1097-1097: LGTM - Clear security documentationThe documentation correctly specifies the key length requirements for AES encryption standards. This is important security information for users implementing the EncryptCookie middleware.
1130-1135: Good documentation of the extractor pattern migrationThe documentation clearly explains the transition from string-based
KeyLookupto the newExtractorfield pattern. The introduction of the sharedextractorpackage is well documented, showing the reusable helpers available.middleware/session/extractors.go (4)
7-7: Good import aliasing approachThe
intextractoralias clearly distinguishes between the shared extractor package and the local session extractor types, avoiding naming conflicts.
62-68: Excellent refactoring to shared extractor patternThe refactoring consistently delegates extraction logic to the shared
extractorpackage while preserving session-specific behavior through theSourcetypes. This eliminates code duplication and centralizes extraction logic.The Source type mapping is correct:
SourceCookieandSourceHeaderfor extractors that can set response valuesSourceOtherfor read-only extractors (params, forms, queries)Also applies to: 81-87, 100-106, 119-125, 138-144
171-191: Well-implemented Chain function refactoringThe refactoring effectively leverages the base package's
Chainfunctionality while adding session-specific enhancements:
- Converts local extractors to base extractors for delegation
- Adds empty value validation specific to session requirements
- Preserves the first extractor's
SourceandKeyfor response handling- Maintains the original extractors in the
Chainfield for metadataThe approach reduces complexity while maintaining full functionality.
1-193: Excellent architectural refactoringThis refactoring successfully centralizes extraction logic in the shared
extractorpackage while preserving all session-specific functionality. Key benefits:
- Eliminates code duplication - Common extraction patterns are now shared
- Maintains backward compatibility - All public APIs remain unchanged
- Preserves domain logic - Session-specific
Sourcetypes and response handling are intact- Improves maintainability - Single source of truth for extraction logic
This change aligns perfectly with the extractor pattern migration documented in the PR and represents a significant architectural improvement.
middleware/keyauth/keyauth_test.go (4)
70-74: LGTM! Proper error handling for extractor creation in tests.The inline function pattern effectively handles the error from
DefaultExtractorwhile keeping the test configuration concise.
146-146: LGTM! Correct test setup for panic conditions.Using an empty
intextractor.Extractor{}with nil Extract function properly tests the panic behavior when the extractor is not configured.Also applies to: 159-159
180-181: LGTM! Function name and error message updated correctly.The changes align with the refactoring from
DefaultKeyLookuptoDefaultExtractor.
197-198: LGTM! Proper error handling for MultipleKeySourceLookup.The error from
MultipleKeySourceLookupis correctly checked before using the extractor.Also applies to: 201-201
middleware/csrf/extractors.go (3)
78-84: LGTM! Clean delegation to base extractor.The refactoring correctly delegates extraction logic to the base extractor while preserving CSRF-specific metadata.
100-106: LGTM! Consistent refactoring pattern across all extractors.All extractor functions (FromForm, FromHeader, FromQuery) follow the same clean delegation pattern, maintaining consistency and preserving CSRF-specific metadata.
Also applies to: 122-128, 144-150
170-203: LGTM! Well-implemented Chain function with proper error handling.The Chain function correctly:
- Handles empty extractor list edge case
- Converts between CSRF and base extractors
- Adds proper error handling for empty token values
- Preserves metadata from the first extractor
middleware/keyauth/keyauth.go (9)
39-41: LGTM! Simplified extractor validation.The validation correctly checks for nil Extract function and sets a sensible default using the Authorization header.
51-51: LGTM! Direct extractor usage.The extraction logic correctly uses the configured extractor.
76-88: LGTM! Clean implementation of multiple key source lookup.The function correctly creates and chains multiple extractors with proper error handling.
91-93: LGTM! Convenient wrapper for Chain functionality.The wrapper function provides a clean API for keyauth users.
95-112: LGTM! Well-implemented DefaultExtractor with improved error handling.The function correctly parses key lookup strings and returns appropriate extractors with clear error messages.
115-146: LGTM! Robust header extraction with auth scheme support.The implementation provides comprehensive validation for header-based authentication with proper scheme handling and error reporting.
149-172: LGTM! Consistent error handling for query and form extractors.Both implementations correctly wrap the base extractors with standardized error handling.
175-186: LGTM! Custom param extraction with URL unescaping.The implementation correctly handles URL-encoded parameters with proper unescaping and validation.
189-199: LGTM! Consistent cookie extraction implementation.The implementation follows the established pattern with proper error handling.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 5f0d6e3 | Previous: b446888 | Ratio |
|---|---|---|---|
BenchmarkAppendMsgdata |
0.6241 ns/op 1602.41 MB/s 0 B/op 0 allocs/op |
0.3112 ns/op 3213.48 MB/s 0 B/op 0 allocs/op |
2.01 |
BenchmarkAppendMsgdata - ns/op |
0.6241 ns/op |
0.3112 ns/op |
2.01 |
This comment was automatically generated by workflow using github-action-benchmark.
…uthentication handling
… in authentication handling
sixcolors
left a comment
sixcolors
left a comment
There was a problem hiding this comment.
Decision: No Shared Extractor Utilities
After review, this PR should not introduce shared extractor utilities across middleware packages. Each middleware should maintain its own extractor logic and error handling to ensure clearer separation, better domain-specific error reporting, and a more flexible API surface for future changes.
I’ll begin committing the necessary changes. If anyone has concerns or prefers a different approach, please comment here!
…uth and session middleware - Deleted the extractor package and its associated documentation. - Updated keyauth middleware to include built-in extractors with improved error handling and metadata. - Refactored session middleware to use custom extractor functions instead of the deleted extractor package. - Enhanced tests for keyauth extractors to cover missing and successful extraction scenarios. - Updated documentation to reflect changes in keyauth middleware and the removal of the extractor package.
…mproved consistency
…ng and default error responses
… handling in keyauth middleware tests
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3651 +/- ##
==========================================
+ Coverage 91.91% 92.01% +0.10%
==========================================
Files 113 114 +1
Lines 11496 11494 -2
==========================================
+ Hits 10566 10576 +10
+ Misses 667 659 -8
+ Partials 263 259 -4
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.
Pull Request Overview
This PR modernizes the KeyAuth middleware by replacing the legacy string-based configuration (KeyLookup, AuthScheme, CustomKeyLookup) with a type-safe, extractor-based system. This improves clarity, composability, and consistency while maintaining proper error handling and authentication flows.
Key changes:
- Replaces string-based configuration with explicit
Extractorfunctions for API key retrieval - Adds new extractor utilities (
FromHeader,FromAuthHeader,FromCookie, etc.) and chaining capabilities - Updates error handling to use new error types and consistent messaging
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| middleware/keyauth/keyauth.go | Core middleware refactored to use the new extractor system and simplified WWW-Authenticate header handling |
| middleware/keyauth/extractors.go | New file with extractor utilities and composable API key extraction functions |
| middleware/keyauth/config.go | Updated configuration to use Extractor field instead of string-based lookups |
| middleware/keyauth/keyauth_test.go | Comprehensive test refactoring to use new extractor functions and updated error messages |
| middleware/keyauth/extractors_test.go | New test file for extractor functionality and chaining |
| middleware/keyauth/config_test.go | New test file for configuration validation and defaults |
| docs/middleware/keyauth.md | Updated documentation with new extractor examples and migration guide |
| docs/middleware/encryptcookie.md | Minor documentation update to reflect proper key size constraints |
| docs/whats_new.md | Added migration guide and feature description for the new extractor system |
| app.Use(New(Config{ | ||
| Next: func(c fiber.Ctx) bool { | ||
| return c.OriginalURL() != "/auth1" | ||
| return c.Path() != "/auth1" |
There was a problem hiding this comment.
Using c.Path() instead of c.OriginalURL() changes the behavior of the middleware. c.Path() returns only the path portion without query parameters, while c.OriginalURL() includes the full URL with query parameters. This could break functionality if the original test relied on checking query parameters.
| return c.Path() != "/auth1" | |
| u, err := url.Parse(c.OriginalURL()) | |
| if err != nil { | |
| // fallback: skip auth if cannot parse | |
| return true | |
| } | |
| return u.Path != "/auth1" |
| app.Use(New(Config{ | ||
| Next: func(c fiber.Ctx) bool { | ||
| return c.OriginalURL() != "/auth2" | ||
| return c.Path() != "/auth2" |
There was a problem hiding this comment.
Same issue as above - using c.Path() instead of c.OriginalURL() changes the behavior and could break functionality that depends on checking the full URL including query parameters.
| return c.Path() != "/auth2" | |
| return c.OriginalURL() != "/auth2" |
| require.NoError(t, err, test.description) | ||
|
|
||
| // test the body of the request | ||
| body, err := io.ReadAll(res.Body) |
There was a problem hiding this comment.
The variable body is declared but never used in the scope where it's needed for assertions. The variable should be declared before the conditional block and used in the assertions.
| func FromQuery(param string) Extractor { | ||
| return Extractor{ | ||
| Extract: func(c fiber.Ctx) (string, error) { | ||
| apiKey := fiber.Query[string](c, param) |
There was a problem hiding this comment.
[nitpick] Using fiber.Query[string](c, param) is inconsistent with other extractors that use methods like c.Get(), c.Cookies(), etc. Consider using c.Query(param) for consistency with the Fiber API patterns used elsewhere.
| apiKey := fiber.Query[string](c, param) | |
| apiKey := c.Query(param) |
| expectedBody = "Cannot GET /" | ||
| } | ||
| require.Equal(t, expectedCode, res.StatusCode, test.description) | ||
| require.Equal(t, expectedBody, string(body), test.description) |
There was a problem hiding this comment.
The variable body is used here but was declared inside a conditional block above, making it unavailable in this scope. This will cause a compilation error.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring of the keyauth middleware, replacing the string-based KeyLookup with a more robust and flexible Extractor pattern. The changes improve type safety, composability, and overall developer experience. The tests have been thoroughly updated and new ones added to cover the new functionality. My review includes a suggestion to enhance the default error handler to provide more specific error messages from the new extractors, and a corresponding documentation update to ensure examples are accurate.
|
@ReneWerner87, @gaby, @efectn - Error Values: The error returned for a missing key is always “Invalid or expired API Key” unless it’s a specific missing or malformed error. Now that we have |
|
Yes pls |
|
👍 yes for me too |
…r/fiber into codex/2025-08-01-14-16-14
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
middleware/keyauth/config.go (1)
44-64: LGTM!The default error handler now provides specific error messages for different extraction sources, which improves debugging experience as intended by the PR objectives.
middleware/keyauth/extractors.go (1)
178-199: Consider using c.Query for consistencyThe
FromQueryextractor usesfiber.Query[string](c, param)while other extractors use direct methods on the context (e.g.,c.Get(),c.Cookies()). Consider usingc.Query(param)for consistency with the Fiber API patterns.func FromQuery(param string) Extractor { return Extractor{ Extract: func(c fiber.Ctx) (string, error) { - apiKey := fiber.Query[string](c, param) + apiKey := c.Query(param) if apiKey == "" { return "", ErrMissingAPIKeyInQuery } return apiKey, nil }, Key: param, Source: SourceQuery, } }middleware/keyauth/keyauth_test.go (1)
276-276: Path comparison change is appropriate for the test scenario.Using
c.Path()instead ofc.OriginalURL()is correct here since the tests are only checking route paths without query parameters. This simplifies the comparison logic without losing test coverage.Also applies to: 290-290
🧹 Nitpick comments (2)
middleware/keyauth/config.go (1)
66-77: Consider providing a more helpful error message for missing configThe panic message at line 70 could be misleading when
New()is called without any config. The issue is not just a missing validator, but a missing config altogether.func configDefault(config ...Config) Config { // Return default config if nothing provided if len(config) < 1 { - panic("fiber: keyauth middleware requires a validator function") + panic("fiber: keyauth middleware requires a config with a validator function") }middleware/keyauth/extractors_test.go (1)
66-122: Consider improving test cleanup with deferMultiple
defer app.ReleaseCtx(ctx)statements are scattered throughout the test. Consider grouping related tests to minimize context acquisition/release overhead.While the current approach works, you could improve readability by grouping the tests that share the same context pattern:
// FromForm ctx := app.AcquireCtx(&fasthttp.RequestCtx{}) -defer app.ReleaseCtx(ctx) ctx.Request().Header.SetContentType(fiber.MIMEApplicationForm) ctx.Request().SetBodyString("api_key=token_from_form") token, err := FromForm("api_key").Extract(ctx) require.NoError(t, err) require.Equal(t, "token_from_form", token) +app.ReleaseCtx(ctx) // FromQuery ctx = app.AcquireCtx(&fasthttp.RequestCtx{}) -defer app.ReleaseCtx(ctx) ctx.Request().SetRequestURI("/?api_key=token_from_query") token, err = FromQuery("api_key").Extract(ctx) require.NoError(t, err) require.Equal(t, "token_from_query", token) +app.ReleaseCtx(ctx)This makes it clearer that each test case is independent and releases its context immediately after use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/middleware/keyauth.md(8 hunks)docs/whats_new.md(3 hunks)middleware/keyauth/config.go(2 hunks)middleware/keyauth/config_test.go(1 hunks)middleware/keyauth/extractors.go(1 hunks)middleware/keyauth/extractors_test.go(1 hunks)middleware/keyauth/keyauth.go(3 hunks)middleware/keyauth/keyauth_test.go(21 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/whats_new.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/middleware/keyauth.md
🧠 Learnings (6)
📓 Common learnings
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-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-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.
📚 Learning: 2025-07-27T17:28:53.403Z
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.
Applied to files:
middleware/keyauth/extractors.gomiddleware/keyauth/keyauth_test.gomiddleware/keyauth/keyauth.godocs/middleware/keyauth.mdmiddleware/keyauth/config.go
📚 Learning: 2024-07-26T21:00:12.902Z
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.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
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.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: 2024-12-13T08:14:22.851Z
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`.
Applied to files:
middleware/keyauth/keyauth_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
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.
Applied to files:
docs/middleware/keyauth.md
🧬 Code Graph Analysis (6)
middleware/keyauth/config_test.go (3)
middleware/keyauth/keyauth.go (1)
New(23-55)middleware/keyauth/config.go (2)
Config(10-42)ConfigDefault(45-64)middleware/keyauth/extractors.go (2)
Extractor(42-48)FromHeader(164-176)
middleware/keyauth/extractors_test.go (1)
middleware/keyauth/extractors.go (16)
FromParam(118-130)ErrMissingAPIKeyInParam(54-54)FromForm(141-153)ErrMissingAPIKeyInForm(55-55)FromQuery(187-199)ErrMissingAPIKeyInQuery(53-53)FromHeader(164-176)ErrMissingAPIKeyInHeader(52-52)FromAuthHeader(61-84)FromCookie(95-107)ErrMissingAPIKeyInCookie(56-56)Chain(211-252)ErrMissingAPIKey(51-51)Extractor(42-48)Source(12-12)SourceCustom(38-38)
middleware/keyauth/extractors.go (3)
middleware/csrf/extractors.go (11)
Source(11-11)Chain(187-228)Extractor(45-50)SourceCookie(37-37)FromParam(76-88)FromForm(103-115)FromHeader(130-142)SourceHeader(16-16)FromQuery(157-169)SourceQuery(24-24)SourceCustom(41-41)middleware/session/extractors.go (10)
Source(14-14)Chain(183-224)Extractor(36-41)FromCookie(61-73)SourceCookie(20-20)FromParam(85-97)FromForm(109-121)FromHeader(133-145)SourceHeader(25-25)FromQuery(157-169)middleware/keyauth/keyauth.go (1)
New(23-55)
middleware/keyauth/keyauth_test.go (3)
middleware/keyauth/keyauth.go (1)
New(23-55)middleware/keyauth/config.go (1)
Config(10-42)middleware/keyauth/extractors.go (4)
Extractor(42-48)FromHeader(164-176)FromAuthHeader(61-84)ErrMissingAPIKeyInHeader(52-52)
middleware/keyauth/keyauth.go (3)
middleware/keyauth/config.go (1)
Config(10-42)middleware/keyauth/extractors.go (4)
Extractor(42-48)Source(12-12)SourceAuthHeader(20-20)Chain(211-252)constants.go (1)
HeaderWWWAuthenticate(166-166)
middleware/keyauth/config.go (3)
middleware/keyauth/extractors.go (4)
Extractor(42-48)ErrMissingAPIKey(51-51)ErrMissingAPIKeyInHeader(52-52)FromAuthHeader(61-84)middleware/keyauth/keyauth.go (1)
ErrMissingOrMalformedAPIKey(20-20)constants.go (2)
StatusUnauthorized(74-74)HeaderAuthorization(163-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (37)
middleware/keyauth/config_test.go (5)
1-11: LGTM!The package declaration and imports are appropriate for the test file.
12-20: LGTM!The test correctly validates that
New()panics with the expected message when called without any configuration.
22-28: LGTM!The test correctly validates that
configDefaultpanics whenValidatoris nil.
30-43: LGTM!The test thoroughly validates that default values are correctly applied when only a validator is provided.
45-71: LGTM!The test comprehensively validates that custom configuration values are preserved through
configDefault. Usingreflect.ValueOfto compare function pointers is the appropriate approach for verifying function preservation.middleware/keyauth/config.go (2)
9-42: LGTM!The refactored
Configstruct clearly defines the new extractor-based API with appropriate documentation for each field.
79-94: LGTM!The default value assignment logic is clean and correctly handles the new extractor-based configuration.
middleware/keyauth/extractors_test.go (4)
1-12: LGTM!The package declaration and imports are appropriate for the test file.
14-55: LGTM!The test comprehensively validates that each extractor returns the correct error when the API key is missing from its respective source.
57-64: LGTM!The helper function is well-implemented with proper error handling via panic for test setup issues.
124-175: LGTM!The test thoroughly validates the chain extractor behavior including edge cases like empty chains and extractors returning empty strings with nil errors.
docs/middleware/keyauth.md (6)
18-61: LGTM!The basic example clearly demonstrates the new extractor-based API with the
FromCookieextractor.
81-165: LGTM!The example clearly demonstrates how to use the
Nextfunction for selective endpoint authentication.
167-222: LGTM!The example effectively shows how to apply middleware to specific routes using the default
FromAuthHeaderextractor.
224-254: LGTM!The new sections on Key Extractors and Chaining Extractors provide clear documentation of the new extractor-based API.
256-290: LGTM!The configuration documentation and default config are well-documented and accurately reflect the implementation.
63-77: Error message consistency verifiedThe
ErrMissingAPIKeyInCookiedefinition inmiddleware/keyauth/extractors.gouses the message"missing api key in cookie", which exactly matches the documentation example. No further changes needed.middleware/keyauth/extractors.go (10)
1-9: LGTM!Package declaration and imports are appropriate.
10-39: LGTM!The
Sourcetype and constants are well-documented with security considerations noted for URL-based extraction methods.
41-48: LGTM!The
Extractorstruct is well-designed with clear field documentation.
50-57: LGTM!The error variables are well-defined and provide specific error messages for each extraction source.
59-84: LGTM!The
FromAuthHeaderfunction correctly handles authentication schemes with proper case-insensitive comparison and validation.
86-107: LGTM!The
FromCookieextractor is well-implemented with comprehensive documentation.
109-130: LGTM!The
FromParamextractor is correctly implemented with appropriate documentation.
132-153: LGTM!The
FromFormextractor is properly implemented with clear documentation.
155-176: LGTM!The
FromHeaderextractor is correctly implemented with appropriate documentation.
201-252: LGTM!The
Chainfunction is well-implemented with proper error handling and comprehensive documentation. The logic correctly returns the first successful extraction or the last error encountered.middleware/keyauth/keyauth.go (5)
16-16: LGTM!Good use of
iotafor the context key constant, which is a Go best practice for defining sequential constants.
27-28: Good architectural choice for WWW-Authenticate support.The approach of determining the auth scheme from the extractor at initialization time is efficient and clean. This enables the middleware to properly set the WWW-Authenticate header when authentication fails.
38-53: Consider returning extractor-specific errors to clients as agreed.Based on the PR discussion, maintainers agreed to return extractor-specific errors instead of a generic error message. The current implementation returns the original error from the extractor, which aligns with the maintainers' request.
The error flow correctly handles both extraction and validation errors, maintaining proper authentication semantics.
49-51: Proper WWW-Authenticate header implementation.The WWW-Authenticate header is correctly set with the auth scheme and realm when authentication fails, following RFC 7235 standards. This provides proper authentication challenge responses to clients.
67-79: Well-designed helper function for auth scheme extraction.The
getAuthSchemefunction efficiently inspects both the primary extractor and its chain to find auth scheme information. This supports complex chained extractor configurations while maintaining clean separation of concerns.middleware/keyauth/keyauth_test.go (5)
24-31: Good use of constants for extractor names.Using named constants for extractor types improves code readability and maintainability across the test suite.
79-86: Pragmatic approach to handling special characters in tests.Using simplified keys for param and cookie extractors avoids URL encoding issues in test setup while still maintaining test coverage. This is a reasonable testing trade-off.
88-90: Good implementation of error handler for testing.The error handler correctly returns the actual error message from the extractor, which aligns with the maintainers' decision to expose extractor-specific errors to clients.
175-190: Comprehensive error message testing for each extractor type.The test correctly verifies that each extractor returns its specific error message when the API key is missing, ensuring proper error reporting for different authentication sources.
800-897: Excellent comprehensive test coverage for WWW-Authenticate header.The test thoroughly covers all WWW-Authenticate header scenarios including:
- Default and custom realms
- Different extractor types (auth header vs non-auth header)
- Chained extractors
- Success vs failure cases
This ensures the new WWW-Authenticate functionality works correctly across all configurations.
|
PR is a complete rewrite. Closing and will re-open new. |
Refactor KeyAuth Middleware: Extractor-Based Configuration and Enhanced Flexibility
This PR modernizes the
keyauthmiddleware by replacing the legacy string-based key extraction configuration (KeyLookup,AuthScheme,CustomKeyLookup) with a robust, type-safe extractor system. This update improves flexibility, composability, and clarity for users implementing API key authentication in Fiber.Key Changes
Extractor-Based API Key Retrieval:
Extractorfield for configuration, allowing explicit selection of where to extract the API key (header, auth header, query, param, cookie, form).FromHeader(header string)FromAuthHeader(header, scheme string)FromCookie(name string)FromQuery(param string)FromParam(param string)FromForm(name string)Chain(...)for chaining multiple extractors.Config Refactoring:
KeyLookup,AuthScheme, andCustomKeyLookup.Validatorrequired; panics if missing.WWW-Authenticate Header and Realm:
WWW-Authenticateheader on authentication failure when using an auth header extractor.Realmsupport.Comprehensive Tests and Documentation:
Migration Guide
Before:
After:
You can combine multiple sources:
Why?
Explicit extractor configuration avoids fragile string parsing and makes it easier to extend/customize key source logic.
Easily combine multiple sources for API key retrieval using
Chain.Consistent error messages and
WWW-Authenticatesupport.No ambiguous string formats; everything is explicit and documented.
Breaking Changes:
KeyLookup,AuthScheme, andCustomKeyLookuphave been removed.Extractorconfiguration with provided helper functions.See updated documentation and migration notes for further details.
Summary
extractorutilitieswhats_newTesting
go build ./...golangci-lint run ./...(fails: unsupported version of configuration)https://chatgpt.com/codex/tasks/task_e_688cbe2da4ec83268f6d3d6edc7ea367