OAuth2AccessTokenResponse.expiresIn() is ignored when initialized from another response

[benba]

Describe the bug
OAuth2AccessTokenResponse doesn't use values provided to expiresIn() method when it was originally built from a previous response (through the withResponse() method).

Expected behavior
The newly created token response should update expiresAt using the value the provided to expiresIn(), or maybe throw an exception if keeping the original expiresAt value is done on purpose.

Sample

@Test
public void test() {
    var sourceTokenResponse = OAuth2AccessTokenResponse.withToken("abcd")
            .tokenType(OAuth2AccessToken.TokenType.BEARER)
            .expiresIn(10)
            .build();
    var modifiedTokenResponse = OAuth2AccessTokenResponse
            .withResponse(sourceTokenResponse)
            .expiresIn(60)
            .build();
    Assertions.assertEquals(
            modifiedTokenResponse.getAccessToken().getIssuedAt().plusSeconds(60),
            modifiedTokenResponse.getAccessToken().getExpiresAt());
}

Note
May be linked to #8696 ?

[jgrandja]

@benba It doesn't make sense to call OAuth2AccessTokenResponse.withResponse(sourceTokenResponse).expiresIn(60), since the supplied sourceTokenResponse provides the expiresAt. If you want to explicitly configure expiresIn() then call OAuth2AccessTokenResponse.withToken() instead.

[benba]

@jgrandja sure, it's just that you need to copy all the entries manually:

OAuth2AccessTokenResponse.withToken(accessToken.getTokenValue())
                .tokenType(accessToken.getTokenType())
                .scopes(accessToken.getScopes())
                .expiresIn(Duration.ofDays(1).toSeconds())
                .refreshToken(original.getRefreshToken().getTokenValue())
                .additionalParameters(original.getAdditionalParameters())
                .build();

Maybe I understand this wrong but I thought of the withResponse()or withToken() as a way to copy values from a response or a token to the Builder, and then being able to change them.
Anyway, why not throwing an exception if expiresIn() is called while expiresAtis already set, instead of silently ignoring the error?

[jgrandja]

@benba I see your point...

why not throwing an exception if expiresIn() is called while expiresAt is already set, instead of silently ignoring the error?

However, throwing an exception would be too much I think. How about setting this.expiresAt = null when expiresIn() is called? Would you be interested in submitting a PR for this?

[benba]

@jgrandja PR submitted