API product access control conflicts with "Apigee Edge API product RBAC"

[arlina-espinoza]

Describe the bug
Enabling monetization disables the API product entity access control provided by the apigee_edge_apiproduct_rbac module.

To Reproduce
Steps to reproduce the behavior:

1. On a clean install do a developer sync to bring in some users.
2. Enable the apigee_edge and apigee_edge_apiproduct_rbac modules.
3. Configure the API products RBAC at /admin/config/apigee-edge/developer-settings/access-control.
4. Log in as non-admin user and go to create a developer app (/user/{{user_id}}/create-app). Only the API products that the user's roles has access to are displayed.
5. Enable apigee_m10n.
6. Log in as non-admin user and go to create a developer app (/user/{{user_id}}/create-app).
7. The list of API products available for the app doesn't respect the RBAC settings.

Expected behavior
Only the API products that the user's roles has access to (as configured on the RBAC settings page) should be displayed.

Version Info
Latest apigee_edge 8.x-1.x and apigee_m10n 8.x-1.x.

[shadcn]

@arlina-espinoza For edge, enabling RBAC removes the custom access hook so that control is handed over to RBAC. Should we do same for m10n?

/**
 * Implements hook_module_implements_alter().
 */
function apigee_edge_apiproduct_rbac_module_implements_alter(&$implementations, $hook) {
  if ($hook == 'api_product_access') {
    // Disable API Product access provided by Apigee Edge module when
    // this module is enabled. API product visibility based access control
    // and role based access control provided by this module is incompatible
    // with each other.
    unset($implementations['apigee_edge']);
  }
}

This is what m10n is doing right now:

/**
 * Implements hook_ENTITY_TYPE_access().
 */
function apigee_m10n_api_product_access(EntityInterface $entity, $operation, AccountInterface $account) {
  // For the assignment operation a product must either be free or purchased.
  if ($operation == 'assign') {
    return \Drupal::service('apigee_m10n.monetization')
      ->apiProductAssignmentAccess($entity, $account);
  }

  // No opinion on other operations.
  return AccessResult::neutral();
}

[arlina-espinoza]

I've been thinking about this, and in the end it seems that it is doing what is correct, and perhaps we should just document that apigee_edge_apiproduct_rbac is incompatible with monetization or at least explain how they restrict each other. My reasoning is:

If monetization is enabled, the "assign" operation (used when adding or updating an API product on an app) should only show API products that a developer is eligible to access, which is what \Drupal::service('apigee_m10n.monetization') ->apiProductAssignmentAccess($entity, $account) is already doing (according to the docs):

• Allow API products for which a developer has accepted a rate plan.
• Allow API products that are public and do not have a published rate plan.
• Explicitly forbid access to API products that are not in the categories above.

It would not make sense to have monetization enabled, and then have another module allow apps with API products that a developer has not purchased.

If RBAC is also enabled, it might be confusing, because the way access control works is by gathering all the entity access hooks results, and:

• If any result is forbidden, access is forbidden.
• If no result is forbidden, then if any is explicitly allowed, access is allowed.
• If all results are neutral, access is denied.

This could mean some confusing scenarios, like:

• RBAC allows an API product, but monetization doesn't: it will not be shown.
• RBAC disallows a product, but monetization does because the developer has bought a rate plan with it: it will not be shown.

Let me know your thoughts @arshad / @cnovak / @arunz6161 .

[shadcn]

I agree with what @arlina-espinoza said above.

If both m10n and RBAC are enabled, m10n should win access control.

Let's add this to the documentation (and m10n readme).

We can also add a note to the Status report page if a site has both enabled.

[cnovak]

I agree on Monetized API Products, I think what is missing is documentation on how you do RBAC when M10n is enabled. Instead of having access control on the API Product, you instead use the "Audience" on the rate plan to only allow certain developers to purchase a rate plan.

However, does the API product RBAC module still work with API Products that are not monetized? I would think that the RBAC module would still allow non-monetized API Product to be configured. Can the API product RBAC module show the monetized API Products disabled in the admin UI with info saying that the API Products that are monetized needs to be configured at the rate plan in the Edge UI for access control?

[arunz6161]

Need to discuss this in deep dive- 08/10

[arunz6161]

Need to discuss this in deep dive- 08/24

[cnovak]

After discussing with @arlina-espinoza , we realized that we have two different access control systems: 1) The RBAC submodule in Apigee Edge or the simple access control using audience in Apigee Edge, and 2) access control for API products which are monetized. The access control for API products which are monetized have access control configured when you set up the rate plan.

Our approach will be:

1. If an API product is non monetized, the RBAC module or Apigee Edge simple access control will determine if it is visible.
2. For monetized API Products, show all API Products which the developer has accepted the rate plan.
3. In the admin side, the simple Apigee Edge module audience access control and the RBAC submodule should show monetized API Products as disabled with a note saying that "API Products that are monetized cannot be configured. Use the access control in the rate plan setup to control what developers can see monetized API Products."

[shadcn]

@cnovak @arlina-espinoza Are we going to break this down into smaller tickets/how do we proceed?

[cnovak]

@arlina-espinoza @arshad we can just break this down into three tickets as listed above, or do we need more than that?

[arlina-espinoza]

@cnovak I'll create a ticket for 1 and 2, and leave 3 on it's own. I'll assign to myself, as @arshad already has a heavier load this sprint.

[arlina-espinoza]

Follow up issues: #250 and #251