Commit Graph

229 Commits

Author SHA1 Message Date
aram price
cff2dc1379 Reorder functions 2020-12-18 15:08:55 -08:00
Ryan Richard
fc250f98d0 Adjust func grouping 2020-12-18 14:58:39 -08:00
Aram Price
b3e428c9de Several more controllers Sync less often by adjusting their filters
- JWKSWriterController
- JWKSObserverController
- FederationDomainSecretsController for HMAC keys
- FederationDomainSecretsController for state signature key
- FederationDomainSecretsController for state encryption key

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-18 14:55:05 -08:00
Ryan Richard
1056cef384 Sync garbage collector controller less often by adjusting its filters
- Only sync on add/update of secrets in the same namespace which
  have the "storage.pinniped.dev/garbage-collect-after" annotation, and
  also during a full resync of the informer whenever secrets in the
  same namespace with that annotation exist.
- Ignore deleted secrets to avoid having this controller trigger itself
  unnecessarily when it deletes a secret. This controller is never
  interested in deleted secrets, since its only job is to delete
  existing secrets.
- No change to the self-imposed rate limit logic. That still applies
  because secrets with this annotation will be created and updated
  regularly while the system is running (not just during rare system
  configuration steps).
2020-12-18 09:36:28 -08:00
aram price
187bd9060c All FederationDomain Secrets have distinct Types
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-17 17:07:38 -08:00
aram price
587cced768 Add extra type info where SecretType is used 2020-12-17 15:43:20 -08:00
Ryan Richard
50964c6677 Supervisor CSRF Secret has unique Type
Signed-off-by: aram price <pricear@vmware.com>
2020-12-17 15:30:26 -08:00
Ryan Richard
b27e3e1a89 Put a Type on the Secrets that we create for FederationDomain JWKS
Signed-off-by: Aram Price <pricear@vmware.com>
2020-12-17 14:48:49 -08:00
Aram Price
55483b726b More "op" and "opc" local variable renames
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-17 13:49:53 -08:00
Ryan Richard
b96d49df0f Rename all "op" and "opc" usages
Signed-off-by: Aram Price <pricear@vmware.com>
2020-12-17 11:34:49 -08:00
Matt Moyer
b60542f0d1
Clean this test up a trivial amount using require.Implementsf().
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-17 08:38:16 -06:00
Monis Khan
dc8e7a2f39
Enable cache mutation detector in unit tests
Signed-off-by: Monis Khan <mok@vmware.com>
2020-12-17 08:38:15 -06:00
Andrew Keesler
04d54e622a
Only set single secret status field in FederationDomainSecretsController
This implementation is janky because I wanted to make the smallest change
possible to try to get the code back to stable so we can release.

Also deep copy an object so we aren't mutating the cache.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-17 07:41:53 -05:00
Margo Crawford
196e43aa48 Rename off of main
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-16 14:27:09 -08:00
Matt Moyer
7dae166a69
Merge branch 'main' into username-and-subject-claims 2020-12-16 15:23:19 -06:00
Margo Crawford
1d4012cabf jwtcachefiller_test.go: don't assert about time zones in errors
Because the library that we are using which returns that error
formats the timestamp in localtime, which is LMT when running
on a laptop, but is UTC when running in CI.

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-16 10:17:17 -08:00
Ryan Richard
dcb19150fc Nest claim configs one level deeper in JWTAuthenticatorSpec
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-16 09:42:19 -08:00
Margo Crawford
a10d219049 Pass through custom groups claim and username claim
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-15 16:11:53 -08:00
Ryan Richard
05ab8f375e Default to "username" claim in jwtcachefiller
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-15 14:37:38 -08:00
Margo Crawford
720bc7ae42 jwtcachefiller_test.go: refactor and remove "if short skip" check
- Refactor the test to avoid testing a private method and instead
  always test the results of running the controller.
