stytchauth/stytch-java

Sessions.authenticateJwt should return JWTNullResponse

Closed this issue · 4 comments

cosmin-marginean commented

According to the design of JWTResponse (here) we assumed that when authentication fails for Sessions.authenticateJwt, we'd get a JWTNullResponse.

However, the implementation returns a plain null in that case, as you can see here: JWTNullResponse

This isn't really a big issue we can check for null instead of JWTNullResponse, but it creates confusion around the API contract and we propose that we either:

A) address the implementation by making authenticateJwt return StytchResult<JWTResponse> instead of the nullable StytchResult<JWTResponse?> and the line above corrected to

else -> StytchResult.Success(JWTNullResponse)

or

B) remove JWTNullResponse so that consumer never expects that in the result.

jhaven-stytch commented

Yep, this looks like a regression. It should be option A, returning a JWTNullResponse. If you want to make a PR, I can get that merged in, otherwise I'll pick it up in this coming cycle. Thanks for catching this!

cosmin-marginean commented

Great, thanks, PR here: #23

logan-stytch commented

Fixed in #23
This will go out with the 2.3.0 release which is expected to land in the next hour or two (related PR).

  • 👍1
logan-stytch commented

Thanks for your contribution, @cosmin-marginean!

  • 👍1