feat(jwt) add oauth2 token to ngx.ctx

[ikogan]

Summary

Store the Access Token in the context so other plugins can have access to it without reparsing it and without breaking "hide credentials".

Full changelog

• Store the access token in the context along with the credential.

[p0pr0ck5]

@ikogan can you enlighten us a bit to your use case here? Are there third-party plugins that would make use of this value?

Should we also consider extending this logic to other authentication plugins?

[ikogan]

I'm currently working on a plugin that exposes an API to allow administrators to impersonate users logged in through OAuth. The way this is done is by adding an entry in a new table connected to the OAuth token with which the user logs in, and updates the authenticated_userid header, after the OAuth plugin sets it, to the desired user we want to impersonate.

While I could pull out the token from the headers, re-parse it, and search that way; it's significantly slower, performance wise, duplicates header parsing code, and creates several edge cases (what if the OAuth plugin is hiding credentials?).

[p0pr0ck5]

Should we also consider extending this logic to other authentication plugins?

@ikogan any comment ^?

[ikogan]

Well, I suppose the answer to that is "perhaps"? I guess that depends on the question too. Are we talking about simply looking at each plugin and seeing what additional information they should store or are we talking about more generic logic to store plugin specific things?

I think the scope of the question is a bit broad. For example, I would love if the ldap-auth plugin did a search and stored user attributes. In fact, most of https://github.com/ohioit/kong-userinfo-plugin could be super ceded by that kind of logic but again, it seems like something that would have to be decided on for each plugin.

Spring Security has the concept of an "authentication context" which, in the Java world, is a base class that authentication libraries can extend to provide their own functionality. While that may be a useful model, it wouldn't translate directly here. What might; though, is to simply store the decoded authentication information in such a context. This could include the HMAC parameters or JWT claims? For BASIC and DIGEST auth, should it include the user's password though?

I guess one of my issues is that I'm not sufficiently familiar with the architectural direction of Kong to really make any kind of call like that. Could this be done for other authentication plugins, sure? Should it be? I'm not sure if I'm qualified to answer that.

[albertored]

I just saw this PR, but yesterday I opened an analogue PR for the jwt plugin (#2988).

Since it is exactly the same change with exactly the same reasons I think we can use the same notation in both plugins.

I agree on moving the assignation into the set_consumer function but maybe we can use different names for the ngx.ctx token key so that a successive plugin can know which type of token is present. For example ngx.ctx.authenticated_oauth2_token and ngx.ctx.authenticated_jwt_token.

Let me know what do you think.

[ikogan]

Is there a situation where we wouldn't care about the token type and it would be useful to reference it in a generic way?

[albertored]

Maybe yes but in that case the plugin that needs to access the token from ngx.ctx can implement its own logic for retrieving one of the possible many tokens.

If instead we use the same key the plugin that needs to access the token from ngx.ctx should implement some more complex logic to detect the token type (if it needs to know the type).

[ikogan]

Yeah, I agree, I've updated the PR with your suggestion. Based on my longer answer above, a more generic implementation would need a lot more design and direction so I think this is significantly simpler.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[hbagdi]

Closing because of lack of activity. We would like to accept this contribution if anyone wishes to pick this back up.