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>
- AudienceMatchingStrategy: we want to use the default matcher from
fosite, so remove that line
- AllowedPromptValues: We can use the default if we add a small
change to the auth_handler.go to account for it (in a future commit)
- MinParameterEntropy: Use the fosite default to make it more likely
that off the shelf OIDC clients can work with the supervisor
Signed-off-by: Ryan Richard <richardry@vmware.com>
- 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>
When we try to decode with the wrong decryption key, we could get any number of
error messages, depending on what failure mode we are in (couldn't authenticate
plaintext after decryption, couldn't deserialize, etc.). This change makes the
test weaker, but at least we know we will get an error message in the case where
the decryption key is wrong.
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
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>
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>
Note that we don't cache the securecookie.SecureCookie that we use in our
implementation. This was purely because of laziness. We should think about
caching this value in the future.
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
- Make it more likely that the end user will get the more specific error
message saying that their refresh token has expired the first time
that they try to use an expired refresh token
Signed-off-by: Ryan Richard <richardry@vmware.com>
- This struct represents the configuration of all timeouts. These
timeouts are all interrelated to declare them all in one place.
This should also make it easier to allow the user to override
our defaults if we would like to implement such a feature in the
future.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
- This is to make it easier for the token exchange branch to also edit
this test without causing a lot of merge conflicts with the
refresh token branch, to enable parallel development of closely
related stories.
- This refactor will allow us to add new test tables for the
refresh and token exchange requests, which both must come after
an initial successful authcode exchange has already happened
Signed-off-by: Margo Crawford <margaretc@vmware.com>
`token_endpoint_auth_signing_alg_values_supported` is only related to
private_key_jwt and client_secret_jwt client authentication methods
at the token endpoint, which we do not support. See
https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
for more details.
Signed-off-by: Aram Price <pricear@vmware.com>
This refactors the `UpstreamOIDCIdentityProviderI` interface and its implementations to pass ID token claims through a `*oidctypes.Token` return parameter rather than as a third return parameter.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
TokenURL is used by Fosite to validate clients authenticating with the
private_key_jwt method. We don't have any use for this right now, so just leave
this blank until we need it.
See when Ryan brought this up in
https://github.com/vmware-tanzu/pinniped/pull/239#discussion_r528022162.
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
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>
I'm worried that these errors are going to be really burried from the user, so
add some log statements to try to make them a tiny bit more observable.
Also follow some of our error message convetions by using lowercase error
messages.
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This CSRF cookie needs to be included on the request to the callback endpoint triggered by the redirect from the OIDC upstream provider. This is not allowed by `Same-Site=Strict` but is allowed by `Same-Site=Lax` because it is a "cross-site top-level navigation" [1].
We didn't catch this earlier with our Dex-based tests because the upstream and downstream issuers were on the same parent domain `*.svc.cluster.local` so the cookie was allowed even with `Strict` mode.
[1]: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00#section-3.2
Signed-off-by: Matt Moyer <moyerm@vmware.com>
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>