- Also remove the `if testing.Short()` check because it will always
  be short when running unit tests. This prevented the unit test
  from ever running, both locally and in CI.

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-15 13:33:49 -08:00
Andrew Keesler
35bb76ea82
Ensure labels are set correct on generated Supervisor secret
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 15:55:14 -05:00
Andrew Keesler
9d9040944a Secrets owned by Deployment have Controller: false
- This is to prevent K8s internal Deployment controller from trying to
manage these objects

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 12:12:47 -08:00
Andrew Keesler
7320928235
Get rid of TODOs in code by punting on them
We will do these later; they have been recorded in a work tracking record.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 09:58:46 -05:00
Andrew Keesler
82ae98d9d0
Set secret names on OIDCProvider status field
We believe this API is more forwards compatible with future secrets management
use cases. The implementation is a cry for help, but I was trying to follow the
previously established pattern of encapsulating the secret generation
functionality to a single group of packages.

This commit makes a breaking change to the current OIDCProvider API, but that
OIDCProvider API was added after the latest release, so it is technically still
in development until we release, and therefore we can continue to thrash on it.

I also took this opportunity to make some things private that didn't need to be
public.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 09:13:01 -05:00
Andrew Keesler
60d4a7beac
Test more filters in SupervisorSecretsController (see 6e8d564013)
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 07:58:33 -05:00
aram price
e03e344dcd SecretHelper depends less on OIDCProvider
This should allow the helper to be more generic so that it can be used
with the SupervisorSecretsController
2020-12-14 19:35:45 -08:00
aram price
bf86bc3383 Rename for clarity 2020-12-14 18:36:56 -08:00
aram price
b799515f84 Pull symmetricsecrethelper package up to generator
- rename symmetricsecrethelper.New => generator.NewSymmetricSecretHelper
2020-12-14 17:41:02 -08:00
aram price
b1ee434ddf Rename in preparation for refactor 2020-12-14 16:44:27 -08:00
aram price
6e8d564013 Test filters in SupervisorSecretsController 2020-12-14 16:08:48 -08:00
Andrew Keesler
9c79adcb26 Rename and move some code to perpare for refactor
Signed-off-by: aram price <pricear@vmware.com>
2020-12-14 14:24:13 -08:00
Aram Price
5b7a86ecc1
Integration test for Supervisor secret controllers
This forced us to add labels to the CSRF cookie secret, just as we do
for other Supervisor secrets. Yay tests.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-14 15:53:12 -05:00
Andrew Keesler
cae0023234
Merge remote-tracking branch 'upstream/main' into secret-generation
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-14 11:44:01 -05:00
Andrew Keesler
e3ea141bf3
Reuse helper filter in generic secret gen controller
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-14 10:37:27 -05:00
Andrew Keesler
b043dae149
Finish first implementation of generic secret generator controller
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-14 10:36:45 -05:00
aram price
3ca877f1df
WIP - preliminary OIDCProviderSecrets controller
Tests not yet passing, controller is incomplete and expectations may be
incorrect.
2020-12-13 17:37:49 -05:00
aram price
3e31668eb0
Refactor some utilitiy methods for sharing. 2020-12-13 17:37:48 -05:00
aram price
9e2213cbae
Rename for clarity
- makes space for OIDCPrivder related controller
2020-12-13 17:37:48 -05:00
Ryan Richard
baa1a4a2fc Supervisor storage garbage collection controller enabled in production
- Also add more log statements to the controller
- Also have the controller apply a rate limit to itself, to avoid
  having a very chatty controller that runs way more often than is
  needed.
- Also add an integration test for the controller's behavior.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-11 15:21:34 -08:00
Andrew Keesler
022dcd1909
Update secretgenerator controller after synchronous review
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-11 15:37:10 -05:00
Andrew Keesler
e17bc31b29
Pass CSRF cookie signing key from controller to cache
This also sets the CSRF cookie Secret's OwnerReference to the Pod's grandparent
Deployment so that when the Deployment is cleaned up, then the Secret is as
well.

Obviously this controller implementation has a lot of issues, but it will at
least get us started.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-11 11:49:27 -05:00
Margo Crawford
ed9b3ffce5 Add controller for garbage collecting secrets
Signed-off-by: Ryan Richard <rrichard@vmware.com>
2020-12-10 17:34:05 -08:00
aram price
ccac124b7a Fix broken test
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-10 17:32:55 -08:00
Andrew Keesler
c3f73ffb57 Check in some musings on a symmetric key generator controller
There is still a test failing, but I am sure it is a simple fix hiding in the
code. I think this is the general shape of the controller that we want.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-10 17:32:55 -08:00
Andrew Keesler
381a2e749a
impotent -> idempotent
These words do not mean the same thing...

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-08 15:41:49 -05:00
Aram Price
9ed5dcb031
Only create underlying jwt authenticator when spec has changed
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-08 15:41:49 -05:00
Andrew Keesler
e0ee18a993
Always close JWTAuthenticator underlying authenticator
Otherwise we will leak goroutines.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-08 15:41:48 -05:00
Andrew Keesler
57103e0a9f
Add JWTAuthenticator controller
See https://github.com/vmware-tanzu/pinniped/issues/260 for UX bummer.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-08 15:41:48 -05:00
Andrew Keesler
58237d0e7d
WIP: start to wire signing key into token handler
This commit includes a failing test (amongst other compiler failures) for the
dynamic signing key fetcher that we will inject into fosite. We are checking it
in so that we can pass the WIP off.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-03 15:37:25 -05:00
Matt Moyer
4fe691de92
Save an http.Client with each upstreamoidc.ProviderConfig object.
This allows the token exchange request to be performed with the correct TLS configuration.

We go to a bit of extra work to make sure the `http.Client` object is cached between reconcile operations so that connection pooling works as expected.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-02 15:55:33 -06:00
Matt Moyer
d64acbb5a9
Add upstreamoidc.ProviderConfig type implementing provider.UpstreamOIDCIdentityProviderI.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-30 15:22:56 -06:00
Ryan Richard
227fbd63aa Use an interface instead of a concrete type for UpstreamOIDCIdentityProvider
Because we want it to implement an AuthcodeExchanger interface and
do it in a way that will be more unit test-friendly than the underlying
library that we intend to use inside its implementation.
2020-11-18 13:38:13 -08:00
Matt Moyer
ee978fdde8
Add controller support for spec.tls field.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-16 20:23:20 -06:00
Matt Moyer
c10393b495
Mask the raw error messages from go-oidc, since they are dangerous.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-13 16:22:34 -06:00
Matt Moyer
cbd71df574
Add "upstream-watcher" controller to supervisor.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-13 12:30:38 -06:00
Monis Khan
9c8b081906
Prevent multiple pinnipeds from thrashing on the API service
Signed-off-by: Monis Khan <mok@vmware.com>
2020-11-11 20:09:49 -05:00
Monis Khan
15a5332428
Reduce log spam
Signed-off-by: Monis Khan <mok@vmware.com>
2020-11-10 10:22:27 -05:00
Matt Moyer
4da3d93f6e
The supervisor JWKS observer and TLS cert controllers use the ctx after all, whoops.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-04 13:08:50 -06:00
Monis Khan
418f4d20ae
Use parent func to indicate when the controller queue is a singleton
This prevents unnecessary sync loop runs when the controller is
running with a single worker.  When the controller is running with
more than one worker, it prevents subtle bugs that can cause the
controller to go "back in time."

Signed-off-by: Monis Khan <mok@vmware.com>
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-04 11:08:10 -06:00
Matt Moyer
2bf5c8b48b
Replace the OIDCProvider field SNICertificateSecretName with a TLS.SecretName field.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-02 18:15:03 -06:00
Matt Moyer
2b8773aa54
Rename OIDCProviderConfig to OIDCProvider.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-02 17:40:39 -06:00
Matt Moyer
59263ea733
Rename CredentialIssuerConfig to CredentialIssuer.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-02 17:39:42 -06:00
Ryan Richard
75c35e74cc Refactor and add unit tests for previous commit to run agent pod as root 2020-11-02 15:03:37 -08:00
Ryan Richard
a01921012d
kubecertagent: explicitly run as root
We need root here because the files that this pod reads are
most likely restricted to root access.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-02 16:33:46 -05:00
Ryan Richard
ab5c04b1f3
Merge pull request #176 from vmware-tanzu/agent_pod_additional_label_handling
Handle custom labels better in the agent pod controllers
2020-11-02 09:08:42 -08:00
Ryan Richard
7597b12a51 Small unit test changes for deleter_test.go 2020-11-02 08:40:39 -08:00
Ryan Richard
f76b9857da Don't use custom labels when selecting an agent pod
And delete the agent pod when it needs its custom labels to be
updated, so that the creator controller will notice that it is missing
and immediately create it with the new custom labels.
2020-10-30 17:41:17 -07:00
Matt Moyer
9e1922f1ed
Split the config CRDs into two API groups.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 19:22:46 -05:00
Matt Moyer
34da8c7877
Rename existing references to "IDP" and "Identity Provider".
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 15:12:01 -05:00
Matt Moyer
f3a83882a4
Rename the IdentityProvider field to Authenticator in TokenCredentialRequest.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 15:11:53 -05:00
Matt Moyer
0f25657a35
Rename WebhookIdentityProvider to WebhookAuthenticator.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 15:11:53 -05:00
Matt Moyer
e69183aa8a
Rename idp.concierge.pinniped.dev to authentication.concierge.pinniped.dev.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 14:07:40 -05:00
Matt Moyer
81390bba89
Rename idp.pinniped.dev to idp.concierge.pinniped.dev.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 14:07:39 -05:00
Matt Moyer
f0320dfbd8
Rename login API to login.concierge.pinniped.dev.
This is the first of a few related changes that re-organize our API after the big recent changes that introduced the supervisor component.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-10-30 09:58:28 -05:00
Ryan Richard
29e0ce5662 Configure name of the supervisor default TLS cert secret via ConfigMap
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-28 11:56:50 -07:00
Ryan Richard
170d3a3993 Forgot to commit some test fixtures in a prior commit 2020-10-27 17:00:00 -07:00
Ryan Richard
38802c2184 Add a way to set a default supervisor TLS cert for when SNI won't work
- Setting a Secret in the supervisor's namespace with a special name
  will cause it to get picked up and served as the supervisor's TLS
  cert for any request which does not have a matching SNI cert.
- This is especially useful for when there is no DNS record for an
  issuer and the user will be accessing it via IP address. This
  is not how we would expect it to be used in production, but it
  might be useful for other cases.
- Includes a new integration test
- Also suppress all of the warnings about ignoring the error returned by
  Close() in lines like `defer x.Close()` to make GoLand happier
2020-10-27 16:33:08 -07:00
Ryan Richard
eeb110761e Rename secretName to SNICertificateSecretName in OIDCProviderConfig 2020-10-26 17:25:45 -07:00
Ryan Richard
8b7c30cfbd Supervisor listens for HTTPS on port 443 with configurable TLS certs
- TLS certificates can be configured on the OIDCProviderConfig using
  the `secretName` field.
- When listening for incoming TLS connections, choose the TLS cert
  based on the SNI hostname of the incoming request.
- Because SNI hostname information on incoming requests does not include
  the port number of the request, we add a validation that
  OIDCProviderConfigs where the issuer hostnames (not including port
  number) are the same must use the same `secretName`.
- Note that this approach does not yet support requests made to an
  IP address instead of a hostname. Also note that `localhost` is
  considered a hostname by SNI.
- Add port 443 as a container port to the pod spec.
- A new controller watches for TLS secrets and caches them in memory.
  That same in-memory cache is used while servicing incoming connections
  on the TLS port.
- Make it easy to configure both port 443 and/or port 80 for various
  Service types using our ytt templates for the supervisor.
- When deploying to kind, add another nodeport and forward it to the
  host on another port to expose our new HTTPS supervisor port to the
  host.
