From 40c8031441a258920c871158b3e9208bb1856312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 1 Jul 2021 23:14:25 +0200 Subject: [PATCH 01/11] add claims policy selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/claims-policy-selector.md | 5 +++++ ocis-pkg/oidc/claims.go | 20 +++++++++++-------- proxy/pkg/config/config.go | 7 +++++++ proxy/pkg/middleware/account_resolver.go | 2 ++ proxy/pkg/proxy/policy/selector.go | 19 ++++++++++++++++++ 5 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 changelog/unreleased/claims-policy-selector.md diff --git a/changelog/unreleased/claims-policy-selector.md b/changelog/unreleased/claims-policy-selector.md new file mode 100644 index 0000000000..25eb2ccf37 --- /dev/null +++ b/changelog/unreleased/claims-policy-selector.md @@ -0,0 +1,5 @@ +Enhancement: Proxy: Add claims policy selector + +Using the proxy config file, it is now possible to let let the IdP determine the routing policy by sending an `ocis.routing.policy` claim. Its value will be used to determine the set of routes for the logged in user. + +https://github.com/owncloud/ocis/pull/2248 diff --git a/ocis-pkg/oidc/claims.go b/ocis-pkg/oidc/claims.go index a35d4322e3..815281fc94 100644 --- a/ocis-pkg/oidc/claims.go +++ b/ocis-pkg/oidc/claims.go @@ -1,15 +1,16 @@ package oidc const ( - Iss = "iss" - Sub = "sub" - Email = "email" - Name = "name" + Iss = "iss" + Sub = "sub" + Email = "email" + Name = "name" PreferredUsername = "preferred_username" - UIDNumber = "uidnumber" - GIDNumber = "gidnumber" - Groups = "groups" - OwncloudUUID = "ownclouduuid" + UIDNumber = "uidnumber" + GIDNumber = "gidnumber" + Groups = "groups" + OwncloudUUID = "ownclouduuid" + OcisRoutingPolicy = "ocis.routing.policy" ) // The ProviderMetadata describes an idp. @@ -192,4 +193,7 @@ type StandardClaims struct { // OcisID is a unique, persistent, non reassignable user id OcisID string `json:"ownclouduuid,omitempty"` + + // OcisRoutingPolicy is used to specify the routing policy to use for the ocis proxy + OcisRoutingPolicy string `json:"ocis.routing.policy,omitempty"` } diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index a0b20b2d0f..4077f99921 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -141,6 +141,7 @@ type OIDC struct { type PolicySelector struct { Static *StaticSelectorConf Migration *MigrationSelectorConf + Claims *ClaimsSelectorConf } // StaticSelectorConf is the config for the static-policy-selector @@ -166,6 +167,12 @@ type MigrationSelectorConf struct { UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"` } +// ClaimsSelectorConf is the config for the claims-selector +type ClaimsSelectorConf struct { + DefaultPolicy string `mapstructure:"default_policy"` + UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"` +} + // New initializes a new configuration func New() *Config { return &Config{ diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index a216829a47..d89ced2606 100644 --- a/proxy/pkg/middleware/account_resolver.go +++ b/proxy/pkg/middleware/account_resolver.go @@ -83,6 +83,8 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { } m.logger.Debug().Interface("claims", claims).Msg("Autoprovisioning user") u, err = m.userProvider.CreateUserFromClaims(req.Context(), claims) + // TODO instead of creating an account create a personal storage via the CS3 admin api? + // see https://cs3org.github.io/cs3apis/#cs3.admin.user.v1beta1.CreateUserRequest } if errors.Is(err, backend.ErrAccountDisabled) { diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 10e259d194..11abfe6f52 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -67,6 +67,10 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) { accounts.NewAccountsService("com.owncloud.accounts", grpc.NewClient())), nil } + if cfg.Claims != nil { + return NewClaimsSelector(cfg.Claims), nil + } + return nil, ErrUnexpectedConfigError } @@ -117,3 +121,18 @@ func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.Account } } + +// NewClaimsSelector selects the policy based on the "ocis.routing.policy" claim +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 { + if p, ok := claims[oidc.OcisRoutingPolicy].(string); ok && p != "" { + // TODO check we know the routing policy? + return p, nil + } + return cfg.DefaultPolicy, nil + } + + return cfg.UnauthenticatedPolicy, nil + } +} From 4385d3dacb32571370ff28e0f7f2aac52c7cad1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 2 Jul 2021 08:54:15 +0200 Subject: [PATCH 02/11] add RegexSelector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/config/config.go | 8 +++ proxy/pkg/middleware/account_resolver.go | 13 ++-- proxy/pkg/proxy/policy/selector.go | 81 ++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index 4077f99921..b1c61664d8 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -142,6 +142,7 @@ type PolicySelector struct { Static *StaticSelectorConf Migration *MigrationSelectorConf Claims *ClaimsSelectorConf + Regex *RegexSelectorConf } // StaticSelectorConf is the config for the static-policy-selector @@ -173,6 +174,13 @@ type ClaimsSelectorConf struct { UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"` } +// 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"` +} + // New initializes a new configuration func New() *Config { return &Config{ diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index d89ced2606..12262a5b7e 100644 --- a/proxy/pkg/middleware/account_resolver.go +++ b/proxy/pkg/middleware/account_resolver.go @@ -26,7 +26,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl "expires": int64(60), }) if err != nil { - logger.Fatal().Err(err).Msgf("Could not initialize token-manager") + logger.Fatal().Err(err).Msg("Could not initialize token-manager") } return &accountResolver{ @@ -53,8 +53,9 @@ type accountResolver struct { // TODO do not use the context to store values: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39 func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { - claims := oidc.FromContext(req.Context()) - u, ok := revauser.ContextGetUser(req.Context()) + ctx := req.Context() + claims := oidc.FromContext(ctx) + u, ok := revauser.ContextGetUser(ctx) if claims == nil && !ok { m.next.ServeHTTP(w, req) @@ -99,6 +100,10 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } + // add user to context for selectors + ctx = revauser.ContextSetUser(ctx, u) + req = req.WithContext(ctx) + m.logger.Debug().Interface("claims", claims).Interface("user", u).Msg("associated claims with user") } @@ -107,7 +112,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { m.logger.Error().Err(err).Msg("could not get owner scope") return } - token, err := m.tokenManager.MintToken(req.Context(), u, s) + token, err := m.tokenManager.MintToken(ctx, u, s) if err != nil { m.logger.Error().Err(err).Msg("could not mint token") w.WriteHeader(http.StatusInternalServerError) diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 11abfe6f52..ac0d3f5366 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -4,8 +4,10 @@ import ( "context" "fmt" "net/http" + "regexp" "github.com/asim/go-micro/plugins/client/grpc/v3" + revauser "github.com/cs3org/reva/pkg/user" accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/ocis-pkg/oidc" "github.com/owncloud/ocis/proxy/pkg/config" @@ -71,6 +73,10 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) { return NewClaimsSelector(cfg.Claims), nil } + if cfg.Regex != nil { + return NewRegexSelector(cfg.Regex), nil + } + return nil, ErrUnexpectedConfigError } @@ -123,6 +129,15 @@ func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.Account } // NewClaimsSelector selects the policy based on the "ocis.routing.policy" claim +// The policy for corner cases is configurable: +// "policy_selector": { +// "migration": { +// "default_policy" : "ocis", +// "unauthenticated_policy": "oc10" +// } +// }, +// +// 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 { @@ -136,3 +151,69 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector { return cfg.UnauthenticatedPolicy, nil } } + +// NewRegexSelector selects the policy based on a user property +// 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" +// }, +// }, +// "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 + } + 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) { + if u, ok := revauser.ContextGetUser(ctx); ok { + if u.Mail != "" { + for r, p := range mailRegexPolicies { + if r.MatchString(u.Mail) { + return p, nil + } + } + } + if u.Username != "" { + for r, p := range usernameRegexPolicies { + if r.MatchString(u.Username) { + return p, nil + } + } + } + if u.Id != nil && u.Id.OpaqueId != "" { + for r, p := range idRegexPolicies { + if r.MatchString(u.Id.OpaqueId) { + return p, nil + } + } + } + return cfg.DefaultPolicy, nil + } + + return cfg.UnauthenticatedPolicy, nil + } +} From b540ccdf10bafe556ac9080552f5b131223c09b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 2 Jul 2021 13:41:51 +0200 Subject: [PATCH 03/11] add regex rule priority, drop request from selector signature, add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/config/config.go | 16 +++- proxy/pkg/proxy/policy/selector.go | 114 +++++++++++++----------- proxy/pkg/proxy/policy/selector_test.go | 102 +++++++++++++++++---- proxy/pkg/proxy/proxy.go | 2 +- 4 files changed, 160 insertions(+), 74 deletions(-) diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index b1c61664d8..4015daa949 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -1,6 +1,8 @@ package config -import "context" +import ( + "context" +) // Log defines the available logging configuration. type Log struct { @@ -176,9 +178,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 diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index ac0d3f5366..3ced3d18af 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -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" @@ -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") ) @@ -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 } @@ -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 } } @@ -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 claims map[string]interface{} - if claims = oidc.FromContext(r.Context()); claims == nil { + if claims = oidc.FromContext(ctx); claims == nil { return cfg.UnauthenticatedPolicy, nil } @@ -139,8 +153,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 p, ok := claims[oidc.OcisRoutingPolicy].(string); ok && p != "" { // TODO check we know the routing policy? return p, nil @@ -156,58 +170,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, ®exRule{ + 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 } } } @@ -217,3 +219,9 @@ func NewRegexSelector(cfg *config.RegexSelectorConf) Selector { return cfg.UnauthenticatedPolicy, nil } } + +type regexRule struct { + property string + rule *regexp.Regexp + policy string +} diff --git a/proxy/pkg/proxy/policy/selector_test.go b/proxy/pkg/proxy/policy/selector_test.go index 09127a9dcc..a22dba988e 100644 --- a/proxy/pkg/proxy/policy/selector_test.go +++ b/proxy/pkg/proxy/policy/selector_test.go @@ -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" @@ -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) } @@ -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) } @@ -67,7 +71,7 @@ func TestStaticSelector(t *testing.T) { } } -type testCase struct { +type migrationTestCase struct { AccSvcShouldReturnError bool Claims map[string]interface{} Expected string @@ -79,7 +83,7 @@ func TestMigrationSelector(t *testing.T) { AccNotFoundPolicy: "not_found", UnauthenticatedPolicy: "unauth", } - var tests = []testCase{ + var tests = []migrationTestCase{ {true, map[string]interface{}{oidc.PreferredUsername: "Hans"}, "not_found"}, {true, map[string]interface{}{oidc.Email: "hans@example.test"}, "not_found"}, {false, map[string]interface{}{oidc.PreferredUsername: "Hans"}, "found"}, @@ -87,15 +91,11 @@ func TestMigrationSelector(t *testing.T) { } 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) } @@ -103,7 +103,6 @@ func TestMigrationSelector(t *testing.T) { if got != tc.Expected { t.Errorf("Expected Policy %v got %v", tc.Expected, got) } - //}) } } @@ -123,3 +122,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(), map[string]interface{}{oidc.OcisRoutingPolicy: ""}), "default"}, + {"claim-value", oidc.NewContext(context.Background(), map[string]interface{}{oidc.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) + } + }) + } +} diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index d722439e05..b4871617a1 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -110,7 +110,7 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { } func (p *MultiHostReverseProxy) directorSelectionDirector(r *http.Request) { - pol, err := p.PolicySelector(r.Context(), r) + pol, err := p.PolicySelector(r.Context()) if err != nil { p.logger.Error().Msgf("Error while selecting pol %v", err) return From b0c742cb2f79191b78b3c1137f2dfd9dbe4a21bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 5 Jul 2021 13:19:33 +0200 Subject: [PATCH 04/11] fix docs typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/proxy/policy/selector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 3ced3d18af..bc6fa67ef2 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -169,7 +169,7 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector { // NewRegexSelector selects the policy based on a user property // The policy for each case is configurable: // "policy_selector": { -// "migration": { +// "regex": { // "matches_policies": [ // {"priority": 10, "property": "mail", "match": "marie@example.org", "policy": "ocis"}, // {"priority": 20, "property": "mail", "match": "[^@]+@example.org", "policy": "oc10"}, From 2583c6a1a71acfa07f798909333772c991e6fabb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 5 Jul 2021 15:43:08 +0200 Subject: [PATCH 05/11] remove comma MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/proxy/policy/selector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index bc6fa67ef2..a1c58fa7c8 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -176,7 +176,7 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector { // {"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"}, +// {"priority": 60, "property": "id", "match": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", "policy": "oc10"} // ], // "unauthenticated_policy": "oc10" // } From 5c024b4543600e5da3b06c5037219b4e796af09a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 5 Jul 2021 15:43:50 +0200 Subject: [PATCH 06/11] use context from middleware in proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/proxy/proxy.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index b4871617a1..0d4e9cfd79 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -1,7 +1,6 @@ package proxy import ( - "context" "crypto/tls" "net" "net/http" @@ -213,12 +212,12 @@ func (p *MultiHostReverseProxy) AddHost(policy string, target *url.URL, rt confi } func (p *MultiHostReverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { - ctx := context.Background() + ctx := r.Context() var span *trace.Span // Start root span. if p.config.Tracing.Enabled { - ctx, span = trace.StartSpan(context.Background(), r.URL.String()) + ctx, span = trace.StartSpan(ctx, r.URL.String()) defer span.End() p.propagator.SpanContextToRequest(span.SpanContext(), r) } From 0a632df7d2f114d378e13edce665ca24271cf3f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 5 Jul 2021 15:49:57 +0200 Subject: [PATCH 07/11] update proxy config json example files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/config/proxy-example-migration.json | 5 +- proxy/config/proxy-example-regex.json | 167 ++++++++++++++++++++++ proxy/config/proxy-example.json | 5 +- 3 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 proxy/config/proxy-example-regex.json diff --git a/proxy/config/proxy-example-migration.json b/proxy/config/proxy-example-migration.json index 886e6dd31e..f44c9f49a0 100644 --- a/proxy/config/proxy-example-migration.json +++ b/proxy/config/proxy-example-migration.json @@ -1,6 +1,7 @@ { - "HTTP": { - "Namespace": "com.owncloud" + "http": { + "addr": "0.0.0.0:9200", + "root": "/" }, "oidc": { "issuer": "https://localhost:9200", diff --git a/proxy/config/proxy-example-regex.json b/proxy/config/proxy-example-regex.json new file mode 100644 index 0000000000..542726c974 --- /dev/null +++ b/proxy/config/proxy-example-regex.json @@ -0,0 +1,167 @@ +{ + "http": { + "addr": "0.0.0.0:9200", + "root": "/" + }, + "oidc": { + "issuer": "https://localhost:9200", + "insecure": true + }, + "policy_selector": { + "regex": { + "default_policy": "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" + } + }, + "policies": [ + { + "name": "ocis", + "routes": [ + { + "endpoint": "/", + "backend": "http://localhost:9100" + }, + { + "endpoint": "/.well-known/", + "backend": "http://localhost:9130" + }, + { + "endpoint": "/konnect/", + "backend": "http://localhost:9130" + }, + { + "endpoint": "/signin/", + "backend": "http://localhost:9130" + }, + { + "type": "regex", + "endpoint": "/ocs/v[12].php/cloud/(users?|groups)", + "backend": "http://localhost:9110" + }, + { + "endpoint": "/ocs/", + "backend": "http://localhost:9140" + }, + { + "type": "query", + "endpoint": "/remote.php/?preview=1", + "backend": "http://localhost:9115" + }, + { + "endpoint": "/remote.php/", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/dav/", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/webdav/", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/status.php", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/index.php/", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/data", + "backend": "http://localhost:9140" + }, + { + "endpoint": "/graph/", + "backend": "http://localhost:9120" + }, + { + "endpoint": "/graph-explorer/", + "backend": "http://localhost:9135" + }, + { + "endpoint": "/api/v0/accounts", + "backend": "http://localhost:9181" + }, + { + "endpoint": "/accounts.js", + "backend": "http://localhost:9181" + }, + { + "endpoint": "/api/v0/settings", + "backend": "http://localhost:9190" + }, + { + "endpoint": "/settings.js", + "backend": "http://localhost:9190" + }, + { + "endpoint": "/onlyoffice.js", + "backend": "http://localhost:9220" + } + ] + }, + { + "name": "oc10", + "routes": [ + { + "endpoint": "/", + "backend": "http://localhost:9100" + }, + { + "endpoint": "/.well-known/", + "backend": "http://localhost:9130" + }, + { + "endpoint": "/konnect/", + "backend": "http://localhost:9130" + }, + { + "endpoint": "/signin/", + "backend": "http://localhost:9130" + }, + { + "endpoint": "/ocs/", + "backend": "https://demo.owncloud.com", + "apache-vhost": true + }, + { + "endpoint": "/remote.php/", + "backend": "https://demo.owncloud.com", + "apache-vhost": true + }, + { + "endpoint": "/dav/", + "backend": "https://demo.owncloud.com", + "apache-vhost": true + }, + { + "endpoint": "/webdav/", + "backend": "https://demo.owncloud.com", + "apache-vhost": true + }, + { + "endpoint": "/status.php", + "backend": "https://demo.owncloud.com" + }, + { + "endpoint": "/index.php/", + "backend": "https://demo.owncloud.com" + }, + { + "endpoint": "/data", + "backend": "https://demo.owncloud.com", + "apache-vhost": true + } + ] + } + ] +} diff --git a/proxy/config/proxy-example.json b/proxy/config/proxy-example.json index 166015d950..5347b4538c 100644 --- a/proxy/config/proxy-example.json +++ b/proxy/config/proxy-example.json @@ -1,6 +1,7 @@ { - "HTTP": { - "Namespace": "com.owncloud" + "http": { + "addr": "0.0.0.0:9200", + "root": "/" }, "policy_selector": { "static": { From cb70f4882f93f7b95fcb04e7b9e05ea5edf1cb13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 13 Jul 2021 09:43:25 +0000 Subject: [PATCH 08/11] add todo for x-access-header handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/middleware/account_resolver.go | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index 12262a5b7e..e7d6aed68b 100644 --- a/proxy/pkg/middleware/account_resolver.go +++ b/proxy/pkg/middleware/account_resolver.go @@ -56,6 +56,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { ctx := req.Context() claims := oidc.FromContext(ctx) u, ok := revauser.ContextGetUser(ctx) + // TODO what if an X-Access-Token is set? happens eg for download requests to the /data endpoint in the reva frontend if claims == nil && !ok { m.next.ServeHTTP(w, req) From a0dce56480c8880354f17987da24b7d233ce6859 Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Wed, 21 Jul 2021 18:34:34 +0200 Subject: [PATCH 09/11] use cookie to enforce routing for regex and claim selector --- proxy/config/proxy-example-regex.json | 1 + proxy/pkg/command/server.go | 6 ++ proxy/pkg/config/config.go | 2 + proxy/pkg/middleware/options.go | 12 +++- proxy/pkg/middleware/selector_cookie.go | 78 +++++++++++++++++++++++++ proxy/pkg/proxy/policy/selector.go | 35 +++++++++-- proxy/pkg/proxy/proxy.go | 2 +- 7 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 proxy/pkg/middleware/selector_cookie.go diff --git a/proxy/config/proxy-example-regex.json b/proxy/config/proxy-example-regex.json index 542726c974..ba4aee182e 100644 --- a/proxy/config/proxy-example-regex.json +++ b/proxy/config/proxy-example-regex.json @@ -9,6 +9,7 @@ }, "policy_selector": { "regex": { + "selector_cookie_name": "owncloud-selector", "default_policy": "oc10", "matches_policies": [ {"priority": 10, "property": "mail", "match": "marie@example.org", "policy": "ocis"}, diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index 0fbc30abe1..9546b92abb 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -221,6 +221,12 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic middleware.AutoprovisionAccounts(cfg.AutoprovisionAccounts), ), + middleware.SelectorCookie( + middleware.Logger(l), + middleware.UserProvider(userProvider), + middleware.PolicySelectorConfig(*cfg.PolicySelector), + ), + // finally, trigger home creation when a user logs in middleware.CreateHome( middleware.Logger(l), diff --git a/proxy/pkg/config/config.go b/proxy/pkg/config/config.go index 4015daa949..e263302741 100644 --- a/proxy/pkg/config/config.go +++ b/proxy/pkg/config/config.go @@ -174,6 +174,7 @@ type MigrationSelectorConf struct { type ClaimsSelectorConf struct { DefaultPolicy string `mapstructure:"default_policy"` UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"` + SelectorCookieName string `mapstructure:"selector_cookie_name"` } // RegexSelectorConf is the config for the regex-selector @@ -181,6 +182,7 @@ type RegexSelectorConf struct { DefaultPolicy string `mapstructure:"default_policy"` MatchesPolicies []RegexRuleConf `mapstructure:"matches_policies"` UnauthenticatedPolicy string `mapstructure:"unauthenticated_policy"` + SelectorCookieName string `mapstructure:"selector_cookie_name"` } type RegexRuleConf struct { Priority int `mapstructure:"priority"` diff --git a/proxy/pkg/middleware/options.go b/proxy/pkg/middleware/options.go index e0144ca8e7..3f91cf1c27 100644 --- a/proxy/pkg/middleware/options.go +++ b/proxy/pkg/middleware/options.go @@ -1,10 +1,11 @@ package middleware import ( - "github.com/owncloud/ocis/proxy/pkg/user/backend" "net/http" "time" + "github.com/owncloud/ocis/proxy/pkg/user/backend" + settings "github.com/owncloud/ocis/settings/pkg/proto/v0" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -23,6 +24,8 @@ type Options struct { Logger log.Logger // TokenManagerConfig for communicating with the reva token manager TokenManagerConfig config.TokenManager + // PolicySelectorConfig for using the policy selector + PolicySelector config.PolicySelector // HTTPClient to use for communication with the oidcAuth provider HTTPClient *http.Client // AccountsClient for resolving accounts @@ -82,6 +85,13 @@ func TokenManagerConfig(cfg config.TokenManager) Option { } } +// PolicySelectorConfig provides a function to set the policy selector config option. +func PolicySelectorConfig(cfg config.PolicySelector) Option { + return func(o *Options) { + o.PolicySelector = cfg + } +} + // HTTPClient provides a function to set the http client config option. func HTTPClient(c *http.Client) Option { return func(o *Options) { diff --git a/proxy/pkg/middleware/selector_cookie.go b/proxy/pkg/middleware/selector_cookie.go new file mode 100644 index 0000000000..1ef8a70c39 --- /dev/null +++ b/proxy/pkg/middleware/selector_cookie.go @@ -0,0 +1,78 @@ +package middleware + +import ( + "net/http" + + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/ocis-pkg/oidc" + "github.com/owncloud/ocis/proxy/pkg/config" + "github.com/owncloud/ocis/proxy/pkg/proxy/policy" +) + +// SelectorCookie provides a middleware which +func SelectorCookie(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + logger := options.Logger + policySelector := options.PolicySelector + + return func(next http.Handler) http.Handler { + return &selectorCookie{ + next: next, + logger: logger, + policySelector: policySelector, + } + } +} + +type selectorCookie struct { + next http.Handler + logger log.Logger + policySelector config.PolicySelector +} + +func (m selectorCookie) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if m.policySelector.Regex == nil && m.policySelector.Claims == nil { + // only set selector cookie for regex and claim selectors + m.next.ServeHTTP(w, req) + return + } + + ctx := req.Context() + claims := oidc.FromContext(ctx) + + selectorCookieName := "" + if m.policySelector.Regex != nil { + selectorCookieName = m.policySelector.Regex.SelectorCookieName + } else if m.policySelector.Claims != nil { + selectorCookieName = m.policySelector.Claims.SelectorCookieName + } + + _, err := req.Cookie(selectorCookieName) + if err != nil { + // no cookie there - try to add one + if claims != nil { + + selectorFunc, err := policy.LoadSelector(&m.policySelector) + if err != nil { + m.logger.Err(err) + } + + selector, err := selectorFunc(ctx, req) + if err != nil { + m.logger.Err(err) + } + + cookie := http.Cookie{ + Name: selectorCookieName, + Value: selector, + Domain: req.Host, + Path: "/", + MaxAge: 60 * 60, + HttpOnly: true, + } + http.SetCookie(w, &cookie) + } + } + + m.next.ServeHTTP(w, req) +} diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index a1c58fa7c8..966f105264 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -3,6 +3,7 @@ package policy import ( "context" "fmt" + "net/http" "regexp" "sort" @@ -22,6 +23,10 @@ var ( ErrUnexpectedConfigError = fmt.Errorf("could not initialize policy-selector for given config") ) +const ( + SelectorCookieName = "owncloud-selector" +) + // Selector is a function which selects a proxy-policy based on the request. // // A policy is a random name which identifies a set of proxy-routes: @@ -47,7 +52,7 @@ var ( // } // ] //} -type Selector func(ctx context.Context) (string, error) +type Selector func(ctx context.Context, r *http.Request) (string, error) // LoadSelector constructs a specific policy-selector from a given configuration func LoadSelector(cfg *config.PolicySelector) (Selector, error) { @@ -84,10 +89,16 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) { } if cfg.Claims != nil { + if cfg.Claims.SelectorCookieName == "" { + cfg.Claims.SelectorCookieName = SelectorCookieName + } return NewClaimsSelector(cfg.Claims), nil } if cfg.Regex != nil { + if cfg.Regex.SelectorCookieName == "" { + cfg.Regex.SelectorCookieName = SelectorCookieName + } return NewRegexSelector(cfg.Regex), nil } @@ -102,7 +113,7 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) { // "static": {"policy" : "ocis"} // }, func NewStaticSelector(cfg *config.StaticSelectorConf) Selector { - return func(ctx context.Context) (s string, err error) { + return func(ctx context.Context, r *http.Request) (s string, err error) { return cfg.Policy, nil } } @@ -121,7 +132,7 @@ 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) (s string, err error) { + return func(ctx context.Context, r *http.Request) (s string, err error) { var claims map[string]interface{} if claims = oidc.FromContext(ctx); claims == nil { return cfg.UnauthenticatedPolicy, nil @@ -153,7 +164,14 @@ 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) (s string, err error) { + return func(ctx context.Context, r *http.Request) (s string, err error) { + // use cookie first if provided + selectorCookie, err := r.Cookie(cfg.SelectorCookieName) + if err == nil { + return selectorCookie.Value, nil + } + + // if no cookie is present, try to route by selector if claims := oidc.FromContext(ctx); claims != nil { if p, ok := claims[oidc.OcisRoutingPolicy].(string); ok && p != "" { // TODO check we know the routing policy? @@ -195,7 +213,14 @@ func NewRegexSelector(cfg *config.RegexSelectorConf) Selector { policy: cfg.MatchesPolicies[i].Policy, }) } - return func(ctx context.Context) (s string, err error) { + return func(ctx context.Context, r *http.Request) (s string, err error) { + // use cookie first if provided + selectorCookie, err := r.Cookie(cfg.SelectorCookieName) + if err == nil { + return selectorCookie.Value, nil + } + + // if no cookie is present, try to route by selector if u, ok := revauser.ContextGetUser(ctx); ok { for i := range regexRules { switch regexRules[i].property { diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index 0d4e9cfd79..bb8c0c76a9 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -109,7 +109,7 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { } func (p *MultiHostReverseProxy) directorSelectionDirector(r *http.Request) { - pol, err := p.PolicySelector(r.Context()) + pol, err := p.PolicySelector(r.Context(), r) if err != nil { p.logger.Error().Msgf("Error while selecting pol %v", err) return From bea986fe268cfd2862086d97d46a9db678f321d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 23 Jul 2021 09:03:14 +0000 Subject: [PATCH 10/11] pass only request instead of context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/middleware/selector_cookie.go | 7 ++----- proxy/pkg/proxy/policy/selector.go | 19 +++++++++---------- proxy/pkg/proxy/policy/selector_test.go | 21 ++++++++++++++------- proxy/pkg/proxy/proxy.go | 16 +++++++++------- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/proxy/pkg/middleware/selector_cookie.go b/proxy/pkg/middleware/selector_cookie.go index 1ef8a70c39..04cf8cfbc3 100644 --- a/proxy/pkg/middleware/selector_cookie.go +++ b/proxy/pkg/middleware/selector_cookie.go @@ -37,9 +37,6 @@ func (m selectorCookie) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - ctx := req.Context() - claims := oidc.FromContext(ctx) - selectorCookieName := "" if m.policySelector.Regex != nil { selectorCookieName = m.policySelector.Regex.SelectorCookieName @@ -50,14 +47,14 @@ func (m selectorCookie) ServeHTTP(w http.ResponseWriter, req *http.Request) { _, err := req.Cookie(selectorCookieName) if err != nil { // no cookie there - try to add one - if claims != nil { + if oidc.FromContext(req.Context()) != nil { selectorFunc, err := policy.LoadSelector(&m.policySelector) if err != nil { m.logger.Err(err) } - selector, err := selectorFunc(ctx, req) + selector, err := selectorFunc(req) if err != nil { m.logger.Err(err) } diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 966f105264..ef5a1b556a 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -1,7 +1,6 @@ package policy import ( - "context" "fmt" "net/http" "regexp" @@ -52,7 +51,7 @@ const ( // } // ] //} -type Selector func(ctx context.Context, r *http.Request) (string, error) +type Selector func(r *http.Request) (string, error) // LoadSelector constructs a specific policy-selector from a given configuration func LoadSelector(cfg *config.PolicySelector) (Selector, error) { @@ -113,7 +112,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(r *http.Request) (s string, err error) { return cfg.Policy, nil } } @@ -132,9 +131,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(r *http.Request) (s string, err error) { var claims map[string]interface{} - if claims = oidc.FromContext(ctx); claims == nil { + if claims = oidc.FromContext(r.Context()); claims == nil { return cfg.UnauthenticatedPolicy, nil } @@ -145,7 +144,7 @@ func NewMigrationSelector(cfg *config.MigrationSelectorConf, ss accounts.Account return cfg.AccNotFoundPolicy, nil } - if _, err := acc.GetAccount(ctx, &accounts.GetAccountRequest{Id: userID}); err != nil { + if _, err := acc.GetAccount(r.Context(), &accounts.GetAccountRequest{Id: userID}); err != nil { return cfg.AccNotFoundPolicy, nil } return cfg.AccFoundPolicy, nil @@ -164,7 +163,7 @@ 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) { + return func(r *http.Request) (s string, err error) { // use cookie first if provided selectorCookie, err := r.Cookie(cfg.SelectorCookieName) if err == nil { @@ -172,7 +171,7 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector { } // if no cookie is present, try to route by selector - if claims := oidc.FromContext(ctx); claims != nil { + if claims := oidc.FromContext(r.Context()); claims != nil { if p, ok := claims[oidc.OcisRoutingPolicy].(string); ok && p != "" { // TODO check we know the routing policy? return p, nil @@ -213,7 +212,7 @@ func NewRegexSelector(cfg *config.RegexSelectorConf) Selector { policy: cfg.MatchesPolicies[i].Policy, }) } - return func(ctx context.Context, r *http.Request) (s string, err error) { + return func(r *http.Request) (s string, err error) { // use cookie first if provided selectorCookie, err := r.Cookie(cfg.SelectorCookieName) if err == nil { @@ -221,7 +220,7 @@ func NewRegexSelector(cfg *config.RegexSelectorConf) Selector { } // if no cookie is present, try to route by selector - if u, ok := revauser.ContextGetUser(ctx); ok { + if u, ok := revauser.ContextGetUser(r.Context()); ok { for i := range regexRules { switch regexRules[i].property { case "mail": diff --git a/proxy/pkg/proxy/policy/selector_test.go b/proxy/pkg/proxy/policy/selector_test.go index a22dba988e..5e65674c95 100644 --- a/proxy/pkg/proxy/policy/selector_test.go +++ b/proxy/pkg/proxy/policy/selector_test.go @@ -3,6 +3,7 @@ package policy import ( "context" "fmt" + "net/http/httptest" "testing" "github.com/asim/go-micro/v3/client" @@ -47,9 +48,9 @@ func TestLoadSelector(t *testing.T) { func TestStaticSelector(t *testing.T) { sel := NewStaticSelector(&config.StaticSelectorConf{Policy: "ocis"}) - ctx := context.Background() + req := httptest.NewRequest("GET", "https://example.org/foo", nil) want := "ocis" - got, err := sel(ctx) + got, err := sel(req) if got != want { t.Errorf("Expected policy %v got %v", want, got) } @@ -61,7 +62,7 @@ func TestStaticSelector(t *testing.T) { sel = NewStaticSelector(&config.StaticSelectorConf{Policy: "foo"}) want = "foo" - got, err = sel(ctx) + got, err = sel(req) if got != want { t.Errorf("Expected policy %v got %v", want, got) } @@ -93,9 +94,11 @@ func TestMigrationSelector(t *testing.T) { for _, tc := range tests { tc := tc sut := NewMigrationSelector(&cfg, mockAccSvc(tc.AccSvcShouldReturnError)) - ctx := oidc.NewContext(context.Background(), tc.Claims) + r := httptest.NewRequest("GET", "https://example.com", nil) + ctx := oidc.NewContext(r.Context(), tc.Claims) + nr := r.WithContext(ctx) - got, err := sut(ctx) + got, err := sut(nr) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -141,7 +144,9 @@ func TestClaimsSelector(t *testing.T) { {"claim-value", oidc.NewContext(context.Background(), map[string]interface{}{oidc.OcisRoutingPolicy: "ocis.routing.policy-value"}), "ocis.routing.policy-value"}, } for _, tc := range tests { - got, err := sel(tc.Context) + r := httptest.NewRequest("GET", "https://example.com", nil) + nr := r.WithContext(tc.Context) + got, err := sel(nr) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -182,7 +187,9 @@ func TestRegexSelector(t *testing.T) { for _, tc := range tests { tc := tc // capture range variable t.Run(tc.Name, func(t *testing.T) { - got, err := sel(tc.Context) + r := httptest.NewRequest("GET", "https://example.com", nil) + nr := r.WithContext(tc.Context) + got, err := sel(nr) if err != nil { t.Errorf("Unexpected error: %v", err) } diff --git a/proxy/pkg/proxy/proxy.go b/proxy/pkg/proxy/proxy.go index bb8c0c76a9..9193252e13 100644 --- a/proxy/pkg/proxy/proxy.go +++ b/proxy/pkg/proxy/proxy.go @@ -66,7 +66,7 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { if options.Config.PolicySelector == nil { firstPolicy := options.Config.Policies[0].Name - rp.logger.Warn().Msgf("policy-selector not configured. Will always use first policy: '%v'", firstPolicy) + rp.logger.Warn().Str("policy", firstPolicy).Msg("policy-selector not configured. Will always use first policy") options.Config.PolicySelector = &config.PolicySelector{ Static: &config.StaticSelectorConf{ Policy: firstPolicy, @@ -91,9 +91,10 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { uri, err := url.Parse(route.Backend) if err != nil { rp.logger. - Fatal(). + Fatal(). // fail early on misconfiguration Err(err). - Msgf("malformed url: %v", route.Backend) + Str("backend", route.Backend). + Msg("malformed url") } rp.logger. @@ -109,16 +110,17 @@ func NewMultiHostReverseProxy(opts ...Option) *MultiHostReverseProxy { } func (p *MultiHostReverseProxy) directorSelectionDirector(r *http.Request) { - pol, err := p.PolicySelector(r.Context(), r) + pol, err := p.PolicySelector(r) if err != nil { - p.logger.Error().Msgf("Error while selecting pol %v", err) + p.logger.Error().Err(err).Msg("Error while selecting pol") return } if _, ok := p.Directors[pol]; !ok { p.logger. Error(). - Msgf("policy %v is not configured", pol) + Str("policy", pol). + Msg("policy is not configured") return } @@ -247,7 +249,7 @@ func (p MultiHostReverseProxy) queryRouteMatcher(endpoint string, target url.URL func (p *MultiHostReverseProxy) regexRouteMatcher(pattern string, target url.URL) bool { matched, err := regexp.MatchString(pattern, target.String()) if err != nil { - p.logger.Warn().Err(err).Msgf("regex with pattern %s failed", pattern) + p.logger.Warn().Err(err).Str("pattern", pattern).Msg("regex with pattern failed") } return matched } From 07bb4a179ee613e8bbff7c422bb8a9c26eb942f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 23 Jul 2021 09:42:40 +0000 Subject: [PATCH 11/11] clarify cs3 claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- proxy/pkg/flagset/flagset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/pkg/flagset/flagset.go b/proxy/pkg/flagset/flagset.go index cf6efe7614..79522c4e14 100644 --- a/proxy/pkg/flagset/flagset.go +++ b/proxy/pkg/flagset/flagset.go @@ -254,7 +254,7 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { &cli.StringFlag{ Name: "user-cs3-claim", Value: flags.OverrideDefaultString(cfg.UserCS3Claim, "mail"), - Usage: "The claim to use when looking up a user in the CS3 API, eg. 'userid' or 'mail'", + Usage: "The CS3 claim to use when looking up a user in the CS3 users API, eg. 'userid', 'username' or 'mail'", EnvVars: []string{"PROXY_USER_CS3_CLAIM"}, Destination: &cfg.UserCS3Claim, },