From d413a3f82a0a421cf30228aee6ca0fd829b31373 Mon Sep 17 00:00:00 2001 From: khoipham Date: Fri, 27 Dec 2019 15:49:58 +0700 Subject: [PATCH] Fix RemoveFilteredPolicy --- README.md | 8 +++++ adapter.go | 43 ++++++++++++----------- adapter_test.go | 60 ++++++++++++++++++++++++++++---- docker-compose.yml | 3 +- postgres/Dockerfile | 3 ++ postgres/enable_log_statement.sh | 5 +++ 6 files changed, 94 insertions(+), 28 deletions(-) create mode 100644 postgres/Dockerfile create mode 100755 postgres/enable_log_statement.sh diff --git a/README.md b/README.md index 7161570..892878a 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,14 @@ func main() { } ``` +## Run all tests + + docker-compose run --rm go + +## Debug tests + + docker-compose run --rm go dlv test github.com/pckhoi/casbin-pg-adapter + ## Getting Help - [Casbin](https://github.com/casbin/casbin) diff --git a/adapter.go b/adapter.go index 7eade0d..ab966a1 100644 --- a/adapter.go +++ b/adapter.go @@ -32,9 +32,6 @@ type Adapter struct { db *pg.DB } -// finalizer is the destructor for Adapter. -func finalizer(a *Adapter) {} - // NewAdapter is the constructor for Adapter. // arg should be a PostgreS URL string or of type *pg.Options // The adapter will create a DB named "casbin" if it doesn't exist @@ -149,6 +146,13 @@ func (a *Adapter) LoadPolicy(model model.Model) error { return nil } +func policyID(ptype string, rule []string) string { + data := strings.Join(append([]string{ptype}, rule...), ",") + sum := make([]byte, 64) + sha3.ShakeSum128(sum, []byte(data)) + return fmt.Sprintf("%x", sum) +} + func savePolicyLine(ptype string, rule []string) *CasbinRule { line := &CasbinRule{PType: ptype} @@ -172,10 +176,7 @@ func savePolicyLine(ptype string, rule []string) *CasbinRule { line.V5 = rule[5] } - data := strings.Join(append([]string{ptype}, rule...), ",") - sum := make([]byte, 64) - sha3.ShakeSum128(sum, []byte(data)) - line.ID = fmt.Sprintf("%x", sum) + line.ID = policyID(ptype, rule) return line } @@ -222,28 +223,28 @@ func (a *Adapter) RemovePolicy(sec string, ptype string, rule []string) error { // RemoveFilteredPolicy removes policy rules that match the filter from the storage. func (a *Adapter) RemoveFilteredPolicy(sec string, ptype string, fieldIndex int, fieldValues ...string) error { - line := &CasbinRule{PType: ptype} + query := a.db.Model((*CasbinRule)(nil)).Where("p_type = ?", ptype) idx := fieldIndex + len(fieldValues) - if fieldIndex <= 0 && idx > 0 { - line.V0 = fieldValues[0-fieldIndex] + if fieldIndex <= 0 && idx > 0 && fieldValues[0-fieldIndex] != "" { + query = query.Where("v0 = ?", fieldValues[0-fieldIndex]) } - if fieldIndex <= 1 && idx > 1 { - line.V1 = fieldValues[1-fieldIndex] + if fieldIndex <= 1 && idx > 1 && fieldValues[1-fieldIndex] != "" { + query = query.Where("v1 = ?", fieldValues[1-fieldIndex]) } - if fieldIndex <= 2 && idx > 2 { - line.V2 = fieldValues[2-fieldIndex] + if fieldIndex <= 2 && idx > 2 && fieldValues[2-fieldIndex] != "" { + query = query.Where("v2 = ?", fieldValues[2-fieldIndex]) } - if fieldIndex <= 3 && idx > 3 { - line.V3 = fieldValues[3-fieldIndex] + if fieldIndex <= 3 && idx > 3 && fieldValues[3-fieldIndex] != "" { + query = query.Where("v3 = ?", fieldValues[3-fieldIndex]) } - if fieldIndex <= 4 && idx > 4 { - line.V4 = fieldValues[4-fieldIndex] + if fieldIndex <= 4 && idx > 4 && fieldValues[4-fieldIndex] != "" { + query = query.Where("v4 = ?", fieldValues[4-fieldIndex]) } - if fieldIndex <= 5 && idx > 5 { - line.V5 = fieldValues[5-fieldIndex] + if fieldIndex <= 5 && idx > 5 && fieldValues[5-fieldIndex] != "" { + query = query.Where("v5 = ?", fieldValues[5-fieldIndex]) } - err := a.db.Delete(line) + _, err := query.Delete() return err } diff --git a/adapter_test.go b/adapter_test.go index 99754ae..15df695 100644 --- a/adapter_test.go +++ b/adapter_test.go @@ -18,6 +18,7 @@ type AdapterTestSuite struct { } func (s *AdapterTestSuite) testGetPolicy(res [][]string) { + s.T().Helper() myRes := s.e.GetPolicy() s.Assert().True(util.Array2DEquals(res, myRes), "Policy Got: %v, supposed to be %v", myRes, res) } @@ -27,8 +28,7 @@ func (s *AdapterTestSuite) dropCasbinDB() { s.Require().NoError(err) db := pg.Connect(opts) defer db.Close() - _, err = db.Exec("DROP DATABASE casbin") - s.Require().NoError(err) + db.Exec("DROP DATABASE casbin") } func (s *AdapterTestSuite) SetupTest() { @@ -41,9 +41,9 @@ func (s *AdapterTestSuite) SetupTest() { // This is a trick to save the current policy to the DB. // We can't call e.SavePolicy() because the adapter in the enforcer is still the file adapter. // The current policy means the policy in the Casbin enforcer (aka in memory). - s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") + e, err := casbin.NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") s.Require().NoError(err) - err = s.a.SavePolicy(s.e.GetModel()) + err = s.a.SavePolicy(e.GetModel()) s.Require().NoError(err) s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", s.a) @@ -79,11 +79,59 @@ func (s *AdapterTestSuite) TestAutoSave() { // Because AutoSave is enabled, the policy change not only affects the policy in Casbin enforcer, // but also affects the policy in the storage. - s.e.AddPolicy("alice", "data1", "write") + _, err = s.e.AddPolicy("alice", "data1", "write") + s.Require().NoError(err) // Reload the policy from the storage to see the effect. - s.e.LoadPolicy() + err = s.e.LoadPolicy() + s.Require().NoError(err) // The policy has a new rule: {"alice", "data1", "write"}. s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}, {"alice", "data1", "write"}}) + + // Aditional AddPolicy have no effect + _, err = s.e.AddPolicy("alice", "data1", "write") + s.Require().NoError(err) + err = s.e.LoadPolicy() + s.Require().NoError(err) + s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}, {"alice", "data1", "write"}}) + s.Require().NoError(err) +} + +func (s *AdapterTestSuite) TestConstructorOptions() { + opts, err := pg.ParseURL(os.Getenv("PG_CONN")) + s.Require().NoError(err) + + a, err := NewAdapter(opts) + s.Require().NoError(err) + defer a.Close() + + s.e, err = casbin.NewEnforcer("examples/rbac_model.conf", a) + s.Require().NoError(err) + + s.testGetPolicy([][]string{{"alice", "data1", "read"}, {"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}}) +} + +func (s *AdapterTestSuite) TestRemovePolicy() { + _, err := s.e.RemovePolicy("alice", "data1", "read") + s.Require().NoError(err) + + s.testGetPolicy([][]string{{"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}}) + + err = s.e.LoadPolicy() + s.Require().NoError(err) + + s.testGetPolicy([][]string{{"bob", "data2", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}}) +} + +func (s *AdapterTestSuite) TestRemoveFilteredPolicy() { + _, err := s.e.RemoveFilteredPolicy(0, "", "data2") + s.Require().NoError(err) + + s.testGetPolicy([][]string{{"alice", "data1", "read"}}) + + err = s.e.LoadPolicy() + s.Require().NoError(err) + + s.testGetPolicy([][]string{{"alice", "data1", "read"}}) } func TestAdapterTestSuite(t *testing.T) { diff --git a/docker-compose.yml b/docker-compose.yml index 428543a..a0387b5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -2,7 +2,8 @@ version: "3.4" services: postgres: - image: postgres:12 + build: + context: ./postgres ports: - 5432:5432 environment: diff --git a/postgres/Dockerfile b/postgres/Dockerfile new file mode 100644 index 0000000..dc9b7b5 --- /dev/null +++ b/postgres/Dockerfile @@ -0,0 +1,3 @@ +FROM postgres:12-alpine + +ADD . /docker-entrypoint-initdb.d diff --git a/postgres/enable_log_statement.sh b/postgres/enable_log_statement.sh new file mode 100755 index 0000000..be83490 --- /dev/null +++ b/postgres/enable_log_statement.sh @@ -0,0 +1,5 @@ +#!/bin/sh +set -x + +PGCONF=/var/lib/postgresql/data/postgresql.conf +sed -i "s/#log_statement = 'none'.*/log_statement = 'all'/g" $PGCONF