vert-x3 / issues

Apache License 2.0
36 stars 7 forks source link

Possible JWT handler regression #562

Closed jponge closed 3 years ago

jponge commented 3 years ago

Running the code of https://github.com/jponge/vertx-in-action/tree/master/part2-steps-challenge I have failures with the JWT web handler (all other tests pass).

The tests that fail are those where a user shall be identified using JWT:

Screenshot 2020-11-09 at 17 37 46

Digging into what happens:

private void checkUser(RoutingContext ctx) {
  String subject = ctx.user().principal().getString("sub")
  if (!ctx.pathParam("username").equals(subject)) {
    sendStatusCode(ctx, 403);
  } else {
    ctx.next();
  }

The call to principal returns a JsonObject that does not contain the JWT token data. Instead it has a single access_token entry whose value is the token string. This is a regression, and likely a bug, or I am just missing something obvious but that code used to work.

pmlopes commented 3 years ago

@jponge the goal to keep all user object internal state consistent between all auth modules was that:

This means that the property you're looking for, now lives on ctx.user().attributes() I can try to revert it back/keep it back on principal()

https://github.com/vert-x3/vertx-auth/blob/master/vertx-auth-jwt/src/main/java/io/vertx/ext/auth/jwt/impl/JWTAuthProviderImpl.java#L193

But I would need to do it too for OAuth2. What's your opinion?

jponge commented 3 years ago

These are book examples, I cannot make a fix since that code is in print, so we need to have principal return the token attributes entries like it used to be a few weeks back.

It's probably worth being consistent in OAuth as well.

The best is to keep the old behaviour, introduce .attributes(), and document that principal will no longer return the entries at some point in the future.

jponge commented 3 years ago

Can we have it in 4.0.0.CR2?

pmlopes commented 3 years ago

Sure, I'll get it done today

jponge commented 3 years ago

Fixed in vertx-auth / master, waiting for 4.0.0.CR2