Commit Graph

27 Commits

Author SHA1 Message Date
aram price
e111ca02da Use the narrowest possible interface 2020-12-09 17:34:02 -08:00
Matt Moyer
644cb687b9
Grant the Pinniped STS scope in authorize/callback handlers.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-09 09:36:45 -06:00
Aram Price
d91baba240 authorize and callback endpoints now handle the offline_access scope
- This is in preparation for the token endpoint to support the refresh
  grant

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-12-07 17:22:34 -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
f0ebd808d7
Switch CSRF cookie from Same-Site=Strict to Same-Site=Lax.
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>
2020-12-03 21:30:00 -06:00
Matt Moyer
c23c54f500
Add an explicit Path=/; to our CSRF cookie, per the spec.
> [...] a cookie named "__Host-cookie1" MUST contain a "Path" attribute with a value of "/".

https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00#section-3.2

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-12-02 15:55:33 -06:00
Ryan Richard
ccddeb4cda Merge branch 'main' into callback-endpoint 2020-11-20 15:13:25 -08:00
Ryan Richard
c4ff1ca304 auth_handler.go: Ignore invalid CSRF cookies rather than return error
Generate a new cookie for the user and move on as if they had not sent
a bad cookie. Hopefully this will make the user experience better if,
for example, the server rotated cookie signing keys and then a user
submitted a very old cookie.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-20 13:56:35 -08:00
Ryan Richard
72321fc106
Use /callback (without IDP name) path for callback endpoint (part 1)
This is much nicer UX for an administrator installing a UpstreamOIDCProvider
CRD. They don't have to guess as hard at what the callback endpoint path should
be for their UpstreamOIDCProvider.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-20 16:14:45 -05:00
Ryan Richard
ee84f31f42 callback_handler.go: Add JWT Issuer claim to storage 2020-11-19 08:35:23 -08: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
e0a9bef6ce
Move ./internal/oidcclient to ./pkg/oidcclient.
This will allow it to be imported by Go code outside of our repository, which was something we have planned for since this code was written.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2020-11-17 14:53:32 -06:00
Ryan Richard
052cdc40dc
callback_handler.go: add CSRF and version state validations
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-16 14:41:00 -05:00
Andrew Keesler
4138c9244f
callback_handler.go: write 2 invalid cookie tests
Also common-ize some more constants shared between the auth and callback
endpoints.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-16 11:47:49 -05: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
080bb594b2 Supervisor authorize endpoint reuses existing CSRF cookies and signs new ones
- To better support having multiple downstream providers configured,
  the authorize endpoint will share a CSRF cookie between all
  downstream providers' authorize endpoints. The first time a
  user's browser hits the authorize endpoint of any downstream
  provider, that endpoint will set the cookie. Then if the user
  starts an authorize flow with that same downstream provider or with
  any other downstream provider which shares the same domain name
  (i.e. differentiated by issuer path), then the same cookie will be
  submitted and respected.
- Just in case we are sharing the domain name with some other app,
  we sign the value of any new CSRF cookie and check the signature
  when we receive the cookie. This wasn't strictly necessary since
  we probably won't share a domain name with other apps, but it
  wasn't hard to add this cookie signing.

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-12 15:36:59 -08:00
Andrew Keesler
c2262773e6 Finish the WIP from the previous commit for saving authorize endpoint state
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-11 12:29:14 -08:00
Monis Khan
dd190dede6 WIP for saving authorize endpoint state into upstream state param
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-10 17:58:00 -08:00
Andrew Keesler
005225d5f9 Use the new plog pkg in auth_handler.go
- Add a new helper method to plog to make a consistent way to log
  expected errors at the info level (as opposed to unexpected
  system errors that would be logged using plog.Error)

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-10 10:33:52 -08:00
Ryan Richard
246471bc91 Also run OIDC validations in supervisor authorize endpoint
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-06 14:44:58 -08:00
Ryan Richard
33ce79f89d Expose the Supervisor OIDC authorization endpoint to the public 2020-11-04 17:06:47 -08:00
Andrew Keesler
a36f7c6c07 Test that the port of localhost redirect URI is ignored during validation
Also move definition of our oauth client and the general fosite
configuration to a helper so we can use the same config to construct
the handler for both test and production code.

Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-04 15:04:50 -08:00
Andrew Keesler
2564d1be42 Supervisor authorize endpoint errors when missing PKCE params
Signed-off-by: Ryan Richard <richardry@vmware.com>
2020-11-04 12:19:07 -08:00
Andrew Keesler
6fe455c687
auth_handler.go: comment out currently unused fosite wiring
See e8f4336 for why this is here in the first place.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-04 11:20:03 -05:00
Andrew Keesler
e8f433643f
auth_handler.go: only inject oauth store into handler
Previously we were injecting the whole oauth handler chain into this function,
which meant we were essentially writing unit tests to test our tests. Let's push
some of this logic into the source code.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-04 10:35:26 -05:00
Andrew Keesler
259ffb5267
Checkpoint: write a single negative test using fosite
Bringing in fosite to our go.mod introduced those other go.mod changes.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-04 10:15:19 -05:00
Ryan Richard
c34e5a727d Starting the implementation of an OIDC authorization endpoint handler
Does not validate incoming request parameters yet. Also is not
served on the http/https ports yet. Those will come in future commits.

Signed-off-by: Andrew Keesler <akeesler@vmware.com>
2020-11-03 16:17:38 -08:00