Commit Graph

342 Commits

Author SHA1 Message Date
Joshua Casey
02ed2d9c95 Backfill tests 2023-01-04 13:29:14 -06:00
Ryan Richard
cd17bdb5f7 POC of OIDCIdentityProvider.spec.claims.additionalClaimMappings
Proof-of-concept implementation of a potential new Supervisor feature
which allows arbitrary upstream ID token claims to be mapped into a
new top-level claim in the ID tokens issued by the Supervisor.
2022-12-21 07:13:26 -06:00
Ryan Richard
976035115e Stop using pointer pkg functions that were deprecated by dependency bump 2022-12-14 08:47:16 -08:00
Ryan Richard
85b67f254c Add more assertion to token_handler_test.go for token exchange exp claim 2022-12-14 08:47:16 -08:00
Ryan Richard
e1a0367b03 Upgrade project Go dependencies
Most of the changes in this commit are because of these fosite PRs
which changed behavior and/or APIs in fosite:
- https://github.com/ory/fosite/pull/667
- https://github.com/ory/fosite/pull/679 (from me!)
- https://github.com/ory/fosite/pull/675
- https://github.com/ory/fosite/pull/688

Due to the changes in fosite PR #688, we need to bump our storage
version for anything which stores the DefaultSession struct as JSON.
2022-12-14 08:47:16 -08:00
Ryan Richard
1c296e5c4c Implement the OIDCClientSecretRequest API
This commit is a WIP commit because it doesn't include many tests
for the new feature.

Co-authored-by: Ryan Richard <richardry@vmware.com>
Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
2022-09-21 15:15:07 -07:00
Ryan Richard
8d8f980e86 Merge branch 'main' into dynamic_clients 2022-08-26 11:35:35 -07:00
Ryan Richard
a5ac710831 Add unit test for expired refresh tokens used in refresh flow 2022-08-26 09:23:25 -07:00
Ryan Richard
65197d0f9d Merge branch 'main' into access_token_validation 2022-08-24 16:41:12 -07:00
Ryan Richard
c6c2c525a6 Upgrade the linter and fix all new linter warnings
Also fix some tests that were broken by bumping golang and dependencies
in the previous commits.

Note that in addition to changes made to satisfy the linter which do not
impact the behavior of the code, this commit also adds ReadHeaderTimeout
to all usages of http.Server to satisfy the linter (and because it
seemed like a good suggestion).
2022-08-24 14:45:55 -07:00
Ryan Richard
1384f75731 Improve token exchange error messages and error test cases 2022-08-23 17:20:30 -07:00
Ryan Richard
0bb2c7beb7 Always add the azp claim to ID tokens to show the original client ID
When the token exchange grant type is used to get a cluster-scoped
ID token, the returned token has a new audience value. The client ID
of the client which performed the authorization was lost. This didn't
matter before, since the only client was `pinniped-cli`, but now that
dynamic clients can be registered, the information would be lost in the
cluster-scoped ID token. It could be useful for logging, tracing, or
auditing, so preserve the information by putting the client ID into the
`azp` claim in every ID token (authcode exchange, clsuter-scoped, and
refreshed ID tokens).
2022-08-09 16:07:23 -07:00
Ryan Richard
22fbced863 Create username scope, required for clients to get username in ID token
- For backwards compatibility with older Pinniped CLIs, the pinniped-cli
  client does not need to request the username or groups scopes for them
  to be granted. For dynamic clients, the usual OAuth2 rules apply:
  the client must be allowed to request the scopes according to its
  configuration, and the client must actually request the scopes in the
  authorization request.
- If the username scope was not granted, then there will be no username
  in the ID token, and the cluster-scoped token exchange will fail since
  there would be no username in the resulting cluster-scoped ID token.
- The OIDC well-known discovery endpoint lists the username and groups
  scopes in the scopes_supported list, and lists the username and groups
  claims in the claims_supported list.
- Add username and groups scopes to the default list of scopes
  put into kubeconfig files by "pinniped get kubeconfig" CLI command,
  and the default list of scopes used by "pinniped login oidc" when
  no list of scopes is specified in the kubeconfig file
- The warning header about group memberships changing during upstream
  refresh will only be sent to the pinniped-cli client, since it is
  only intended for kubectl and it could leak the username to the
  client (which may not have the username scope granted) through the
  warning message text.
- Add the user's username to the session storage as a new field, so that
  during upstream refresh we can compare the original username from the
  initial authorization to the refreshed username, even in the case when
  the username scope was not granted (and therefore the username is not
  stored in the ID token claims of the session storage)
- Bump the Supervisor session storage format version from 2 to 3
  due to the username field being added to the session struct
- Extract commonly used string constants related to OIDC flows to api
  package.
- Change some import names to make them consistent:
  - Always import github.com/coreos/go-oidc/v3/oidc as "coreosoidc"
  - Always import go.pinniped.dev/generated/latest/apis/supervisor/oidc
    as "oidcapi"
  - Always import go.pinniped.dev/internal/oidc as "oidc"
2022-08-08 16:29:22 -07:00
Ryan Richard
88f611d31a Be extra defensive and don't lookup dynamic client ID's lacking prefix 2022-07-22 15:19:19 -07:00
Ryan Richard
0495286f97 Fix lint error and remove accidental direct dep on ory/x
Fixing some mistakes from previous commit on feature branch.
2022-07-21 13:50:33 -07:00
Ryan Richard
e42f5488fa More unit tests for dynamic clients
- Add dynamic client unit tests for the upstream OIDC callback and
  POST login endpoints.
- Enhance a few log statements to print the full fosite error messages
  into the logs where they were previously only printing the name of
  the error type.
2022-07-21 09:26:00 -07:00
Ryan Richard
34509e7430 Add more unit tests for dynamic clients and enhance token exchange
- Enhance the token exchange to check that the same client is used
  compared to the client used during the original authorization and
  token requests, and also check that the client has the token-exchange
  grant type allowed in its configuration.
- Reduce the minimum required bcrypt cost for OIDCClient secrets
  because 15 is too slow for real-life use, especially considering
  that every login and every refresh flow will require two client auths.
- In unit tests, use bcrypt hashes with a cost of 4, because bcrypt
  slows down by 13x when run with the race detector, and we run our
  tests with the race detector enabled, causing the tests to be
  unacceptably slow. The production code uses a higher minimum cost.
- Centralize all pre-computed bcrypt hashes used by unit tests to a
  single place. Also extract some other useful test helpers for
  unit tests related to OIDCClients.
- Add tons of unit tests for the token endpoint related to dynamic
  clients for authcode exchanges, token exchanges, and refreshes.
2022-07-20 13:55:56 -07:00
Ryan Richard
e0ecdc004b Allow dynamic clients to be used in downstream OIDC flows
This is only a first commit towards making this feature work.
- Hook dynamic clients into fosite by returning them from the storage
  interface (after finding and validating them)
- In the auth endpoint, prevent the use of the username and password
  headers for dynamic clients to force them to use the browser-based
  login flows for all the upstream types
- Add happy path integration tests in supervisor_login_test.go
- Add lots of comments (and some small refactors) in
  supervisor_login_test.go to make it much easier to understand
- Add lots of unit tests for the auth endpoint regarding dynamic clients
  (more unit tests to be added for other endpoints in follow-up commits)
- Enhance crud.go to make lifetime=0 mean never garbage collect,
  since we want client secret storage Secrets to last forever
- Move the OIDCClient validation code to a package where it can be
  shared between the controller and the fosite storage interface
- Make shared test helpers for tests that need to create OIDC client
  secret storage Secrets
- Create a public const for "pinniped-cli" now that we are using that
  string in several places in the production code
2022-07-14 09:51:11 -07:00
Margo Crawford
a010e72b29 Merge branch 'dynamic_clients' into require-groups-scope 2022-06-22 14:27:06 -07:00
Margo Crawford
dac0395680 Add a couple tests, address pr comments
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-06-22 14:19:55 -07:00
Margo Crawford
f2005b4c7f Merge branch 'dynamic_clients' into require-groups-scope 2022-06-22 12:30:54 -07:00
Margo Crawford
c70a0b99a8 Don't do ldap group search when group scope not specified
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-06-22 10:58:08 -07:00
Margo Crawford
9903c5f79e Handle refresh requests without groups scope
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-06-22 08:21:16 -07:00
Monis Khan
59d67322d3
Static validation for OIDC clients
The following validation is enforced:

1. Names must start with client.oauth.pinniped.dev-
2. Redirect URIs must start with https://
   or http://127.0.0.1
   or http://::1
3. All spec lists must not have duplicates

Added an integration test to assert all static validations.

