vmware-tanzu / pinniped

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

Make ID token lifetime configurable for `OIDCClients` #1914

Closed cfryanr closed 4 months ago

cfryanr commented 4 months ago

This PR adds a new configuration option to OIDCClient resources to allow the ID tokens issued by authcode flows and refresh flows to have a custom lifetime, within certain reasonable limits of min/max allowed lifetimes. See the new field introduced in types_oidcclient.go.tmpl in the diffs of this PR.

Note that this PR borrows heavily from the proof-of-concept PR https://github.com/vmware-tanzu/pinniped/pull/1857. The difference is that this PR is intended to be production-quality and to be eventually merged. This PR does not include anything from the POC PR about adjusting token lifetimes for sessions started using GitHub PATs, but that could be added later after this PR is merged, if desired.

Release note:

TBD
codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 86.51163% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 38.45%. Comparing base (5fe94c4) to head (57a07a4).

Files Patch % Lines
...derationdomain/idtokenlifespan/idtoken_lifespan.go 44.44% 10 Missing :warning:
...upervisor/config/v1alpha1/zz_generated.deepcopy.go 56.25% 7 Missing :warning:
internal/federationdomain/oidc/oidc.go 94.17% 6 Missing :warning:
.../federationdomain/endpoints/token/token_handler.go 83.33% 2 Missing and 1 partial :warning:
internal/testutil/oidcclient.go 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1914 +/- ## ========================================== - Coverage 38.63% 38.45% -0.18% ========================================== Files 349 350 +1 Lines 44511 44680 +169 ========================================== - Hits 17196 17182 -14 - Misses 26799 26983 +184 + Partials 516 515 -1 ```

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

cfryanr commented 4 months ago

Although this is covered by integration tests, it probably also deserves some manual testing before merge.