feat: Add support for Introspection based on headers #2585
Conversation
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
|
@tuunit ping |
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
|
Starting to create some tests. |
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
| if opts.SkipJwtBearerTokens { | ||
| sessionLoaders := []middlewareapi.TokenToSessionFunc{ | ||
| provider.CreateSessionFromToken, | ||
| if opts.SkipJwtBearerTokens || opts.SkipBearerTokens { |
There was a problem hiding this comment.
We already considered renaming this flag in another PR. Please don't introduce a second SkipBearerToken. Just remove JWT from the current flag.
There was a problem hiding this comment.
On the other hand this will break current functionality. So maybe we need to keep the old one and mark it as deprecated.
There was a problem hiding this comment.
I put it in based on a comment in the PR, based on your suggestion I think :-)
| @@ -35,6 +35,10 @@ type RequestScope struct { | |||
| // ClearSession indicates whether the user should be logged out or not. | |||
| ClearSession bool | |||
|
|
|||
| // IntrospectToken indicates whether to introspect the session token. | |||
| // This is set if the request has the header `X-Oauth2-Proxy-Introspect-Token`. | |||
There was a problem hiding this comment.
Is this approach wise? Shouldn't the introspection always happen? This way the client decides when to redo the introspection, right?
There was a problem hiding this comment.
Unless the client is Nginx, my use case here is to choose what to introspect. I haven't given much thought about cli options with this flag, maybe add --introspection-enable-header which enables this behavior.
E.g. if you want to introspect all write queries, but not read. Or just /admin.
There was a problem hiding this comment.
I added the flag, make sense!
I guess it should however also enable the sessiontoken creater.
There was a problem hiding this comment.
I made it such that my header introspection updates the session.
Could we enable that always for --introspect-token and skip the createSessionFromIntrospectedToken?
There was a problem hiding this comment.
I made a patch doing that. It's wip though, needs testing obviously.
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
Signed-off-by: Josef Johansson <[email protected]>
|
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
|
Hello, Thank you very much all for your work. Thanks in advance :-) |
|
Nice feature! It would be amazing to get this merged! |
|
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
|
Should really see this through. The only thing left is to add some test.. not sure how to do them though. |
|
@tuunit I'm unsure how to make this merge-able. Would like to help me out a bit? What tests should I implement? |
isodude
changed the title
Set `X-Oauth2-Proxy-Introspect-Token: true' on requests that you want to be introspected. Will affect latency.
Description
Adding the ability to introspect the token when a specific header is set in the incoming request. From what I have come to understand, it's not possible to use Introspection on PKCE (https://salesforce.stackexchange.com/a/299171).
This uses the introspection url already provided by the OIDC endpoint.
Motivation and Context
In e.g. Nginx this makes it possible to make really sure that certain operations validate the token the whole way.
How Has This Been Tested?
Checklist: