From 882b7dfd280a4ccb1054aed839e51a5bb2dfad01 Mon Sep 17 00:00:00 2001 From: surtur Date: Sun, 10 Sep 2023 14:12:13 +0200 Subject: [PATCH] go: add more logs on unauthorised access * log details about unauthorised access * return semantically correct 403 (instead of 401) on unauthorised access * allow read-only admin access to "hibp breach details" endpoint --- handlers/handlers.go | 2 ++ handlers/manage-apikeys.go | 27 +++++++++++++++++++++----- handlers/manage-user.go | 28 +++++++++++++++------------ handlers/search-hibp.go | 8 ++++++-- handlers/user-init-password-change.go | 8 ++++++-- handlers/view-hibp.go | 7 ------- 6 files changed, 52 insertions(+), 28 deletions(-) diff --git a/handlers/handlers.go b/handlers/handlers.go index f64651c..f723b31 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -10,6 +10,8 @@ import ( "github.com/labstack/echo/v4" ) +const unauthorisedAccess = "Unauthorised access detected" + func Index() echo.HandlerFunc { return func(c echo.Context) error { defer addHeaders(c) diff --git a/handlers/manage-apikeys.go b/handlers/manage-apikeys.go index ad29599..d837668 100644 --- a/handlers/manage-apikeys.go +++ b/handlers/manage-apikeys.go @@ -13,15 +13,32 @@ func ManageAPIKeys() echo.HandlerFunc { return func(c echo.Context) error { addHeaders(c) - u := c.Get("sessUsr") + u, ok := c.Get("sessUsr").(moduser.User) + if !ok { + return renderErrorPage( + c, + http.StatusUnauthorized, + http.StatusText(http.StatusUnauthorized), + "username was nil", + ) + } + + if !u.IsAdmin { + c.Logger().Debug("this is a restricted endpoint", "endpoint", "/manage/api-keys", "user", u.Username, "isAdmin", u.IsAdmin) + + status := http.StatusForbidden + msg := http.StatusText(status) + + return renderErrorPage( + c, status, msg+": You should not be here", "Restricted endpoint", + ) + } + p := newPage() p.Title = "Manage API Keys" p.Current = "api-keys" - - if _, ok := u.(moduser.User); ok { - p.User = u.(moduser.User) - } + p.User = u if setting.APIKeyHIBP() != "" { p.Data["hibpApiKey"] = setting.APIKeyHIBP() diff --git a/handlers/manage-user.go b/handlers/manage-user.go index 9772543..9fc9a3b 100644 --- a/handlers/manage-user.go +++ b/handlers/manage-user.go @@ -46,9 +46,9 @@ func ManageUsers() echo.HandlerFunc { } if !u.IsAdmin { - c.Logger().Debug("this is a restricted endpoint") + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) - status := http.StatusUnauthorized + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -177,9 +177,9 @@ func CreateUser() echo.HandlerFunc { //nolint:gocognit } if !u.IsAdmin { - c.Logger().Debug("this is a restricted endpoint") + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) - status := http.StatusUnauthorized + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -283,9 +283,9 @@ func ViewUser() echo.HandlerFunc { } if !u.IsAdmin { - c.Logger().Debug("this is a restricted endpoint") + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) - status := http.StatusUnauthorized + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -420,9 +420,9 @@ func EditUser() echo.HandlerFunc { } if !u.IsAdmin { - c.Logger().Debug("this is a restricted endpoint") + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) - status := http.StatusUnauthorized + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -522,9 +522,9 @@ func UpdateUser() echo.HandlerFunc { //nolint:gocognit } if !u.IsAdmin { - c.Logger().Debug("this is a restricted endpoint") + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) - status := http.StatusUnauthorized + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -756,7 +756,9 @@ func DeleteUserConfirmation() echo.HandlerFunc { "username was nil", ) } else if !u.IsAdmin { - status := http.StatusUnauthorized + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -855,7 +857,9 @@ func DeleteUser() echo.HandlerFunc { "username was nil", ) } else if !u.IsAdmin { - status := http.StatusUnauthorized + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( diff --git a/handlers/search-hibp.go b/handlers/search-hibp.go index 13d03a4..f40eec8 100644 --- a/handlers/search-hibp.go +++ b/handlers/search-hibp.go @@ -23,7 +23,9 @@ func GetSearchHIBP() echo.HandlerFunc { if !ok { c.Logger().Warnf("Error getting user from session cookie") } else if u.IsAdmin { - status := http.StatusUnauthorized + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -102,7 +104,9 @@ func SearchHIBP() echo.HandlerFunc { // ) c.Logger().Warnf("Error getting user from session cookie") } else if u.IsAdmin { - status := http.StatusUnauthorized + log.Warn(unauthorisedAccess, "endpoint", c.Path(), "method", c.Request().Method, "user", u.Username, "isAdmin", u.IsAdmin) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( diff --git a/handlers/user-init-password-change.go b/handlers/user-init-password-change.go index 6519d6c..94bc891 100644 --- a/handlers/user-init-password-change.go +++ b/handlers/user-init-password-change.go @@ -26,7 +26,9 @@ func InitialPasswordChangePost() echo.HandlerFunc { "Username was nil", ) } else if u.IsAdmin { - status := http.StatusUnauthorized + log.Warn("this is a restricted endpoint", "endpoint", "/user/initial-password-change", "user", u.Username, "isAdmin", u.IsAdmin, "route name", c.Path()) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( @@ -145,7 +147,9 @@ func InitialPasswordChange() echo.HandlerFunc { "Username was nil", ) } else if u.IsAdmin { - status := http.StatusUnauthorized + log.Warn("this is a restricted endpoint", "endpoint", "/user/initial-password-change", "user", u.Username, "isAdmin", u.IsAdmin, "route name", c.Path()) + + status := http.StatusForbidden msg := http.StatusText(status) return renderErrorPage( diff --git a/handlers/view-hibp.go b/handlers/view-hibp.go index 5058f0d..54dbfcc 100644 --- a/handlers/view-hibp.go +++ b/handlers/view-hibp.go @@ -19,13 +19,6 @@ func ViewHIBP() echo.HandlerFunc { u, ok := c.Get("sessUsr").(moduser.User) if !ok { c.Logger().Warnf("Error getting user from session cookie") - } else if u.IsAdmin { - status := http.StatusUnauthorized - msg := http.StatusText(status) - - return renderErrorPage( - c, status, msg+": You should not be here", "This endpoint is for users only", - ) } h := new(hibpBreachDetail)