Commit Graph

124 Commits

Author SHA1 Message Date
Ryan Richard 2633d72ce2 Change some test variable names related to additional claims
Co-authored-by: Ryan Richard <richardry@vmware.com>
Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>
2023-01-13 14:59:59 -08:00
Ryan Richard 8ff6ef32e9 Allow additional claims to map into an ID token issued by the supervisor
- Specify mappings on OIDCIdentityProvider.spec.claims.additionalClaimMappings
- Advertise additionalClaims in the OIDC discovery endpoint under claims_supported

Co-authored-by: Ryan Richard <richardry@vmware.com>
Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>
2023-01-13 14:59:50 -08: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 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 8d8f980e86 Merge branch 'main' into dynamic_clients 2022-08-26 11:35:35 -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 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 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 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 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 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 4c44f583e9 Don't add pinniped_idp_name pinniped_idp_type params into upstream state 2022-05-06 12:00:46 -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 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
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 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
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 b0ea7063c7 Supervisor should emit a warning when access token lifetime is too short 2022-01-20 13:48:50 -08:00
Ryan Richard 651d392b00 Refuse logins when no upstream refresh token and no userinfo endpoint
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-01-12 18:03:25 -08:00
Margo Crawford 6f3977de9d Store access token when refresh not available for authcode flow.
Also refactor oidc downstreamsessiondata code to be shared between
callback handler and auth handler.

Signed-off-by: Ryan Richard <richardry@vmware.com>
2022-01-12 18:03:25 -08:00
Ryan Richard 91924ec685 Revert adding allowAccessTokenBasedRefresh flag to OIDCIdentityProvider
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-01-12 18:03:25 -08:00
Margo Crawford 683a2c5b23 WIP adding access token to storage upon login 2022-01-12 18:03:25 -08:00
Margo Crawford 2958461970 Addressing PR feedback
store issuer and subject in storage for refresh
Clean up some constants

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-01-10 11:03:37 -08:00
Monis Khan 9599ffcfb9
Update all deps to latest where possible, bump Kube deps to v0.23.1
Highlights from this dep bump:

1. Made a copy of the v0.4.0 github.com/go-logr/stdr implementation
   for use in tests.  We must bump this dep as Kube code uses a
   newer version now.  We would have to rewrite hundreds of test log
   assertions without this copy.
2. Use github.com/felixge/httpsnoop to undo the changes made by
   ory/fosite#636 for CLI based login flows.  This is required for
   backwards compatibility with older versions of our CLI.  A
   separate change after this will update the CLI to be more
   flexible (it is purposefully not part of this change to confirm
   that we did not break anything).  For all browser login flows, we
   now redirect using http.StatusSeeOther instead of http.StatusFound.
3. Drop plog.RemoveKlogGlobalFlags as klog no longer mutates global
   process flags
4. Only bump github.com/ory/x to v0.0.297 instead of the latest
   v0.0.321 because v0.0.298+ pulls in a newer version of
   go.opentelemetry.io/otel/semconv which breaks k8s.io/apiserver.
   We should update k8s.io/apiserver to use the newer code.
5. Migrate all code from k8s.io/apimachinery/pkg/util/clock to
   k8s.io/utils/clock and k8s.io/utils/clock/testing
6. Delete testutil.NewDeleteOptionsRecorder and migrate to the new
   kubetesting.NewDeleteActionWithOptions
7. Updated ExpectedAuthorizeCodeSessionJSONFromFuzzing caused by
   fosite's new rotated_secrets OAuth client field.  This new field
   is currently not relevant to us as we have no private clients.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-12-16 21:15:27 -05:00
Margo Crawford 59d999956c Move ad specific stuff to controller
also make extra refresh attributes a separate field rather than part of
Extra

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-12-09 16:16:36 -08:00
Margo Crawford acaad05341 Make pwdLastSet stuff more generic and not require parsing the timestamp
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-12-09 16:16:36 -08:00
Margo Crawford f988879b6e Addressing code review changes
- changed to use custom authenticators.Response rather than the k8s one
  that doesn't include space for a DN
- Added more checking for correct idp type in token handler
- small style changes

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-11-05 14:22:43 -07:00
Margo Crawford 8396937503 Updates to tests and some error assertions 2021-11-05 14:22:43 -07:00
Margo Crawford 2c4dc2951d resolved a couple of testing related todos 2021-11-05 14:22:43 -07:00
Margo Crawford 7a58086040 Check that username and subject remain the same for ldap refresh 2021-11-05 14:22:43 -07:00
Margo Crawford 19281313dd Basic upstream LDAP/AD refresh
This stores the user DN in the session data upon login and checks that
the entry still exists upon refresh. It doesn't check anything
else about the entry yet.
2021-11-05 14:22:42 -07:00
Ryan Richard dec43289f6 Lots of small updates based on PR feedback 2021-10-20 15:53:25 -07:00
Margo Crawford 1bd346cbeb Require refresh tokens for upstream OIDC and save more session data
- Requiring refresh tokens to be returned from upstream OIDC idps
- Storing refresh tokens (for oidc) and idp information (for all idps) in custom session data during authentication
- Don't pass access=offline all the time
2021-10-08 15:48:21 -07:00
Margo Crawford 43244b6599 Do not pass through downstream prompt param
- throw an error when prompt=none because the spec says we can't ignore
  it
- ignore the other prompt params

Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-10-06 16:30:30 -07:00
Ryan Richard c6f1d29538 Use PinnipedSession type instead of fosite's DefaultSesssion type
This will allow us to store custom data inside the fosite session
storage for all downstream OIDC sessions.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-10-06 15:28:13 -07:00
Margo Crawford 1c5a2b8892 Add a couple more unit tests 2021-08-25 11:33:42 -07: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
Ryan Richard 211f4b23d1 Log auth endpoint errors with stack traces 2021-08-20 14:41:02 -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
Ryan Richard 964d16110e Some refactors based on PR feedback from @enj 2021-08-17 13:14:09 -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
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 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