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

JWT claim check policy: uri was not escape correctly #1428

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

tkan145
Copy link
Contributor

@tkan145 tkan145 commented Nov 19, 2023

What:

Fix THREESCALE-10308

The uri was not escaped correctly with JWT claim check policy so the mapping rule will not work as expected.

For example: Mapping rule: /api/{id}/whatever is supposed to match /api/ 123/whatever and perform the JWT token check but due to the special character (space) the request will not match on resource and APIcast
skips the JWT token check.

Comparison the behavior before and after this PR

  • Mapping rule with capture group

Mapping rule:  /foo{id}/bar

Request Direct to APICast JWT Claim Check policy PR#1428
GET /foo/bar False False False
GET /foo1/bar True True True
GET /foo%201/bar True False True
  • Mapping rule with % character

Mapping rule:  /foo%20/bar

Request Direct to APICast JWT Claim Check policy PR#1428
GET /foo/bar False False False
GET /foo1/bar False False False
GET /foo%20/bar True False True
  • Normal mapping rule

Mapping rule:  /foo/bar

Request Direct to APICast JWT Claim Check policy PR#1428
GET /foo/bar True True True
GET /foo1/bar False False False
GET /foo%20/bar False False False

With:

True   : mapping rules match

False : mapping rules do not match

Verification steps

/groups/{groupid}/foo$
  • Configure a JWT Claim check policy with the following:
{                                                                           
  "name": "apicast.policy.jwt_claim_check",                                 
  "configuration": {                                                         
    "rules" : [{                                                             
        "operations": [                                                     
            {"op": "matches", "jwt_claim": "{{roles | join: '|'}}", "jwt_claim_type": "liquid",
            "value": "invalid"}                               
        ],                                                                   
        "combine_op": "and",                                                 
        "methods": ["GET"],                                                 
        "resource": "/groups/{groupid}/foo$"           
    }]                                                                       
  }
},     
  • Start dev environment
make development
make dependencies
  • Run apicast locally
THREESCALE_DEPLOYMENT_ENV=staging APICAST_LOG_LEVEL=debug APICAST_WORKER=1 APICAST_CONFIGURATION_LOADER=lazy APICAST_CONFIGURATION_CACHE=0 THREESCALE_PORTAL_ENDPOINT=https://token@3scale-admin.example.com ./bin/apicast
  • Generate token from a realm A
  • Run query with the valid jwt
# capture apicast IP
APICAST_IP=$(docker inspect apicast_build_0-development-1 | yq e -P '.[0].NetworkSettings.Networks.apicast_build_0_default.IPAddress' -)

curl -v -k -H "Host: example.com:443" -H "Accept: application/json" -H "Authorization: Bearer ${ACCESS_TOKEN}" "http://${APICAST_IP}:8080/groups/%201/foo"
  • The response should be HTTP/1.1 403 Forbidden
< HTTP/1.1 403 Forbidden                     
< Server: openresty                          
< Date: Tue, 28 Nov 2023 02:44:27 GMT        
< Content-Type: text/plain                   
< Transfer-Encoding: chunked                 
< Connection: keep-alive                     
<                                            
Request blocked due to JWT claim policy      
* Connection #0 to host 127.0.0.1 left intact

@tkan145 tkan145 marked this pull request as ready for review November 28, 2023 03:01
@tkan145 tkan145 requested a review from a team as a code owner November 28, 2023 03:01
@tkan145 tkan145 requested a review from eguzki November 28, 2023 03:02
@tkan145
Copy link
Contributor Author

tkan145 commented Nov 28, 2023

It seems like this is the only place we use context:get_uri(), does anyone know why we use context:get_uri() instead of ngx.var.uri?

@eguzki
Copy link
Member

eguzki commented Jan 15, 2024

It seems like this is the only place we use context:get_uri(), does anyone know why we use context:get_uri() instead of ngx.var.uri?

I wonder if this is the method being called https://github.com/3scale/APIcast/blob/master/gateway/src/apicast/policy/load_configuration/load_configuration.lua#L19-L23

get_uri = function(self)
      if self.route_upstream then
        return self.route_upstream.uri.path or "/"
      end
      return ngx.var.uri
    end,

So the routing policy could override the uri path?

Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

Verification steps worked.

I have also run same verification steps with the APIcast version of current master branch and the request hits upstream service

The uri was not escaped correctly so the mapping rule will not work
as expected.
@tkan145
Copy link
Contributor Author

tkan145 commented Jan 30, 2024

I wonder if this is the method being called https://github.com/3scale/APIcast/blob/master/gateway/src/apicast/policy/load_configuration/load_configuration.lua#L19-L23

Yes. that is the one.

Thank you for the code-review. I will merge this now.

@tkan145 tkan145 merged commit 29f8755 into 3scale:master Jan 30, 2024
12 checks passed
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.

2 participants