Skip to content
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

fix(middleware/cors): Handling and wildcard subdomain matching #2915

Merged
merged 34 commits into from Mar 17, 2024

Conversation

sixcolors
Copy link
Member

@sixcolors sixcolors commented Mar 11, 2024

PR Description

Summary of Changes

This pull request enhances the CORS middleware in the Fiber web framework by introducing improved origin matching, subdomain support, and comprehensive documentation updates.

Changes Made

  1. Origin Matching Improvements:

    • Refactored the utils.validateDomain function to handle wildcard subdomain patterns more accurately.
    • Added tests to ensure correct validation of various origin patterns.
  2. Subdomain Support:

    • Modified the CORS middleware to support subdomain matching for the AllowOrigins configuration.
    • Added tests to verify the behavior of subdomain matching.
  3. Documentation Updates:

    • Added detailed documentation explaining how the CORS middleware works, including the handling of different AllowOrigins configurations.
    • Highlighted security considerations and best practices to avoid potential vulnerabilities.
    • Included information on handling misconfigurations related to wildcard origins and credentials.
    • Updated the documentation to reflect changes in the middleware's behavior.
  4. Improved CORS Handling:

    • Introduced the setCORSHeaders function to streamline the management of common behaviors in both simple and pre-flight requests.
    • Restructured the handler function to consistently apply CORS rules through the newly added setCORSHeaders function.

How It Works Section

  • Added a new section in the documentation explaining the functionality of the CORS middleware.
  • Described the process of adding CORS headers to responses based on the configuration.
  • Clarified the handling of preflight requests and the dynamic evaluation of allowed origins.

Security Considerations Section

  • Introduced a section emphasizing security considerations when configuring CORS.
  • Highlighted potential risks of allowing all origins, credentials exposure, and exposing sensitive headers.
  • Provided guidance on proper configurations to mitigate security threats.

Testing Updates

  • Expanded test coverage to include scenarios related to subdomains, origin validation, and wildcard patterns.
  • Added tests for improved coverage of the CORS middleware.

Notes for Reviewers

  • Please review the changes made to the utils.validateDomain function and ensure accuracy in handling wildcard subdomain patterns.
  • Verify the correctness of the subdomain matching behavior in the CORS middleware.
  • Verify the correctness of the middleware behavior when making simple requests.
  • Thoroughly review the documentation updates for clarity, completeness, and adherence to best practices.
  • This is targeting v2 branch, we will also need to cherry pick and merge into v3/main.

This pull request aims to enhance the functionality, security, and documentation of the CORS middleware in Fiber. Your feedback and suggestions are highly appreciated.

Commits

fix(middleware/cors): handling and wildcard subdomain matching
docs(middleware/cors): add 'How it works' and 'Security Considerations'
chore: grammar
chore: fix misspelling
test(middleware/cors): combine Invalid_Origins tests
refactor(middleware/cors): headers handling

Summary by CodeRabbit

  • New Features
    • Enhanced CORS middleware support including subdomain matching and dynamic origin evaluation.
  • Bug Fixes
    • Corrected Access-Control-Allow-Origin header handling based on AllowCredentials.
  • Refactor
    • Improved CORS middleware code organization with a new function for setting headers.
  • Tests
    • Added and adjusted tests for various CORS configurations and scenarios.
  • Documentation
    • Updated CORS middleware documentation with security considerations and configuration guidance.

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes #2882
docs(middleware/cors): add How it works and Security Considerations
Copy link
Contributor

coderabbitai bot commented Mar 11, 2024

Warning

Rate Limit Exceeded

@sixcolors has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 6 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between cc8c559 and 87e7cad.

Walkthrough

These updates enhance the CORS middleware by improving subdomain matching, refining method handling, and introducing dynamic origin evaluation. Emphasis on security is evident through caution against wildcard origins with credentials and the careful exposure of headers. Code organization benefits from the separation of concerns, specifically in setting CORS headers, while tests ensure robustness against invalid origins and verify correct header behavior.

Changes

Files Summary
docs/api/middleware/cors.md Enhanced AllowOrigins support, refined AllowMethods, added AllowOriginsFunc, and security considerations.
middleware/cors/cors.go Adjusted Access-Control-Allow-Origin handling, added warnings for unsafe configurations, and improved code organization with setCORSHeaders function.
middleware/cors/cors_test.go Extended tests for origin handling, including invalid patterns, subdomain matching, and header verification.
middleware/cors/utils.go Improved wildcard subdomain handling and added checks against wildcards with protocols.

