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

Security Filter ignores configured matchers when deciding on a rule #710

Open
TWJoachim opened this issue Oct 3, 2024 · 3 comments
Open

Comments

@TWJoachim
Copy link

Hello,

its basically the same issue from here: https://groups.google.com/g/pac4j-users/c/vCZ9-YItIUg .When the security filter config for pac4j defines two rules that would match the same request, but uses "matchers" (e.g. HttpMethodMatcher, one POST, one GET) to define a more specific rule matching behavior, the current org.pac4j.play.filters.SecurityFilter ignores this in its "apply" method.

The logic in "findRule" does only apply regex matching on the path and is not taking any matchers into account. So the matchers are evaluated later when the chosen endpoint is executed - but to my understanding of the docs they should also be relevant in choosing the right rule.

(see https://github.com/pac4j/play-pac4j/blob/master/shared/src/main/scala/org/pac4j/play/filters/SecurityFilter.scala , method "private def findRule(..)"

Thanks a lot,
Joachim

@leleuj
Copy link
Member

leleuj commented Oct 7, 2024

Indeed, there is a flaw here, only the path is taken into account. This should be fixed. As I said, I'm not fluent in Scala. A pull request is welcome here. Thanks

@TWJoachim
Copy link
Author

I'm also not fluent in Scala. Has the Play-Pac4j integration active maintainers? If not, I can close this issue (and we may look into rewriting the SecurityFilter in Java).

@leleuj
Copy link
Member

leleuj commented Oct 8, 2024

I'm the main maintainer and there are also occasional contributors. But unfortunately, I'm not fluent in Scala either.

Rewriting the filter in Java is a great idea (as filters can be used both in Java and Scala), it would ease its maintenance. Keep the issue open if you intend to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants