Commit Graph

128 Commits

Author SHA1 Message Date
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
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
c12ffad29e Add integration test for failed client auth for a dynamic client 2022-07-21 10:13:34 -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
f5f55176af Enhance integration tests for OIDCClients in supervisor_login_test.go 2022-07-14 18:50:23 -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
f2005b4c7f Merge branch 'dynamic_clients' into require-groups-scope 2022-06-22 12:30:54 -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
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
Mo Khan
c77bee67c1
Merge pull request #1189 from vmware-tanzu/token_exchange_aud
Disallow certain requested audience strings in token exchange
2022-06-14 16:41:51 -04: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
ec533cd781 Skip some recently added integration tests when LDAP is unavailable
Also refactor to use shared test helper for skipping LDAP and AD tests.
2022-06-08 12:57:00 -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
Ryan Richard
8170889aef Update CSP header expectations in TestSupervisorLogin_Browser int test 2022-06-07 11:20:59 -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
0b106c245e Add LDAP browser flow login test to supervisor_login_test.go 2022-05-10 12:54:40 -07:00
Ryan Richard
a4e32d8f3d Extract browsertest.LoginToUpstreamLDAP() integration test helper 2022-05-09 15:43:36 -07:00
Ryan Richard
53348b8464 Add custom prefix to downstream access and refresh tokens and authcodes 2022-04-13 10:13:27 -07:00
Ryan Richard
fffcb7f5b4 Update to github.com/golangci/golangci-lint/cmd/golangci-lint@v1.44.2
- Two of the linters changed their names
- Updated code and nolint comments to make all linters pass with 1.44.2
- Added a new hack/install-linter.sh script to help developers install
  the expected version of the linter for local development
2022-03-08 12:28:09 -08:00
Margo Crawford
f6ad5d5c45 Add group change warning test for Active Directory
Also refactor some of the AD test helper functions

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2022-03-02 11:54:36 -08:00
Margo Crawford
e2c6dcd6e6 Add integration test 2022-02-17 12:50:28 -08:00
Margo Crawford
662f2cef9c Integration test for updating group search base
Also a small change to a comment
2022-02-17 11:29:59 -08:00
Margo Crawford
cd7538861a Add integration test where we don't get groups back 2022-02-17 11:29:59 -08:00
Margo Crawford
013b521838 Upstream ldap group refresh:
- Doing it inline on the refresh request
2022-02-17 11:29:59 -08:00
Monis Khan
b8202d89d9
Enforce naming convention for browser based tests
This allows us to target browser based tests with the regex:

go test -v -race -count 1 -timeout 0 ./test/integration -run '/_Browser'

New tests that call browsertest.Open will automatically be forced to
follow this convention.

Signed-off-by: Monis Khan <mok@vmware.com>
2022-02-16 09:20:28 -05:00
Margo Crawford
513c943e87 Keep all scopes except offline_access in integration test 2022-01-19 13:29:26 -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
Margo Crawford
0cd086cf9c Check username claim is unchanged for oidc.
Also add integration tests for claims changing.
2022-01-10 11:03:37 -08: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
65f3464995 Fix issue with very high integer value parsing, add unit tests
also add comment about urgent replication
2021-12-09 16:16:36 -08:00
Margo Crawford
ee4f725209 Incorporate PR feedback 2021-12-09 16:16:36 -08:00
Margo Crawford
ef5a04c7ce Check for locked users on ad upstream refresh
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-12-09 16:16:36 -08:00
Margo Crawford
f62e9a2d33 Active directory checks for deactivated user
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-12-09 16:16:36 -08:00
Margo Crawford
da9b4620b3 Active Directory checks whether password has changed recently during
upstream refresh

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-12-09 16:16:35 -08:00
Margo Crawford
cb60a44f8a extract ldap refresh search into helper function
also added an integration test for refresh failing after updating the username attribute
2021-11-05 14:22:43 -07:00
Margo Crawford
b5b8cab717 Refactors:
- pull construction of authenticators.Response into searchAndBindUser
- remove information about the identity provider in the error that gets
  returned to users. Put it in debug instead, where it may show up in
  logs.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-11-05 14:22:43 -07:00
Margo Crawford
722b5dcc1b Test for change to stored username or subject.
All of this is still done staticly.
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
9e05d175a7 Add integration test: upstream refresh failure during downstream refresh 2021-10-13 15:12:19 -07:00
Margo Crawford
05f5bac405 ValidatedSettings is all or nothing
If either the search base or the tls settings is invalid, just
recheck everything.
2021-09-07 13:09:35 -07:00
Margo Crawford
27c1d2144a Make sure search base in the validatedSettings cache is properly updated when the bind secret changes 2021-09-07 13:09:35 -07:00
Margo Crawford
6f221678df Change sAMAccountName env vars to userPrincipalName
and add E2E ActiveDirectory test
also fixed regexes in supervisor_login_test to be anchored to the
beginning and end
2021-08-26 16:18:05 -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
Margo Crawford
5e9087263d Increase timeout for activedirectoryidentityprovider to be loaded 2021-08-18 16:24:05 -07:00
Margo Crawford
a20aee5f18 Update test assertions to reflect userPrincipalName as username 2021-08-18 13:18:53 -07:00
Margo Crawford
26c47d564f Make new combined sAMAccountName@domain attribute the group name
Also change default username attribute to userPrincipalName
2021-08-17 16:53:26 -07:00