"In the world of code, where changes abound, 🌍
A rabbit hopped in, with updates profound. 🐰
CORS now refined, with security tight,
Subdomains and methods, now perfectly right. 🛠️
Let's celebrate the code, so cleverly penned,
For a web that's more secure, in the end. 🎉"
~ @coderabbitai

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d2b19e2 and af930bd.
Files selected for processing (5)
  • docs/api/middleware/cors.md (2 hunks)
  • middleware/cors/cors.go (2 hunks)
  • middleware/cors/cors_test.go (6 hunks)
  • middleware/cors/utils.go (2 hunks)
  • middleware/cors/utils_test.go (1 hunks)
Additional comments: 10
middleware/cors/utils.go (2)
  • 27-33: The implementation for handling wildcard subdomain patterns in validateDomain looks correct. It properly checks if the domain matches the pattern by trimming the leading dot from the pattern and then checking if the domain ends with the trimmed pattern prefixed by a dot. This approach effectively supports wildcard subdomain matching.
  • 70-76: The addition of a check in normalizeOrigin to disallow wildcards with a protocol is a crucial security measure. It prevents the misuse of wildcards in origins, which could lead to security vulnerabilities. This change aligns with the objective of enhancing security in the CORS middleware.
middleware/cors/cors.go (1)
  • 206-209: The logic to handle the Access-Control-Allow-Origin header when AllowCredentials is true and AllowOrigins is set to '*' has been updated. This change correctly addresses the security concern of using a wildcard origin with credentials. However, it's important to ensure that this warning is visible to developers during the configuration phase to prevent misconfiguration.
Verification successful

The warning log regarding the insecure setup of 'AllowCredentials' being true while 'AllowOrigins' is set to a wildcard is effectively visible to developers during the configuration phase, as indicated by the presence of a panic in middleware/cors/cors.go. This ensures immediate awareness and prevention of misconfiguration related to CORS policies.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that the warning log is effectively visible to developers during configuration
rg --type go --context 5 "Insecure setup, 'AllowCredentials' is set to true, and 'AllowOrigins' is set to a wildcard."

Length of output: 844

middleware/cors/utils_test.go (1)
  • 94-101: The updates to the validateOrigin function's test cases improve the coverage for wildcard subdomain matching and TLD comparisons. These changes ensure that the behavior of origin validation is correctly tested, especially in scenarios involving wildcard subdomains and different TLDs. It's important to ensure that these test cases cover all relevant scenarios to maintain the robustness of the CORS middleware.
docs/api/middleware/cors.md (3)
  • 73-79: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [9-9]

In the context of parsing AllowOrigins, the term is correctly used as a variable name and should not be split into two words. However, the use of "matching" in "matching allowed origin" is correct as it is, indicating that the origin being checked matches one of the allowed origins. No change is necessary here.

  • 76-76: The documentation now specifies that AllowOrigins supports subdomain matching with a syntax like "https://.example.com". This is a significant enhancement for flexibility in CORS policy configuration. Ensure that examples or additional guidance on using this feature responsibly, considering the security implications, are provided to help developers avoid common pitfalls.
  • 131-151: The "Security Considerations" section is crucial for guiding developers in configuring CORS policies securely. It's well-explained, emphasizing the risks associated with allowing all origins, credentials, and exposing headers. However, it might be beneficial to include examples or more detailed explanations of how these configurations can lead to security vulnerabilities, providing developers with a clearer understanding of the risks and how to mitigate them.
middleware/cors/cors_test.go (3)
  • 195-231: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [228-242]

The updates to Test_CORS_Subdomain correctly implement and verify the enhanced subdomain matching functionality. The test cases for both allowed and disallowed origins are well-constructed and align with the PR's objectives.

  • 277-298: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [280-325]

The expanded test cases in Test_CORS_AllowOriginScheme provide comprehensive coverage of various scenarios, including different schemes, subdomains, and port numbers. These additions ensure the CORS middleware's enhanced functionality is thoroughly tested.

  • 367-393: The addition of Test_CORS_AllowOriginHeader_NoMatch correctly verifies the behavior of the CORS middleware when no allowed origins match the request's origin. This test enhances the middleware's test coverage and ensures its reliability.

