remove saml_idp_service_provider_labels and `saml_idp_service_provi…

…der_labels_expression` (#40047) * reserve 40, 41 tag numbers for RoleConditions * remove GetSAMLIdPServiceProviderLabels and SetSAMLIdPServiceProviderLabels * remove SAMLIdPServiceProviderLabels from label matchers and rolev6 test * run make -C integrations/operator manifests
gravitational · Mar 29, 2024 · bdba1a6 · bdba1a6
1 parent 630a465
commit bdba1a6
Show file tree

Hide file tree

Showing 10 changed files with 1,736 additions and 2,090 deletions.
diff --git a/api/proto/teleport/legacy/types/types.proto b/api/proto/teleport/legacy/types/types.proto
@@ -3053,16 +3053,10 @@ message RoleConditions {
   // SPIFFE is used to allow or deny access to a role holder to generating a
   // SPIFFE SVID.
   repeated SPIFFERoleCondition SPIFFE = 39 [(gogoproto.jsontag) = "spiffe,omitempty"];
-  // SAMLIdPServiceProviderLabels is a labels map used in RBAC system to allow/deny access
-  // to saml_idp_service_provider resource.
-  wrappers.LabelValues SAMLIdPServiceProviderLabels = 40 [
-    (gogoproto.nullable) = false,
-    (gogoproto.jsontag) = "saml_idp_service_provider_labels,omitempty",
-    (gogoproto.customtype) = "Labels"
-  ];
-  // SAMLIdPServiceProviderLabelsExpression is a predicate expression used to allow/deny
-  // access to saml_idp_service_provider resource.
-  string SAMLIdPServiceProviderLabelsExpression = 41 [(gogoproto.jsontag) = "saml_idp_service_provider_labels_expression,omitempty"];
+  reserved 40; // removed saml_idp_service_provider_labels in favor of using app_labels.
+  reserved "SAMLIdPServiceProviderLabels";
+  reserved 41; // removed saml_idp_service_provider_labels_expression in favor of using app_labels_expression.
+  reserved "SAMLIdPServiceProviderLabelsExpression";
 }
 
 // SPIFFERoleCondition sets out which SPIFFE identities this role is allowed or

diff --git a/api/types/role.go b/api/types/role.go
@@ -266,13 +266,6 @@ type Role interface {
 	GetSPIFFEConditions(rct RoleConditionType) []*SPIFFERoleCondition
 	// SetSPIFFEConditions sets the allow or deny SPIFFERoleCondition.
 	SetSPIFFEConditions(rct RoleConditionType, cond []*SPIFFERoleCondition)
-
-	// GetSAMLIdPServiceProviderLabels gets the map of saml_idp_service_provider resource
-	// labels this role is allowed or denied access to.
-	GetSAMLIdPServiceProviderLabels(RoleConditionType) Labels
-	// SetSAMLIdPServiceProviderLabels sets the map of saml_idp_service_provider resource
-	// labels this role is allowed or denied access to.
-	SetSAMLIdPServiceProviderLabels(RoleConditionType, Labels)
 }
 
 // NewRole constructs new standard V7 role.
@@ -950,25 +943,6 @@ func (r *RoleV6) SetSPIFFEConditions(rct RoleConditionType, cond []*SPIFFERoleCo
 	}
 }
 
