Commit Graph

766 Commits

Author SHA1 Message Date
Margo Crawford
19100d68ef Merge branch 'main' of github.com:vmware-tanzu/pinniped into active-directory-identity-provider 2021-08-26 20:42:16 -07:00
Mayank Bhatt
68547f767d Copy hostNetwork field for kube-cert-agent
For clusters where the control plane nodes aren't running a CNI, the
kube-cert-agent pods deployed by concierge cannot be scheduled as they
don't know to use `hostNetwork: true`. This change allows embedding the
host network setting in the Concierge configuration. (by copying it from
the kube-controller-manager pod spec when generating the kube-cert-agent
Deployment)

Also fixed a stray double comma in one of the nearby tests.
2021-08-26 17:09:59 -07:00
Margo Crawford
2d32e0fa7d Merge branch 'main' of github.com:vmware-tanzu/pinniped into active-directory-identity-provider 2021-08-26 16:21:08 -07:00
Margo Crawford
6f221678df Change sAMAccountName env vars to userPrincipalName
and add E2E ActiveDirectory test
also fixed regexes in supervisor_login_test to be anchored to the
beginning and end
2021-08-26 16:18:05 -07:00
Monis Khan
74daa1da64
test/integration: run parallel tests concurrently with serial tests
Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-26 12:59:52 -04:00
Margo Crawford
1c5a2b8892 Add a couple more unit tests 2021-08-25 11:33:42 -07:00
Monis Khan
c71ffdcd1e
leader election: use better duration defaults
OpenShift has good defaults for these duration fields that we can
use instead of coming up with them ourselves:

e14e06ba8d/pkg/config/leaderelection/leaderelection.go (L87-L109)

Copied here for easy future reference:

// We want to be able to tolerate 60s of kube-apiserver disruption without causing pod restarts.
// We want the graceful lease re-acquisition fairly quick to avoid waits on new deployments and other rollouts.
// We want a single set of guidance for nearly every lease in openshift.  If you're special, we'll let you know.
// 1. clock skew tolerance is leaseDuration-renewDeadline == 30s
// 2. kube-apiserver downtime tolerance is == 78s
//      lastRetry=floor(renewDeadline/retryPeriod)*retryPeriod == 104
//      downtimeTolerance = lastRetry-retryPeriod == 78s
// 3. worst non-graceful lease acquisition is leaseDuration+retryPeriod == 163s
// 4. worst graceful lease acquisition is retryPeriod == 26s
if ret.LeaseDuration.Duration == 0 {
	ret.LeaseDuration.Duration = 137 * time.Second
}

if ret.RenewDeadline.Duration == 0 {
	// this gives 107/26=4 retries and allows for 137-107=30 seconds of clock skew
	// if the kube-apiserver is unavailable for 60s starting just before t=26 (the first renew),
	// then we will retry on 26s intervals until t=104 (kube-apiserver came back up at 86), and there will
	// be 33 seconds of extra time before the lease is lost.
	ret.RenewDeadline.Duration = 107 * time.Second
}
if ret.RetryPeriod.Duration == 0 {
	ret.RetryPeriod.Duration = 26 * time.Second
}

Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-24 16:21:53 -04: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
Monis Khan
c0617ceda4
leader election: in-memory leader status is stopped before release
This change fixes a small race condition that occurred when the
current leader failed to renew its lease.  Before this change, the
leader would first release the lease via the Kube API and then would
update its in-memory status to reflect that change.  Now those
events occur in the reverse (i.e. correct) order.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-24 15:02:56 -04:00
Mo Khan
3077034b2d
Merge branch 'main' into oidc_password_grant 2021-08-24 12:23:52 -04:00
Ryan Richard
211f4b23d1 Log auth endpoint errors with stack traces 2021-08-20 14:41:02 -07: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
05afae60c2 Review comments--
- Change list of attributeParsingOverrides to a map
- Add unit test for sAMAccountName as group name without the override
- Change some comments in the the type definition.
2021-08-19 14:21:18 -07:00
Ryan Richard
6239a567a8 remove one nolint:unparam comment 2021-08-19 10:57:00 -07:00
Ryan Richard
e4d418a076 Merge branch 'main' into oidc_password_grant 2021-08-19 10:55:54 -07:00
Ryan Richard
b4a39ba3c4 Remove unparam linter
We decided that this linter does not provide very useful feedback
for our project.
2021-08-19 10:20:24 -07:00
Margo Crawford
1c5da35527 Merge remote-tracking branch 'origin' into active-directory-identity-provider 2021-08-18 12:44:12 -07:00
Ryan Richard
61c21d2977 Refactor some authorize and callback error handling, and add more tests 2021-08-18 12:06:46 -07:00
Ryan Richard
04b8f0b455 Extract Supervisor authorize endpoint string constants into apis pkg 2021-08-18 10:20:33 -07:00
Margo Crawford
8657b0e3e7 Cleanup new group attribute behavior and add test coverage 2021-08-18 10:11:18 -07:00
Ryan Richard
0089540b07 Extract Supervisor IDP discovery endpoint string constants into apis pkg 2021-08-17 17:50:02 -07:00
Margo Crawford
26c47d564f Make new combined sAMAccountName@domain attribute the group name
Also change default username attribute to userPrincipalName
2021-08-17 16:53:26 -07:00
Ryan Richard
62c6d53a21 Merge branch 'main' into oidc_password_grant 2021-08-17 15:23:29 -07:00
Ryan Richard
96474b3d99 Extract Supervisor IDP discovery endpoint types into apis package 2021-08-17 15:23:03 -07:00
Ryan Richard
964d16110e Some refactors based on PR feedback from @enj 2021-08-17 13:14:09 -07:00
Monis Khan
e0901f4fe5
dynamiccert: prevent misuse of NewServingCert
The Kube API server code that we use will cast inputs in an attempt
to see if they implement optional interfaces.  This change adds a
simple wrapper struct to prevent such casts from causing us any
issues.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-17 12:58:32 -04:00
Ryan Richard
52409f86e8 Merge branch 'main' into oidc_password_grant 2021-08-16 15:17:55 -07:00
Ryan Richard
91c8a3ebed Extract private helper in auth_handler.go 2021-08-16 15:17:30 -07:00
Ryan Richard
52cb0bbc07 More unit tests and small error handling changes for OIDC password grant 2021-08-16 14:27:40 -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
5b96d014b4 Merge branch 'main' into oidc_password_grant 2021-08-12 11:12:57 -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
Monis Khan
4a17e1e736
impersonator: update tests for new Impersonate-Uid code
Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-09 19:16:54 -04:00
Monis Khan
a027f1ae2c
jwtcachefiller: update to use CAContentProvider
Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-09 19:16:25 -04:00
Matt Moyer
58bbffded4
Switch to a slimmer distroless base image.
At a high level, it switches us to a distroless base container image, but that also includes several related bits:

