SEC-2836: If token not present into request generate the InvalidCsrfTokenException always

[kazuki43zoo]

I modified that if token not present into request generate the InvalidCsrfTokenException always.
Please review and merge this pull-request, if possible.

https://jira.spring.io/browse/SEC-2836

I have signed and agree to the terms of the Spring Individual Contributor License Agreement.

[rwinch]

@kazuki43zoo I responded via JIRA https://jira.spring.io/browse/SEC-2836

[kazuki43zoo]

@rwinch

Thank you for comment.
I did feedback to JIRA. Please review again.

[kazuki43zoo]

resolved a conflict.

[pivotal-issuemaster]

@kazuki43zoo Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@kazuki43zoo Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@kazuki43zoo Thank you for signing the Contributor License Agreement!

[jgrandja]

Hi @kazuki43zoo. I reviewed the PR as well as the comments in #3062.

I'm not sure if the extra check if (actualToken != null makes sense here.

            if (actualToken != null && missingToken) {
                accessDeniedHandler.handle(request, response,
                        new MissingCsrfTokenException(actualToken));

If missingToken is true then it's a MissingCsrfTokenException. Regardless if the token was passed in the request or not. More specifically, if the CsrfToken is not found in the CsrfTokenRepository (for example, expired session) than there is nothing to compare with even if the token was passed in the request. So it makes total sense to handle MissingCsrfTokenException.

If missingToken is false then we have a CsrfToken to compare with on the incoming request. This is really the only case where InvalidCsrfTokenException should get handled because of a comparison failure.

So I feel that extra check if (actualToken != null is not necessary here. However, I may be missing something from my understanding. Please clarify if I am.
Thanks.

[jgrandja]

Hi @kazuki43zoo, I haven't heard back from you based on my last comment so I'm assuming you're good with my explanation.
I'm going to close this PR but if you have more feedback around this I'd be happy to re-open it.
Thanks.