auth0 / auth0-java-mvc-common

Contains common helper classes and api client logic that are used across our Java MVC libraries
MIT License
43 stars 39 forks source link

[SDK-4670] Improved state handling errors #140

Closed jimmyjames closed 9 months ago

jimmyjames commented 10 months ago

Changes

This PR provides additional information to the error message received when a state validation error occurs. To minimize any potential breaking changes, it:

Note that the SDK currently needs to support the deprecated methods that use the session for auth state param storage (both for building the auth URL as well as processing the callback). In v2, we should:

Note that this change also includes additional tests for functionality that was not changed, in addition to tests for the changes. The first commit adds tests to cover all paths prior to the change, as well as updates tests to the new expected behavor. This commit provides the functional change.

evansims commented 10 months ago

Failures look OK. Just a dupe Snyk webhook causing that one failure; fixed that. And Codecov is weird, as usual.

jimmyjames commented 9 months ago

Codecov was failing because there was a check in RandomStorage#checkSessionState for a null session value but non-null request state value. This is now being handled in RequestProcessor#assertValidState so there was a drop in coverage. Rather than updating checkSessionState, I've kept that logic but added a unit test for that specific case, just to ensure the changes are minimized. Codecov is now fixed.

@evansims I see the security/snyk (auth0-sdks) check is still expected and not reporting (security/snyk (DX) is passing). Did the webhook change you make intend to remove the duplicated check, security/snyk (auth0-sdks)?