- Add a writable /tmp but make the rest of our filesystems read-only at runtime.

- Condense our main server binaries into a single pinniped-server binary. This saves a bunch of space in
  the image due to duplicated library code. The correct behavior is dispatched based on `os.Args[0]`, and
  the `pinniped-server` binary is symlinked to `pinniped-concierge` and `pinniped-supervisor`.

- Strip debug symbols from our binaries. These aren't really useful in a distroless image anyway and all the
  normal stuff you'd expect to work, such as stack traces, still does.

- Add a separate `pinniped-concierge-kube-cert-agent` binary with "sleep" and "print" functionality instead of
  using builtin /bin/sleep and /bin/cat for the kube-cert-agent. This is split from the main server binary
  because the loading/init time of the main server binary was too large for the tiny resource footprint we
  established in our kube-cert-agent PodSpec. Using a separate binary eliminates this issue and the extra
  binary adds only around 1.5MiB of image size.

- Switch the kube-cert-agent code to use a JSON `{"tls.crt": "<b64 cert>", "tls.key": "<b64 key>"}` format.
  This is more robust to unexpected input formatting than the old code, which simply concatenated the files
  with some extra newlines and split on whitespace.

- Update integration tests that made now-invalid assumptions about the `pinniped-server` image.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-08-09 15:05:13 -04:00
Matt Moyer
5f679059d5
Add ClusterIP service to impersonator-config-controller informer.
Prior to this fix, this controller did not correctly react to changes to the ClusterIP service. It would still eventually react with a long delay due to our 5 minute resync interval.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-07-28 11:57:18 -05:00
Monis Khan
8b4ed86071
certs_expirer: be specific about what secret to delete
This change fixes a race that can occur because we have multiple
writers with no leader election lock.

1. TestAPIServingCertificateAutoCreationAndRotation/automatic
   expires the current serving certificate
2. CertsExpirerController 1 deletes expired serving certificate
3. CertsExpirerController 2 starts deletion of expired serving
   certificate but has not done so yet
4. CertsManagerController 1 creates new serving certificate
5. TestAPIServingCertificateAutoCreationAndRotation/automatic
   records the new serving certificate
6. CertsExpirerController 2 finishes deletion, and thus deletes the
   newly created serving certificate instead of the old one
7. CertsManagerController 2 creates new serving certificate
8. TestAPIServingCertificateAutoCreationAndRotation/automatic keeps
   running and eventually times out because it is expecting the
   serving certificate created by CertsManagerController 2 to match
   the value it recorded from CertsManagerController 1 (which will
   never happen since that certificate was incorrectly deleted).

Signed-off-by: Monis Khan <mok@vmware.com>
2021-07-28 09:56:05 -04:00
Matt Moyer
727035a2dc
Fix form_post CSS styling in Firefox and Safari.
This functioned fine, but did not have the intended visual appearance when it came to how the text of the auth code wrapped inside the copy button in the manual flow.

The new styling behaves correctly on at least Chrome, Firefox, and Safari on macOS.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-07-28 08:09:20 -05:00
Margo Crawford
474266f918 Merge branch 'main' of github.com:vmware-tanzu/pinniped into active-directory-identity-provider 2021-07-27 15:06:58 -07:00
Margo Crawford
bbaa820278 parsing objectGUID as human-readable string version 2021-07-27 11:08:23 -07:00
Margo Crawford
287a5d225a Change SearchBaseFound condition success reason to be a string constant 2021-07-27 10:23:05 -07:00
Ryan Richard
f17f7c0c6a Small refactors in impersonator_config.go suggested by @mattmoyer 2021-07-26 17:46:06 -07:00
Margo Crawford
53b58f65b2 Add integration test for wrong password with ldap 2021-07-26 16:32:46 -07:00
Margo Crawford
cc3875f048 PR feedback 2021-07-26 16:03:12 -07:00
Margo Crawford
5d23068690 Removed a todo that was resolved 2021-07-23 13:01:41 -07:00
Margo Crawford
91085e68f9 Refactoring defaulting logic 2021-07-23 13:01:41 -07:00
Margo Crawford
f99f7be836 Default values for ad usersearch and groupsearch 2021-07-23 13:01:41 -07:00
Margo Crawford
890d9c3216 resolve some todos about error handling search base discovery results 2021-07-23 13:01:41 -07:00
Margo Crawford
cb0ee07b51 Fetch AD search base from defaultNamingContext when not specified 2021-07-23 13:01:41 -07:00