Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[management] Better select plan based on subscription #7824

Closed
jhaeyaert opened this issue Jun 13, 2022 · 3 comments
Closed

[management] Better select plan based on subscription #7824

jhaeyaert opened this issue Jun 13, 2022 · 3 comments

Comments

@jhaeyaert
Copy link

Since #6528, selection of the subscription during the security chain execution is not perfect.

When dealing with multiple plans based on the same security type, the policy is applied prior to retrieve the associated subscription that needs to checked (same security type -> JWT and Oauth2 are both based on Authorization bearer token).

This is not optimal because, in some circumstances, it can select a subscription that is not linked to the plan that has been executed.
We must rethink the way plans are selected and executed by reordering the different steps:

  1. extract the information which identify the caller (clientId, apikey, ...)
  2. identify the subscription
  3. identify the plan associated to the subscription
  4. execute the plan
@mouligno mouligno added this to the APIM - 3.10.x milestone Jun 13, 2022
@mouligno mouligno added the p2 label Jun 13, 2022
@marcambier marcambier self-assigned this Jul 4, 2022
marcambier pushed a commit to gravitee-io/gravitee-api-management that referenced this issue Jul 19, 2022
Before this change, when dealing with multiple plans based on the same security type, the policy is applied prior to retrieve the associated subscription that needs to checked (same security type -> JWT and Oauth2 are both based on Authorization bearer token).
This is not optimal because, in some circumstances, it can select a subscription that is not linked to the plan that has been executed.

This change refactor the plan selection workflow by reordering the different steps.

For JWT and Oauth2 plans, subscription has to be searched with client_id, to see if a plan can handle the incoming request.

For Oauth2, client_id is retreived from token introspection, so we can't check subscription outside of policy chain execution, where the auth server is called.

For JWT, client_id is always present in incoming request token claims. So we can fetch the subscription prior of policy chain execution.

JWT are now processed prior to Oauth2 plans.
Having multiple Oauth2 plans on the same API will still require selection rules to apply to right one.

gravitee-io/issues#7824
@marcambier
Copy link
Contributor

marcambier commented Jul 19, 2022

@RubenMMSantos @LiliaEn, a simple test case for this ticket :

  • api1 with 2 JWT plans : JWT1 and JWT2

  • app1 is subscribed to JWT1

  • app2 is subscribed to JWT2

  • call the API using app1 subscription : it works

  • call the API using app2 subscription : it works

  • add a selection rule on JWT1

  • call the API using app1 subscription, matching selection rule : it works

  • call the API using app1 subscription, not matching selection rule : unauthorized

  • remove selection rule on JWT1

  • add a selection rule on JWT2

  • call the API using app2 subscription, matching selection rule : it works

  • call the API using app2 subscription, not matching selection rule : unauthorized

  • remove selection rule on JWT2

Additionnally, we have to check non-regression of all that is about JWT / Oauth2 plans and their selection rules.

gaetanmaisse pushed a commit to gravitee-io/gravitee-api-management that referenced this issue Jul 27, 2022
Before this change, when dealing with multiple plans based on the same security type, the policy is applied prior to retrieve the associated subscription that needs to checked (same security type -> JWT and Oauth2 are both based on Authorization bearer token).
This is not optimal because, in some circumstances, it can select a subscription that is not linked to the plan that has been executed.

This change refactor the plan selection workflow by reordering the different steps.

For JWT and Oauth2 plans, subscription has to be searched with client_id, to see if a plan can handle the incoming request.

For Oauth2, client_id is retreived from token introspection, so we can't check subscription outside of policy chain execution, where the auth server is called.

For JWT, client_id is always present in incoming request token claims. So we can fetch the subscription prior of policy chain execution.

JWT are now processed prior to Oauth2 plans.
Having multiple Oauth2 plans on the same API will still require selection rules to apply to right one.

gravitee-io/issues#7824
@LiliaEn
Copy link
Contributor

LiliaEn commented Jul 29, 2022

The following scenarios were covered:

  • JWT+JWT plans on the same API
    The correct plan is selected when a using a selection rule.
    When there is no selection rule applied, depending on the token used, the appropriate plan gets executed.
    When the selection rule is not matched, the appropriate error is displayed
  • JWT+oAUTH2 plans on the same API
    The correct plan is selected when a using a selection rule.
    When there is no selection rule applied, depending on the token used, the appropriate plan gets executed.
    When the selection rule is not matched, the appropriate error is displayed
  • JWT+oAUTH2+ API Key
    The correct plan is selected when a using a selection rule.
    When there is no selection rule applied, depending on the token/ api key used, the appropriate plan gets executed.
    When the selection rule is not matched, the appropriate error is displayed

@RubenMMSantos
Copy link

RubenMMSantos commented Aug 1, 2022

Oauth2_plan+ Oauth2_plan scenario not working as expected, 1st plan is always overwriting the 2nd one in terms of behaviour (in terms of policies and selection_rules).
Confirmed with @marcambier that this will be fixed on Jupiter via ticket #7991 but cannot be fixed on existing V3 gateway so moving the ticket to done so it does not block the other fixes in the ticket

@gaetanmaisse gaetanmaisse changed the title Better select plan based on subscription [management] Better select plan based on subscription Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants