THREESCALE-11887 Add configurable whitelist_deny_unmatched policy option

[borisurbanik]

Fixes:

• THREESCALE-11887 - Add option for limiting Keycloak Role Check to only operate on described resources

Verification:

Deploy 3Scale and Keycloak using the operators.

The default 3Scale configuration comes with API Product and Developer account that has an echo backend mapped to /echo path.

Configure 3Scale OIDC integration for the API Product following the documentation:
https://docs.redhat.com/en/documentation/red_hat_3scale_api_management/2.16/html/administering_the_api_gateway/integrating-threescale-with-an-openid-connect-identity-provider#integrating-threescale-with-rhsso-as-the-openid-connect-identity-provider_oidc

Add new application under Developer account, set client id and client password environment variables:

CLIENT_ID='(your client id)'
CLIENT_SECRET='(your client secret)'
ACCESS_TOKEN="$(curl -k -X POST "https://keycloak-3scale-keycloak.apps.burbanik-3scale2.cp.fyre.ibm.com/auth/realms/apicast/protocol/openid-connect/token" \
  -u "$CLIENT_ID:$CLIENT_SECRET" \
  -H "Content-Type: application/x-www-form-urlencoded" \
  -d "grant_type=client_credentials"  | jq -r '.access_token')"

Verify that the integration is setup correctly:

curl -k -i -H "Authorization: Bearer $ACCESS_TOKEN" https://api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com/echo

To test authorized user, add another application and assign a role "my-role" to the corresponding client (this will work with configuration below).

Scenarios tested

1. test policy with previous version of apicast:

Policy created with original version:

curl -k "https://$MASTER_TOKEN@master.apps.burbanik-3scale2.cp.fyre.ibm.com/master/api/proxy/configs/staging.json?host=api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com" | jq -r '.proxy_configs[0].proxy_config.content.proxy.policy_chain | map(select(.name=="keycloak_role_check")) | .[]'

{
  "name": "keycloak_role_check",
  "version": "builtin",
  "configuration": {
    "scopes": [
      {
        "realm_roles": [],
        "resource": "/echo/protected",
        "methods": [
          "ANY"
        ],
        "client_roles": [
          {
            "client": "{{ jwt.azp }}",
            "name": "my-role",
            "name_type": "plain",
            "client_type": "liquid"
          }
        ],
        "resource_type": "plain"
      }
    ],
    "type": "whitelist"
  }
}

make runtime-image REGISTRY=quay.io/burbanik IMAGE_NAME=apicast-runtime-image:latest
make push REGISTRY=quay.io/burbanik IMAGE_NAME=apicast-runtime-image:latest
oc patch apimanager 3scale -n 3scale-test --type='json' -p='[{"op": "add", "path": "/spec/apicast/image", "value":"quay.io/burbanik/apicast-runtime-image:latest"}]'

curl -k -i -H "Authorization: Bearer $ACCESS_TOKEN" https://api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com/echo/protected
curl -k -i -H "Authorization: Bearer $ACCESS_TOKEN" https://api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com/echo

With authorized application: 200 response for /echo/protected, 403 for /echo
With not-authorized application: 403 for both

2. update + publish the policy after switching the boolean to false:

curl -k "https://$MASTER_TOKEN@master.apps.burbanik-3scale2.cp.fyre.ibm.com/master/api/proxy/configs/staging.json?host=api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com" | jq -r '.proxy_configs[0].proxy_config.content.proxy.policy_chain | map(select(.name=="keycloak_role_check")) | .[]'

{
  "name": "keycloak_role_check",
  "version": "builtin",
  "configuration": {
    "type": "whitelist",
    "whitelist_deny_unmatched": false,
    "scopes": [
      {
        "client_roles": [
          {
            "client": "{{ jwt.azp }}",
            "name": "my-role",
            "name_type": "plain",
            "client_type": "liquid"
          }
        ],
        "realm_roles": [],
        "resource": "/echo/protected",
        "methods": [
          "ANY"
        ],
        "resource_type": "plain"
      }
    ]
  }
}

curl -k -i -H "Authorization: Bearer $ACCESS_TOKEN" https://api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com/echo/protected
curl -k -i -H "Authorization: Bearer $ACCESS_TOKEN" https://api-3scale-apicast-staging.apps.burbanik-3scale2.cp.fyre.ibm.com/echo

With authorized application: 200 response for both
With not-authorized application: 403 for /echo/protected, 200 response for /echo

[tkan145]

The current PR shape is unnecessary complicated and confused. For example, if I have type set to black_list then what is the different between "type_defined" and "allow"?

Typically, you would only want the policy react on what is being configured, the black_list is behave as expected and I don't see the need to touch it. The problem this JIRA is trying to address is that when using whitelist, for paths not configured, the policy is making a big assumption that no roles are whitelisted and denied all traffics.

Arguably, a better assumption might be that if an admin didn't configure a path, there is no explicit rules for the path. With no explicit rules, the policy should ignore paths not matching any scope.

So, for this PR, I suggest to add a simple boolean value to the whitelist and enable by default (to maintain the current behavior). When set to false, the policy will only deny access to the configured path and allow access to other paths.

[borisurbanik]

Arguably, a better assumption might be that if an admin didn't configure a path, there is no explicit rules for the path. With no explicit rules, the policy should ignore paths not matching any scope.

So, for this PR, I suggest to add a simple boolean value to the whitelist and enable by default (to maintain the current behavior). When set to false, the policy will only deny access to the configured path and allow access to other paths.

Yes, that's a good simplification, the blacklist path is the confusing one. I've updated the policy to use a whitelist specific boolean and ignore it in the blacklist path.

[tkan145]

No spec or .t test covers "missing JWT + whitelist_deny_unmatched: false + a request path that matches a configured scope."

[tkan145]

Why are we doing this? This is authentication bypass

[borisurbanik]

Thank you for the review! To answer your question - this PR intentionally preserves the existing functionality when type is blacklist. The nil return was introduced to implement the whitelist_deny_unmatched flag where it needs to distinguish between no path matched and path matched but roles failed. You can verify that both nil and false result produce the same outcome in the blacklist branch compared to master in all configurations except when whitelist path is unmatched.

I agree that the blacklist implementation is very permissive, but it works in tandem with jwt_parser. Enforcing this in the keycloak policy would be inconsistent with jwt_parser.required: false. Note that if a service is using oauth, JWT will always be present. This code path is only possible if customers are explicitly injecting JWT with jwt_parser to requests authenticated with api key.

Please open a separate JIRA if the policy should deny blacklist when no JWT is in the context.

[borisurbanik]

No spec or .t test covers "missing JWT + whitelist_deny_unmatched: false + a request path that matches a configured scope."

Thanks for the review, I've added missing JWT tests to specs for whitelist for combinations of matched/unmatched paths and whitelist_deny_unmatched configurations.