From b3f138679cff12e0453581625019045232de7eca Mon Sep 17 00:00:00 2001 From: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> Date: Thu, 28 Jul 2022 09:47:53 -0700 Subject: [PATCH] identity/oidc: allow filtering the list providers response by an allowed_client_id (#16181) * identity/oidc: allow filtering the list providers response by an allowed_client_id * adds changelog * adds api documentation * use identity store view in list provider test --- changelog/16181.txt | 3 + http/logical.go | 2 + vault/identity_store_oidc_provider.go | 58 +++++++++++-- vault/identity_store_oidc_provider_test.go | 83 +++++++++++++++++++ .../secret/identity/oidc-provider.mdx | 5 ++ 5 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 changelog/16181.txt diff --git a/changelog/16181.txt b/changelog/16181.txt new file mode 100644 index 000000000..1e97d1e15 --- /dev/null +++ b/changelog/16181.txt @@ -0,0 +1,3 @@ +```release-note:improvement +identity/oidc: allows filtering the list providers response by an allowed_client_id +``` diff --git a/http/logical.go b/http/logical.go index 2de6c3295..34c60d18e 100644 --- a/http/logical.go +++ b/http/logical.go @@ -178,6 +178,8 @@ func buildLogicalRequestNoAuth(perfStandby bool, w http.ResponseWriter, r *http. path += "/" } + data = parseQuery(r.URL.Query()) + case "OPTIONS", "HEAD": default: return nil, nil, http.StatusMethodNotAllowed, nil diff --git a/vault/identity_store_oidc_provider.go b/vault/identity_store_oidc_provider.go index 3867c2095..a7422fc6d 100644 --- a/vault/identity_store_oidc_provider.go +++ b/vault/identity_store_oidc_provider.go @@ -135,6 +135,19 @@ type provider struct { effectiveIssuer string } +// allowedClientID returns true if the given client ID is in +// the provider's set of allowed client IDs or its allowed client +// IDs contains the wildcard "*" char. +func (p *provider) allowedClientID(clientID string) bool { + for _, allowedID := range p.AllowedClientIDs { + switch allowedID { + case "*", clientID: + return true + } + } + return false +} + type providerDiscovery struct { Issuer string `json:"issuer"` Keys string `json:"jwks_uri"` @@ -356,6 +369,14 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path { }, { Pattern: "oidc/provider/?$", + Fields: map[string]*framework.FieldSchema{ + "allowed_client_id": { + Type: framework.TypeString, + Description: "Filters the list of OIDC providers to those " + + "that allow the given client ID in their set of allowed_client_ids.", + Query: true, + }, + }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ListOperation: &framework.PathOperation{ Callback: i.pathOIDCListProvider, @@ -1301,6 +1322,34 @@ func (i *IdentityStore) pathOIDCListProvider(ctx context.Context, req *logical.R if err != nil { return nil, err } + + // If allowed_client_id is provided as a query parameter, filter the set of + // returned OIDC providers to those that allow the given value in their set + // of allowed_client_ids. + if clientIDRaw, ok := d.GetOk("allowed_client_id"); ok { + clientID := clientIDRaw.(string) + if clientID == "" { + return logical.ListResponse(providers), nil + } + + filtered := make([]string, 0) + for _, name := range providers { + provider, err := i.getOIDCProvider(ctx, req.Storage, name) + if err != nil { + return nil, err + } + if provider == nil { + continue + } + + if provider.allowedClientID(clientID) { + filtered = append(filtered, name) + } + } + + providers = filtered + } + return logical.ListResponse(providers), nil } @@ -1592,8 +1641,7 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ if client == nil { return authResponse("", state, ErrAuthInvalidClientID, "client with client_id not found") } - if !strutil.StrListContains(provider.AllowedClientIDs, "*") && - !strutil.StrListContains(provider.AllowedClientIDs, clientID) { + if !provider.allowedClientID(clientID) { return authResponse("", state, ErrAuthUnauthorizedClient, "client is not authorized to use the provider") } @@ -1812,8 +1860,7 @@ func (i *IdentityStore) pathOIDCToken(ctx context.Context, req *logical.Request, } // Validate that the client is authorized to use the provider - if !strutil.StrListContains(provider.AllowedClientIDs, "*") && - !strutil.StrListContains(provider.AllowedClientIDs, clientID) { + if !provider.allowedClientID(clientID) { return tokenResponse(nil, ErrTokenInvalidClient, "client is not authorized to use the provider") } @@ -2133,8 +2180,7 @@ func (i *IdentityStore) pathOIDCUserInfo(ctx context.Context, req *logical.Reque } // Validate that the client is authorized to use the provider - if !strutil.StrListContains(provider.AllowedClientIDs, "*") && - !strutil.StrListContains(provider.AllowedClientIDs, clientID) { + if !provider.allowedClientID(clientID) { return userInfoResponse(nil, ErrUserInfoAccessDenied, "client is not authorized to use the provider") } diff --git a/vault/identity_store_oidc_provider_test.go b/vault/identity_store_oidc_provider_test.go index d3a47dfca..a923c31d1 100644 --- a/vault/identity_store_oidc_provider_test.go +++ b/vault/identity_store_oidc_provider_test.go @@ -3350,6 +3350,89 @@ func TestOIDC_Path_OIDC_Provider_List(t *testing.T) { expectStrings(t, respListProvidersAfterDelete.Data["keys"].([]string), expectedStrings) } +func TestOIDC_Path_OIDC_Provider_List_Filter(t *testing.T) { + c, _, _ := TestCoreUnsealed(t) + ctx := namespace.RootContext(nil) + + // Create providers with different allowed_client_ids values + providers := []struct { + name string + allowedClientIDs []string + }{ + {name: "p0", allowedClientIDs: []string{"*"}}, + {name: "p1", allowedClientIDs: []string{"abc"}}, + {name: "p2", allowedClientIDs: []string{"abc", "def"}}, + {name: "p3", allowedClientIDs: []string{"abc", "def", "ghi"}}, + {name: "p4", allowedClientIDs: []string{"ghi"}}, + {name: "p5", allowedClientIDs: []string{"jkl"}}, + } + for _, p := range providers { + resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/provider/" + p.name, + Operation: logical.CreateOperation, + Storage: c.identityStore.view, + Data: map[string]interface{}{ + "allowed_client_ids": p.allowedClientIDs, + }, + }) + expectSuccess(t, resp, err) + } + + tests := []struct { + name string + clientIDFilter string + expectedProviders []string + }{ + { + name: "list providers with client_id filter subset 1", + clientIDFilter: "abc", + expectedProviders: []string{"default", "p0", "p1", "p2", "p3"}, + }, + { + name: "list providers with client_id filter subset 2", + clientIDFilter: "def", + expectedProviders: []string{"default", "p0", "p2", "p3"}, + }, + { + name: "list providers with client_id filter subset 3", + clientIDFilter: "ghi", + expectedProviders: []string{"default", "p0", "p3", "p4"}, + }, + { + name: "list providers with client_id filter subset 4", + clientIDFilter: "jkl", + expectedProviders: []string{"default", "p0", "p5"}, + }, + { + name: "list providers with client_id filter only matching glob", + clientIDFilter: "globmatch_only", + expectedProviders: []string{"default", "p0"}, + }, + { + name: "list providers with empty client_id filter returns all", + clientIDFilter: "", + expectedProviders: []string{"default", "p0", "p1", "p2", "p3", "p4", "p5"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // List providers with the allowed_client_id query parameter + resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/provider", + Operation: logical.ListOperation, + Storage: c.identityStore.view, + Data: map[string]interface{}{ + "allowed_client_id": tc.clientIDFilter, + }, + }) + expectSuccess(t, resp, err) + + // Assert the filtered set of providers is returned + require.Equal(t, tc.expectedProviders, resp.Data["keys"]) + }) + } +} + // TestOIDC_Path_OpenIDProviderConfig tests read operations for the // openid-configuration path func TestOIDC_Path_OpenIDProviderConfig(t *testing.T) { diff --git a/website/content/api-docs/secret/identity/oidc-provider.mdx b/website/content/api-docs/secret/identity/oidc-provider.mdx index ed41ca863..e38a36f97 100644 --- a/website/content/api-docs/secret/identity/oidc-provider.mdx +++ b/website/content/api-docs/secret/identity/oidc-provider.mdx @@ -84,6 +84,11 @@ This endpoint returns a list of all OIDC providers. | :----- | :------------------------------ | | `LIST` | `/identity/oidc/provider` | +### Query Parameters + +- `allowed_client_id` `(string: )` – Filters the list of OIDC providers to those + that allow the given client ID in their set of [allowed_client_ids](/api-docs/secret/identity/oidc-provider#allowed_client_ids). + ### Sample Request ```shell-session