diff --git a/.changelog/17755.txt b/.changelog/17755.txt new file mode 100644 index 000000000..7edf7b26e --- /dev/null +++ b/.changelog/17755.txt @@ -0,0 +1,3 @@ +```release-note:improvement +mesh: Stop jwt providers referenced by intentions from being deleted. +``` \ No newline at end of file diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index 340a53f11..9abaafc39 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -634,6 +634,12 @@ func validateProposedConfigEntryInGraph( case structs.TCPRoute: case structs.RateLimitIPConfig: case structs.JWTProvider: + if newEntry == nil && existingEntry != nil { + err := validateJWTProviderIsReferenced(tx, kindName, existingEntry) + if err != nil { + return err + } + } default: return fmt.Errorf("unhandled kind %q during validation of %q", kindName.Kind, kindName.Name) } @@ -704,6 +710,66 @@ func getReferencedProviderNames(j *structs.IntentionJWTRequirement, s []*structs return providerNames } +// validateJWTProviderIsReferenced iterates over intentions to determine if the provider being +// deleted is referenced by any intention. +// +// This could be an expensive operation based on the number of intentions. We purposely set this to only +// run on delete and don't expect this to be called often. +func validateJWTProviderIsReferenced(tx ReadTxn, kn configentry.KindName, ce structs.ConfigEntry) error { + meta := acl.NewEnterpriseMetaWithPartition( + kn.EnterpriseMeta.PartitionOrDefault(), + acl.DefaultNamespaceName, + ) + entry, ok := ce.(*structs.JWTProviderConfigEntry) + if !ok { + return fmt.Errorf("invalid jwt provider config entry: %T", entry) + } + + _, ixnEntries, err := configEntriesByKindTxn(tx, nil, structs.ServiceIntentions, &meta) + if err != nil { + return err + } + + err = findJWTProviderNameReferences(ixnEntries, entry.Name) + if err != nil { + return err + } + + return nil +} + +func findJWTProviderNameReferences(entries []structs.ConfigEntry, pName string) error { + errMsg := "cannot delete jwt provider config entry referenced by an intention. Provider name: %s, intention name: %s" + for _, entry := range entries { + ixn, ok := entry.(*structs.ServiceIntentionsConfigEntry) + if !ok { + return fmt.Errorf("type %T is not a service intentions config entry", entry) + } + + if ixn.JWT != nil { + for _, prov := range ixn.JWT.Providers { + if prov.Name == pName { + return fmt.Errorf(errMsg, pName, ixn.Name) + } + } + } + + for _, s := range ixn.Sources { + for _, perm := range s.Permissions { + if perm.JWT == nil { + continue + } + for _, prov := range perm.JWT.Providers { + if prov.Name == pName { + return fmt.Errorf(errMsg, pName, ixn.Name) + } + } + } + } + } + return nil +} + // This fetches all the jwt-providers config entries and iterates over them // to validate that any provider referenced exists. // This is okay because we assume there are very few jwt-providers per partition diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index 572719dc4..d72f12c87 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -3714,3 +3714,178 @@ func TestStateStore_DiscoveryChain_AttachVirtualIPs(t *testing.T) { require.Equal(t, []string{"2.2.2.2", "3.3.3.3"}, chain.ManualVirtualIPs) } + +func TestFindJWTProviderNameReferences(t *testing.T) { + oktaProvider := structs.IntentionJWTProvider{Name: "okta"} + auth0Provider := structs.IntentionJWTProvider{Name: "auth0"} + cases := map[string]struct { + entries []structs.ConfigEntry + providerName string + expectedError string + }{ + "no jwt at any level": { + entries: []structs.ConfigEntry{}, + providerName: "okta", + }, + "provider not referenced": { + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: "service-intentions", + Name: "api-intention", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&oktaProvider, &auth0Provider}, + }, + }, + }, + providerName: "fake-provider", + }, + "only top level jwt with no permissions": { + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: "service-intentions", + Name: "api-intention", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&oktaProvider, &auth0Provider}, + }, + }, + }, + providerName: "okta", + expectedError: "cannot delete jwt provider config entry referenced by an intention. Provider name: okta, intention name: api-intention", + }, + "top level jwt with permissions": { + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: "service-intentions", + Name: "api-intention", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&oktaProvider}, + }, + Sources: []*structs.SourceIntention{ + { + Name: "api", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + { + Action: "allow", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&oktaProvider}, + }, + }, + }, + }, + { + Name: "serv", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + { + Action: "allow", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&auth0Provider}, + }, + }, + }, + }, + { + Name: "web", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + {Action: "allow"}, + }, + }, + }, + }, + }, + providerName: "auth0", + expectedError: "cannot delete jwt provider config entry referenced by an intention. Provider name: auth0, intention name: api-intention", + }, + "no top level jwt and existing permissions": { + entries: []structs.ConfigEntry{ + &structs.ServiceIntentionsConfigEntry{ + Kind: "service-intentions", + Name: "api-intention", + Sources: []*structs.SourceIntention{ + { + Name: "api", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + { + Action: "allow", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&oktaProvider}, + }, + }, + }, + }, + { + Name: "serv", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + { + Action: "allow", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{&auth0Provider}, + }, + }, + }, + }, + { + Name: "web", + Action: "allow", + Permissions: []*structs.IntentionPermission{ + {Action: "allow"}, + }, + }, + }, + }, + }, + providerName: "okta", + expectedError: "cannot delete jwt provider config entry referenced by an intention. Provider name: okta, intention name: api-intention", + }, + } + + for name, tt := range cases { + tt := tt + t.Run(name, func(t *testing.T) { + err := findJWTProviderNameReferences(tt.entries, tt.providerName) + + if tt.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestStore_ValidateJWTProviderIsReferenced(t *testing.T) { + s := testStateStore(t) + + // First create a config entry + provider := &structs.JWTProviderConfigEntry{ + Kind: structs.JWTProvider, + Name: "okta", + } + require.NoError(t, s.EnsureConfigEntry(0, provider)) + + // create a service intention referencing the config entry + ixn := &structs.ServiceIntentionsConfigEntry{ + Name: "api", + JWT: &structs.IntentionJWTRequirement{ + Providers: []*structs.IntentionJWTProvider{ + {Name: provider.Name}, + }, + }, + } + require.NoError(t, s.EnsureConfigEntry(1, ixn)) + + // attempt deleting a referenced provider + err := s.DeleteConfigEntry(0, structs.JWTProvider, provider.Name, nil) + require.Error(t, err) + require.Contains(t, err.Error(), `cannot delete jwt provider config entry referenced by an intention. Provider name: okta, intention name: api`) + + // delete the intention + require.NoError(t, s.DeleteConfigEntry(1, structs.ServiceIntentions, ixn.Name, nil)) + // successfully delete the provider after deleting the intention + require.NoError(t, s.DeleteConfigEntry(0, structs.JWTProvider, provider.Name, nil)) +}