Commit Graph

2924 Commits

Author SHA1 Message Date
Matt Moyer
01b6bf7850
Tweak timeouts in oidcclient package.
- The overall timeout for logins is increased to 90 minutes.
- The timeout for token refresh is increased from 30 seconds to 60 seconds to be a bit more tolerant of extremely slow networks.
- A new, matching timeout of 60 seconds has been added for the OIDC discovery, auth code exchange, and RFC8693 token exchange operations.

The new code uses the `http.Client.Timeout` field rather than managing contexts on individual requests. This is easier because the OIDC package stores a context at creation time and tries to use it later when performing key refresh operations.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-16 13:47:08 -06:00
Matt Moyer
2840e4e152
Merge pull request #288 from mattmoyer/fixup-securityheaders
Fix a regression in securityheaders package and add tests.
2020-12-16 13:46:28 -06: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
Matt Moyer
24c01d3e54
Add an integration test to verify security headers on the supervisor authorize endpoint.
It would be great to do this for the supervisor's callback endpoint as well, but it's difficult to get at those since the request happens inside the spawned browser.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-16 12:41:06 -06:00
Matt Moyer
74e52187a3
Simplify securityheader package by merging header fields.
From RFC2616 (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2):
 > It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair,
 > without changing the semantics of the message, by appending each subsequent field-value to the first,
 > each separated by a comma.

This was correct before, but this simplifes a bit and shaves off a few bytes from the response.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-16 12:41:05 -06:00
Matt Moyer
602f3c59ba
Fix a regression in securityheader package.
The bug itself has to do with when headers are streamed to the client. Once a wrapped handler has sent any bytes to the `http.ResponseWriter`, the value of the map returned from `w.Header()` no longer matters for the response. The fix is fairly trivial, which is to add those response headers before invoking the wrapped handler.

The existing unit test didn't catch this due to limitations in `httptest.NewRecorder()`. It is now replaced with a new test that runs a full HTTP test server, which catches the previous bug.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-16 12:41:05 -06:00
Aram Price
a33dace80b
Upgrade golang (1.15.5 -> 1.15.6)
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-16 13:31:54 -05: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
Matt Moyer
bc1dc0805e
Merge pull request #289 from mattmoyer/fix-secret-type-doc-comment
Fix documentation comment for the UpstreamOIDCProvider's spec.client.secretName type.
2020-12-16 10:09:05 -06:00
Andrew Keesler
fec80113c7
Revert "Retry a couple of times if we fail to get a token from the Supervisor"
This reverts commit be4e34d0c0.

