You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/16 11:35:35 UTC

[GitHub] [apisix] jenskeiner opened a new issue #2764: bug: OIDC plugin may not be setting request headers correctly.

jenskeiner opened a new issue #2764:
URL: https://github.com/apache/apisix/issues/2764


   ### Issue description
   When using the OIDC plugin, it looks like request headers with the user name/access token/id_token may not be set correctly in the code path where an incoming request is successfully authenticated against an OIDC server.
   
   According to line https://github.com/apache/apisix/blob/4d316bc0681c80435ab1a3fc113c3c6ea3c82831/apisix/plugins/openid-connect.lua#L161 in the code, the `openidc.authenticate` method is called with two expected return values, a response and an error object. The user name and tokens are then extracted from the response object and set as headers on the HTTP request.
   
   However, the method that is called actually seems to return four values. If the incoming request had no access token set in a header field or in a cookie, the return value seems to come from https://github.com/zmartzone/lua-resty-openidc/blob/df75c6e26cb7dbc9d78927d989e98be04173f10b/lib/resty/openidc.lua#L1189 where the first two return values are actually `nil`, the third is original request URL, and the fourth is the session object.
   
   The session object turns out to actually hold the user name and tokens.
   
   Essentially, the current plugin code seems to incorrectly look in `nil` values for the user name and tokens to add them as request headers, where it should actually be checking the returned session object.
   
   ### Environment
   
   * apisix version (cmd: `apisix version`): 2.0
   * OS: Alpine Linux Docker image
   
   ### Minimal test code / Steps to reproduce the issue
   1. Add the openid-connect plugin to protect any route.
   2. In the plugin configuration, specify the fields `client_id`, `client_secret`, `discovery`, `real`, `introspection_endpoint`, and `redirect_uri` accordingly.
   3. Verify that the plugin doesn't go through the code path where it would extract the user name and tokens and add corresponding headers; see https://github.com/apache/apisix/blob/4d316bc0681c80435ab1a3fc113c3c6ea3c82831/apisix/plugins/openid-connect.lua.
   
   ### What's the actual result? (including assertion message & call stack if applicable)
   Expected headers don't get add to HTTP request, as explained above.
   
   
   ### What's the expected result?
   HTTP headers for user name, access_token, and id_token are added to the request.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] jenskeiner commented on issue #2764: bug: OIDC plugin may not be setting request headers correctly.

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on issue #2764:
URL: https://github.com/apache/apisix/issues/2764#issuecomment-728837269


   @spacewander I'm open to submitting a PR, but would need to set up a proper development environment on my end first. I was hoping original developers would be quicker to fix for that reason, but I'll see what I can do. Can't guess an ETA at the moment.
   
   Also, the reason I noticed the issue was when I tried to combine this plugin with the `authz-keycloak` one. Once a request is authenticated, subsequent requests from the user should contain an access and/or ID token in suitable headers or a cookie header. However, `authz-keycloak` only looks for a bearer token in the `Authorization` header and doesn't seem to consider cookie headers or the `X-Access-Token` header used by this plugin.
   
   Another issue to consider in a separate ticket is that `authz-keycloak` runs in the rewrite phase, so always before this plugin which runs in the access phase. This means that unauthenticated requests don't go through the OIDC Relying Party flow so that an access token can be obtained, but are rejected by `authz-keycloak` right away. I think `authz-keycloak` should always run after this one, and both plugins need to use a consistent scheme of handling tokens and other information.
   
   In the end, my goal is to have a clean flow that can re-direct unauthenticated requests to an identity provider and obtain tokens et cetera transparently, and after that, for authenticated requests, can use token contents and Keycloak to check authorization. I imagine this is a fairly common case for securing APIs.
   
   Any thoughts on these other issues?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] jenskeiner commented on issue #2764: bug: OIDC plugin may not be setting request headers correctly.

Posted by GitBox <gi...@apache.org>.
jenskeiner commented on issue #2764:
URL: https://github.com/apache/apisix/issues/2764#issuecomment-734480738


   I'll close this one as I've since confirmed that the headers are actually set correctly.
   
   I'll probably open a few more tickets and PRs to address other issues in the area.
   
   Thanks for help.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] jenskeiner closed issue #2764: bug: OIDC plugin may not be setting request headers correctly.

Posted by GitBox <gi...@apache.org>.
jenskeiner closed issue #2764:
URL: https://github.com/apache/apisix/issues/2764


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on issue #2764: bug: OIDC plugin may not be setting request headers correctly.

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #2764:
URL: https://github.com/apache/apisix/issues/2764#issuecomment-728750519


   @jenskeiner 
   Thanks for your detailed report. Would you submit a bugfix for it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on issue #2764: bug: OIDC plugin may not be setting request headers correctly.

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #2764:
URL: https://github.com/apache/apisix/issues/2764#issuecomment-728877972


   I suggest you to divide it into separate issues so that we can track them correctly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org