Improve OIDC/OAuth2 Flow discovery

[pendula95]

Describe the feature

As explained here we should improve the discovery mechanism to support all possible flows enabled by the provider. We should not force user to choose the flow when creating a provider but check if the request is possible when request is made. Store possible flows in in a list.

https://github.com/vert-x3/vertx-web/pull/2016#issuecomment-895141723

[pendula95]

is this still up for taking?

[pmlopes]

@pendula95 yes, I did try to see if quickly remove the single entry would be trivial, but the assumption that there is a single flow is used internally in several switch statements which will require some real refactor.

If you're interested in working on this, lets start a discussion here on the required changes and you can start a pull request we can work together

[pendula95]

If I am interpreting this right the first step would be to remove OAuth2Options setFlow(OAuth2FlowType flow) from OAuth2Options and flow should be passed in authenticate(JsonObject credentials) from AuthenticationProvider or even to be automatically concluded by the content of credentials?

[pmlopes]

Yes.

• https://github.com/vert-x3/vertx-auth/blob/master/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/Oauth2Credentials.java should have a new field to specify the flow
• https://github.com/vert-x3/vertx-auth/blob/master/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/OAuth2Options.java should deprecate setting the flow

With those in place, start replacing all usages of the deprecated getter with the getter from the credentials object.

A second action is to start working on the discovery feature updates. During the discovery, we should collect the allowed flows. Then during the calls https://github.com/vert-x3/vertx-auth/blob/master/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/impl/OAuth2API.java ensure that the flow in the credentials is permitted.

[pmlopes]

@pendula95 I believe this is wrong:

https://github.com/vert-x3/vertx-auth/blob/04ac36df802b8c09d4a15400fb5885bc092ad7ab/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/impl/OAuth2AuthProviderImpl.java#L304-L316

Let me explain, when working in On-Behalf-Of mode the method will attempt to trade an existing token by a new one. However, I don't think it should be the responsibility of this code to perform the conversion. Instead, the user should use the right type from start:

    // OBO should use the OAuth2Credentials to customize the right request
    Credentials credentials = new Oauth2Credentials()
      .setAssertion("head.body.signature") // the source token
      .setScopes(Arrays.asList("a", "b"));

    oauth2.authenticate(credentials)
      .onSuccess(user -> {
        should.assertNotNull(token);
        should.assertNotNull(token.principal());
        test.complete();
      });

This way we can remove that block and decouple the flow from the provider and add it to the credentials object.

@photomorre You're using this functionality, could you help us out testing if you replace in a test environment from:

// FROM:
Credentials credentials = new TokenCredentials("head.body.signature") // the source token
      .setScopes(Arrays.asList("a", "b")); // this depends on your case use case

// TO:
Credentials credentials = new Oauth2Credentials()
      .setAssertion("head.body.signature") // the source token
      .setScopes(Arrays.asList("a", "b"));

If it would still work? If it does, we can work safely note that we will introduce a breaking change for the future and have 1 blocker less for this feature!

[photomorre]

I will set up a new test test environment over the holidays. I'll ping you when things are up and running and I can test the above.

[pmlopes]

This has been fixed.