Rename scopes to scopes_supported for OIDC providers (#12851)

This commit is contained in:
Austin Gebauer 2021-10-15 19:33:32 -07:00 committed by GitHub
parent ae79afdd26
commit 4e5b865c4f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 79 additions and 51 deletions

View File

@ -141,7 +141,7 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
// Create the OIDC provider
_, err = active.Logical().Write("identity/oidc/provider/test-provider", map[string]interface{}{
"allowed_client_ids": []string{clientID},
"scopes": []string{"user", "groups"},
"scopes_supported": []string{"user", "groups"},
})
require.NoError(t, err)

View File

@ -33,6 +33,9 @@ const (
scopesDelimiter = " "
accessTokenScopesMeta = "scopes"
accessTokenClientIDMeta = "client_id"
clientIDLength = 32
clientSecretLength = 64
clientSecretPrefix = "hvo_secret_"
// Storage path constants
oidcProviderPrefix = "oidc_provider/"
@ -41,10 +44,6 @@ const (
clientPath = oidcProviderPrefix + "client/"
providerPath = oidcProviderPrefix + "provider/"
clientIDLength = 32
clientSecretLength = 64
clientSecretPrefix = "hvo_secret_"
// Error constants used in the Authorization Endpoint. See details at
// https://openid.net/specs/openid-connect-core-1_0.html#AuthError.
ErrAuthUnsupportedResponseType = "unsupported_response_type"
@ -108,7 +107,7 @@ type client struct {
type provider struct {
Issuer string `json:"issuer"`
AllowedClientIDs []string `json:"allowed_client_ids"`
Scopes []string `json:"scopes"`
ScopesSupported []string `json:"scopes_supported"`
// effectiveIssuer is a calculated field and will be either Issuer (if
// that's set) or the Vault instance's api_addr.
@ -303,9 +302,9 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Type: framework.TypeCommaStringSlice,
Description: "The client IDs that are permitted to use the provider",
},
"scopes": {
"scopes_supported": {
Type: framework.TypeCommaStringSlice,
Description: "The scopes available for requesting on the provider",
Description: "The scopes supported for requesting on the provider",
},
},
Operations: map[logical.Operation]framework.OperationHandler{
@ -344,8 +343,10 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Description: "Name of the provider",
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.ReadOperation: i.pathOIDCProviderDiscovery,
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
Callback: i.pathOIDCProviderDiscovery,
},
},
HelpSynopsis: "Query OIDC configurations",
HelpDescription: "Query this path to retrieve the configured OIDC Issuer and Keys endpoints, response types, subject types, and signing algorithms used by the OIDC backend.",
@ -358,8 +359,10 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
Description: "Name of the provider",
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.ReadOperation: i.pathOIDCReadProviderPublicKeys,
Operations: map[logical.Operation]framework.OperationHandler{
logical.ReadOperation: &framework.PathOperation{
Callback: i.pathOIDCReadProviderPublicKeys,
},
},
HelpSynopsis: "Retrieve public keys",
HelpDescription: "Returns the public portion of keys for a named OIDC provider. Clients can use them to validate the authenticity of an ID token.",
@ -589,7 +592,7 @@ func (i *IdentityStore) providersReferencingTargetScopeName(ctx context.Context,
if err := entry.DecodeJSON(&tempProvider); err != nil {
return nil, err
}
for _, a := range tempProvider.Scopes {
for _, a := range tempProvider.ScopesSupported {
if a == targetScopeName {
providers = append(providers, providerName)
}
@ -620,13 +623,13 @@ func (i *IdentityStore) pathOIDCCreateUpdateAssignment(ctx context.Context, req
if entitiesRaw, ok := d.GetOk("entity_ids"); ok {
assignment.EntityIDs = entitiesRaw.([]string)
} else if req.Operation == logical.CreateOperation {
assignment.EntityIDs = d.GetDefaultOrZero("entity_ids").([]string)
assignment.EntityIDs = d.Get("entity_ids").([]string)
}
if groupsRaw, ok := d.GetOk("group_ids"); ok {
assignment.GroupIDs = groupsRaw.([]string)
} else if req.Operation == logical.CreateOperation {
assignment.GroupIDs = d.GetDefaultOrZero("group_ids").([]string)
assignment.GroupIDs = d.Get("group_ids").([]string)
}
// remove duplicates and lowercase entities and groups
@ -748,13 +751,13 @@ func (i *IdentityStore) pathOIDCCreateUpdateScope(ctx context.Context, req *logi
if descriptionRaw, ok := d.GetOk("description"); ok {
scope.Description = descriptionRaw.(string)
} else if req.Operation == logical.CreateOperation {
scope.Description = d.GetDefaultOrZero("description").(string)
scope.Description = d.Get("description").(string)
}
if templateRaw, ok := d.GetOk("template"); ok {
scope.Template = templateRaw.(string)
} else if req.Operation == logical.CreateOperation {
scope.Template = d.GetDefaultOrZero("template").(string)
scope.Template = d.Get("template").(string)
}
// Attempt to decode as base64 and use that if it works
@ -1093,24 +1096,24 @@ func (i *IdentityStore) pathOIDCCreateUpdateProvider(ctx context.Context, req *l
if issuerRaw, ok := d.GetOk("issuer"); ok {
provider.Issuer = issuerRaw.(string)
} else if req.Operation == logical.CreateOperation {
provider.Issuer = d.GetDefaultOrZero("issuer").(string)
provider.Issuer = d.Get("issuer").(string)
}
if allowedClientIDsRaw, ok := d.GetOk("allowed_client_ids"); ok {
provider.AllowedClientIDs = allowedClientIDsRaw.([]string)
} else if req.Operation == logical.CreateOperation {
provider.AllowedClientIDs = d.GetDefaultOrZero("allowed_client_ids").([]string)
provider.AllowedClientIDs = d.Get("allowed_client_ids").([]string)
}
if scopesRaw, ok := d.GetOk("scopes"); ok {
provider.Scopes = scopesRaw.([]string)
if scopesRaw, ok := d.GetOk("scopes_supported"); ok {
provider.ScopesSupported = scopesRaw.([]string)
} else if req.Operation == logical.CreateOperation {
provider.Scopes = d.GetDefaultOrZero("scopes").([]string)
provider.ScopesSupported = d.Get("scopes_supported").([]string)
}
// remove duplicate allowed client IDs and scopes
provider.AllowedClientIDs = strutil.RemoveDuplicates(provider.AllowedClientIDs, false)
provider.Scopes = strutil.RemoveDuplicates(provider.Scopes, false)
provider.ScopesSupported = strutil.RemoveDuplicates(provider.ScopesSupported, false)
if provider.Issuer != "" {
// verify that issuer is the correct format:
@ -1143,7 +1146,7 @@ func (i *IdentityStore) pathOIDCCreateUpdateProvider(ctx context.Context, req *l
}
scopeTemplateKeyNames := make(map[string]string)
for _, scopeName := range provider.Scopes {
for _, scopeName := range provider.ScopesSupported {
scope, err := i.getOIDCScope(ctx, req.Storage, scopeName)
if err != nil {
return nil, err
@ -1223,7 +1226,7 @@ func (i *IdentityStore) pathOIDCReadProvider(ctx context.Context, req *logical.R
Data: map[string]interface{}{
"issuer": provider.Issuer,
"allowed_client_ids": provider.AllowedClientIDs,
"scopes": provider.Scopes,
"scopes_supported": provider.ScopesSupported,
},
}, nil
}
@ -1286,7 +1289,7 @@ func (i *IdentityStore) pathOIDCProviderDiscovery(ctx context.Context, req *logi
}
// the "openid" scope is reserved and is included for every provider
scopes := append(p.Scopes, openIDScope)
scopes := append(p.ScopesSupported, openIDScope)
disc := providerDiscovery{
Issuer: p.effectiveIssuer,
@ -1455,7 +1458,7 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
// Scope values that are not supported by the provider should be ignored
scopes := make([]string, 0)
for _, scope := range requestedScopes {
if strutil.StrListContains(provider.Scopes, scope) && scope != openIDScope {
if strutil.StrListContains(provider.ScopesSupported, scope) && scope != openIDScope {
scopes = append(scopes, scope)
}
}
@ -1513,12 +1516,15 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
}
// Validate that there is an identity entity associated with the request
if req.EntityID == "" {
return authResponse("", state, ErrAuthAccessDenied, "identity entity must be associated with the request")
}
entity, err := i.MemDBEntityByID(req.EntityID, false)
if err != nil {
return authResponse("", state, ErrAuthServerError, err.Error())
}
if entity == nil {
return authResponse("", state, ErrAuthAccessDenied, "identity entity must be associated with the request")
return authResponse("", state, ErrAuthAccessDenied, "identity entity associated with the request not found")
}
// Validate that the entity is a member of the client's assignments
@ -1733,7 +1739,7 @@ func (i *IdentityStore) pathOIDCToken(ctx context.Context, req *logical.Request,
return tokenResponse(nil, ErrTokenServerError, err.Error())
}
if entity == nil {
return tokenResponse(nil, ErrTokenInvalidRequest, "identity entity associated with request not found")
return tokenResponse(nil, ErrTokenInvalidRequest, "identity entity associated with the request not found")
}
// Validate that the entity is a member of the client's assignments
@ -1935,13 +1941,16 @@ func (i *IdentityStore) pathOIDCUserInfo(ctx context.Context, req *logical.Reque
return userInfoResponse(nil, ErrUserInfoAccessDenied, "client not found")
}
// Get the entity associated with the request
// Validate that there is an identity entity associated with the request
if req.EntityID == "" {
return userInfoResponse(nil, ErrUserInfoAccessDenied, "identity entity must be associated with the request")
}
entity, err := i.MemDBEntityByID(req.EntityID, false)
if err != nil {
return userInfoResponse(nil, ErrUserInfoServerError, err.Error())
}
if entity == nil {
return userInfoResponse(nil, ErrUserInfoAccessDenied, "identity entity must be associated with the request")
return userInfoResponse(nil, ErrUserInfoAccessDenied, "identity entity associated with the request not found")
}
// Validate that the entity is a member of the client's assignments
@ -1959,14 +1968,22 @@ func (i *IdentityStore) pathOIDCUserInfo(ctx context.Context, req *logical.Reque
}
// Get the scopes for the access token
scopes, ok := te.InternalMeta[accessTokenScopesMeta]
if !ok || len(scopes) == 0 {
tokenScopes, ok := te.InternalMeta[accessTokenScopesMeta]
if !ok || len(tokenScopes) == 0 {
return userInfoResponse(claims, "", "")
}
parsedScopes := strutil.ParseStringSlice(scopes, scopesDelimiter)
parsedScopes := strutil.ParseStringSlice(tokenScopes, scopesDelimiter)
// Scope values that are not supported by the provider should be ignored
scopes := make([]string, 0)
for _, scope := range parsedScopes {
if strutil.StrListContains(provider.ScopesSupported, scope) {
scopes = append(scopes, scope)
}
}
// Populate each of the token's scope templates
templates, conflict, err := i.populateScopeTemplates(ctx, req.Storage, ns, entity, parsedScopes...)
templates, conflict, err := i.populateScopeTemplates(ctx, req.Storage, ns, entity, scopes...)
if !conflict && err != nil {
return userInfoResponse(nil, ErrUserInfoServerError, err.Error())
}

View File

@ -576,6 +576,17 @@ func TestOIDC_Path_OIDC_Authorize(t *testing.T) {
},
wantErr: ErrAuthRequestURINotSupported,
},
{
name: "invalid authorize request with identity entity not associated with the request",
args: args{
entityID: "",
clientReq: testClientReq(s),
providerReq: testProviderReq(s, clientID),
assignmentReq: testAssignmentReq(s, entityID, groupID),
authorizeReq: testAuthorizeReq(s, clientID),
},
wantErr: ErrAuthAccessDenied,
},
{
name: "invalid authorize request with identity entity ID not found",
args: args{
@ -944,7 +955,7 @@ func testProviderReq(s logical.Storage, clientID string) *logical.Request {
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"allowed_client_ids": []string{clientID},
"scopes": []string{"test-scope", "conflict"},
"scopes_supported": []string{"test-scope", "conflict"},
},
}
}
@ -2111,7 +2122,7 @@ func TestOIDC_Path_OIDC_ProviderScope_DeleteWithExistingProvider(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope"},
"scopes_supported": []string{"test-scope"},
},
Storage: storage,
})
@ -2465,7 +2476,7 @@ func TestOIDC_Path_OIDCProvider(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope"},
"scopes_supported": []string{"test-scope"},
},
Storage: storage,
})
@ -2494,7 +2505,7 @@ func TestOIDC_Path_OIDCProvider(t *testing.T) {
expected := map[string]interface{}{
"issuer": "",
"allowed_client_ids": []string{},
"scopes": []string{},
"scopes_supported": []string{},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2518,7 +2529,7 @@ func TestOIDC_Path_OIDCProvider(t *testing.T) {
Operation: logical.UpdateOperation,
Data: map[string]interface{}{
"allowed_client_ids": []string{"test-client-id"},
"scopes": []string{"test-scope"},
"scopes_supported": []string{"test-scope"},
},
Storage: storage,
})
@ -2534,7 +2545,7 @@ func TestOIDC_Path_OIDCProvider(t *testing.T) {
expected = map[string]interface{}{
"issuer": "",
"allowed_client_ids": []string{"test-client-id"},
"scopes": []string{"test-scope"},
"scopes_supported": []string{"test-scope"},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2577,7 +2588,7 @@ func TestOIDC_Path_OIDCProvider(t *testing.T) {
expected = map[string]interface{}{
"issuer": "https://example.com:8200",
"allowed_client_ids": []string{"test-client-id"},
"scopes": []string{"test-scope"},
"scopes_supported": []string{"test-scope"},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2639,7 +2650,7 @@ func TestOIDC_Path_OIDCProvider_DuplicateTemplateKeys(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope1", "test-scope2"},
"scopes_supported": []string{"test-scope1", "test-scope2"},
},
Storage: storage,
})
@ -2664,7 +2675,7 @@ func TestOIDC_Path_OIDCProvider_DuplicateTemplateKeys(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope1", "test-scope2"},
"scopes_supported": []string{"test-scope1", "test-scope2"},
},
Storage: storage,
})
@ -2695,7 +2706,7 @@ func TestOIDC_Path_OIDCProvider_Deduplication(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope1", "test-scope1"},
"scopes_supported": []string{"test-scope1", "test-scope1"},
"allowed_client_ids": []string{"test-id1", "test-id2", "test-id1"},
},
Storage: storage,
@ -2712,7 +2723,7 @@ func TestOIDC_Path_OIDCProvider_Deduplication(t *testing.T) {
expected := map[string]interface{}{
"issuer": "",
"allowed_client_ids": []string{"test-id1", "test-id2"},
"scopes": []string{"test-scope1"},
"scopes_supported": []string{"test-scope1"},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2747,7 +2758,7 @@ func TestOIDC_Path_OIDCProvider_Update(t *testing.T) {
expected := map[string]interface{}{
"issuer": "https://example.com:8200",
"allowed_client_ids": []string{"test-client-id"},
"scopes": []string{},
"scopes_supported": []string{},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2774,7 +2785,7 @@ func TestOIDC_Path_OIDCProvider_Update(t *testing.T) {
expected = map[string]interface{}{
"issuer": "https://changedurl.com",
"allowed_client_ids": []string{"test-client-id"},
"scopes": []string{},
"scopes_supported": []string{},
}
if diff := deep.Equal(expected, resp.Data); diff != nil {
t.Fatal(diff)
@ -2856,7 +2867,7 @@ func TestOIDC_Path_OpenIDProviderConfig(t *testing.T) {
Path: "oidc/provider/test-provider",
Operation: logical.CreateOperation,
Data: map[string]interface{}{
"scopes": []string{"test-scope-1"},
"scopes_supported": []string{"test-scope-1"},
},
Storage: storage,
})
@ -2910,8 +2921,8 @@ func TestOIDC_Path_OpenIDProviderConfig(t *testing.T) {
Operation: logical.UpdateOperation,
Storage: storage,
Data: map[string]interface{}{
"issuer": testIssuer,
"scopes": []string{"test-scope-2"},
"issuer": testIssuer,
"scopes_supported": []string{"test-scope-2"},
},
})
expectSuccess(t, resp, err)