Make extraFields check configurable by the user

[viveksyngh]

Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

We are using Pinniped Concierge to validate webhook tokens and our webhook authenticator returns extra fields in TokenReview as user metadata. We have seen that the authentication fails because of these extra fields. Below is the piece of code which checks and fails if these fields are set. https://github.com/vmware-tanzu/pinniped/blob/main/internal/registry/credentialrequest/rest.go#L176-L179

We are using it for something nothing to do with Pinniped, we offload auth to our webhook for another service, and use extra info for that service. Dropping it makes no difference to the privileges you get in k8s. This just means we have to make less desirable changes to our code just for Pinniped, which we don't use in all environments.

Describe the solution you'd like

It would be really nice if we can make the above check configurable. The check is enabled by default so it is secure by default but can be configured by the users to disable these checks if they are not using these fields for any kind of privileges in Kubernetes.

Describe alternatives you've considered

NA

Are you considering submitting a PR for this feature?

• How will this project improvement be tested?
• How does this change the current architecture?
• How will this change be backwards compatible?
• How will this feature be documented?

Additional context

Add any other context or screenshots about the feature request here.

[cfryanr]

Hi @viveksyngh,

If you're interested in working on a PR for this, here are some pointers to help you get started. I would expect that this would be an optional setting on the spec of the WebhookAuthenticator. Let me know if you had something else in mind.

1. The WebhookAuthenticator CRD is defined here. After making any changes to a .tmpl file, run ./hack/update.sh to invoke the code generator.
2. This controller watches all WebhookAuthenticator CRs in the appropriate namespace and loads them into an in-memory cache whenever they change.
3. The in-memory cache of authenticators implements a function which, given the name of an authenticator inside a TokenCredentialRequest, chooses the authenticator from the cache and uses it to authenticate the token from the TokenCredentialRequest. The trick is that the cache can also include other kinds of authenticators aside from WebhookAuthenticators (e.g. JWTAuthenticators, and maybe more in the future).
4. The rest.go file that you already mentioned implements the TokenCredentialRequest API by asking the authenticators cache mentioned above to perform the authentication.

So the trick would be to come up with a way to express an interface that is only needed today by a single type of authenticator (WebhookAuthenticator) to express that the individual instance of the authenticator would like to skip (turn off) the userinfo extras validation. This would keep that cache code generic, while allowing other instances of the same type of authenticator (other WebhookAuthenticators) to keep the check on (the default), and allowing other kinds of authenticators (JWTAuthenticator and future authenticators) to always keep the check on (with no configurable way to turn it off).

Of course, many of these files have units tests that would need to be updated, and it may be valuable to enhance the integration tests to make sure that the feature works. Although I might suggest proposing a possible implementation plan before investing a lot of effort in writing tests.