JWT not being re-generated by default

[jvictorcf]

What would you like to change?

We had some issues regarding JWT not being re-generated when our JWT didn't contain the "exp" property in the payload.
Since this is used in conjunction with a one-time password, I believe that in the case of the JWT not containing the "exp" property, we should always re-generate it (i.e. re-generating should be the default behaviour).
My fear is that if we ever handle a payload that doesn't have an "exp" property, we'll be always returning the same token until spring is restarted.

I'd like to know what the team thinks about it. This is only my interpretation.

References:
One-time password:

service-auth-provider-java-client/src/main/java/uk/gov/hmcts/reform/authorisation/generators/ServiceAuthTokenGenerator.java

Line 36 in a3deb2b

signInDetails.put("oneTimePassword", oneTimePassword);

Re-generation logic:

service-auth-provider-java-client/src/main/java/uk/gov/hmcts/reform/authorisation/generators/AutorefreshingJwtAuthTokenGenerator.java

Line 57 in a3deb2b

return expDate != null && Date.from(now().plus(refreshTimeDelta)).after(expDate);

How do you think that would improve the project?

I think it could prevent problems in production and make functional testing less dependant on testers using the "exp" property.

If this entry is related to a bug, please provide the steps to reproduce it

1. Mock the return of the "/lease" endpoint to be a valid JWT with no "exp" property.
2. Generate the token (AuthTokenGenerator.generate())
3. Reset the mock
4. Mock it again with another valid JWT token also with no "exp" property.
5. Generate the token (AuthTokenGenerator.generate())
   You will see that steps 2 and 5 will generate the same token (i.e. the service no longer queries the "/lease" endpoint.

[timja]

When does it not have the exp property?

[jvictorcf]

I'm not sure, @timja. This comes from IDAM and it seems the property is there at the moment. But from this component's perspective, we can never be sure.
And If the property is ever missing, does that mean the token never expires or does it mean it's a one-time token (and should, therefore, be re-generated every time the method is called)?

[timja]

It's part of the API contract though, it should always be there =/.

[jvictorcf]

Is it? If that's the case, then might not be a problem while the ServiceAuthTokenGenerator implementation is used.
I guess I'm thinking of service-auth-provider-java-client as a separate component that have its own rules regardless of whether "exp" is provided.

[timja]

Are you mocking the value that's being returned by the s2s server?

If so it sounds like you aren't mocking it correctly.

[jvictorcf]

Hi @timja. We are mocking it and you're right, the value was wrong (as it didn't have the "exp" property).
That's fixed now. My only concern (and reason for raising this discussion) is what would happen if this payload came from production without the "exp" property.
If the current behaviour (not refreshing unless there is an "exp" property) is expected, then that's fine. Happy to close this.

[timja]

It's part of the API contract, so no not an issue