fix: openid-connect plugin check token request against granted scopes #9127

whatvn · 2023-03-21T07:12:55Z

Description

Current openid connect plugin does not check for access token scope permission. For example, a client-id clientX has been granted for scope-a, apisix with openid connect has 2 routes set up with scope-a and scope-b for different client-id.

ClientX then use client-id and client-secret to get access-token to request for service defined with scope-a, this access token can still be used to access service defined with scope-b since this plugin does not check for scope when doing oidc introspect.

For example, a client request with token ory_at_l3PFHHNGOrGKGaM8t0arZFCV4_UFZv5630Zn60E-6JQ.O7eE2aLhB4bkGgJ8QbeHamc2sDJj3aTYKtUpkiCSgsM with following configuration, accessing route with defined scope default-nginx-page-3

{
  "set_access_token_header": true,
  "client_secret": "apisix-secret",
  "access_token_in_authorization_header": false,
  "scope": "default-nginx-page-3",
  "set_id_token_header": true,
  "unauth_action": "auth",
  "bearer_only": false,
  "set_userinfo_header": true,
  "set_refresh_token_header": false,
  "ssl_verify": "no",
  "timeout": 3000,
  "realm": "master",
  "logout_path": "/logout",
  "introspection_endpoint_auth_method": "client_secret_basic",
  "redirect_uri": "https://httpbin.org/get",
  "discovery": "http://127.0.0.1:4444/.well-known/openid-configuration",
  "client_id": "apisix-id",
  "introspection_endpoint": "http://127.0.0.1:4445/admin/oauth2/introspect",
  "use_pkce": false,
  "session": {
    "secret": "Qt1Wx+r8sl6GnFMzqfyZyzbpPHkbhN1b4wjkU451L7Q="
  }
}

Route configuration is as following

{
      "modifiedIndex": 87,
      "key": "/apisix/routes/2",
      "value": {
        "create_time": 1679369373,
        "upstream_id": "3",
        "methods": [
          "GET"
        ],
        "priority": 0,
        "id": "2",
        "update_time": 1679375413,
        "status": 1,
        "uri": "/index3.html",
        "name": "Route with OIDC plugin",
        "plugins": {
          "openid-connect": {
            "set_access_token_header": true,
            "client_secret": "apisix-secret",
            "access_token_in_authorization_header": false,
            "scope": "default-nginx-page-3",
            "set_id_token_header": true,
            "unauth_action": "auth",
            "bearer_only": false,
            "set_userinfo_header": true,
            "set_refresh_token_header": false,
            "ssl_verify": false,
            "timeout": 3,
            "realm": "master",
            "logout_path": "/logout",
            "introspection_endpoint_auth_method": "client_secret_basic",
            "redirect_uri": "https://httpbin.org/get",
            "discovery": "http://127.0.0.1:4444/.well-known/openid-configuration",
            "client_id": "apisix-id",
            "introspection_endpoint": "http://127.0.0.1:4445/admin/oauth2/introspect",
            "use_pkce": false,
            "session": {
              "secret": "Qt1Wx+r8sl6GnFMzqfyZyzbpPHkbhN1b4wjkU451L7Q="
            }
          }
        }
      },
      "createdIndex": 19
    }

While client-id who owns this access token does not have scope default-nginx-page-3 granted.

CLIENT ID				CLIENT SECRET	GRANT TYPES		RESPONSE TYPES	SCOPE			AUDIENCE	REDIRECT URIS
b01d7b27-68dc-441c-afd3-7c828c7c479d			client_credentials	code		default-nginx-page  default-nginx-page-2

Since it can access route which defined with scope default-nginx-page and default-nginx-page-2, it also can access route configured with other scope.

curl http://127.0.0.1:9080/index3.html -i -H "Authorization: Bearer ory_at_l3PFHHNGOrGKGaM8t0arZFCV4_UFZv5630Zn60E-6JQ.O7eE2aLhB4bkGgJ8QbeHamc2sDJj3aTYKtUpkiCSgsM"
HTTP/1.1 404 Not Found
Content-Type: text/html; charset=utf-8
Content-Length: 146
Connection: keep-alive
Date: Tue, 21 Mar 2023 05:45:35 GMT
Server: APISIX/3.2.0

<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>

This fixes issue mentioned above by also checking allowed scope of access token with granted scope in oidc. With this applied, above request will return 401

curl http://127.0.0.1:9080/index3.html -i -H "Authorization: Bearer ory_at_l3PFHHNGOrGKGaM8t0arZFCV4_UFZv5630Zn60E-6JQ.O7eE2aLhB4bkGgJ8QbeHamc2sDJj3aTYKtUpkiCSgsM"
HTTP/1.1 401 Unauthorized
Date: Tue, 21 Mar 2023 05:59:53 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 176
Connection: keep-alive
Server: APISIX/3.2.0

<html>
<head><title>401 Authorization Required</title></head>
<body>
<center><h1>401 Authorization Required</h1></center>
<hr><center>openresty</center>
</body>
</html>

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

…heck

whatvn · 2023-03-22T03:07:01Z

@starsz @spacewander please take a look, thank!

