Skip to content

Commit

Permalink
add regex rule priority, drop request from selector signature, add un…
Browse files Browse the repository at this point in the history 
…it tests

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
@butonic
butonic committed Jul 2, 2021
1 parent 81614db commit dc71a60
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 74 deletions.
16 changes: 12 additions & 4 deletions proxy/pkg/config/config.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package config

import "context"
import (
"context"
)

// Log defines the available logging configuration.
type Log struct {
Expand Down Expand Up @@ -174,9 +176,15 @@ type ClaimsSelectorConf struct {

// RegexSelectorConf is the config for the regex-selector
type RegexSelectorConf struct {
DefaultPolicy string `mapstructure:"default_policy"`
MatchesPolicies map[string]map[string]string `mapstructure:"matches_policies"`
UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"`
DefaultPolicy string `mapstructure:"default_policy"`
MatchesPolicies []RegexRuleConf `mapstructure:"matches_policies"`
UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"`
}
type RegexRuleConf struct {
Priority int `mapstructure:"priority"`
Property string `mapstructure:"property"`
Match string `mapstructure:"match"`
Policy string `mapstructure:"policy"`
}

// New initializes a new configuration
Expand Down
114 changes: 61 additions & 53 deletions proxy/pkg/proxy/policy/selector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package policy
import (
"context"
"fmt"
"net/http"
"regexp"
"sort"

"github.com/asim/go-micro/plugins/client/grpc/v3"
revauser "github.com/cs3org/reva/pkg/user"
Expand All @@ -15,9 +15,9 @@ import (

var (
// ErrMultipleSelectors in case there is more then one selector configured.
ErrMultipleSelectors = fmt.Errorf("only one type of policy-selector (static or migration) can be configured")
ErrMultipleSelectors = fmt.Errorf("only one type of policy-selector (static, migration, claim or regex) can be configured")
// ErrSelectorConfigIncomplete if policy_selector conf is missing
ErrSelectorConfigIncomplete = fmt.Errorf("missing either \"static\" or \"migration\" configuration in policy_selector config ")
ErrSelectorConfigIncomplete = fmt.Errorf("missing either \"static\", \"migration\", \"claim\" or \"regex\" configuration in policy_selector config ")
// ErrUnexpectedConfigError unexpected config error
ErrUnexpectedConfigError = fmt.Errorf("could not initialize policy-selector for given config")
)
Expand Down Expand Up @@ -47,15 +47,29 @@ var (
// }
// ]
//}
type Selector func(ctx context.Context, r *http.Request) (string, error)
type Selector func(ctx context.Context) (string, error)

// LoadSelector constructs a specific policy-selector from a given configuration
func LoadSelector(cfg *config.PolicySelector) (Selector, error) {
if cfg.Migration != nil && cfg.Static != nil {
selCount := 0

if cfg.Migration != nil {
selCount++
}
if cfg.Static != nil {
selCount++
}
if cfg.Claims != nil {
selCount++
}
if cfg.Regex != nil {
selCount++
}
if selCount > 1 {
return nil, ErrMultipleSelectors
}

if cfg.Migration == nil && cfg.Static == nil {
if cfg.Migration == nil && cfg.Static == nil && cfg.Claims == nil && cfg.Regex == nil {
return nil, ErrSelectorConfigIncomplete
}

Expand Down Expand Up @@ -88,7 +102,7 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) {
// "static": {"policy" : "ocis"}
// },
func NewStaticSelector(cfg *config.StaticSelectorConf) Selector {
return func(ctx context.Context, r *http.Request) (s string, err error) {
return func(ctx context.Context) (s string, err error) {
return cfg.Policy, nil
}
}
Expand All @@ -107,9 +121,9 @@ func NewStaticSelector(cfg *config.StaticSelectorConf) Selector {
// thus have an entry in ocis-accounts. All users without accounts entry are routed to the legacy ownCloud10 instance.
func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.AccountsService) Selector {
var acc = ss
return func(ctx context.Context, r *http.Request) (s string, err error) {
return func(ctx context.Context) (s string, err error) {
var userID string
if claims := oidc.FromContext(r.Context()); claims != nil {
if claims := oidc.FromContext(ctx); claims != nil {
userID = claims.PreferredUsername
if _, err := acc.GetAccount(ctx, &accounts.GetAccountRequest{Id: userID}); err != nil {
return cfg.AccNotFoundPolicy, nil
Expand All @@ -133,8 +147,8 @@ func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.Account
//
// This selector can be used in migration-scenarios where some users have already migrated from ownCloud10 to OCIS and
func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector {
return func(ctx context.Context, r *http.Request) (s string, err error) {
if claims := oidc.FromContext(r.Context()); claims != nil {
return func(ctx context.Context) (s string, err error) {
if claims := oidc.FromContext(ctx); claims != nil {
if claims.OcisRoutingPolicy == "" {
return cfg.DefaultPolicy, nil
}
Expand All @@ -150,58 +164,46 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector {
// The policy for each case is configurable:
// "policy_selector": {
// "migration": {
// "matches_policies": {
// "mail": {
// "marie@example.com": "oc10"
// "[^@]+@example.com": "ocis"
// },
// "username": {
// "(einstein|feynman)": "ocis"
// "marie": "oc10"
// },
// "id": {
// "4c510ada-c86b-4815-8820-42cdf82c3d51": "ocis"
// "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c": "oc10"
// },
// },
// "matches_policies": [
// {"priority": 10, "property": "mail", "match": "marie@example.org", "policy": "ocis"},
// {"priority": 20, "property": "mail", "match": "[^@]+@example.org", "policy": "oc10"},
// {"priority": 30, "property": "username", "match": "(einstein|feynman)", "policy": "ocis"},
// {"priority": 40, "property": "username", "match": ".+", "policy": "oc10"},
// {"priority": 50, "property": "id", "match": "4c510ada-c86b-4815-8820-42cdf82c3d51", "policy": "ocis"},
// {"priority": 60, "property": "id", "match": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", "policy": "oc10"},
// ],
// "unauthenticated_policy": "oc10"
// }
// },
//
// This selector can be used in migration-scenarios where some users have already migrated from ownCloud10 to OCIS and
func NewRegexSelector(cfg *config.RegexSelectorConf) Selector {
var mailRegexPolicies map[*regexp.Regexp]string
for m, p := range cfg.MatchesPolicies["mail"] {
mailRegexPolicies[regexp.MustCompile(m)] = p
regexRules := []*regexRule{}
sort.Slice(cfg.MatchesPolicies, func(i, j int) bool {
return cfg.MatchesPolicies[i].Priority < cfg.MatchesPolicies[j].Priority
})
for i := range cfg.MatchesPolicies {
regexRules = append(regexRules, &regexRule{
property: cfg.MatchesPolicies[i].Property,
rule: regexp.MustCompile(cfg.MatchesPolicies[i].Match),
policy: cfg.MatchesPolicies[i].Policy,
})
}
var usernameRegexPolicies map[*regexp.Regexp]string
for m, p := range cfg.MatchesPolicies["username"] {
usernameRegexPolicies[regexp.MustCompile(m)] = p
}
var idRegexPolicies map[*regexp.Regexp]string
for m, p := range cfg.MatchesPolicies["id"] {
usernameRegexPolicies[regexp.MustCompile(m)] = p
}
return func(ctx context.Context, r *http.Request) (s string, err error) {
return func(ctx context.Context) (s string, err error) {
if u, ok := revauser.ContextGetUser(ctx); ok {
if u.Mail != "" {
for r, p := range mailRegexPolicies {
if r.MatchString(u.Mail) {
return p, nil
for i := range regexRules {
switch regexRules[i].property {
case "mail":
if regexRules[i].rule.MatchString(u.Mail) {
return regexRules[i].policy, nil
}
}
}
if u.Username != "" {
for r, p := range usernameRegexPolicies {
if r.MatchString(u.Username) {
return p, nil
case "username":
if regexRules[i].rule.MatchString(u.Username) {
return regexRules[i].policy, nil
}
}
}
if u.Id != nil && u.Id.OpaqueId != "" {
for r, p := range idRegexPolicies {
if r.MatchString(u.Id.OpaqueId) {
return p, nil
case "id":
if u.Id != nil && regexRules[i].rule.MatchString(u.Id.OpaqueId) {
return regexRules[i].policy, nil
}
}
}
Expand All @@ -211,3 +213,9 @@ func NewRegexSelector(cfg *config.RegexSelectorConf) Selector {
return cfg.UnauthenticatedPolicy, nil
}
}

type regexRule struct {
property string
rule *regexp.Regexp
policy string
}
102 changes: 86 additions & 16 deletions proxy/pkg/proxy/policy/selector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package policy
import (
"context"
"fmt"
"net/http/httptest"
"testing"

"github.com/asim/go-micro/v3/client"
userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
revauser "github.com/cs3org/reva/pkg/user"
"github.com/owncloud/ocis/accounts/pkg/proto/v0"
"github.com/owncloud/ocis/ocis-pkg/oidc"
"github.com/owncloud/ocis/proxy/pkg/config"
Expand All @@ -23,29 +24,32 @@ func TestLoadSelector(t *testing.T) {
AccNotFoundPolicy: "not_found",
UnauthenticatedPolicy: "unauth",
}
ccfg := &config.ClaimsSelectorConf{}
rcfg := &config.RegexSelectorConf{}

table := []test{
{cfg: &config.PolicySelector{Static: sCfg, Migration: mcfg}, expectedErr: ErrMultipleSelectors},
{cfg: &config.PolicySelector{Static: sCfg, Claims: ccfg, Regex: rcfg}, expectedErr: ErrMultipleSelectors},
{cfg: &config.PolicySelector{}, expectedErr: ErrSelectorConfigIncomplete},
{cfg: &config.PolicySelector{Static: sCfg}, expectedErr: nil},
{cfg: &config.PolicySelector{Migration: mcfg}, expectedErr: nil},
{cfg: &config.PolicySelector{Claims: ccfg}, expectedErr: nil},
{cfg: &config.PolicySelector{Regex: rcfg}, expectedErr: nil},
}

for _, test := range table {
_, err := LoadSelector(test.cfg)
if err != test.expectedErr {
t.Fail()
t.Errorf("Unexpected error %v", err)
}
}
}

func TestStaticSelector(t *testing.T) {
ctx := context.Background()
req := httptest.NewRequest("GET", "https://example.org/foo", nil)
sel := NewStaticSelector(&config.StaticSelectorConf{Policy: "ocis"})

ctx := context.Background()
want := "ocis"
got, err := sel(ctx, req)
got, err := sel(ctx)
if got != want {
t.Errorf("Expected policy %v got %v", want, got)
}
Expand All @@ -57,7 +61,7 @@ func TestStaticSelector(t *testing.T) {
sel = NewStaticSelector(&config.StaticSelectorConf{Policy: "foo"})

want = "foo"
got, err = sel(ctx, req)
got, err = sel(ctx)
if got != want {
t.Errorf("Expected policy %v got %v", want, got)
}
Expand All @@ -67,7 +71,7 @@ func TestStaticSelector(t *testing.T) {
}
}

type testCase struct {
type migrationTestCase struct {
AccSvcShouldReturnError bool
Claims *oidc.StandardClaims
Expected string
Expand All @@ -79,30 +83,25 @@ func TestMigrationSelector(t *testing.T) {
AccNotFoundPolicy: "not_found",
UnauthenticatedPolicy: "unauth",
}
var tests = []testCase{
var tests = []migrationTestCase{
{true, &oidc.StandardClaims{PreferredUsername: "Hans"}, "not_found"},
{false, &oidc.StandardClaims{PreferredUsername: "Hans"}, "found"},
{false, nil, "unauth"},
}

for _, tc := range tests {
//t.Run(fmt.Sprintf("#%v", k), func(t *testing.T) {
// t.Parallel()
tc := tc
sut := NewMigrationSelector(&cfg, mockAccSvc(tc.AccSvcShouldReturnError))
r := httptest.NewRequest("GET", "https://example.com", nil)
ctx := oidc.NewContext(r.Context(), tc.Claims)
nr := r.WithContext(ctx)
ctx := oidc.NewContext(context.Background(), tc.Claims)

got, err := sut(ctx, nr)
got, err := sut(ctx)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

if got != tc.Expected {
t.Errorf("Expected Policy %v got %v", tc.Expected, got)
}
//})
}
}

Expand All @@ -122,3 +121,74 @@ func mockAccSvc(retErr bool) proto.AccountsService {
}

}

type testCase struct {
Name string
Context context.Context
Expected string
}

func TestClaimsSelector(t *testing.T) {
sel := NewClaimsSelector(&config.ClaimsSelectorConf{
DefaultPolicy: "default",
UnauthenticatedPolicy: "unauthenticated",
})

var tests = []testCase{
{"unatuhenticated", context.Background(), "unauthenticated"},
{"default", oidc.NewContext(context.Background(), &oidc.StandardClaims{OcisRoutingPolicy: ""}), "default"},
{"claim-value", oidc.NewContext(context.Background(), &oidc.StandardClaims{OcisRoutingPolicy: "ocis.routing.policy-value"}), "ocis.routing.policy-value"},
}
for _, tc := range tests {
got, err := sel(tc.Context)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

if got != tc.Expected {
t.Errorf("Expected Policy %v got %v", tc.Expected, got)
}
}
}

func TestRegexSelector(t *testing.T) {
sel := NewRegexSelector(&config.RegexSelectorConf{
DefaultPolicy: "default",
MatchesPolicies: []config.RegexRuleConf{
{Priority: 10, Property: "mail", Match: "marie@example.org", Policy: "ocis"},
{Priority: 20, Property: "mail", Match: "[^@]+@example.org", Policy: "oc10"},
{Priority: 30, Property: "username", Match: "(einstein|feynman)", Policy: "ocis"},
{Priority: 40, Property: "username", Match: ".+", Policy: "oc10"},
{Priority: 50, Property: "id", Match: "4c510ada-c86b-4815-8820-42cdf82c3d51", Policy: "ocis"},
{Priority: 60, Property: "id", Match: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", Policy: "oc10"},
},
UnauthenticatedPolicy: "unauthenticated",
})

var tests = []testCase{
{"unauthenticated", context.Background(), "unauthenticated"},
{"default", revauser.ContextSetUser(context.Background(), &userv1beta1.User{}), "default"},
{"mail-ocis", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "marie@example.org"}), "ocis"},
{"mail-oc10", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "einstein@example.org"}), "oc10"},
{"username-einstein", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Username: "einstein"}), "ocis"},
{"username-feynman", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Username: "feynman"}), "ocis"},
{"username-marie", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Username: "marie"}), "oc10"},
{"id-nil", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{}}), "default"},
{"id-1", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51"}}), "ocis"},
{"id-2", revauser.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}}), "oc10"},
}

for _, tc := range tests {
tc := tc // capture range variable
t.Run(tc.Name, func(t *testing.T) {
got, err := sel(tc.Context)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

if got != tc.Expected {
t.Errorf("Expected Policy %v got %v", tc.Expected, got)
}
})
}
}
Loading

0 comments on commit dc71a60

Please sign in to comment.