Signed-off-by: Monis Khan <mok@vmware.com>
2022-06-15 15:09:40 -04:00
Margo Crawford
4d0c2e16f4 require groups scope to get groups back from supervisor
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-06-15 08:00:17 -07:00
Ryan Richard
b9272b2729 Reserve all of *.pinniped.dev for requested aud in token exchanges
Our previous plan was to reserve only *.oauth.pinniped.dev but we
changed our minds during PR review.
2022-06-13 12:08:11 -07:00
Ryan Richard
321abfc98d Merge branch 'dynamic_clients' into token_exchange_aud 2022-06-08 09:03:29 -07:00
Ryan Richard
ea45e5dfef Disallow certain requested audience strings in token exchange 2022-06-07 16:32:19 -07:00
Mo Khan
472ab229e7
Merge branch 'main' into auth_handler_form_post_csp 2022-06-07 18:26:52 -04:00
Ryan Richard
7751c0bf59
Bump project deps, including kube 0.23.6->0.24.1 and Go 1.18.1->1.18.3
Several API changes in Kube required changes in Pinniped code.

Signed-off-by: Monis Khan <mok@vmware.com>
2022-06-07 15:26:30 -04:00
Ryan Richard
b99c4773a2 Use CSP headers in auth handler response
When response_mode=form_post is requested, some error cases will be
returned to the client using the form_post web page to POST the result
back to the client's redirect URL.
2022-06-02 09:23:34 -07:00
Ryan Richard
39fd9ba270 Small refactors and comments for LDAP/AD UI 2022-05-19 16:02:08 -07:00
Ryan Richard
aa732a41fb Add LDAP browser flow login failure tests to supervisor_login_test.go
Also do some refactoring to share more common test setup code in
supervisor_login_test.go.
2022-05-10 16:28:08 -07:00
Ryan Richard
4c44f583e9 Don't add pinniped_idp_name pinniped_idp_type params into upstream state 2022-05-06 12:00:46 -07:00
Ryan Richard
ec22b5715b Add Pinniped favicon to login UI page 🦭 2022-05-05 14:46:07 -07:00
Ryan Richard
cffa353ffb Login page styling/structure for users, screen readers, passwd managers
Also:
- Add CSS to login page
- Refactor login page HTML and CSS into a new package
- New custom CSP headers for the login page, because the requirements
  are different from the form_post page
2022-05-05 13:13:25 -07:00
Ryan Richard
6ca7c932ae Add unit test for rendering form_post response from POST /login 2022-05-05 13:13:25 -07:00
Ryan Richard
2e031f727b Use security headers for the form_post page in the POST /login endpoint
Also use more specific test assertions where security headers are
expected. And run the unit tests for the login package in parallel.
2022-05-03 16:46:09 -07:00
Margo Crawford
388cdb6ddd Fix bug where form was posting to the wrong path
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-05-03 15:18:38 -07:00
Ryan Richard
69e5169fc5 Implement post_login_handler.go to accept form post and auth to LDAP/AD
Also extract some helpers from auth_handler.go so they can be shared
with the new handler.
2022-04-29 16:02:00 -07:00
Margo Crawford
646c6ec9ed Show error message on login page
Also add autocomplete attribute and title element

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-29 10:36:13 -07:00
Margo Crawford
453c69af7d Fix some errors and pass state as form element
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-28 12:07:04 -07:00
Margo Crawford
07b2306254 Add basic outline of login get handler
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-28 11:51:36 -07:00
Margo Crawford
ae60d4356b Some refactoring of shared code between OIDC and LDAP browser flows
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-27 08:51:37 -07:00
Margo Crawford
379a803509 when password header but not username is sent to password grant, error
also add more unit tests

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-26 16:46:58 -07:00
Ryan Richard
65eed7e742 Implement login_handler.go to defer to other handlers
The other handlers for GET and POST requests are not yet implemented in
this commit. The shared handler code in login_handler.go takes care of
things checking the method, checking the CSRF cookie, decoding the state
param, and adding security headers on behalf of both the GET and POST
handlers.

Some code has been extracted from callback_handler.go to be shared.
2022-04-26 15:37:30 -07:00
Margo Crawford
eb1d3812ec Update authorization endpoint to redirect to new login page
Also fix some test failures on the callback handler, register the
new login handler in manager.go and add a (half baked) integration test

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-26 12:51:56 -07:00
Margo Crawford
8832362b94 WIP: Add login handler for LDAP/AD web login
Also change state param to include IDP type
2022-04-25 16:41:55 -07:00
Margo Crawford
694e4d6df6 Advertise browser_authcode flow in ldap idp discovery
To keep this backwards compatible, this PR changes how
the cli deals with ambiguous flows. Previously, if there
was more than one flow advertised, the cli would require users
to set the flag --upstream-identity-provider-flow. Now it
chooses the first one in the list.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-04-25 14:54:21 -07:00
Ryan Richard
132d2aac72 add a code comment 2022-04-19 11:35:46 -07:00