When backend receives a valid Oauth2 Client, headers that are null should not be set and passed.
One other thing I wanted to bring up, X-Consumer-Groups is not a valuable header to us, would kong accept a PR boolean to Oauth2 that disables this header? Think about it. A consumer at scale with an ACL to each proxy(if you do it like that) ends up being 100-200+ uuids thrown in this header, not super performant. Would Kong accept a PR to enable/disable that in the Oauth2 schema?
X-Consumer-ID: 8aa31296-a4ec-4cb4-bfc7-64a5428b3c65
X-Consumer-Custom-ID: userdata: NULL
X-Consumer-Username: admin
X-Consumer-Groups: c3af0477-4747-4700-b9bc-520d158fdc15, 614c8995-a153-408b-b67d-06f0fc524898
~X-Authenticated-Groups(?), do you mean X-Authenticated-Scope?~ okay got it, you mean ACLplugin.
@jeremyjpj0916 I would give a + for such PR on ACL plugin. I think we also need ngx.ctx.authenticated_groups that any plugin can set (such as OpenID Connect or LDAP plugins), so that they can also be used together with ACL plugin (ACL plugin then first checks that, and after that it checks if there is a consumer, like it is now).
@bungle yeah I was looking at the OAuth2 code the other day and realized X-Consumer-Groups header had to deal with ACL plguin code and not OAuth2 in my suggestion. So glad to hear you think that would be an appropriate change as each API Provider does not need to know all the groups(which really represent either access to one or many proxies) a consumer has access to(must be a specific use case yall were thinking about when it was designed). And as for X-Consumer-Custom-ID: userdata: NULL being populated by the OAuth2 plugin you would consider that to be a bug right and a PR to the OAuth2 plugin checking for nil before assignment is appropriate?
Yes, that is a bug, that is lurking around in a lot of places as new dao (kong.db) returns ngx.null (that doesn't evaluate false). And consumer entity was just moved to new. Old dao returned nil in such cases, and evaluated false.
Edit... the placeholder for the comment I had here is already resolved with merged PR! null in the other header value this issue was originally opened for may still persist.
@bungle Just checking, your PR here: https://github.com/Kong/kong/pull/3710 fixes this issue as well as #3617 right? Just if you wanted to bundle and close them together for that same PR for documentation purposes.
@jeremyjpj0916, Yes, I think so. Thanks!