fix: issuedAt now validated with leeway in Oauth2ExpirationIssuedAtValidationRule

[richardtreier]

What this PR changes/adds

As discussed here, added a configurable leeway to the validation of the issuedAt claim in oauth2-core.

Why it does that

The validation was added due to the sensitivity to clock skew of the aforementioned issuedAt claim.

See the #3692 for more detail.

Further notes

• Added no validation for the expiresAt claim as it is less sensitive to clock skew.

Linked Issue(s)

Closes #3692

[github-actions]

We are always happy to welcome new contributors ❤️ To make things easier for everyone, please make sure to follow our contribution guidelines, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.

[jimmarino]

Please have the validation leeway an opt-in and adjust the tests accordingly. Also include a test for the case where the validation leeway is set but has been exceeded.

[richardtreier]

Will post the changes in a sec.

I'd like to warn against not enabling a reasonable leeway (2s-5s) by default:

• Rounding of dates is within-spec in JWT.
• Rounding of dates, whether to round or not is implementation specific.
• Rounding direction of dates is platform and implementation specific.
• Our DAPS is built with Keycloak, so I'd trust them to do JWT by the letter. Same with the Azure cloud, I'd also assume them to do things reasonably.

This error is almost impossible to debug, as it happens only in production.

So I am afraid that keeping this leeway opt-in rather than opt-out might cause people to run into issues they deem impossible to debug and just drop their work. I would personally also argue against a configurability, as any clock skew beyond 1 or 2s (with rounding) could be considered out-of-scope for the eclipse edc connector to deal with.

[richardtreier]

I updated both the default to 0 and the tests to be more explicit.

[jimmarino]

Will post the changes in a sec.

I'd like to warn against not enabling a reasonable leeway (2s-5s) by default:

• Rounding of dates is within-spec in JWT.
• Rounding of dates, whether to round or not is implementation specific.
• Rounding direction of dates is platform and implementation specific.
• Our DAPS is built with Keycloak, so I'd trust them to do JWT by the letter. Same with the Azure cloud, I'd also assume them to do things reasonably.

This error is almost impossible to debug, as it happens only in production.

So I am afraid that keeping this leeway opt-in rather than opt-out might cause people to run into issues they deem impossible to debug and just drop their work. I would personally also argue against a configurability, as any clock skew beyond 1 or 2s (with rounding) could be considered out-of-scope for the eclipse edc connector to deal with.

This needs to be an explicit security decision by the operator. We can issue an INFO (not warning, since that pollutes the log) in the case that this is not set.

[richardtreier]

I added a warning logged with INFO.

[richardtreier]

The code style + message are fixed now.

[richardtreier]

Oh, I had that overlooked. Should be fixed now.

[richardtreier]

I shortened the variables in question.

[richardtreier]

The Setting annotations were updated to use their latest API.

The Apache 2.0 license headers of files that I touched were also updated, I hope correctly. Is this still the way things are done?

[ndr-brt]

as long as checkstyle does not complain about the headers, they are ok.

please update the dependencies file then we'll be ready to go (@jimmarino already approved)

[richardtreier]

The DEPENDENCIES file was updated with the contents output by the CI.