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/27 19:56:56 UTC

[GitHub] [apisix] jenskeiner opened a new issue #2880: feat: Make HTTP headers set by opened-connect plugin more configurable.

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


   ## Background
   The `openid-connect` plugin, when configured accordingly, may obtain an _ID token_, a _user info_ JSON object, and an _access token_, respectively, on behalf of the requestor by automatically redirecting to the ID provider. These tokens are then stored in a session cookie.
   
   Additionally, the plugin sets the HTTP header `X-ID-Token` to the value of the ID token, the header `X-Access-Token` to the value of the access token, and the header `X-Userinfo` to the user info data, for downstream handlers.
   
   Above code path requires that the respective tokens are already stored in a session (cookie). There is an alternative code path where an incoming request already contains an access (bearer) token in the `Authorization` header. In this case, the token is validated and the plugin then sets the header `X-Userinfo` to either the decoded token value (when validating directly against a public key) or the results of the introspection endpoint of the ID provider.
   
   Importantly, in this code path, I believe it is possible to have different information being put into the header, depending on how the validation is performed. This does not necessarily break since access token and introspection result may contain overlapping and entirely identical information, but an access token is not guaranteed to be identical to the introspection result, so breaks could occur where they differ.
   
   ## Issues with the current solution
   
   1. As noted above, in one code path, the `X-Userinfo` header contains the user info data, and in another one, the same header contains the opaque access token. This looks inconsistent and I would expect downstream handlers to break if they depend on this being a legitimate introspection endpoint response data and the access tokens happen to use an incompatible  format.
   
   2. The code path where the tokens are already stored in a session cookie sets three additional headers (with entirely redundant information from the session cookie) and may increase the request size substantially. This may be inefficient where downstream handlers actually don't need all three tokens. This can also lead to breaks where request header size is constrained.
   
   3. Downstream handlers, e.g. the `authz-keycloak` plugin or actual backend services typically expect an access token to be sent in the `Authorization` header like `Authorization: bearer <token>`, but not the non-standard `X-Access-Token`. As a result, they may fail to extract the token from the incoming request, causing breaks.
   
   ## Suggested changes
   
   1. Make setting the HTTP headers configurable, so that one can choose which tokens (if available) should actually be passed to downstream handlers in the respective headers.
   
   2. Add a configuration option that allows the access token to be set in the `Authorization` header instead of `X-Access-Token`.
   
   3. When the incoming request already contains a bearer token (in the `Authorization` header), additionally set the `X-Access-Token` header to the same value if that's the configured format. Otherwise, don't add the header.
   
   4. When the incoming request already contains a bearer token (in the `Authorization` header), only set the `X-Userinfo` header to the introspection result if the header was enabled as per configuration and the introspection endpoint was actually called. If, however, the token is validated against a public key, then don't set the `X-Userinfo` header to the decoded token value since the token is opaque.
   
   I would appreciate any thoughts on above issues and suggested solution.


----------------------------------------------------------------
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 #2880: feat: Make HTTP headers set by opened-connect plugin more configurable.

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


   PR is welcome.


----------------------------------------------------------------
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 #2880: feat: Make HTTP headers set by openid-connect plugin more configurable.

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


   Solved by #2903.


----------------------------------------------------------------
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 closed issue #2880: feat: Make HTTP headers set by openid-connect plugin more configurable.

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


   


----------------------------------------------------------------
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 #2880: feat: Make HTTP headers set by openid-connect plugin more configurable.

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


   > PR is welcome.
   
   Ok, will open one.


----------------------------------------------------------------
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