starsz · 2023-03-22T09:42:03Z

@starsz @spacewander please take a look, thank!

Hi, @whatvn I don't see any doc to introduce that we can add scope in the request body of the introspection request.
But I can see that we can get the scope from the introspection response, and it's optional.

Can you give me some information about this? Thanks very much.

whatvn · 2023-03-22T10:35:07Z

@starsz according to rfc7662 https://www.rfc-editor.org/rfc/rfc7662#section-2.1

The introspection endpoint MAY accept other OPTIONAL parameters to
   provide further context to the query.  For instance, an authorization
   server may desire to know the IP address of the client accessing the
   protected resource to determine if the correct client is likely to be
   presenting the token.  The definition of this or any other parameters
   are outside the scope of this specification, to be defined by service
   documentation or extensions to this specification.  If the
   authorization server is unable to determine the state of the token
   without additional information, it SHOULD return an introspection
   response indicating the token is not active as described in
   [Section 2.2](https://www.rfc-editor.org/rfc/rfc7662#section-2.2).

Client can also provide additional parameter in order to let authorization server to optimize search query. That's why in oidc resty module which we are using, it allows us to add scope to introspect the token.

With your question, I think I may also add another check, to check in case authorization (other than Ory Hydra that I am testing) does not support for scope in request, we still can be able to check granted scope. What do you think?

starsz · 2023-03-23T02:30:02Z

https://www.rfc-editor.org/rfc/rfc7662#section-2.1

Thanks for providing the RFC docs. I agree with you.
But maybe it's a broken change, so we should add an option to control this changes.

whatvn · 2023-03-23T07:19:11Z

@starsz I add options to control this change, also update doc (en) to clarify new options and behaviour. Please take a look, thank!

starsz · 2023-03-24T03:53:27Z

@starsz I add options to control this change, also update doc (en) to clarify new options and behaviour. Please take a look, thank!

Hi, @whatvn Can you add test case of this?

whatvn · 2023-03-24T09:09:28Z

@starsz let me try again, I tried to run current test cases on my dev machine followed document to setup test but it failed everytime.

starsz · 2023-03-28T09:07:46Z

@starsz let me try again, I tried to run current test cases on my dev machine followed document to setup test but it failed everytime.

What's the error that your dev machine report?

whatvn · 2023-03-30T08:26:46Z

@starsz please find output of command in attached file.

prove -I. -I../test-nginx/inc -I../test-nginx/lib -r t/plugin/openid-connect.t  >t.log 2>&1

t.log

Also there's error about random seed in error log

2023/03/30 15:41:23 [debug] 22379#22379: [lua] patch.lua:114: randomseed(): Random seed has been inited
stack traceback:
        /root/hungnv4/apisix/deps/share/lua/5.1/resty/jit-uuid.lua:64: in function 'seed'
        /root/hungnv4/apisix/apisix/core/id.lua:81: in function 'init'
        /root/hungnv4/apisix/apisix/init.lua:85: in function 'http_init'
        init_by_lua:15: in main chunk
2023/03/30 15:41:23 [notice] 22379#22379: [lua] id.lua:83: init(): not found apisix uid, generate a new one: 8eec179e-973a-47b7-ba06-141438cfb406
....
....
2023/03/30 15:41:23 [debug] 22381#22381: *2 [lua] patch.lua:114: randomseed(): Random seed has been inited
stack traceback:
        ...ngnv4/apisix/deps/share/lua/5.1/resty/rocketmq/utils.lua:15: in main chunk
        [C]: in function 'require'
        ...ungnv4/apisix/deps/share/lua/5.1/resty/rocketmq/core.lua:2: in main chunk
        [C]: in function 'require'
        ...v4/apisix/deps/share/lua/5.1/resty/rocketmq/producer.lua:1: in main chunk
        [C]: in function 'require'
        /root/hungnv4/apisix/apisix/plugins/rocketmq-logger.lua:19: in main chunk
        [C]: at 0x7fdc741dd740
        [C]: in function 'pcall'
        /root/hungnv4/apisix/apisix/plugin.lua:131: in function 'load_plugin'
        /root/hungnv4/apisix/apisix/plugin.lua:221: in function 'load'
        /root/hungnv4/apisix/apisix/plugin.lua:340: in function 'load'
        /root/hungnv4/apisix/apisix/plugin.lua:747: in function 'init_worker'
        /root/hungnv4/apisix/apisix/init.lua:148: in function 'http_init_worker'
        init_worker_by_lua:2: in main chunk
2023/03/30 15:41:23 [debug] 22382#22382: *1 [lua] patch.lua:114: randomseed(): Random seed has been inited
stack traceback:
        ...ngnv4/apisix/deps/share/lua/5.1/resty/rocketmq/utils.lua:15: in main chunk
        [C]: in function 'require'
        ...ungnv4/apisix/deps/share/lua/5.1/resty/rocketmq/core.lua:2: in main chunk
        [C]: in function 'require'
        ...v4/apisix/deps/share/lua/5.1/resty/rocketmq/producer.lua:1: in main chunk
        [C]: in function 'require'
        /root/hungnv4/apisix/apisix/plugins/rocketmq-logger.lua:19: in main chunk
        [C]: at 0x7fdc741dd740
        [C]: in function 'pcall'
        /root/hungnv4/apisix/apisix/plugin.lua:131: in function 'load_plugin'
        /root/hungnv4/apisix/apisix/plugin.lua:221: in function 'load'
        /root/hungnv4/apisix/apisix/plugin.lua:340: in function 'load'
        /root/hungnv4/apisix/apisix/plugin.lua:747: in function 'init_worker'
        /root/hungnv4/apisix/apisix/init.lua:148: in function 'http_init_worker'
        init_worker_by_lua:2: in main chunk

whatvn · 2023-04-03T08:30:16Z

@starsz I built and run test again on different clean dev machine, still got this error. Any idea?

starsz · 2023-04-03T09:46:50Z

t.log

Look strange, can you merge the master branch and try again?

whatvn · 2023-04-04T04:01:56Z

@starsz I run the test on master, not my branch. Just merge master again with updates and test is still fail

whatvn · 2023-04-05T09:18:50Z

@starsz question, how do we setup keycloak to perform the test? Do we have document on how to setup test for openid-connect? Thank

starsz · 2023-04-06T02:08:11Z

@starsz question, how do we setup keycloak to perform the test? Do we have document on how to setup test for openid-connect? Thank

Hi, @whatvn. As you can see in the workflow:

https://github.com/apache/apisix/blob/master/ci/pod/docker-compose.plugin.yml

APISIX will start keycloak by docker-compose.
The docker-compose file is here: https://github.com/apache/apisix/blob/master/ci/pod/docker-compose.plugin.yml

Do we have document on how to setup test for openid-connect.

We have a blog of Getting Started with APISIX Test.

github-actions · 2023-11-20T10:04:40Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

csotiriou · 2023-11-27T18:49:08Z

Maybe this PR is less relevant now that #10493 has been merged?

github-actions · 2024-01-28T10:04:15Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

shreemaan-abhishek · 2024-02-02T02:11:35Z

closing in favour of #10493

hungnv added 2 commits March 21, 2023 13:56

verify access token against granted scopes for request client-id

d65f928

remove local cjson and use core json

bdc0b1e

whatvn changed the title ~~openid-connect plugin - check token request against granted scopes~~ fix: openid-connect plugin check token request against granted scopes Mar 21, 2023

Nguyễn Văn Hưng added 2 commits March 22, 2023 10:01

include introspection_params in request to remove unneccessary loop c…

df8a1e6

…heck

remove new local variable

a4cb671

add configuration for scope checking, update doc (en)

1598030

Merge branch 'apache:master' into master

0455f55

monkeyDluffy6017 added the need test cases label May 18, 2023

github-actions bot added the stale label Nov 20, 2023

github-actions bot removed the stale label Nov 28, 2023

github-actions bot added the stale label Jan 28, 2024

shreemaan-abhishek closed this Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: openid-connect plugin check token request against granted scopes #9127

fix: openid-connect plugin check token request against granted scopes #9127

whatvn commented Mar 21, 2023 •

edited

Loading

whatvn commented Mar 22, 2023

starsz commented Mar 22, 2023 •

edited

Loading

whatvn commented Mar 22, 2023

starsz commented Mar 23, 2023

whatvn commented Mar 23, 2023

starsz commented Mar 24, 2023

whatvn commented Mar 24, 2023 •

edited

Loading

starsz commented Mar 28, 2023

whatvn commented Mar 30, 2023 •

edited

Loading

whatvn commented Apr 3, 2023

starsz commented Apr 3, 2023

whatvn commented Apr 4, 2023

whatvn commented Apr 5, 2023

starsz commented Apr 6, 2023

github-actions bot commented Nov 20, 2023

csotiriou commented Nov 27, 2023

github-actions bot commented Jan 28, 2024

shreemaan-abhishek commented Feb 2, 2024

fix: openid-connect plugin check token request against granted scopes #9127

fix: openid-connect plugin check token request against granted scopes #9127

Conversation

whatvn commented Mar 21, 2023 • edited Loading

Description

Checklist

whatvn commented Mar 22, 2023

starsz commented Mar 22, 2023 • edited Loading

whatvn commented Mar 22, 2023

starsz commented Mar 23, 2023

whatvn commented Mar 23, 2023

starsz commented Mar 24, 2023

whatvn commented Mar 24, 2023 • edited Loading

starsz commented Mar 28, 2023

whatvn commented Mar 30, 2023 • edited Loading

whatvn commented Apr 3, 2023

starsz commented Apr 3, 2023

whatvn commented Apr 4, 2023

whatvn commented Apr 5, 2023

starsz commented Apr 6, 2023

github-actions bot commented Nov 20, 2023

csotiriou commented Nov 27, 2023

github-actions bot commented Jan 28, 2024

shreemaan-abhishek commented Feb 2, 2024

whatvn commented Mar 21, 2023 •

edited

Loading

starsz commented Mar 22, 2023 •

edited

Loading

whatvn commented Mar 24, 2023 •

edited

Loading

whatvn commented Mar 30, 2023 •

edited

Loading