From a91c0c5cfeaf6855c6738aa80d68b9d8e0f21978 Mon Sep 17 00:00:00 2001 From: peusebiu Date: Thu, 12 Oct 2023 16:37:55 +0300 Subject: [PATCH] fix(authn): create sessions only if UI header value is supplied (#1919) Signed-off-by: Petu Eusebiu --- pkg/api/authn.go | 15 +- pkg/api/controller_test.go | 461 ++++++++++++++++++++++--------------- 2 files changed, 279 insertions(+), 197 deletions(-) diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 741db65b7..8a8b42849 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -127,9 +127,11 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce userAc.AddGroups(groups) userAc.SaveOnRequest(request) - // saved logged session - if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { - return false, err + // saved logged session only if the request comes from web (has UI session header value) + if hasSessionHeader(request) { + if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { + return false, err + } } // we have already populated the request context with userAc @@ -163,8 +165,11 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce userAc.AddGroups(groups) userAc.SaveOnRequest(request) - if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { - return false, err + // saved logged session only if the request comes from web (has UI session header value) + if hasSessionHeader(request) { + if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil { + return false, err + } } // we have already populated the request context with userAc diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index a7c0bb726..05b53f125 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -2686,238 +2686,297 @@ func TestOpenIDMiddleware(t *testing.T) { for _, testcase := range testCases { t.Run(testcase.testCaseName, func(t *testing.T) { - dir := t.TempDir() - - ctlr.Config.Storage.RootDirectory = dir - ctlr.Config.HTTP.ExternalURL = testcase.externalURL - ctlr.Config.HTTP.Address = testcase.address - cm := test.NewControllerManager(ctlr) + Convey("make controller", t, func() { + dir := t.TempDir() - cm.StartServer() - defer cm.StopServer() - test.WaitTillServerReady(baseURL) - - Convey("browser client requests", t, func() { - Convey("login with no provider supplied", func() { - client := resty.New() - client.SetRedirectPolicy(test.CustomRedirectPolicy(20)) - // first login user - resp, err := client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - SetQueryParam("provider", "unknown"). - Get(baseURL + constants.LoginPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) - }) + ctlr.Config.Storage.RootDirectory = dir + ctlr.Config.HTTP.ExternalURL = testcase.externalURL + ctlr.Config.HTTP.Address = testcase.address + cm := test.NewControllerManager(ctlr) - Convey("login with openid and get catalog with session", func() { - client := resty.New() - client.SetRedirectPolicy(test.CustomRedirectPolicy(20)) + cm.StartServer() + defer cm.StopServer() + test.WaitTillServerReady(baseURL) - Convey("with callback_ui value provided", func() { + Convey("browser client requests", func() { + Convey("login with no provider supplied", func() { + client := resty.New() + client.SetRedirectPolicy(test.CustomRedirectPolicy(20)) // first login user resp, err := client.R(). SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - SetQueryParam("provider", "oidc"). - SetQueryParam("callback_ui", baseURL+"/v2/"). + SetQueryParam("provider", "unknown"). Get(baseURL + constants.LoginPath) So(err, ShouldBeNil) So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest) + }) + + //nolint: dupl + Convey("make sure sessions are not used without UI header value", func() { + sessionsNo, err := getNumberOfSessions(conf.Storage.RootDirectory) + So(err, ShouldBeNil) + So(sessionsNo, ShouldEqual, 0) + + client := resty.New() + + // without header should not create session + resp, err := client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + sessionsNo, err = getNumberOfSessions(conf.Storage.RootDirectory) + So(err, ShouldBeNil) + So(sessionsNo, ShouldEqual, 0) + + client.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue) + + resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + sessionsNo, err = getNumberOfSessions(conf.Storage.RootDirectory) + So(err, ShouldBeNil) + So(sessionsNo, ShouldEqual, 1) + + // set cookies + client.SetCookies(resp.Cookies()) + + // should get same cookie + resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + sessionsNo, err = getNumberOfSessions(conf.Storage.RootDirectory) + So(err, ShouldBeNil) + So(sessionsNo, ShouldEqual, 1) + + resp, err = client.R(). + SetBasicAuth(htpasswdUsername, passphrase). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + client.SetCookies(resp.Cookies()) + + // call endpoint with session, without credentials, (added to client after previous request) + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + sessionsNo, err = getNumberOfSessions(conf.Storage.RootDirectory) + So(err, ShouldBeNil) + So(sessionsNo, ShouldEqual, 1) }) - // first login user - resp, err := client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - SetQueryParam("provider", "oidc"). - Get(baseURL + constants.LoginPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + Convey("login with openid and get catalog with session", func() { + client := resty.New() + client.SetRedirectPolicy(test.CustomRedirectPolicy(20)) + client.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue) + + Convey("with callback_ui value provided", func() { + // first login user + resp, err := client.R(). + SetQueryParam("provider", "oidc"). + SetQueryParam("callback_ui", baseURL+"/v2/"). + Get(baseURL + constants.LoginPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + }) - client.SetCookies(resp.Cookies()) + // first login user + resp, err := client.R(). + SetQueryParam("provider", "oidc"). + Get(baseURL + constants.LoginPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) - // call endpoint with session (added to client after previous request) - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + client.SetCookies(resp.Cookies()) - // logout with options method for coverage - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Options(baseURL + constants.LogoutPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) + // call endpoint with session (added to client after previous request) + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // logout user - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Post(baseURL + constants.LogoutPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // logout with options method for coverage + resp, err = client.R(). + Options(baseURL + constants.LogoutPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) - // calling endpoint should fail with unauthorized access - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - }) + // logout user + resp, err = client.R(). + Post(baseURL + constants.LogoutPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - //nolint: dupl - Convey("login with basic auth(htpasswd) and get catalog with session", func() { - client := resty.New() + // calling endpoint should fail with unauthorized access + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) - // without creds, should get access error - resp, err := client.R().Get(baseURL + "/v2/") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - var e apiErr.Error - err = json.Unmarshal(resp.Body(), &e) - So(err, ShouldBeNil) + //nolint: dupl + Convey("login with basic auth(htpasswd) and get catalog with session", func() { + client := resty.New() + client.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue) - // first login user - // with creds, should get expected status code - resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // without creds, should get access error + resp, err := client.R().Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + var e apiErr.Error + err = json.Unmarshal(resp.Body(), &e) + So(err, ShouldBeNil) - resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // first login user + // with creds, should get expected status code + resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - resp, err = client.R(). - SetBasicAuth(htpasswdUsername, passphrase). - Get(baseURL + constants.FullMgmt) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + resp, err = client.R().SetBasicAuth(htpasswdUsername, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - client.SetCookies(resp.Cookies()) + resp, err = client.R(). + SetBasicAuth(htpasswdUsername, passphrase). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // call endpoint with session, without credentials, (added to client after previous request) - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + client.SetCookies(resp.Cookies()) - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + constants.FullMgmt) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // call endpoint with session, without credentials, (added to client after previous request) + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // logout user - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Post(baseURL + constants.LogoutPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + resp, err = client.R(). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // calling endpoint should fail with unauthorized access - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - }) + // logout user + resp, err = client.R(). + Post(baseURL + constants.LogoutPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - //nolint: dupl - Convey("login with ldap and get catalog", func() { - client := resty.New() + // calling endpoint should fail with unauthorized access + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) - // without creds, should get access error - resp, err := client.R().Get(baseURL + "/v2/") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - var e apiErr.Error - err = json.Unmarshal(resp.Body(), &e) - So(err, ShouldBeNil) + //nolint: dupl + Convey("login with ldap and get catalog", func() { + client := resty.New() + client.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue) - // first login user - // with creds, should get expected status code - resp, err = client.R().SetBasicAuth(username, passphrase).Get(baseURL) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // without creds, should get access error + resp, err := client.R().Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + var e apiErr.Error + err = json.Unmarshal(resp.Body(), &e) + So(err, ShouldBeNil) - resp, err = client.R().SetBasicAuth(username, passphrase).Get(baseURL + "/v2/") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // first login user + // with creds, should get expected status code + resp, err = client.R().SetBasicAuth(username, passphrase).Get(baseURL) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - resp, err = client.R(). - SetBasicAuth(username, passphrase). - Get(baseURL + constants.FullMgmt) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + resp, err = client.R().SetBasicAuth(username, passphrase).Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - client.SetCookies(resp.Cookies()) + resp, err = client.R(). + SetBasicAuth(username, passphrase). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // call endpoint with session, without credentials, (added to client after previous request) - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + client.SetCookies(resp.Cookies()) - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + constants.FullMgmt) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // call endpoint with session, without credentials, (added to client after previous request) + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // logout user - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Post(baseURL + constants.LogoutPath) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + resp, err = client.R(). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - // calling endpoint should fail with unauthorized access - resp, err = client.R(). - SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) - }) + // logout user + resp, err = client.R(). + Post(baseURL + constants.LogoutPath) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) - Convey("unauthenticated catalog request", func() { - client := resty.New() + // calling endpoint should fail with unauthorized access + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) - // mgmt should work both unauthenticated and authenticated - resp, err := client.R(). - Get(baseURL + constants.FullMgmt) - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusOK) + Convey("unauthenticated catalog request", func() { + client := resty.New() - // call endpoint without session - resp, err = client.R(). - Get(baseURL + "/v2/_catalog") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + // mgmt should work both unauthenticated and authenticated + resp, err := client.R(). + Get(baseURL + constants.FullMgmt) + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + + // call endpoint without session + resp, err = client.R(). + Get(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized) + }) }) }) }) @@ -3273,6 +3332,7 @@ func TestAuthnSessionErrors(t *testing.T) { }() client := resty.New() + client.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue) // first htpasswd saveSessionLoggedUser() error resp, err := client.R(). @@ -9764,3 +9824,20 @@ func getEmptyImageConfig() ([]byte, godigest.Digest) { return configBlobContent, configBlobDigestRaw } + +func getNumberOfSessions(rootDir string) (int, error) { + rootDirContents, err := os.ReadDir(path.Join(rootDir, "_sessions")) + if err != nil { + return -1, err + } + + sessionsNo := 0 + + for _, file := range rootDirContents { + if !file.IsDir() && strings.HasPrefix(file.Name(), "session_") { + sessionsNo += 1 + } + } + + return sessionsNo, nil +}