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

[THREESCALE-2236] Add methods option on Keycloak policy. #1039

Merged
merged 3 commits into from
May 21, 2019

Conversation

eloycoto
Copy link
Contributor

This commits add the methods option on the keycloack policy. That
allow users to define a new policy from any jwt claim, resource and
method.

To be backwards compatible 'ANY' method is in place, and if methods are
not defined this global method will be used and all will work as normal.

Example policy:

"policy_chain": [
  {
    "name": "apicast.policy.keycloak_role_check",
    "configuration": {
      "scopes": [
        {
          "realm_roles": [ { "name": "director" } ],
          "resource": "/confidential",
          "methods": ["POST"]
        }
      ],
      "type": "blacklist"
    }
  },
  { "name": "apicast.policy.apicast" }
]

Signed-off-by: Eloy Coto [email protected]

@eloycoto eloycoto changed the title [THREESCALE-2236] Add methods option on keycloack policy. [THREESCALE-2236] Add methods option on keycloak policy. May 15, 2019
@eloycoto eloycoto force-pushed the THREESCALE-2236 branch 3 times, most recently from a175403 to ef24d2d Compare May 15, 2019 15:31
@eloycoto eloycoto changed the title [THREESCALE-2236] Add methods option on keycloak policy. [THREESCALE-2236] Add methods option on Keycloak policy. May 15, 2019
@eloycoto eloycoto marked this pull request as ready for review May 15, 2019 15:37
@eloycoto eloycoto requested a review from a team as a code owner May 15, 2019 15:37
@@ -393,3 +393,160 @@ yay, api backend
--- no_error_log
[error]
oauth failed with


=== TEST6: Role check with allow methods
Copy link
Contributor

@davidor davidor May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a space between TEST and 6 ? It's just because I don't know if that can cause some issue with the script that we have to automatically number tests correctly. https://github.com/3scale/APIcast/blob/master/script/reorder-tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated all the test.

@davidor davidor added this to the 3.6 milestone May 16, 2019
@eloycoto eloycoto force-pushed the THREESCALE-2236 branch 2 times, most recently from 2861336 to 12e595b Compare May 17, 2019 07:41
@davidor
Copy link
Contributor

davidor commented May 17, 2019

Good job @eloycoto 👍

I think that extracting a Scope module might simplify some code and also the tests, but there's no need to do that now. We can evaluate that in the future if we need to add more functionality to this policy.

I think the PR could be merged after #1041

eloycoto added 3 commits May 21, 2019 07:31
This commits add the `methods` option on the Keycloak policy. That
allow users to define a new policy from any jwt claim, resource and
method.

To be backwards compatible 'ANY' method is in place, and if methods are
not defined this global method will be used and all will work as normal.

Example policy:

```
"policy_chain": [
  {
    "name": "apicast.policy.keycloak_role_check",
    "configuration": {
      "scopes": [
        {
          "realm_roles": [ { "name": "director" } ],
          "resource": "/confidential",
          "methods": ["POST"]
        }
      ],
      "type": "blacklist"
    }
  },
  { "name": "apicast.policy.apicast" }
]
```

Signed-off-by: Eloy Coto <[email protected]>
This commits adds a ANY method to match any method in the mapping_rule
so allow all and only block by the request uri.

Signed-off-by: Eloy Coto <[email protected]>
@eloycoto
Copy link
Contributor Author

Code rebased with master branch, should be ready to merge.

@davidor davidor merged commit 9212c8d into 3scale:master May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants