Commit Graph

59 Commits

Author SHA1 Message Date
Ryan Richard
cec9f3c4d7 Improve the selectors of Deployments and Services
Fixes #801. The solution is complicated by the fact that the Selector
field of Deployments is immutable. It would have been easy to just
make the Selectors of the main Concierge Deployment, the Kube cert agent
Deployment, and the various Services use more specific labels, but
that would break upgrades. Instead, we make the Pod template labels and
the Service selectors more specific, because those not immutable, and
then handle the Deployment selectors in a special way.

For the main Concierge and Supervisor Deployments, we cannot change
their selectors, so they remain "app: app_name", and we make other
changes to ensure that only the intended pods are selected. We keep the
original "app" label on those pods and remove the "app" label from the
pods of the Kube cert agent Deployment. By removing it from the Kube
cert agent pods, there is no longer any chance that they will
accidentally get selected by the main Concierge Deployment.

For the Kube cert agent Deployment, we can change the immutable selector
by deleting and recreating the Deployment. The new selector uses only
the unique label that has always been applied to the pods of that
deployment. Upon recreation, these pods no longer have the "app" label,
so they will not be selected by the main Concierge Deployment's
selector.

The selector of all Services have been updated to use new labels to
more specifically target the intended pods. For the Concierge Services,
this will prevent them from accidentally including the Kube cert agent
pods. For the Supervisor Services, we follow the same convention just
to be consistent and to help future-proof the Supervisor app in case it
ever has a second Deployment added to it.

The selector of the auto-created impersonation proxy Service was
also previously using the "app" label. There is no change to this
Service because that label will now select the correct pods, since
the Kube cert agent pods no longer have that label. It would be possible
to update that selector to use the new more specific label, but then we
would need to invent a way to pass that label into the controller, so
it seemed like more work than was justified.
2021-09-14 13:35:10 -07:00
Margo Crawford
c590c8ff41 Merge branch 'main' of github.com:vmware-tanzu/pinniped into active-directory-identity-provider 2021-08-24 12:19:29 -07:00
Mo Khan
3077034b2d
Merge branch 'main' into oidc_password_grant 2021-08-24 12:23:52 -04:00
Monis Khan
c356710f1f
Add leader election middleware
Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-20 12:18:25 -04:00
Margo Crawford
1c5da35527 Merge remote-tracking branch 'origin' into active-directory-identity-provider 2021-08-18 12:44:12 -07:00
Ryan Richard
964d16110e Some refactors based on PR feedback from @enj 2021-08-17 13:14:09 -07:00
Ryan Richard
52409f86e8 Merge branch 'main' into oidc_password_grant 2021-08-16 15:17:55 -07:00
Monis Khan
7a812ac5ed
impersonatorconfig: only unload dynamiccert when proxy is disabled
In the upstream dynamiccertificates package, we rely on two pieces
of code:

1. DynamicServingCertificateController.newTLSContent which calls
   - clientCA.CurrentCABundleContent
   - servingCert.CurrentCertKeyContent
2. unionCAContent.VerifyOptions which calls
   - unionCAContent.CurrentCABundleContent

This results in calls to our tlsServingCertDynamicCertProvider and
impersonationSigningCertProvider.  If we Unset these providers, we
subtly break these consumers.  At best this results in test slowness
and flakes while we wait for reconcile loops to converge.  At worst,
it results in actual errors during runtime.  For example, we
previously would Unset the impersonationSigningCertProvider on any
sync loop error (even a transient one caused by a network blip or
a conflict between writes from different replicas of the concierge).
This would cause us to transiently fail to issue new certificates
from the token credential require API.  It would also cause us to
transiently fail to authenticate previously issued client certs
(which results in occasional Unauthorized errors in CI).

Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-16 16:07:46 -04:00
Ryan Richard
50085a505b First unit test for auth endpoint's password grant and related refactor 2021-08-12 17:53:14 -07:00
Ryan Richard
84c3c3aa9c Optionally allow OIDC password grant for CLI-based login experience
- Add `AllowPasswordGrant` boolean field to OIDCIdentityProvider's spec
- The oidc upstream watcher controller copies the value of
  `AllowPasswordGrant` into the configuration of the cached provider
- Add password grant to the UpstreamOIDCIdentityProviderI interface
  which is implemented by the cached provider instance for use in the
  authorization endpoint
- Enhance the IDP discovery endpoint to return the supported "flows"
  for each IDP ("cli_password" and/or "browser_authcode")
- Enhance `pinniped get kubeconfig` to help the user choose the desired
  flow for the selected IDP, and to write the flow into the resulting
  kubeconfg
- Enhance `pinniped login oidc` to have a flow flag to tell it which
  client-side flow it should use for auth (CLI-based or browser-based)
- In the Dex config, allow the resource owner password grant, which Dex
  implements to also return ID tokens, for use in integration tests
- Enhance the authorize endpoint to perform password grant when
  requested by the incoming headers. This commit does not include unit
  tests for the enhancements to the authorize endpoint, which will come
  in the next commit
- Extract some shared helpers from the callback endpoint to share the
  code with the authorize endpoint
- Add new integration tests
2021-08-12 10:45:39 -07:00
Margo Crawford
3899292e89 Advertise Active Directory idps 2021-07-23 13:01:40 -07:00
Matt Moyer
71d4e05fb6
Add custom response_mode=form_post HTML template.
This is a new pacakge internal/oidc/provider/formposthtml containing a number of static files embedded using the relatively recent Go "//go:embed" functionality introduced in Go 1.16 (https://blog.golang.org/go1.16).

The Javascript and CSS files are minifiied and injected to make a single self-contained HTML response. There is a special Content-Security-Policy helper to calculate hash-based script-src and style-src rules.

This new code is covered by a new integration test that exercises the JS/HTML functionality in a real browser outside of the rest of the Supervisor.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-07-09 12:08:43 -05:00
Matt Moyer
6d83ecb420
Unit test response_mode=form_post in internal/oidc/callback.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-07-09 12:08:43 -05:00
Ryan Richard
033e1f0399 Add user search base to downstream subject for upstream LDAP
- Also add some tests about UTF-8 characters in LDAP attributes
2021-05-26 17:04:20 -07:00
Ryan Richard
2014f4623d Move require.NoError() to t.Cleanup() 2021-05-24 14:24:09 -07:00
Ryan Richard
b16e84d90a Add another unit test for the LDAP client code 2021-05-21 12:44:01 -07:00
Ryan Richard
1c66ffd5ff WIP: add supervisor upstream flags to pinniped get kubeconfig
- And perform auto-discovery when the flags are not set
- Several TODOs remain which will be addressed in the next commit

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-04-30 14:28:03 -07:00
Ryan Richard
263a33cc85 Some updates based on PR review 2021-04-27 12:43:09 -07:00
Ryan Richard
f6ded84f07 Implement upstream LDAP support in auth_handler.go
- When the upstream IDP is an LDAP IDP and the user's LDAP username and
  password are received as new custom headers, then authenticate the
  user and, if authentication was successful, return a redirect with
  an authcode. Handle errors according to the OAuth/OIDC specs.
- Still does not support having multiple upstream IDPs defined at the
  same time, which was an existing limitation of this endpoint.
- Does not yet include the actual LDAP authentication, which is
  hidden behind an interface from the point of view of auth_handler.go
- Move the oidctestutil package to the testutil directory.
- Add an interface for Fosite storage to avoid a cyclical test
  dependency.
- Add GetURL() to the UpstreamLDAPIdentityProviderI interface.
- Extract test helpers to be shared between callback_handler_test.go
  and auth_handler_test.go because the authcode and fosite storage
  assertions should be identical.
- Backfill Content-Type assertions in callback_handler_test.go.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-04-08 17:28:01 -07:00
Ryan Richard
c82f568b2c certauthority.go: Refactor issuing client versus server certs
We were previously issuing both client certs and server certs with
both extended key usages included. Split the Issue*() methods into
separate methods for issuing server certs versus client certs so
they can have different extended key usages tailored for each use
case.

Also took the opportunity to clean up the parameters of the Issue*()
methods and New() methods to more closely match how we prefer to call
them. We were always only passing the common name part of the
pkix.Name to New(), so now the New() method just takes the common name
as a string. When making a server cert, we don't need to set the
deprecated common name field, so remove that param. When making a client
cert, we're always making it in the format expected by the Kube API
server, so just accept the username and group as parameters directly.
2021-03-12 16:09:37 -08:00
Ryan Richard
0b300cbe42 Use TokenCredentialRequest instead of base64 token with impersonator
To make an impersonation request, first make a TokenCredentialRequest
to get a certificate. That cert will either be issued by the Kube
API server's CA or by a new CA specific to the impersonator. Either
way, you can then make a request to the impersonator and present
that client cert for auth and the impersonator will accept it and
make the impesonation call on your behalf.

The impersonator http handler now borrows some Kube library code
to handle request processing. This will allow us to more closely
mimic the behavior of a real API server, e.g. the client cert
auth will work exactly like the real API server.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-10 10:30:06 -08:00
Matt Moyer
c4f6fd5b3c
Add a bit nicer assertion helper in testutil/testlogger.
This makes output that's easier to copy-paste into the test. We could also make it ignore the order of key/value pairs in the future.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-03-05 15:49:45 -06:00
Andrew Keesler
069b3fba37
Merge remote-tracking branch 'upstream/main' into impersonation-proxy
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-02-23 12:10:52 -05:00
Ryan Richard
80ff5c1f17 Fix bug which prevented watches from working through impersonator
Also:
- Changed base64 encoding of impersonator bearer tokens to use
  `base64.StdEncoding` to make it easier for users to manually
  create a token using the unix `base64` command
- Test the headers which are and are not passed through to the Kube API
  by the impersonator more carefully in the unit tests
- More WIP on concierge_impersonation_proxy_test.go

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-02-22 17:23:11 -08:00
Andrew Keesler
957cb2d56c
Merge remote-tracking branch 'upstream/main' into impersonation-proxy
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-02-18 13:37:28 -05:00
Matt Moyer
6565265bee
Use new 'go.pinniped.dev/generated/latest' package.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-02-16 13:00:08 -06:00
Andrew Keesler
fdd8ef5835
internal/concierge/impersonator: handle custom login API group
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-02-16 07:55:09 -05:00
Andrew Keesler
0fc1f17866
internal/groupsuffix: mutate TokenCredentialRequest's Authenticator
This is a partial revert of 288d9c999e. For some reason it didn't occur to me
that we could do it this way earlier. Whoops.

This also contains a middleware update: mutation funcs can return an error now
and short-circuit the rest of the request/response flow. The idea here is that
if someone is configuring their kubeclient to use middleware, they are agreeing
to a narrow-er client contract by doing so (e.g., their TokenCredentialRequest's
must have an Spec.Authenticator.APIGroup set).

I also updated some internal/groupsuffix tests to be more realistic.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-02-10 15:53:44 -05:00
Monis Khan
05a471fdf9
Migrate callers to k8s.io/apimachinery/pkg/util/errors.NewAggregate
Signed-off-by: Monis Khan <mok@vmware.com>
2021-02-05 12:56:05 -05:00
Andrew Keesler
62c117421a
internal/kubeclient: fix not found test and request body closing bug
- I realized that the hardcoded fakekubeapi 404 not found response was invalid,
  so we were getting a default error message. I fixed it so the tests follow a
  higher fidelity code path.
- I caved and added a test for making sure the request body was always closed,
  and believe it or not, we were double closing a body. I don't *think* this will
  matter in production, since client-go will pass us ioutil.NopReader()'s, but
  at least we know now.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2021-02-03 08:19:34 -05:00
Monis Khan
efe1fa89fe Allow multiple Pinnipeds to work on same cluster
Yes, this is a huge commit.

The middleware allows you to customize the API groups of all of the
*.pinniped.dev API groups.

Some notes about other small things in this commit:
- We removed the internal/client package in favor of pkg/conciergeclient. The
  two packages do basically the same thing. I don't think we use the former
  anymore.
- We re-enabled cluster-scoped owner assertions in the integration tests.
  This code was added in internal/ownerref. See a0546942 for when this
  assertion was removed.
- Note: the middlware code is in charge of restoring the GV of a request object,
  so we should never need to write mutations that do that.
- We updated the supervisor secret generation to no longer manually set an owner
  reference to the deployment since the middleware code now does this. I think we
  still need some way to make an initial event for the secret generator
  controller, which involves knowing the namespace and the name of the generated
  secret, so I still wired the deployment through. We could use a namespace/name
  tuple here, but I was lazy.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
Co-authored-by: Ryan Richard <richardry@vmware.com>
2021-02-02 15:18:41 -08:00
Matt Moyer
3948bb76d8
Be more lax in some of our test assertions.
Fosite overrides the `Cache-Control` header we set, which is basically fine even though it's not exactly what we want.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-16 13:15:38 -06:00
Ryan Richard
16907e4453 Add Cache-Control, Pragma, Expires, and X-DNS-Prefetch-Control headers
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-14 15:28:32 -08:00
Ryan Richard
858356610c Make assertions about how many secrets were stored by fosite in tests
In both callback_handler_test.go and token_handler_test.go

Signed-off-by: Aram Price <pricear@vmware.com>
2020-12-04 15:40:17 -08:00
Andrew Keesler
03806629b8
Cleanup code via TODOs accumulated during token endpoint work
We opened https://github.com/vmware-tanzu/pinniped/issues/254 for the TODO in
dynamicOpenIDConnectECDSAStrategy.GenerateToken().

This commit also ensures that linting and unit tests are passing again.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-04 10:09:42 -05:00
Matt Moyer
c0f13ef4ac
Merge remote-tracking branch 'origin/main' into callback-endpoint
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-02 16:09:08 -06:00
Matt Moyer
24c4bc0dd4
Tweak some stdlib usage so we compile under Go 1.14.
Mainly, avoid using some `testing` helpers that were added in 1.14, as well as a couple of other niceties we can live without.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-30 10:11:41 -06:00
Andrew Keesler
b25696a1fb callback_handler.go: Prepend iss to sub when making default username
- Also handle several more error cases
- Move RequireTimeInDelta to shared testutils package so other tests
  can also use it
- Move all of the oidc test helpers into a new oidc/oidctestutils
  package to break a circular import dependency. The shared testutil
  package can't depend on any of our other packages or else we
  end up with circular dependencies.
- Lots more assertions about what was stored at the end of the
  request to build confidence that we are going to pass all of the
  right settings over to the token endpoint through the storage, and
  also to avoid accidental regressions in that area in the future

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-19 17:57:07 -08:00
Andrew Keesler
2e62be3ebb
callback_handler.go: assert correct args are passed to token exchange
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-19 10:20:46 -05: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
Andrew Keesler
3ef1171667 Tiny bit more code for Supervisor's callback_handler.go
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-13 15:59:51 -08: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
Andrew Keesler
9e0195e024
kubecertagent: use initial event for when key can't be found
This should fix integration tests running on clusters that don't have
visible controller manager pods (e.g., GKE). Pinniped should boot, not
find any controller manager pods, but still post a status in the CIC.

I also updated a test helper so that we could tell the difference
between when an event was not added and when an event was added with
an empty key.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-09-24 16:54:20 -04:00
Andrew Keesler
406f2723ce
internal/certauthority/dynamiccertauthority: add new dynamic cert issuer
This thing is supposed to be used to help our CredentialRequest handler issue certs with a dynamic
CA keypair.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-09-23 09:53:21 -04:00
Matt Moyer
2d4d7e588a
Add Go vanity import paths.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-09-18 14:56:24 -05:00
Matt Moyer
8c9c1e206d
Update module/package names to match GitHub org switch.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-09-17 12:56:54 -05:00
Andrew Keesler
eab5c2b86b
Save 2 lines by using inline-style comments for Copyright
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-09-16 10:35:19 -04:00
Andrew Keesler
e7b389ae6c
Update copyright to reference Pinniped contributors
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-09-16 10:05:51 -04:00
Matt Moyer
7d8c28a9dc
Extract testutil.TLSTestServer so it can be reused elsewhere.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-09-14 13:23:12 -05:00
Matt Moyer
bbef017989
Add a testlogger util package for testing go-logr.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-09-14 13:23:06 -05:00