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 ?

Comment From: 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.

Comment From: 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?

Comment From: 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?

Comment From: benba

@jgrandja PR submitted