docs/api/middleware/cors.md Outdated Show resolved Hide resolved
middleware/cors/cors_test.go Outdated Show resolved Hide resolved
sixcolors and others added 3 commits March 11, 2024 13:40
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between af930bd and 03e09c9.
Files selected for processing (1)
  • middleware/cors/cors_test.go (6 hunks)
Additional comments: 4
middleware/cors/cors_test.go (4)
  • 198-219: The addition of Test_CORS_Invalid_Origin_URL_Panic effectively tests for panic with an invalid origin URL pattern, ensuring robustness against misconfigurations. This aligns well with the PR's objectives to enhance security.
  • 195-231: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [228-242]

The update to Test_CORS_Subdomain with a more specific subdomain pattern in AllowOrigins is a good enhancement, testing the improved wildcard subdomain matching functionality.

Also applies to: 242-242

  • 280-280: The adjustments and additions to Test_CORS_AllowOriginScheme enhance the test coverage for different origin schemes and patterns, aligning with the PR's objectives to refine CORS policy handling.

Also applies to: 285-285, 290-290, 295-295, 305-305, 310-310, 320-320, 325-325

  • 367-393: The addition of Test_CORS_AllowOriginHeader_NoMatch effectively tests the correct behavior when no origin match is found, ensuring the CORS middleware enforces policies securely.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 03e09c9 and b792951.
Files selected for processing (3)
  • docs/api/middleware/cors.md (3 hunks)
  • middleware/cors/cors.go (1 hunks)
  • middleware/cors/cors_test.go (8 hunks)
Files skipped from review as they are similar to previous changes (1)
  • middleware/cors/cors.go
Additional comments: 7
docs/api/middleware/cors.md (3)
  • 76-76: The documentation now includes support for subdomain matching in AllowOrigins. This is a significant enhancement for flexibility and security in CORS configuration. However, it's crucial to ensure that examples or additional guidance on how to properly format and use subdomain patterns are clear to the users to avoid misconfiguration.

Consider adding examples or more detailed guidance on formatting and using subdomain patterns in AllowOrigins to prevent misconfiguration.

  • 76-76: The documentation mentions the introduction of AllowOriginsFunc for dynamic origin evaluation. This is a powerful feature that allows for runtime determination of allowed origins. However, it's essential to emphasize the security considerations when using this function, especially in production environments.

Highlight the security considerations and best practices when using AllowOriginsFunc, especially the importance of validating origins to prevent security risks.

  • 131-151: The "Security Considerations" section is a valuable addition, emphasizing the importance of careful CORS configuration to avoid security pitfalls. It's well-explained and covers critical aspects like avoiding wildcard origins with credentials and being cautious with exposed headers. This section helps developers understand the potential security risks and how to mitigate them.

The "Security Considerations" section provides essential guidance on avoiding common security pitfalls in CORS configuration. Great addition!

middleware/cors/cors_test.go (4)
  • 175-207: The addition of Test_CORS_Invalid_Origins_Panic is a valuable enhancement to the test suite, ensuring that the middleware correctly handles invalid origin configurations by panicking as expected. This aligns with the PR's objective of enhancing testing around CORS functionality. However, consider adding a comment above the test function explaining the purpose and the expected behavior in more detail for future maintainability.
  • 217-217: The modification of the AllowOrigins configuration in Test_CORS_Subdomain to "http://.example.com" is a crucial update that tests the enhanced subdomain matching functionality. This change effectively tests the middleware's ability to handle requests from subdomains of example.com, aligning with the PR's objective of improving wildcard subdomain matching. It's well-implemented and directly addresses the intended enhancement.
  • 266-287: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [269-314]

The adjustments made to origin patterns in Test_CORS_AllowOriginScheme are well thought out and cover a wide range of scenarios, including different schemes, subdomains, and port numbers. These changes are essential for thoroughly testing the middleware's ability to handle various origin configurations, especially in light of the enhancements made to subdomain matching and scheme handling. This comprehensive approach ensures that the middleware behaves as expected across different use cases, contributing to the overall reliability and security of the CORS functionality.

  • 356-382: The introduction of Test_CORS_AllowOriginHeader_NoMatch is a significant addition to the test suite, specifically targeting the verification of correct header handling when no origin match is found. This test ensures that the Access-Control-Allow-Origin header is not set in response to requests from disallowed origins, which is crucial for maintaining the security and correctness of the CORS policy enforcement. The test setup and validation logic are correctly implemented, effectively addressing one of the PR's objectives related to adjustments in header handling.