-// GetSAMLIdPServiceProviderLabels gets the map of saml_idp_service_provider resource
-// labels this role is allowed or denied access to.
-func (r *RoleV6) GetSAMLIdPServiceProviderLabels(rct RoleConditionType) Labels {
-	if rct == Allow {
-		return r.Spec.Allow.SAMLIdPServiceProviderLabels
-	}
-	return r.Spec.Deny.SAMLIdPServiceProviderLabels
-}
-
-// SetSAMLIdPServiceProviderLabels sets the map of saml_idp_service_provider resource
-// labels this role is allowed or denied access to.
-func (r *RoleV6) SetSAMLIdPServiceProviderLabels(rct RoleConditionType, labels Labels) {
-	if rct == Allow {
-		r.Spec.Allow.SAMLIdPServiceProviderLabels = labels.Clone()
-	} else {
-		r.Spec.Deny.SAMLIdPServiceProviderLabels = labels.Clone()
-	}
-}
-
 // GetPrivateKeyPolicy returns the private key policy enforced for this role.
 func (r *RoleV6) GetPrivateKeyPolicy() keys.PrivateKeyPolicy {
 	switch r.Spec.Options.RequireMFAType {
@@ -1227,7 +1201,6 @@ func (r *RoleV6) CheckAndSetDefaults() error {
 		r.Spec.Allow.DatabaseLabels,
 		r.Spec.Allow.WindowsDesktopLabels,
 		r.Spec.Allow.GroupLabels,
-		r.Spec.Allow.SAMLIdPServiceProviderLabels,
 	} {
 		if err := checkWildcardSelector(labels); err != nil {
 			return trace.Wrap(err)
@@ -1874,8 +1847,6 @@ func (r *RoleV6) GetLabelMatchers(rct RoleConditionType, kind string) (LabelMatc
 		return LabelMatchers{cond.WindowsDesktopLabels, cond.WindowsDesktopLabelsExpression}, nil
 	case KindUserGroup:
 		return LabelMatchers{cond.GroupLabels, cond.GroupLabelsExpression}, nil
-	case KindSAMLIdPServiceProvider:
-		return LabelMatchers{cond.SAMLIdPServiceProviderLabels, cond.SAMLIdPServiceProviderLabelsExpression}, nil
 	}
 	return LabelMatchers{}, trace.BadParameter("can't get label matchers for resource kind %q", kind)
 }
@@ -1926,10 +1897,6 @@ func (r *RoleV6) SetLabelMatchers(rct RoleConditionType, kind string, labelMatch
 		cond.GroupLabels = labelMatchers.Labels
 		cond.GroupLabelsExpression = labelMatchers.Expression
 		return nil
-	case KindSAMLIdPServiceProvider:
-		cond.SAMLIdPServiceProviderLabels = labelMatchers.Labels
-		cond.SAMLIdPServiceProviderLabelsExpression = labelMatchers.Expression
-		return nil
 	}
 	return trace.BadParameter("can't set label matchers for resource kind %q", kind)
 }
@@ -1992,7 +1959,6 @@ var LabelMatcherKinds = []string{
 	KindWindowsDesktop,
 	KindWindowsDesktopService,
 	KindUserGroup,
-	KindSAMLIdPServiceProvider,
 }
 
 const (

diff --git a/api/types/role_test.go b/api/types/role_test.go
@@ -443,7 +443,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 			require.ErrorContains(t, err, contains)
 		}
 	}
-	newRole := func(spec RoleSpecV6) *RoleV6 {
+	newRole := func(t *testing.T, spec RoleSpecV6) *RoleV6 {
 		return &RoleV6{
 			Metadata: Metadata{
 				Name: "test",
@@ -453,14 +453,13 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 	}
 
 	tests := []struct {
-		name                string
-		role                *RoleV6
-		requireError        require.ErrorAssertionFunc
-		compareDefaultValue RoleConditions
+		name         string
+		role         *RoleV6
+		requireError require.ErrorAssertionFunc
 	}{
 		{
 			name: "spiffe: valid",
-			role: newRole(RoleSpecV6{
+			role: newRole(t, RoleSpecV6{
 				Allow: RoleConditions{
 					SPIFFE: []*SPIFFERoleCondition{{Path: "/test"}},
 				},
@@ -469,7 +468,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 		},
 		{
 			name: "spiffe: valid regex path",
-			role: newRole(RoleSpecV6{
+			role: newRole(t, RoleSpecV6{
 				Allow: RoleConditions{
 					SPIFFE: []*SPIFFERoleCondition{{Path: `^\/svc\/foo\/.*\/bar$`}},
 				},
@@ -478,7 +477,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 		},
 		{
 			name: "spiffe: missing path",
-			role: newRole(RoleSpecV6{
+			role: newRole(t, RoleSpecV6{
 				Allow: RoleConditions{
 					SPIFFE: []*SPIFFERoleCondition{{Path: ""}},
 				},
@@ -487,7 +486,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 		},
 		{
 			name: "spiffe: path not prepended",
-			role: newRole(RoleSpecV6{
+			role: newRole(t, RoleSpecV6{
 				Allow: RoleConditions{
 					SPIFFE: []*SPIFFERoleCondition{{Path: "foo"}},
 				},
@@ -496,7 +495,7 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 		},
 		{
 			name: "spiffe: invalid ip cidr",
-			role: newRole(RoleSpecV6{
+			role: newRole(t, RoleSpecV6{
 				Allow: RoleConditions{
 					SPIFFE: []*SPIFFERoleCondition{
 						{
@@ -511,28 +510,6 @@ func TestRoleV6_CheckAndSetDefaults(t *testing.T) {
 			}),
 			requireError: requireBadParameterContains("validating ip_sans[1]: invalid CIDR address: llama"),
 		},
-		{
-			name: "SAMLIdpServiceProviderLabels: valid wildcard labels",
-			role: newRole(RoleSpecV6{
-				Allow: RoleConditions{
-					SAMLIdPServiceProviderLabels: Labels{
-						Wildcard: {Wildcard},
-					},
-				},
-			}),
-			requireError: require.NoError,
-		},
-		{
-			name: "SAMLIdpServiceProviderLabels: invalid labels",
-			role: newRole(RoleSpecV6{
-				Allow: RoleConditions{
-					SAMLIdPServiceProviderLabels: Labels{
-						Wildcard: {"val"},
-					},
-				},
-			}),
-			requireError: requireBadParameterContains("not supported"),
-		},
 	}
 
 	for _, tt := range tests {