Roll back this change that was supposed to make the test more robust. If we
retry multiple token exchanges with the same auth code, of course we are going
to get failures on the second try onwards because the auth code was invalidated
on the first try.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-16 09:04:29 -05:00
Andrew Keesler
5bdbfe1bc6
test/integration: more verbosity to try to track down flakes...
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-16 09:04:25 -05:00
Matt Moyer
404ff93102
Fix documentation comment for the UpstreamOIDCProvider's spec.client.secretName type.
The value is correctly validated as `secrets.pinniped.dev/oidc-client` elsewhere, only this comment was wrong.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 21:52:07 -06:00
aram price
78df80f128 Tests ensure OIDCProvider secrets exist
... whenever one is successfully created.
2020-12-15 18:26:27 -08:00
Ryan Richard
40c6a67631 Merge branch 'main' into username-and-subject-claims 2020-12-15 18:09:44 -08:00
Ryan Richard
91af51d38e Fix integration tests to work with the username and sub claims
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-15 17:16:08 -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
Andrew Keesler
0758ecfea8 Tests wait for OIDCProvider secrets to be set
Signed-off-by: aram price <pricear@vmware.com>
2020-12-15 15:46:55 -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
Aram Price
0bd428e45d
test/integration: more logging to track down flakes
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 16:52:57 -05: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
056afc17bd
Merge remote-tracking branch 'upstream/main' into secret-generation 2020-12-15 15:55:46 -05: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
3d4717b772
Merge pull request #285 from vmware-tanzu/log-unexpected-upstream
Log when unexpected Upstream OIDC Providers found
2020-12-15 15:30:20 -05:00
Andrew Keesler
2b7685fa23
Merge branch 'main' into log-unexpected-upstream 2020-12-15 15:30:05 -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
Matt Moyer
2b2f1bbfc9
Merge pull request #276 from mattmoyer/extended-e2e-cli
Enhance CLI to integrate supervisor and concierge capabilities.
2020-12-15 13:23:51 -06:00
aram price
2edcdc92f4 Log when unexpected Upstream OIDC Providers found
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 10:49:13 -08:00
Ryan Richard
0e60c93cef Add UsernameClaim and GroupsClaim to JWTAuthenticator CRD spec
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2020-12-15 10:36:19 -08:00
Matt Moyer
0b38d6c763
Add TestE2EFullIntegration test which combines supervisor, concierge, and CLI.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:50 -06:00
Matt Moyer
ff49647de4
Add some missing test logs in test/library/client.go.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:50 -06:00
Matt Moyer
e0eba9d5a6
Refactor library.CreateTestJWTAuthenticator() so we can also use the supervisor as an upstream.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:50 -06:00
Matt Moyer
5ad3c65ae1
Close the right pipe output in runPinnipedLoginOIDC.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:50 -06:00
Matt Moyer
aca9af748b
Cleanup TestSuccessfulCredentialRequest and TestCLILoginOIDC a little.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:49 -06:00
Matt Moyer
8cdcb89cef
Add a library.PinnipedCLIPath() test helper, with caching.
Caching saves us a little bit of time now that we're using the CLI in more and more tests.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:49 -06:00
Matt Moyer
70fd330178
Add library.CreateTestClusterRoleBinding test helper.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:49 -06:00
Matt Moyer
ad5e257600
Add a library.RandHex() test helper.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:34:49 -06:00
Matt Moyer
4088793cc5
Add a .ProxyEnv() helper on the test environment.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:04 -06:00
Matt Moyer
b6edc3dc08
Replace TestCLIGetKubeconfig with TestCLIGetKubeconfigStaticToken.
It now tests both the deprecated `pinniped get-kubeconfig` and the new `pinniped get kubeconfig --static-token` flows.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:03 -06:00
Matt Moyer
fe4e2d620d
Update TestCLIGetKubeconfig to ignore stderr output from get-kubeconfig.
This will now have a deprecation warning, so we can't treat is as part of the YAML output.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:03 -06:00
Matt Moyer
f9691208d5
Add library.NewRestConfigFromKubeconfig() test helper.
This is extracted from library.NewClientsetForKubeConfig(). It is useful so you can assert properties of the loaded, parsed kubeconfig.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:03 -06:00
Matt Moyer
71850419c1
Overhaul pinniped CLI subcommands.
- Adds two new subcommands: `pinniped get kubeconfig` and `pinniped login static`
- Adds concierge support to `pinniped login oidc`.
- Adds back wrapper commands for the now deprecated `pinniped get-kubeconfig` and `pinniped exchange-credential` commands. These now wrap `pinniped get kubeconfig` and `pinniped login static` respectively.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:03 -06:00
Matt Moyer
dfbb5b60de
Remove pinniped get-kubeconfig CLI subcommand.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:03 -06:00
Matt Moyer
3b5f00439c
Remove pinniped exchange-credential CLI subcommand.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:02 -06:00
Matt Moyer
9b7fe01648
Add a new ./pkg/conciergeclient package to replace ./internal/client.
This is a slighly evolved version of our previous client package, exported to be public and refactored to use functional options for API maintainability.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-15 12:28:02 -06:00
Andrew Keesler
2e784e006c
Merge remote-tracking branch 'upstream/main' into secret-generation 2020-12-15 13:24:33 -05:00
Andrew Keesler
08cf2f7cd1
Merge pull request #284 from ankeesler/oidcprovider-enum-values
SameIssuerHostMustUseSameSecret is a valid OIDCProvider status, and help some test flakes
2020-12-15 13:23:16 -05:00
Andrew Keesler
be4e34d0c0
Retry a couple of times if we fail to get a token from the Supervisor
I hope this will make TestSupervisorLogin less flaky. There are some instances
where the front half of the OIDC login flow happens so fast that the JWKS
controller doesn't have time to properly generate an asymmetric key.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 11:53:58 -05:00
Andrew Keesler
50f9b434e7
SameIssuerHostMustUseSameSecret is a valid OIDCProvider status
I saw this message in our CI logs, which led me to this fix.
  could not update status: OIDCProvider.config.supervisor.pinniped.dev "acceptance-provider" is invalid: status.status: Unsupported value: "SameIssuerHostMustUseSameSecret": supported values: "Success", "Duplicate", "Invalid"

Also - correct an integration test error message that was misleading.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-12-15 11:53:53 -05:00
Ryan Richard
43bb7117b7 Allow upstream group claim values to be either arrays or strings 2020-12-15 08:34:24 -08:00