@sixcolors
Copy link
Member Author

sixcolors commented Mar 11, 2024

@gaby gaby added the v2 label Mar 12, 2024
@gaby gaby changed the title fix(middleware/cors): handling and wildcard subdomain matching fix(middleware/cors): Handling and wildcard subdomain matching Mar 12, 2024
docs/api/middleware/cors.md Outdated Show resolved Hide resolved
middleware/cors/utils_test.go Outdated Show resolved Hide resolved
@gofiber gofiber deleted a comment from coderabbitai bot Mar 12, 2024
@gofiber gofiber deleted a comment from coderabbitai bot Mar 12, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f12d083 and 79d46e0.
Files selected for processing (1)
  • docs/api/middleware/cors.md (4 hunks)
Additional comments: 8
docs/api/middleware/cors.md (8)
  • 7-7: The introduction clearly explains the purpose of CORS and its role in web security. It's important to emphasize that CORS is not a security feature but a way to relax the same-origin policy for legitimate cross-origin requests.
  • 9-9: The explanation of how the middleware processes the AllowOrigins configuration is clear and concise. It correctly outlines the behavior when a match is found or not, which is crucial for understanding how CORS policies are enforced.
  • 11-11: The documentation highlights the importance of providing correctly formatted origins and the automatic handling of schemes and trailing slashes. This is a valuable detail for users configuring the middleware.
  • 39-39: The example provided for customizing the CORS configuration is straightforward and demonstrates how to set allowed origins and headers. This will help users quickly understand how to customize their CORS policy.
  • 57-57: The introduction of AllowOriginsFunc for dynamic origin validation is a significant enhancement. This allows for more flexible and secure configurations by enabling runtime decisions on whether to allow specific origins.
  • 58-58: The documentation now explicitly mentions support for subdomain matching in AllowOrigins, which is a key feature of this update. The explanation is clear, but it could benefit from a brief mention of the security considerations when using wildcard subdomains.

Consider adding a note on the security implications of using wildcard subdomains, especially in relation to credential handling and potential exposure to less secure subdomains.

  • 87-99: The "Subdomain Matching" section provides a clear example of how to configure AllowOrigins to allow requests from any subdomain of example.com. This practical example enhances the documentation by showing users exactly how to use this new feature.
  • 127-151: The "Security Considerations" section is an excellent addition, offering secure configurations and highlighting common pitfalls. This guidance is crucial for helping users configure CORS in a way that balances functionality with security.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 10

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 79d46e0 and 6860130.
Files selected for processing (1)
  • docs/api/middleware/cors.md (4 hunks)
Additional comments: 1
docs/api/middleware/cors.md (1)
  • 9-9: Ensure the term "matching" is used correctly. It might be more appropriate to use "matching-allowed" if it's part of a compound adjective describing "origin".

Please verify the usage of "matching" in this context to ensure clarity and accuracy.

docs/api/middleware/cors.md Outdated Show resolved Hide resolved
docs/api/middleware/cors.md Outdated Show resolved Hide resolved
docs/api/middleware/cors.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 7

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 6860130 and 2e57b0b.
Files selected for processing (1)
  • docs/api/middleware/cors.md (4 hunks)
Additional comments: 1
docs/api/middleware/cors.md (1)
  • 11-11: The term AllowOrigins is correctly used in the context of CORS configuration. The suggested replacement "Allow Origins" is not applicable as it breaks the specific configuration property naming convention.

docs/api/middleware/cors.md Outdated Show resolved Hide resolved
docs/api/middleware/cors.md Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 8

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 2e57b0b and d220abc.
Files selected for processing (1)
  • docs/api/middleware/cors.md (4 hunks)
Additional comments: 1
docs/api/middleware/cors.md (1)
  • 11-11: The term AllowOrigins is correctly used in the context of CORS configuration and does not represent a spelling mistake.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 8

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d220abc and cc8c559.
Files selected for processing (1)
  • docs/api/middleware/cors.md (4 hunks)
