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

CSRF token validation doesn't work on Wildfly #38

Closed
chkal opened this issue Jan 26, 2019 · 6 comments · Fixed by #39
Closed

CSRF token validation doesn't work on Wildfly #38

chkal opened this issue Jan 26, 2019 · 6 comments · Fixed by #39
Assignees
Labels
bug Something isn't working tck

Comments

@chkal
Copy link
Contributor

chkal commented Jan 26, 2019

I didn't check this in detail, but the TCK tests for validating CSRF tokens in Wildfly are failing. It looks like submitting invalid tokens when CSRF protection is enabled doesn't result in a 401 status code as required by the spec. Perhaps the required validation isn't executed at all?

https://i.imgur.com/4oUl42F.png

It would be great if somebody could have a look at this.

@chkal chkal added bug Something isn't working help wanted Extra attention is needed tck labels Jan 26, 2019
@chkal chkal added this to the 1.0.0-m06 milestone Jan 26, 2019
@chkal
Copy link
Contributor Author

chkal commented Jan 26, 2019

A similar issue has been reported for CXF in #27.

In the CXF case we already know that the ReaderInterceptor isn't invoked by CXF if there is no request entity but only @FormParam annotations (which is typically the case for MVC apps). I brought this up to the JAX-RS EG and this has been clarified (see jakartaee/rest#659) and will be part of JAX-RS 2.2.

I'm not really sure if the Wildfly issue is related to this. But I guess we shouldn't rely on ReaderInterceptor to verify the CXRF token anymore. Any ideas?

@gtudan
Copy link
Contributor

gtudan commented Jan 26, 2019

I'll have a look at it on wildfly.

@chkal
Copy link
Contributor Author

chkal commented Jan 26, 2019

Awesome! Let me know if you need any additional info. I'll try to find some time this weekend to document how to run individual TCK tests if this helps.

@gtudan
Copy link
Contributor

gtudan commented Jan 26, 2019

I ported the CSRF example to the testsuite and it does indeed fail on wildfly. Strange, I'm pretty sure I've seen CSRF errors on wildfly before.

https://travis-ci.org/eclipse-ee4j/krazo/jobs/484771605

@gtudan
Copy link
Contributor

gtudan commented Jan 26, 2019

I had a closer look into this - looks like the CsrfValidationInterceptor never triggers - no idea why, it looks okay to me.

The validation came back after I changed it to a ContainerRequestFilter. Should I dig deeper why it fails as ReaderInterceptor, or shall we just keep it as RequestFilter? Are there any disadvantages?

@chkal
Copy link
Contributor Author

chkal commented Jan 26, 2019

Hmmm. Interesting. Not sure if there are any disadvantages. As mentioned before, we also had some problems with CsrfValidateInterceptor not being invoked with CXF. Maybe using a ContainerResponseFilter instead is fine. It would be interesting to see the TCK results for other containers. If providing a pull request isn't too much work, you could create one which would trigger the TCK run against all other containers. Then we would know more about the impact on other runtimes. Maybe this is the solution to all the CSRF problems.

@chkal chkal removed the help wanted Extra attention is needed label Feb 2, 2019
@chkal chkal closed this as completed in #39 Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tck
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants