From 16f40310855880842b641e246cdecdfa5a0f5baa Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 29 Mar 2025 20:37:24 +0100 Subject: [PATCH] oidc: allow login if the password method is disabled isLoggedInWithOIDC returns false before login so we need to add a specific check Fixes #1879 Signed-off-by: Nicola Murino --- internal/httpd/api_http_user.go | 2 +- internal/httpd/api_utils.go | 6 +++--- internal/httpd/middleware.go | 2 +- internal/httpd/oidc.go | 4 ++-- internal/httpd/oidc_test.go | 3 ++- internal/httpd/server.go | 8 ++++---- internal/httpd/webclient.go | 10 +++++----- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/httpd/api_http_user.go b/internal/httpd/api_http_user.go index 26bc0063..7ba466ba 100644 --- a/internal/httpd/api_http_user.go +++ b/internal/httpd/api_http_user.go @@ -49,7 +49,7 @@ func getUserConnection(w http.ResponseWriter, r *http.Request) (*Connection, err connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%v_%v", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { sendAPIResponse(w, r, err, http.StatusText(http.StatusForbidden), http.StatusForbidden) return nil, err } diff --git a/internal/httpd/api_utils.go b/internal/httpd/api_utils.go index aabea0da..00afa00a 100644 --- a/internal/httpd/api_utils.go +++ b/internal/httpd/api_utils.go @@ -719,7 +719,7 @@ func updateLoginMetrics(user *dataprovider.User, loginMethod, ip string, err err dataprovider.ExecutePostLoginHook(user, loginMethod, ip, protocol, err) } -func checkHTTPClientUser(user *dataprovider.User, r *http.Request, connectionID string, checkSessions bool) error { +func checkHTTPClientUser(user *dataprovider.User, r *http.Request, connectionID string, checkSessions, isOIDCLogin bool) error { if util.Contains(user.Filters.DeniedProtocols, common.ProtocolHTTP) { logger.Info(logSender, connectionID, "cannot login user %q, protocol HTTP is not allowed", user.Username) return util.NewI18nError( @@ -727,7 +727,7 @@ func checkHTTPClientUser(user *dataprovider.User, r *http.Request, connectionID util.I18nErrorProtocolForbidden, ) } - if !isLoggedInWithOIDC(r) && !user.IsLoginMethodAllowed(dataprovider.LoginMethodPassword, common.ProtocolHTTP) { + if !isLoggedInWithOIDC(r) && !isOIDCLogin && !user.IsLoginMethodAllowed(dataprovider.LoginMethodPassword, common.ProtocolHTTP) { logger.Info(logSender, connectionID, "cannot login user %q, password login method is not allowed", user.Username) return util.NewI18nError( fmt.Errorf("login method password is not allowed for user %q", user.Username), @@ -771,7 +771,7 @@ func getActiveUser(username string, r *http.Request) (dataprovider.User, error) if err := user.CheckLoginConditions(); err != nil { return user, util.NewRecordNotFoundError(fmt.Sprintf("user %q cannot login: %v", username, err)) } - if err := checkHTTPClientUser(&user, r, xid.New().String(), false); err != nil { + if err := checkHTTPClientUser(&user, r, xid.New().String(), false, false); err != nil { return user, util.NewRecordNotFoundError(fmt.Sprintf("user %q cannot login: %v", username, err)) } return user, nil diff --git a/internal/httpd/middleware.go b/internal/httpd/middleware.go index 8f18c8aa..822b0c36 100644 --- a/internal/httpd/middleware.go +++ b/internal/httpd/middleware.go @@ -545,7 +545,7 @@ func authenticateUserWithAPIKey(username, keyID string, tokenAuth *jwtauth.JWTAu return err } connectionID := fmt.Sprintf("%v_%v", protocol, xid.New().String()) - if err := checkHTTPClientUser(&user, r, connectionID, true); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, true, false); err != nil { updateLoginMetrics(&user, dataprovider.LoginMethodPassword, ipAddr, err) return err } diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index 93068387..1065b8e8 100644 --- a/internal/httpd/oidc.go +++ b/internal/httpd/oidc.go @@ -392,7 +392,7 @@ func (t *oidcToken) refreshUser(r *http.Request) error { if err := user.CheckLoginConditions(); err != nil { return err } - if err := checkHTTPClientUser(&user, r, xid.New().String(), true); err != nil { + if err := checkHTTPClientUser(&user, r, xid.New().String(), true, false); err != nil { return err } t.Permissions = user.Filters.WebClient @@ -453,7 +453,7 @@ func (t *oidcToken) getUser(r *http.Request) error { return err } connectionID := fmt.Sprintf("%s_%s", common.ProtocolOIDC, xid.New().String()) - if err := checkHTTPClientUser(user, r, connectionID, true); err != nil { + if err := checkHTTPClientUser(user, r, connectionID, true, true); err != nil { updateLoginMetrics(user, dataprovider.LoginMethodIDP, ipAddr, err) return err } diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go index 636c9cfa..c3ae01d5 100644 --- a/internal/httpd/oidc_test.go +++ b/internal/httpd/oidc_test.go @@ -903,7 +903,8 @@ func TestOIDCToken(t *testing.T) { }, Filters: dataprovider.UserFilters{ BaseUserFilters: sdk.BaseUserFilters{ - DeniedProtocols: []string{common.ProtocolHTTP}, + DeniedProtocols: []string{common.ProtocolHTTP}, + DeniedLoginMethods: []string{dataprovider.LoginMethodPassword}, }, }, } diff --git a/internal/httpd/server.go b/internal/httpd/server.go index f50d7d27..0615f82c 100644 --- a/internal/httpd/server.go +++ b/internal/httpd/server.go @@ -269,7 +269,7 @@ func (s *httpdServer) handleWebClientLoginPost(w http.ResponseWriter, r *http.Re return } connectionID := fmt.Sprintf("%v_%v", protocol, xid.New().String()) - if err := checkHTTPClientUser(&user, r, connectionID, true); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, true, false); err != nil { updateLoginMetrics(&user, dataprovider.LoginMethodPassword, ipAddr, err) s.renderClientLoginPage(w, r, util.NewI18nError(err, util.I18nError403Message), ipAddr) return @@ -308,7 +308,7 @@ func (s *httpdServer) handleWebClientPasswordResetPost(w http.ResponseWriter, r return } connectionID := fmt.Sprintf("%v_%v", getProtocolFromRequest(r), xid.New().String()) - if err := checkHTTPClientUser(user, r, connectionID, true); err != nil { + if err := checkHTTPClientUser(user, r, connectionID, true, false); err != nil { s.renderClientResetPwdPage(w, r, util.NewI18nError(err, util.I18nErrorLoginAfterReset), ipAddr) return } @@ -869,7 +869,7 @@ func (s *httpdServer) getUserToken(w http.ResponseWriter, r *http.Request) { return } connectionID := fmt.Sprintf("%v_%v", protocol, xid.New().String()) - if err := checkHTTPClientUser(&user, r, connectionID, true); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, true, false); err != nil { updateLoginMetrics(&user, dataprovider.LoginMethodPassword, ipAddr, err) sendAPIResponse(w, r, err, http.StatusText(http.StatusForbidden), http.StatusForbidden) return @@ -1042,7 +1042,7 @@ func (s *httpdServer) refreshClientToken(w http.ResponseWriter, r *http.Request, logger.Debug(logSender, "", "unable to refresh cookie for user %q: %v", user.Username, err) return } - if err := checkHTTPClientUser(&user, r, xid.New().String(), true); err != nil { + if err := checkHTTPClientUser(&user, r, xid.New().String(), true, false); err != nil { logger.Debug(logSender, "", "unable to refresh cookie for user %q: %v", user.Username, err) return } diff --git a/internal/httpd/webclient.go b/internal/httpd/webclient.go index 4a5f11be..17eb89aa 100644 --- a/internal/httpd/webclient.go +++ b/internal/httpd/webclient.go @@ -871,7 +871,7 @@ func (s *httpdServer) handleWebClientDownloadZip(w http.ResponseWriter, r *http. connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%v_%v", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { s.renderClientForbiddenPage(w, r, err) return } @@ -1160,7 +1160,7 @@ func (s *httpdServer) handleClientGetDirContents(w http.ResponseWriter, r *http. connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%s_%s", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { sendAPIResponse(w, r, err, getI18NErrorString(err, util.I18nErrorDirList403), http.StatusForbidden) return } @@ -1249,7 +1249,7 @@ func (s *httpdServer) handleClientGetFiles(w http.ResponseWriter, r *http.Reques connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%v_%v", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { s.renderClientForbiddenPage(w, r, err) return } @@ -1310,7 +1310,7 @@ func (s *httpdServer) handleClientEditFile(w http.ResponseWriter, r *http.Reques connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%v_%v", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { s.renderClientForbiddenPage(w, r, err) return } @@ -1797,7 +1797,7 @@ func (s *httpdServer) handleClientGetPDF(w http.ResponseWriter, r *http.Request) connID := xid.New().String() protocol := getProtocolFromRequest(r) connectionID := fmt.Sprintf("%v_%v", protocol, connID) - if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil { + if err := checkHTTPClientUser(&user, r, connectionID, false, false); err != nil { s.renderClientForbiddenPage(w, r, err) return }