authz: update representation of allow authenticated in SDK (#5052)

* remove empty principals logic * Update test * minor formatting * resolving comments
grpc · Dec 28, 2021 · fbaf7c5 · fbaf7c5
1 parent 344b93a
commit fbaf7c5
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 46 deletions.
diff --git a/authz/rbac_translator.go b/authz/rbac_translator.go
@@ -157,19 +157,13 @@ func parsePrincipalNames(principalNames []string) []*v3rbacpb.Principal {
 }
 
 func parsePeer(source peer) *v3rbacpb.Principal {
-	if source.Principals == nil {
+	if len(source.Principals) == 0 {
 		return &v3rbacpb.Principal{
 			Identifier: &v3rbacpb.Principal_Any{
 				Any: true,
 			},
 		}
 	}
-	if len(source.Principals) == 0 {
-		return &v3rbacpb.Principal{
-			Identifier: &v3rbacpb.Principal_Authenticated_{
-				Authenticated: &v3rbacpb.Principal_Authenticated{},
-			}}
-	}
 	return principalOr(parsePrincipalNames(source.Principals))
 }
 

diff --git a/authz/rbac_translator_test.go b/authz/rbac_translator_test.go
@@ -205,23 +205,37 @@ func TestTranslatePolicy(t *testing.T) {
 				},
 			},
 		},
-		"empty principal field": {
+		"allow authenticated": {
 			authzPolicy: `{
-				"name": "authz",
-				"allow_rules": [{
-					"name": "allow_authenticated",
-					"source": {"principals":[]}
-				}]
-			}`,
+						"name": "authz",
+						"allow_rules": [
+						{
+							"name": "allow_authenticated",
+							"source": {
+								"principals":["*", ""]
+							}
+						}]
+					}`,
 			wantPolicies: []*v3rbacpb.RBAC{
 				{
 					Action: v3rbacpb.RBAC_ALLOW,
 					Policies: map[string]*v3rbacpb.Policy{
 						"authz_allow_authenticated": {
 							Principals: []*v3rbacpb.Principal{
-								{Identifier: &v3rbacpb.Principal_Authenticated_{
-									Authenticated: &v3rbacpb.Principal_Authenticated{},
-								}},
+								{Identifier: &v3rbacpb.Principal_OrIds{OrIds: &v3rbacpb.Principal_Set{
+									Ids: []*v3rbacpb.Principal{
+										{Identifier: &v3rbacpb.Principal_Authenticated_{
+											Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{
+												MatchPattern: &v3matcherpb.StringMatcher_SafeRegex{SafeRegex: &v3matcherpb.RegexMatcher{Regex: ".+"}},
+											}},
+										}},
+										{Identifier: &v3rbacpb.Principal_Authenticated_{
+											Authenticated: &v3rbacpb.Principal_Authenticated{PrincipalName: &v3matcherpb.StringMatcher{
+												MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: ""},
+											}},
+										}},
+									},
+								}}},
 							},
 							Permissions: []*v3rbacpb.Permission{
 								{Rule: &v3rbacpb.Permission_Any{Any: true}},

diff --git a/authz/sdk_end2end_test.go b/authz/sdk_end2end_test.go
@@ -261,38 +261,14 @@ var sdkTests = map[string]struct {
 		wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
 	},
 	"DeniesRPCRequestWithPrincipalsFieldOnUnauthenticatedConnection": {
-		authzPolicy: `{
-				"name": "authz",
-				"allow_rules":
-				[
-					{
-						"name": "allow_TestServiceCalls",
-						"source": {
-							"principals":
-							[
-								"foo"
-							]
-						},
-						"request": {
-							"paths":
-							[
-								"/grpc.testing.TestService/*"
-							]
-						}
-					}
-				]
-			}`,
-		wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
-	},
-	"DeniesRPCRequestWithEmptyPrincipalsOnUnauthenticatedConnection": {
 		authzPolicy: `{
 				"name": "authz",
 				"allow_rules":
 				[
 					{
 						"name": "allow_authenticated",
 						"source": {
-							"principals": []
+							"principals": ["*", ""]
 						}
 					}
 				]
@@ -386,15 +362,15 @@ func (s) TestSDKStaticPolicyEnd2End(t *testing.T) {
 	}
 }
 
-func (s) TestSDKAllowsRPCRequestWithEmptyPrincipalsOnTLSAuthenticatedConnection(t *testing.T) {
+func (s) TestSDKAllowsRPCRequestWithPrincipalsFieldOnTLSAuthenticatedConnection(t *testing.T) {
 	authzPolicy := `{
 				"name": "authz",
 				"allow_rules":
 				[
 					{
 						"name": "allow_authenticated",
 						"source": {
-							"principals": []
+							"principals": ["*", ""]
 						}
 					}
 				]
@@ -438,15 +414,15 @@ func (s) TestSDKAllowsRPCRequestWithEmptyPrincipalsOnTLSAuthenticatedConnection(
 	}
 }
 
-func (s) TestSDKAllowsRPCRequestWithEmptyPrincipalsOnMTLSAuthenticatedConnection(t *testing.T) {
+func (s) TestSDKAllowsRPCRequestWithPrincipalsFieldOnMTLSAuthenticatedConnection(t *testing.T) {
 	authzPolicy := `{
 				"name": "authz",
 				"allow_rules":
 				[
 					{
 						"name": "allow_authenticated",
 						"source": {
-							"principals": []
+							"principals": ["*", ""]
 						}
 					}
 				]