From 6ada626dda75a2cd9e06e7ca2496405fbc891935 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Wed, 22 Jan 2020 09:19:46 -0800 Subject: [PATCH] fix: impossible to config RBAC if group name includes ',' (#3013) * fix: impossible to config RBAC if group name includes ',' * apply reviewer notes --- util/rbac/rbac.go | 27 +++++++++++++++++++++++---- util/rbac/rbac_test.go | 3 +++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index 386af82d55..0fdfc35edb 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -2,6 +2,7 @@ package rbac import ( "context" + "encoding/csv" "errors" "fmt" "strings" @@ -11,7 +12,6 @@ import ( "github.com/casbin/casbin" "github.com/casbin/casbin/model" - "github.com/casbin/casbin/persist" jwt "github.com/dgrijalva/jwt-go" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" @@ -270,15 +270,34 @@ func newAdapter(builtinPolicy, userDefinedPolicy, runtimePolicy string) *argocdA func (a *argocdAdapter) LoadPolicy(model model.Model) error { for _, policyStr := range []string{a.builtinPolicy, a.userDefinedPolicy, a.runtimePolicy} { for _, line := range strings.Split(policyStr, "\n") { - if line == "" { - continue + if err := loadPolicyLine(strings.TrimSpace(line), model); err != nil { + return err } - persist.LoadPolicyLine(line, model) } } return nil } +// The modified version of LoadPolicyLine function defined in "persist" package of github.com/casbin/casbin. +// Uses CVS parser to correctly handle quotes in policy line. +func loadPolicyLine(line string, model model.Model) error { + if line == "" || strings.HasPrefix(line, "#") { + return nil + } + + reader := csv.NewReader(strings.NewReader(line)) + reader.TrimLeadingSpace = true + tokens, err := reader.Read() + if err != nil { + return err + } + + key := tokens[0] + sec := key[:1] + model[sec][key].Policy = append(model[sec][key].Policy, tokens[1:]) + return nil +} + func (a *argocdAdapter) SavePolicy(model model.Model) error { return errors.New("not implemented") } diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index cd2b83948b..f437a9c504 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -371,6 +371,9 @@ func TestValidatePolicy(t *testing.T) { goodPolicies := []string{ "p, role:admin, projects, delete, *, allow", "", + "#", + `p, "role,admin", projects, delete, *, allow`, + ` p, role:admin, projects, delete, *, allow `, } for _, good := range goodPolicies { assert.Nil(t, ValidatePolicy(good))