ch4mpy / spring-addons

Ease spring OAuth2 resource-servers configuration and testing
Apache License 2.0
521 stars 84 forks source link

Bug? @WithJwt() should validate timestamp instead of throwing #219

Closed cloakedch closed 1 month ago

cloakedch commented 1 month ago

Describe the bug When using spring-addons-starter-oidc-test's @WithJwt() in a test, one can observe the following exception when exp is larger than java's integer max value (2147483647):

{ "exp": 3147483647, }

Exception thrown:

Unable to create SecurityContext using @com.c4_soft.springaddons.security.oauth2.test.annotations.WithJwt...
[...]
Caused by: java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer (java.lang.Long and java.lang.Integer are in module java.base of loader 'bootstrap')

The error is thrown in WithJwt.java on line 158 (final var exp = Optional.ofNullable((Integer) claims.get(JWTClaimNames.EXPIRATION_TIME)).map(Instant::ofEpochSecond).orElse(now.plusSeconds(42));), where an explicit cast to Integer is being done.

I highly suspect the reason for this exception is that the next larger variable type (long) is being used and thus the conversion fails.

Now I don't know what the solution should look like: Should it be possible to supply larger values and thus support timestamps after the year 2038? If not, it would probably be great if the exception is being caught and a more descriptive error message is thrown if the value is larger than Integer's max size.

cloakedch commented 1 month ago

In addition, it seems like exp and iat are not verified in tests setup with @WithJwt(), is this correct and desired? It works fine when running the application, but these fields seem not to be validated in tests.

I suspect this is desired, since the test harness probably doesn't validate the JWT itself (same applies to iss, which will throw if the provider can't be found).

My used test configuration is empty:

@Configuration
@EnableMethodSecurity
public class SecurityConfig {}

Test itself:

@WebMvcTest(controllers = MyController.class)
@AutoConfigureAddonsWebmvcResourceServerSecurity
@Import(SecurityConfig.class)
class MyControllerTest {
    @Autowired
    private MockMvcSupport mockMvc;

    @Test
    @WithJwt("token.json")
    void shouldNotGetMyEndpointWhenTokenIsExpired() throws Exception {
        mockMvc.get("/myendpoint")
                .andExpect(status().isUnauthorized());
    }
}

Token token.json:

{
  "exp": 12341234,
  "iat": 123,
  "jti": "d0a9f7fa-693f-40c8-8af1-8914e9cb444a",
  "iss": "http://localhost:8081/realms/myrealm",
  "sub": "b3407926-5f40-48d8-a115-ec0e19ec40b3",
  "typ": "Bearer",
  "azp": "myrealm",
  "sid": "f58682ed-5627-4247-bcea-99fa77aaae2e",
  "allowed-origins": [
    "/*"
  ],
  "realm_access": {
    "roles": [
      "default-roles-myrealm"
    ]
  },
  "scope": "openid microprofile-jwt",
  "upn": "tokensubject",
}
ch4mpy commented 1 month ago

exp and iat are not verified in tests setup with @WithJwt(), is this correct and desired?

Yes, and yes. No JWT is built: that would be impossible as the authorization server private key would be needed to sign a token and there isn't even an authorization server involved in unit test.

None of the annotations in this lib build an access token or a session cookie, later turned into an Authentication instance by the security filter-chain. Instead, each directly creates this Authentication instance and populates the test security context with it. So, to be more explicit, @WithJwt could be named @WithSomethingExtendingAbstractAuthenticationTokenAndBuiltFromJwtClaimsUsingAConverterBean, but...

I highly suspect the reason for this exception is that the next larger variable type (long) is being used

This is correct. The OAuth2 and OpenID standards state that timestamps in claims should be integer values representing a number of seconds since epoch, but it does not state with how many bytes or a maximum value. JSON deserialization libs build Integer or Long, depending on the serialized value. So, for more flexibility, the Authentication factory behind @WithJwt should adapt to the actual deserialization output. Will fix.

cloakedch commented 1 month ago

That makes sense, I didn't think about the fact that only an incomplete JWT without signature can be built in tests.

thank you for the fast fix.

ch4mpy commented 1 month ago

Tha,k you for taking the time to report.

I just released the fix as part of 7.8.5. It should be available from Maven-central shortly.