2020-10-26 17:03:26 -07:00
Ryan Richard
25a91019c2 Add spec.secretName to OPC and handle case-insensitive hostnames
- When two different Issuers have the same host (i.e. they differ
  only by path) then they must have the same secretName. This is because
  it wouldn't make sense for there to be two different TLS certificates
  for one host. Find any that do not have the same secret name to
  put an error status on them and to avoid serving OIDC endpoints for
  them. The host comparison is case-insensitive.
- Issuer hostnames should be treated as case-insensitive, because
  DNS hostnames are case-insensitive. So https://me.com and
  https://mE.cOm are duplicate issuers. However, paths are
  case-sensitive, so https://me.com/A and https://me.com/a are
  different issuers. Fixed this in the issuer validations and in the
  OIDC Manager's request router logic.
2020-10-23 16:25:44 -07:00
Andrew Keesler
110c72a5d4
dynamiccertauthority: fix cert expiration test failure
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-23 15:34:25 -04:00
Ryan Richard
d9d76726c2 Implement per-issuer OIDC JWKS endpoint 2020-10-16 17:51:40 -07:00
Andrew Keesler
617c5608ca Supervisor controllers apply custom labels to JWKS secrets
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-10-15 12:40:56 -07:00
Ryan Richard
f8e461dfc3 Merge branch 'main' into label_every_resource 2020-10-15 10:19:03 -07:00
Ryan Richard
94f20e57b1 Concierge controllers add labels to all created resources 2020-10-15 10:14:23 -07:00
Andrew Keesler
e05213f9dd
supervisor-generate-key: use EC keys intead of RSA
EC keys are smaller and take less time to generate. Our integration
tests were super flakey because generating an RSA key would take up to
10 seconds *gasp*. The main token verifier that we care about is
Kubernetes, which supports P256, so hopefully it won't be that much of
an issue that our default signing key type is EC. The OIDC spec seems
kinda squirmy when it comes to using non-RSA signing algorithms...

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-15 11:33:08 -04:00
Andrew Keesler
fbcce700dc
Fix whitespace/spelling nits in JWKS controller
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-15 09:22:17 -04:00
Andrew Keesler
31225ac7ae
test/integration: reuse CreateTestOIDCProvider helper
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-15 09:09:49 -04:00
Andrew Keesler
c030551af0
supervisor-generate-key: unit and integration tests
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-14 16:41:16 -04:00
Andrew Keesler
6aed025c79
supervisor-generate-key: initial spike
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-14 09:47:34 -04:00
Andrew Keesler
c555c14ccb
supervisor-oidc: add OIDCProviderConfig.Status.LastUpdateTime
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-09 11:54:50 -04:00
Andrew Keesler
bb015adf4e
Backfill tests to OIDCProviderConfig controller
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-09 10:39:17 -04:00
Ryan Richard
b74486f305 Start back-filling unit tests for OIDCProviderConfigWatcherController
- Left some TODOs for more things that it should test
2020-10-08 17:40:58 -07:00
Ryan Richard
8b7d96f42c Several small refactors related to OIDC providers 2020-10-08 11:28:21 -07:00
Andrew Keesler
da00fc708f
supervisor-oidc: checkpoint: add status to provider CRD
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-10-08 13:27:45 -04:00
Ryan Richard
6b653fc663 Creation and deletion of OIDC Provider discovery endpoints from config
- The OIDCProviderConfigWatcherController synchronizes the
  OIDCProviderConfig settings to dynamically mount and unmount the
  OIDC discovery endpoints for each provider
- Integration test passes but unit tests need to be added still
2020-10-07 19:18:34 -07:00
Andrew Keesler
f48a4e445e
Fix linting and unit tests
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-07 11:48:21 -04:00
Andrew Keesler
019f44982c
supervisor-oidc: checkpoint: controller watches OIDCProviderConfig
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-07 10:54:56 -04:00
Andrew Keesler
fd6a7f5892
supervisor-oidc: hoist OIDC discovery handler for testing
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-10-06 11:16:57 -04:00
Ryan Richard
76bd462cf8 Implement very rough skeleton of the start of a supervisor server
- This is just stab at a starting place because it felt easier to
  put something down on paper than to keep staring at a blank page
2020-10-05 17:28:19 -07:00