Skip to content

Commit

Permalink
Addressed PR comments: moved env variable, removed wantErr from testi…
Browse files Browse the repository at this point in the history 
…ng, changed Errorf to Fatalf for fail to construct engine
  • Loading branch information
Elizabeth Zou committed Aug 18, 2020
1 parent 73e1c18 commit f582070
Showing 1 changed file with 17 additions and 31 deletions.
48 changes: 17 additions & 31 deletions security/authorization/engine/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,6 @@ var (
"deny unknown policy3": errProgram,
"deny unknown policy4": falseProgram,
}}

env, _ = cel.NewEnv(
cel.Declarations(
decls.NewVar("request.url_path", decls.String),
decls.NewVar("request.host", decls.String),
decls.NewVar("request.method", decls.String),
decls.NewVar("request.headers", decls.NewMapType(decls.String, decls.String)),
decls.NewVar("source.address", decls.String),
decls.NewVar("source.port", decls.Int),
decls.NewVar("destination.address", decls.String),
decls.NewVar("destination.port", decls.Int),
decls.NewVar("connection.uri_san_peer_certificate", decls.String),
decls.NewVar("source.principal", decls.String),
),
)
)

func compileCel(env *cel.Env, expr string) (*cel.Ast, error) {
Expand Down Expand Up @@ -338,12 +323,26 @@ func TestAuthorizationEngineEvaluate(t *testing.T) {
}

func TestIntegration(t *testing.T) {
env, _ := cel.NewEnv(
cel.Declarations(
decls.NewVar("request.url_path", decls.String),
decls.NewVar("request.host", decls.String),
decls.NewVar("request.method", decls.String),
decls.NewVar("request.headers", decls.NewMapType(decls.String, decls.String)),
decls.NewVar("source.address", decls.String),
decls.NewVar("source.port", decls.Int),
decls.NewVar("destination.address", decls.String),
decls.NewVar("destination.port", decls.Int),
decls.NewVar("connection.uri_san_peer_certificate", decls.String),
decls.NewVar("source.principal", decls.String),
),
)

tests := map[string]struct {
allow *pb.RBAC
deny *pb.RBAC
authArgs *AuthorizationArgs
wantAuthDecision *AuthorizationDecision
wantErr error
}{
"ALLOW engine: DecisionAllow": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -352,7 +351,6 @@ func TestIntegration(t *testing.T) {
deny: nil,
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"url_path starts with"}},
wantErr: nil,
},
"ALLOW engine: DecisionUnknown": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -362,7 +360,6 @@ func TestIntegration(t *testing.T) {
deny: nil,
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:25"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"url_path and uri_san_peer_certificate"}},
wantErr: nil,
},
"ALLOW engine: DecisionDeny": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -371,7 +368,6 @@ func TestIntegration(t *testing.T) {
deny: nil,
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{}},
wantErr: nil,
},
"DENY engine: DecisionAllow": {
allow: nil,
Expand All @@ -380,7 +376,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{}},
wantErr: nil,
},
"DENY engine: DecisionUnknown": {
allow: nil,
Expand All @@ -390,7 +385,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:25"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"destination address"}},
wantErr: nil,
},
"DENY engine: DecisionDeny": {
allow: nil,
Expand All @@ -400,7 +394,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{"source address or source port"}},
wantErr: nil,
},
"DENY ALLOW engine: DecisionDeny from DENY policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -412,7 +405,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{"source address or source port"}},
wantErr: nil,
},
"DENY ALLOW engine: DecisionUnknown from DENY policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -424,7 +416,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"destination address", "source port and destination port"}},
wantErr: nil,
},
"DENY ALLOW engine: DecisionAllow from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -436,7 +427,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionAllow, policyNames: []string{"method or url_path starts with"}},
wantErr: nil,
},
"DENY ALLOW engine: DecisionUnknown from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -448,7 +438,6 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionUnknown, policyNames: []string{"url_path starts with and method"}},
wantErr: nil,
},
"DENY ALLOW engine: DecisionDeny from ALLOW policy": {
allow: &pb.RBAC{Action: pb.RBAC_ALLOW, Policies: map[string]*pb.Policy{
Expand All @@ -460,21 +449,18 @@ func TestIntegration(t *testing.T) {
}},
authArgs: &AuthorizationArgs{peerInfo: &peer.Peer{Addr: addrMock{addr: "192.0.2.1:8080"}}, fullMethod: "/pkg.service/test/method"},
wantAuthDecision: &AuthorizationDecision{decision: DecisionDeny, policyNames: []string{}},
wantErr: nil,
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
engine, err := NewAuthorizationEngine(tc.allow, tc.deny)
if err != nil {
t.Errorf("Error constructing authorization engine: %v", err)
t.Fatalf("Error constructing authorization engine: %v", err)
}
gotAuthDecision, gotErr := engine.Evaluate(tc.authArgs)
sort.Strings(gotAuthDecision.policyNames)
if tc.wantErr != nil && (gotErr == nil || gotErr.Error() != tc.wantErr.Error()) {
t.Errorf("Expected error to be %v, instead got %v", tc.wantErr, gotErr)
} else if tc.wantErr == nil && (gotErr != nil || gotAuthDecision.decision != tc.wantAuthDecision.decision || !reflect.DeepEqual(gotAuthDecision.policyNames, tc.wantAuthDecision.policyNames)) {
if gotErr != nil || gotAuthDecision.decision != tc.wantAuthDecision.decision || !reflect.DeepEqual(gotAuthDecision.policyNames, tc.wantAuthDecision.policyNames) {
t.Errorf("Expected authorization decision to be (%v, %v), instead got (%v, %v)", tc.wantAuthDecision.decision, tc.wantAuthDecision.policyNames, gotAuthDecision.decision, gotAuthDecision.policyNames)
}
})
Expand Down

0 comments on commit f582070

Please sign in to comment.