Additional comments: 1
docs/api/middleware/cors.md (1)
  • 11-11: The term AllowOrigins is correctly used in the context of CORS configuration. However, the static analysis tool flagged it as a possible spelling mistake. This is a false positive.

docs/api/middleware/cors.md Show resolved Hide resolved
docs/api/middleware/cors.md Show resolved Hide resolved
docs/api/middleware/cors.md Show resolved Hide resolved
docs/api/middleware/cors.md Outdated Show resolved Hide resolved
docs/api/middleware/cors.md Show resolved Hide resolved
docs/api/middleware/cors.md Show resolved Hide resolved
docs/api/middleware/cors.md Show resolved Hide resolved
@sixcolors
Copy link
Member Author

@jub0bs I would appreciate your review as I had gone back to see why some decisions about this middleware were made and #2390 explained some decisions I was questioning.

I think this PR improves the situation while being backward compatible within the documented api. I do plan a complete rewrite for v3.

Thanks!

@sixcolors sixcolors requested a review from gaby March 16, 2024 15:48
@sixcolors sixcolors added this to the Next Release milestone Mar 16, 2024
Copy link
Member

@gaby gaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gaby
Copy link
Member

gaby commented Mar 17, 2024

@sixcolors Should we add the changes from #2908 in this PR?

I'm going to update that PR to rename the fields first.

@ReneWerner87
Copy link
Member

I'm going to update that PR to rename the fields first.

no new features for v2 , only bugfixes and security-related changes

@ReneWerner87 ReneWerner87 merged commit 1aac6f6 into v2 Mar 17, 2024
19 of 20 checks passed
sixcolors added a commit to sixcolors/fiber that referenced this pull request Mar 17, 2024
…er#2915)

* fix: allow origins check

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes gofiber#2882

* test: AllowOrigins with whitespace

* test(middleware/cors): add benchmarks

* chore: fix linter errors

* test(middleware/cors): use h() instead of app.Test()

* test(middleware/cors): add miltiple origins in Test_CORS_AllowOriginScheme

* chore: refactor validate and normalize

* test(cors/middleware): add more benchmarks

* fix(middleware/cors): handling and wildcard subdomain matching

docs(middleware/cors): add How it works and Security Considerations

* chore: grammar

* Apply suggestions from code review

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: fix misspelling

* test(middleware/cors): combine Invalid_Origins tests

* refactor(middleware/cors): headers handling

* docs(middleware/cors): Update AllowOrigins description

* chore: merge

* perf(middleware/cors): optimize handler

* perf(middleware/cors): optimize handler

* chore(middleware/cors): ipdate origin handling logic

* chore(middleware/cors): fix header capitalization

* docs(middleware/cors): improve sercuity notes

* docs(middleware/cors): Improve security notes

* docs(middleware/cors): improve CORS overview

* docs(middleware/cors): fix ordering of how it works

* docs(middleware/cors): add additional info to How to works

* docs(middleware/cors): rm space

* docs(middleware/cors): add validation for AllowOrigins origins to overview

* docs(middleware/cors): update ExposeHeaders and MaxAge descriptions

* docs(middleware/cors): Add dynamic origin validation example

* docs(middleware/cors): Improve security notes and fix header capitalization

* docs(middleware/cors): configuration examples

