vmware-tanzu / pinniped

Pinniped is the easy, secure way to log in to your Kubernetes clusters.
https://pinniped.dev
Apache License 2.0
566 stars 66 forks source link

fix authenticators bug: stop allowing usage when validation fails #2013

Closed cfryanr closed 3 months ago

cfryanr commented 3 months ago

When the spec of a JWTAuthenticator or WebhookAuthenticator changes, and if the resource goes from being valid to being invalid, then that authenticator should not be used anymore. This PR removes it from the in-memory cache so it cannot be used during auth anymore.

This PR also reduces how often the WebhookAuthenticators are revalidated. When the spec has not changed since it was previously validated, then skip performing validations and leave it in the in-memory cache for usage during auth.

These changes also have the side benefit of making the code for controller for WebhookAuthenticators and the controller for JWTAuthenticator more similar/consistent.

Release note:

WebhookAuthenticators and JWTAuthenticators which were previously validated,
and then become invalid due to a spec change, are not considered usable for
authentication anymore. WebhookAuthenticators that are already validated by
a Concierge pod will not be validated again by that same pod unless the spec
changes or the pod restarts, to reduce the number of TCP dials to the remote
webhook server.
codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 98.82353% with 1 line in your changes missing coverage. Please review.

Project coverage is 30.73%. Comparing base (6b722a1) to head (a2be4b7).

Files Patch % Lines
...ler/authenticator/jwtcachefiller/jwtcachefiller.go 97.36% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2013 +/- ## ========================================== + Coverage 30.68% 30.73% +0.04% ========================================== Files 365 365 Lines 60594 60644 +50 ========================================== + Hits 18591 18636 +45 - Misses 41468 41472 +4 - Partials 535 536 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.