From 3d4ee197890b4c39be6ef3de9b05b98237d9feb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 15 Sep 2021 11:56:01 +0200 Subject: [PATCH] allow overriding the cookie based route by claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../proxy-policy-claim-precedence.md | 5 +++ proxy/pkg/proxy/policy/selector.go | 13 ++++---- proxy/pkg/proxy/policy/selector_test.go | 33 +++++++++++-------- 3 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/proxy-policy-claim-precedence.md diff --git a/changelog/unreleased/proxy-policy-claim-precedence.md b/changelog/unreleased/proxy-policy-claim-precedence.md new file mode 100644 index 0000000000..6c0317125c --- /dev/null +++ b/changelog/unreleased/proxy-policy-claim-precedence.md @@ -0,0 +1,5 @@ +Enhancement: allow overriding the cookie based route by claim + +When determining the routing policy we now let the claim override the cookie so that users are routed to the correct backend after login. + +https://github.com/owncloud/ocis/pull/2508 \ No newline at end of file diff --git a/proxy/pkg/proxy/policy/selector.go b/proxy/pkg/proxy/policy/selector.go index 9a9ac5aafa..c877eb083b 100644 --- a/proxy/pkg/proxy/policy/selector.go +++ b/proxy/pkg/proxy/policy/selector.go @@ -164,13 +164,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(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 + // first, 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? @@ -179,6 +174,12 @@ func NewClaimsSelector(cfg *config.ClaimsSelectorConf) Selector { return cfg.DefaultPolicy, nil } + // use cookie if provided + selectorCookie, err := r.Cookie(cfg.SelectorCookieName) + if err == nil { + return selectorCookie.Value, nil + } + return cfg.UnauthenticatedPolicy, nil } } diff --git a/proxy/pkg/proxy/policy/selector_test.go b/proxy/pkg/proxy/policy/selector_test.go index 96da63e695..101eaae73e 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" "net/http/httptest" "testing" @@ -129,6 +130,7 @@ func mockAccSvc(retErr bool) proto.AccountsService { type testCase struct { Name string Context context.Context + Cookie *http.Cookie Expected string } @@ -139,12 +141,17 @@ func TestClaimsSelector(t *testing.T) { }) 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"}, + {"unatuhenticated", context.Background(), nil, "unauthenticated"}, + {"default", oidc.NewContext(context.Background(), map[string]interface{}{oidc.OcisRoutingPolicy: ""}), nil, "default"}, + {"claim-value", oidc.NewContext(context.Background(), map[string]interface{}{oidc.OcisRoutingPolicy: "ocis.routing.policy-value"}), nil, "ocis.routing.policy-value"}, + {"cookie-only", context.Background(), &http.Cookie{Name: SelectorCookieName, Value: "cookie"}, "cookie"}, + {"claim-can-override-cookie", oidc.NewContext(context.Background(), map[string]interface{}{oidc.OcisRoutingPolicy: "ocis.routing.policy-value"}), &http.Cookie{Name: SelectorCookieName, Value: "cookie"}, "ocis.routing.policy-value"}, } for _, tc := range tests { r := httptest.NewRequest("GET", "https://example.com", nil) + if tc.Cookie != nil { + r.AddCookie(tc.Cookie) + } nr := r.WithContext(tc.Context) got, err := sel(nr) if err != nil { @@ -172,16 +179,16 @@ func TestRegexSelector(t *testing.T) { }) var tests = []testCase{ - {"unauthenticated", context.Background(), "unauthenticated"}, - {"default", revactx.ContextSetUser(context.Background(), &userv1beta1.User{}), "default"}, - {"mail-ocis", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "marie@example.org"}), "ocis"}, - {"mail-oc10", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "einstein@example.org"}), "oc10"}, - {"username-einstein", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "einstein"}), "ocis"}, - {"username-feynman", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "feynman"}), "ocis"}, - {"username-marie", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "marie"}), "oc10"}, - {"id-nil", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{}}), "default"}, - {"id-1", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51"}}), "ocis"}, - {"id-2", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}}), "oc10"}, + {"unauthenticated", context.Background(), nil, "unauthenticated"}, + {"default", revactx.ContextSetUser(context.Background(), &userv1beta1.User{}), nil, "default"}, + {"mail-ocis", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "marie@example.org"}), nil, "ocis"}, + {"mail-oc10", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Mail: "einstein@example.org"}), nil, "oc10"}, + {"username-einstein", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "einstein"}), nil, "ocis"}, + {"username-feynman", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "feynman"}), nil, "ocis"}, + {"username-marie", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Username: "marie"}), nil, "oc10"}, + {"id-nil", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{}}), nil, "default"}, + {"id-1", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51"}}), nil, "ocis"}, + {"id-2", revactx.ContextSetUser(context.Background(), &userv1beta1.User{Id: &userv1beta1.UserId{OpaqueId: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}}), nil, "oc10"}, } for _, tc := range tests {