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

[HttpFoundation] Add QueryParameterRequestMatcher #51324

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

alexandre-daubois
Copy link
Contributor

@alexandre-daubois alexandre-daubois commented Aug 9, 2023

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets NA
License MIT
Doc PR Todo

We know we need a authorizationCode query parameter to support a request in our authenticator. We would love to use only the ChainRequestMatcher to do so. I'd like to add this QueryParameterRequestMatcher in order to do the following:

class OurAuthenticator extends AbstractAuthenticator
{
    public function supports(Request $request): ?bool
    {
        return (new ChainRequestMatcher([
            // ...
            new PathRequestMatcher('/sso/provider'),
            new QueryParameterRequestMatcher('authorizationCode')
        ]))->matches($request);
    }

    // ...
}

Which would match the following: /sso/provider?authorizationCode=...

@norkunas
Copy link
Contributor

norkunas commented Aug 10, 2023

While writing github webhook request parser I encountered that they send signature as a header, would it make sense to also add HeaderRequestMatcher to core?

@alexandre-daubois
Copy link
Contributor Author

That would be a big yes for me, definitely! Gave a try here: #51343

Thank you for the idea, it would be even more powerful with those features!

@alexandre-daubois alexandre-daubois force-pushed the query-param-request-matcher branch 2 times, most recently from 6aec919 to 762ae91 Compare August 17, 2023 09:58
@alexandre-daubois
Copy link
Contributor Author

Rebased and conflicts fixed

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@fabpot
Copy link
Member

fabpot commented Feb 3, 2024

Thank you @alexandre-daubois.

@fabpot fabpot merged commit 3feccf7 into symfony:7.1 Feb 3, 2024
5 of 9 checks passed
fabpot added a commit that referenced this pull request Feb 3, 2024
…-daubois)

This PR was merged into the 7.1 branch.

Discussion
----------

[HttpFoundation] Add `HeaderRequestMatcher`

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | _NA_
| License       | MIT
| Doc PR        | Todo

This a follow up to:
- #51324

After `@norkunas`' [comment](#51324 (comment))

Commits
-------

62b5a34 [HttpFoundation] Add `HeaderRequestMatcher`
@fabpot fabpot mentioned this pull request May 2, 2024
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

6 participants