* docs(middleware/cors): `"*"`

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
ReneWerner87 pushed a commit that referenced this pull request Mar 19, 2024
* fix(middleware/cors): Handling and wildcard subdomain matching (#2915)

* fix: allow origins check

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes #2882

* test: AllowOrigins with whitespace

* test(middleware/cors): add benchmarks

* chore: fix linter errors

* test(middleware/cors): use h() instead of app.Test()

* test(middleware/cors): add miltiple origins in Test_CORS_AllowOriginScheme

* chore: refactor validate and normalize

* test(cors/middleware): add more benchmarks

* fix(middleware/cors): handling and wildcard subdomain matching

docs(middleware/cors): add How it works and Security Considerations

* chore: grammar

* Apply suggestions from code review

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: fix misspelling

* test(middleware/cors): combine Invalid_Origins tests

* refactor(middleware/cors): headers handling

* docs(middleware/cors): Update AllowOrigins description

* chore: merge

* perf(middleware/cors): optimize handler

* perf(middleware/cors): optimize handler

* chore(middleware/cors): ipdate origin handling logic

* chore(middleware/cors): fix header capitalization

* docs(middleware/cors): improve sercuity notes

* docs(middleware/cors): Improve security notes

* docs(middleware/cors): improve CORS overview

* docs(middleware/cors): fix ordering of how it works

* docs(middleware/cors): add additional info to How to works

* docs(middleware/cors): rm space

* docs(middleware/cors): add validation for AllowOrigins origins to overview

* docs(middleware/cors): update ExposeHeaders and MaxAge descriptions

* docs(middleware/cors): Add dynamic origin validation example

* docs(middleware/cors): Improve security notes and fix header capitalization

* docs(middleware/cors): configuration examples

* docs(middleware/cors): `"*"`

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* test(middleware/cors): improve test coverage for request types

* chore(middleware/cors): fix v2 merge issues

* test(middleware/cors): Add subdomain matching tests

* fix(middleware/cors): Update Next function signature

* test(middleware/cors): Add benchmark for CORS subdomain matching

* test(middleware/cors): cover additiona test cases

* refactor(middleware/cors): origin validation and normalization

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sixcolors sixcolors deleted the fix-cors-allow-origins branch March 27, 2024 01:15
ReneWerner87 added a commit that referenced this pull request Mar 28, 2024
* Update pull_request_template.md

* Update v3-changes.md

* Update CONTRIBUTING.md (#2752)

Grammar correction.

* chore(encryptcookie)!: update default config (#2753)

* chore(encryptcookie)!: update default config

docs(encryptcookie): enhance documentation and examples

BREAKING CHANGE: removed the hardcoded "csrf_" from the Except.

* docs(encryptcookie): reads or modifies cookies

* chore(encryptcookie): csrf config example

* docs(encryptcookie): md table spacing

* build(deps): bump actions/setup-go from 4 to 5 (#2754)

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4 to 5.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v4...v5)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* 🩹 middleware/logger/: log client IP address by default (#2755)

* middleware/logger: Log client IP address by default.

* Update doc.

* fix: don't constrain middlewares' context-keys to strings 🐛 (#2751)

* Revert "Revert ":bug: requestid.Config.ContextKey is interface{} (#2369)" (#2742)"

This reverts commit 28be17f.

* fix: request ContextKey default value condition

Should check for `nil` since it is `any`.

* fix: don't constrain middlewares' context-keys to strings

`context` recommends using "unexported type" as context keys to avoid
collisions https://pkg.go.dev/github.com/gofiber/fiber/v2#Ctx.Locals.

The official go blog also recommends this https://go.dev/blog/context.

`fiber.Ctx.Locals(key any, value any)` correctly allows consumers to
use unexported types or e.g. strings.

But some fiber middlewares constrain their context-keys to `string` in
their "default config structs", making it impossible to use unexported
types.

This PR removes the `string` _constraint_ from all middlewares, allowing
to now use unexported types as per the official guidelines. However
the default value is still a string, so it's not a breaking change, and
anyone still using strings as context keys is not affected.

* 📚 Update app.md for indentation (#2761)

Update app.md for indentation

* build(deps): bump github.com/google/uuid from 1.4.0 to 1.5.0 (#2762)

Bumps [github.com/google/uuid](https://github.com/google/uuid) from 1.4.0 to 1.5.0.
- [Release notes](https://github.com/google/uuid/releases)
- [Changelog](https://github.com/google/uuid/blob/master/CHANGELOG.md)
- [Commits](google/uuid@v1.4.0...v1.5.0)

---
updated-dependencies:
- dependency-name: github.com/google/uuid
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump github/codeql-action from 2 to 3 (#2763)

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2 to 3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@v2...v3)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Changing default log output (#2730)

changing default log output

Closes #2729

* Update hooks.md

fix wrong hooks signature

* 🩹 Fix: CORS middleware should use the defined AllowedOriginsFunc config when AllowedOrigins is empty (#2771)

* 🐛 [Bug]: Adaptator + otelfiber issue #2641 (#2772)

* 🩹🚨 - fix for redirect with query params (#2748)

* redirect with query params did not work, fix it and add test for it

* redirect middleware - fix test typo

* ♻️ logger/middleware colorize logger error message #2593 (#2773)

* ✨ feat: add liveness and readiness checks (#2509)

* ✨ feat: add liveness and readiness checkers

* 📝 docs: add docs for liveness and readiness

* ✨ feat: add options method for probe checkers

* ✅ tests: add tests for liveness and readiness

* ♻️ refactor: change default endpoint values

* ♻️ refactor: change default value for liveness endpoint

* 📝 docs: add return status for liveness and readiness probes

* ♻️ refactor: change probechecker to middleware

* 📝 docs: move docs to middleware session

* ♻️ refactor: apply gofumpt formatting

* ♻️ refactor: remove unused parameter

* split config and apply a review

* apply reviews and add testcases

* add benchmark

* cleanup

* rename middleware

* fix linter

* Update docs and config values

* Revert change to IsReady

* Updates based on code review

* Update docs to match other middlewares

---------

Co-authored-by: Muhammed Efe Cetin <efectn@protonmail.com>
Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com>
Co-authored-by: Juan Calderon-Perez <jgcalderonperez@protonmail.com>

* prepare release v2.52.0
- add more Parser tests

* fix healthcheck.md

* configure workflows for V2 branch

* configure workflows for V2 branch

* Fix default value to false in docs of QueryBool (#2811)

fix default value to false in docs of QueryBool

* update queryParser config

* Update ctx.md

* Update routing.md

* 📚 Doc: Fix code snippet indentation in /docs/api/middleware/keyauth.md

Removes an an extra level of indentation in line 51 of
`keyauth.md` [here](https://github.com/gofiber/fiber/blob/v2/docs/api/middleware/keyauth.md?plain=1#L51)

* fix: healthcheck middleware not working with route group (#2863)

* fix: healthcheck middleware not working with route group

* perf: change verification method to improve perf

* Update healthcheck_test.go

* test: add not matching route test for strict routing

* add more test cases

* correct tests

* correct test helpers

* correct tests

* correct tests

---------

Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com>
Co-authored-by: René Werner <rene@gofiber.io>

* Merge pull request from GHSA-fmg4-x8pw-hjhg

* Enforce Wildcard Origins with AllowCredentials check

* Expand unit-tests, fix issues with subdomains logic, update docs

* Update cors.md

* Added test using localhost, ipv4, and ipv6 address

* improve documentation markdown

---------

Co-authored-by: René Werner <rene@gofiber.io>

* Update app.go

prepare release v2.52.1

* fix cors domain normalize

* fix sync-docs workflow

* fix sync-docs workflow

* fix(middleware/cors): Validation of multiple Origins (#2883)

* fix: allow origins check

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes #2882

* test: AllowOrigins with whitespace

* test(middleware/cors): add benchmarks

* chore: fix linter errors

* test(middleware/cors): use h() instead of app.Test()

* test(middleware/cors): add miltiple origins in Test_CORS_AllowOriginScheme

* chore: refactor validate and normalize

* test(cors/middleware): add more benchmarks

* prepare release v2.52.2

* refactor(docs): deactivate docs sync for v2

* refactor(docs): deactivate docs sync for v2

* fix(middleware/cors): Handling and wildcard subdomain matching (#2915)

* fix: allow origins check

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes #2882

* test: AllowOrigins with whitespace

* test(middleware/cors): add benchmarks

* chore: fix linter errors

* test(middleware/cors): use h() instead of app.Test()

* test(middleware/cors): add miltiple origins in Test_CORS_AllowOriginScheme

* chore: refactor validate and normalize

* test(cors/middleware): add more benchmarks

* fix(middleware/cors): handling and wildcard subdomain matching

docs(middleware/cors): add How it works and Security Considerations

* chore: grammar

* Apply suggestions from code review

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: fix misspelling

* test(middleware/cors): combine Invalid_Origins tests

* refactor(middleware/cors): headers handling

* docs(middleware/cors): Update AllowOrigins description

* chore: merge

* perf(middleware/cors): optimize handler

* perf(middleware/cors): optimize handler

* chore(middleware/cors): ipdate origin handling logic

* chore(middleware/cors): fix header capitalization

* docs(middleware/cors): improve sercuity notes

* docs(middleware/cors): Improve security notes

* docs(middleware/cors): improve CORS overview

* docs(middleware/cors): fix ordering of how it works

* docs(middleware/cors): add additional info to How to works

* docs(middleware/cors): rm space

* docs(middleware/cors): add validation for AllowOrigins origins to overview

* docs(middleware/cors): update ExposeHeaders and MaxAge descriptions

* docs(middleware/cors): Add dynamic origin validation example

* docs(middleware/cors): Improve security notes and fix header capitalization

* docs(middleware/cors): configuration examples

* docs(middleware/cors): `"*"`

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix(middleware/cors): Categorize requests correctly (#2921)

* fix(middleware/cors): categorise requests correctly

* test(middleware/cors): improve test coverage for request types

* test(middleware/cors): Add subdomain matching tests

* test(middleware/cors): parallel tests for CORS headers based on request type

* test(middleware/cors): Add benchmark for CORS subdomain matching

* test(middleware/cors): cover additiona test cases

* refactor(middleware/cors): origin validation and normalization

* test(middleware/csrf): Fix Benchmark Tests (#2932)

* test(middleware/csrf): fix Benchmark_Middleware_CSRF_*

* fix(middleware/csrf): update refererMatchesHost()

* Prepare release v2.52.3

* fix(middleware/cors): CORS handling (#2937)

* fix(middleware/cors): CORS handling

* fix(middleware/cors): Vary header handling

* test(middleware/cors): Ensure Vary Headers checked

* fix(middleware/cors): Vary header handling non-cors OPTIONS requests (#2939)

* fix(middleware/cors): Vary header handling non-cors OPTIONS requests

* chore(middleware/cors): Add Vary header for non-CORS OPTIONS requests comment

* prepare release v2.52.4

* merge v2 in main(v3)

* merge v2 in main(v3)

* merge v2 in main(v3)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: tokelo-12 <113810058+tokelo-12@users.noreply.github.com>
Co-authored-by: Jason McNeil <sixcolors@mac.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: iRedMail <2048991+iredmail@users.noreply.github.com>
Co-authored-by: Benjamin Grosse <ste3ls@gmail.com>
Co-authored-by: Mehmet Firat KOMURCU <mehmetfiratkomurcu@hotmail.com>
Co-authored-by: Bruno <bdm2943@icloud.com>
Co-authored-by: Muhammad Kholid B <muhammadkholidb@gmail.com>
Co-authored-by: gilwo <gilwo@users.noreply.github.com>
Co-authored-by: Lucas Lemos <lucashenriqueblemos@gmail.com>
Co-authored-by: Muhammed Efe Cetin <efectn@protonmail.com>
Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com>
Co-authored-by: Juan Calderon-Perez <jgcalderonperez@protonmail.com>
Co-authored-by: Jongmin Kim <kjongmin26@gmail.com>
Co-authored-by: Giovanni Rivera <rivera.giovanni271@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
fsandel pushed a commit to stackitcloud/stackit-cert-manager-webhook that referenced this pull request Apr 4, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/gofiber/fiber/v2](https://github.com/gofiber/fiber) | require | patch | `v2.52.2` -> `v2.52.4` |

---

### Release Notes

<details>
<summary>gofiber/fiber (github.com/gofiber/fiber/v2)</summary>

### [`v2.52.4`](https://github.com/gofiber/fiber/releases/tag/v2.52.4)

[Compare Source](gofiber/fiber@v2.52.3...v2.52.4)

### 🐛 Fixes

-   Middleware/cors: CORS handling by [@&#8203;sixcolors](https://github.com/sixcolors) in gofiber/fiber#2937
-   Middleware/cors: Vary header handling non-cors OPTIONS requests by [@&#8203;sixcolors](https://github.com/sixcolors) in gofiber/fiber#2939

**Full Changelog**: gofiber/fiber@v2.52.3...v2.52.4

### [`v2.52.3`](https://github.com/gofiber/fiber/releases/tag/v2.52.3)

[Compare Source](gofiber/fiber@v2.52.2...v2.52.3)

#### 🐛 Fixes

-   Middleware/cors: Handling and wildcard subdomain matching by [@&#8203;sixcolors](https://github.com/sixcolors) in gofiber/fiber#2915
-   Middleware/cors: Categorize requests correctly by [@&#8203;sixcolors](https://github.com/sixcolors) in gofiber/fiber#2921
-   Middleware/csrf: Fix Benchmark Tests by [@&#8203;sixcolors](https://github.com/sixcolors) in gofiber/fiber#2932

**Full Changelog**: gofiber/fiber@v2.52.2...v2.52.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or rename PR to start with "rebase!".

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants