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/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..ba4aee182e --- /dev/null +++ b/proxy/config/proxy-example-regex.json @@ -0,0 +1,168 @@ +{ + "http": { + "addr": "0.0.0.0:9200", + "root": "/" + }, + "oidc": { + "issuer": "https://localhost:9200", + "insecure": true + }, + "policy_selector": { + "regex": { + "selector_cookie_name": "owncloud-selector", + "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": { 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 a0b20b2d0f..e263302741 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 { @@ -141,6 +143,8 @@ type OIDC struct { type PolicySelector struct { Static *StaticSelectorConf Migration *MigrationSelectorConf + Claims *ClaimsSelectorConf + Regex *RegexSelectorConf } // StaticSelectorConf is the config for the static-policy-selector @@ -166,6 +170,27 @@ 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"` + SelectorCookieName string `mapstructure:"selector_cookie_name"` +} + +// RegexSelectorConf is the config for the regex-selector +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"` + Property string `mapstructure:"property"` + Match string `mapstructure:"match"` + Policy string `mapstructure:"policy"` +} + // New initializes a new configuration func New() *Config { return &Config{ 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, }, diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go index a216829a47..e7d6aed68b 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,10 @@ 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) + // 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) @@ -83,6 +85,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) { @@ -97,6 +101,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") } @@ -105,7 +113,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/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..04cf8cfbc3 --- /dev/null +++ b/proxy/pkg/middleware/selector_cookie.go @@ -0,0 +1,75 @@ +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 + } + + 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 oidc.FromContext(req.Context()) != nil { + + selectorFunc, err := policy.LoadSelector(&m.policySelector) + if err != nil { + m.logger.Err(err) + } + + selector, err := selectorFunc(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 10e259d194..ef5a1b556a 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -1,11 +1,13 @@ 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" accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/ocis-pkg/oidc" "github.com/owncloud/ocis/proxy/pkg/config" @@ -13,13 +15,17 @@ 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") ) +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: @@ -45,15 +51,29 @@ var ( // } // ] //} -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) { - 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 } @@ -67,6 +87,20 @@ func LoadSelector(cfg *config.PolicySelector) (Selector, error) { accounts.NewAccountsService("com.owncloud.accounts", grpc.NewClient())), nil } + 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 + } + return nil, ErrUnexpectedConfigError } @@ -78,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 } } @@ -97,7 +131,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, 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(r.Context()); claims == nil { return cfg.UnauthenticatedPolicy, nil @@ -110,10 +144,108 @@ 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 } } + +// 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(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(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 + } +} + +// NewRegexSelector selects the policy based on a user property +// The policy for each case is configurable: +// "policy_selector": { +// "regex": { +// "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 { + 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, + }) + } + return func(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(r.Context()); ok { + for i := range regexRules { + switch regexRules[i].property { + case "mail": + if regexRules[i].rule.MatchString(u.Mail) { + return regexRules[i].policy, nil + } + case "username": + if regexRules[i].rule.MatchString(u.Username) { + return regexRules[i].policy, nil + } + case "id": + if u.Id != nil && regexRules[i].rule.MatchString(u.Id.OpaqueId) { + return regexRules[i].policy, nil + } + } + } + return cfg.DefaultPolicy, nil + } + + 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..5e65674c95 100644 --- a/proxy/pkg/proxy/policy/selector_test.go +++ b/proxy/pkg/proxy/policy/selector_test.go @@ -7,6 +7,8 @@ import ( "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 +25,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"}) - + req := httptest.NewRequest("GET", "https://example.org/foo", nil) want := "ocis" - got, err := sel(ctx, req) + got, err := sel(req) if got != want { t.Errorf("Expected policy %v got %v", want, got) } @@ -57,7 +62,7 @@ func TestStaticSelector(t *testing.T) { sel = NewStaticSelector(&config.StaticSelectorConf{Policy: "foo"}) want = "foo" - got, err = sel(ctx, req) + got, err = sel(req) if got != want { t.Errorf("Expected policy %v got %v", want, got) } @@ -67,7 +72,7 @@ func TestStaticSelector(t *testing.T) { } } -type testCase struct { +type migrationTestCase struct { AccSvcShouldReturnError bool Claims map[string]interface{} Expected string @@ -79,7 +84,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 +92,13 @@ 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) - got, err := sut(ctx, nr) + got, err := sut(nr) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -103,7 +106,6 @@ func TestMigrationSelector(t *testing.T) { if got != tc.Expected { t.Errorf("Expected Policy %v got %v", tc.Expected, got) } - //}) } } @@ -123,3 +125,78 @@ 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 { + 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) + } + + 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) { + 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) + } + + 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..9193252e13 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" @@ -67,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, @@ -92,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. @@ -110,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 } @@ -213,12 +214,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) } @@ -248,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 }