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.

Sign up for GitHub

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

Jump to bottom

Check exposure/enabled for HTTP verb match in management endpoints #1408

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Check exposure/enabled for HTTP verb match in management endpoints #1408

wants to merge 1 commit into from

Conversation

bart-vmware
Copy link
Member

@bart-vmware bart-vmware commented Nov 21, 2024 •
edited
Loading

Description

Instead of mapping endpoints in ASP.NET routing per verb, always map all verbs and then filter inside middleware.

Advantages:

  • Returns 404 when not exposed/enabled with invalid verb, instead of 405
  • Can change verbs at runtime

Disadvantages:

  • Verb information is lost in route mappings endpoint (always shows all verbs)
  • Entries in route mappings with all verbs disabled are no longer hidden

Fixes #1410.

Quality checklist

  • Your code complies with our Coding Style.
  • You've updated unit and/or integration tests for your change, where applicable.
  • You've updated documentation for your change, where applicable.
    If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
  • You've added required license files and/or file headers (explaining where the code came from with proper attribution), where code is copied from StackOverflow, a blog, or OSS.

@bart-vmware bart-vmware mentioned this pull request Nov 21, 2024
Merged
@bart-vmware bart-vmware marked this pull request as ready for review November 21, 2024 11:22
@bart-vmware bart-vmware requested a review from TimHess November 21, 2024 11:23
@bart-vmware
Copy link
Member Author

@TimHess This PR merely exists to explore the impact of the change, we may decide not to merge it.

@bart-vmware bart-vmware marked this pull request as draft November 21, 2024 16:34
@bart-vmware
Copy link
Member Author

Blocked by #1409.

@bart-vmware bart-vmware mentioned this pull request Nov 21, 2024
Open
@bart-vmware bart-vmware added the Component/Management Issues related to Steeltoe Management (actuators) label Nov 21, 2024
@bart-vmware
Check exposure/enabled for HTTP verb match in management endpoints
b03df23 
Instead of mapping endpoints in ASP.NET routing per verb, always map all verbs and then filter inside middleware.

Advantages:
- Returns 404 when not exposed/enabled with invalid verb, instead of 405
- Can change verbs at runtime

Disadvantages:
- Verb information is lost in route mappings endpoint (always shows all verbs)
- Entries in route mappings with all verbs disabled are no longer hidden
@bart-vmware bart-vmware force-pushed the check-exposure-before-verb branch from c9ba059 to b03df23 Compare November 21, 2024 16:46
@bart-vmware bart-vmware removed the request for review from TimHess November 21, 2024 16:46
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Component/Management Issues related to Steeltoe Management (actuators)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect status code when actuator is disabled
1 participant
@bart-vmware