From 355574231643ff105b6ffc74f726c181491f3d85 Mon Sep 17 00:00:00 2001 From: MrMelon54 Date: Fri, 31 May 2024 14:57:54 +0100 Subject: [PATCH] Redesign token authentication --- server/auth.go | 49 ++++++++++++++++---------- server/auth_test.go | 6 ++-- server/db.go | 21 ++++++++--- server/edit.go | 4 +-- server/home.go | 8 ++--- server/login.go | 80 ++++++++++++++++++++++++++++++++++++------ server/manage-apps.go | 12 +++---- server/manage-users.go | 6 ++-- server/oauth.go | 10 +++--- server/otp.go | 10 +++--- server/server.go | 9 ++++- 11 files changed, 152 insertions(+), 63 deletions(-) diff --git a/server/auth.go b/server/auth.go index a57856b..acdd14f 100644 --- a/server/auth.go +++ b/server/auth.go @@ -1,8 +1,7 @@ package server import ( - "github.com/1f349/mjwt" - "github.com/1f349/mjwt/auth" + "errors" "github.com/1f349/tulip/database" "github.com/1f349/tulip/database/types" "github.com/julienschmidt/httprouter" @@ -14,7 +13,7 @@ import ( type UserHandler func(rw http.ResponseWriter, req *http.Request, params httprouter.Params, auth UserAuth) type UserAuth struct { - ID string + Subject string NeedOtp bool } @@ -26,14 +25,16 @@ func (u UserAuth) NextFlowUrl(origin *url.URL) *url.URL { } func (u UserAuth) IsGuest() bool { - return u.ID == "" + return u.Subject == "" } +var ErrAuthHttpError = errors.New("auth http error") + func (h *HttpServer) RequireAdminAuthentication(next UserHandler) httprouter.Handle { return h.RequireAuthentication(func(rw http.ResponseWriter, req *http.Request, params httprouter.Params, auth UserAuth) { var role types.UserRole if h.DbTx(rw, func(tx *database.Queries) (err error) { - role, err = tx.GetUserRole(req.Context(), auth.ID) + role, err = tx.GetUserRole(req.Context(), auth.Subject) return }) { return @@ -59,27 +60,37 @@ func (h *HttpServer) RequireAuthentication(next UserHandler) httprouter.Handle { func (h *HttpServer) OptionalAuthentication(flowPart bool, next UserHandler) httprouter.Handle { return func(rw http.ResponseWriter, req *http.Request, params httprouter.Params) { - authData, err := h.internalAuthenticationHandler(req) - if err == nil { - if n := authData.NextFlowUrl(req.URL); n != nil && !flowPart { - http.Redirect(rw, req, n.String(), http.StatusFound) - return + authData, err := h.internalAuthenticationHandler(rw, req) + if err != nil { + if !errors.Is(err, ErrAuthHttpError) { + http.Error(rw, err.Error(), http.StatusInternalServerError) } + return + } + if n := authData.NextFlowUrl(req.URL); n != nil && !flowPart { + http.Redirect(rw, req, n.String(), http.StatusFound) + return } next(rw, req, params, authData) } } -func (h *HttpServer) internalAuthenticationHandler(req *http.Request) (UserAuth, error) { - if loginCookie, err := req.Cookie("tulip-login-data"); err == nil { - _, b, err := mjwt.ExtractClaims[auth.AccessTokenClaims](h.signingKey, loginCookie.Value) - if err != nil { - return UserAuth{}, err - } - return UserAuth{ID: b.Subject, NeedOtp: b.Claims.Perms.Has("needs-otp")}, nil +func (h *HttpServer) internalAuthenticationHandler(rw http.ResponseWriter, req *http.Request) (UserAuth, error) { + http.SetCookie(rw, &http.Cookie{ + Name: "tulip-login-data", + Path: "/", + MaxAge: -1, + Secure: true, + SameSite: http.SameSiteLaxMode, + }) + + var u UserAuth + err := h.readLoginAccessCookie(rw, req, &u) + if err != nil { + // not logged in + return UserAuth{}, nil } - // not logged in - return UserAuth{}, nil + return u, nil } func PrepareRedirectUrl(targetPath string, origin *url.URL) *url.URL { diff --git a/server/auth_test.go b/server/auth_test.go index bb6356d..87a73ef 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -21,7 +21,7 @@ func TestUserAuth_NextFlowUrl(t *testing.T) { func TestUserAuth_IsGuest(t *testing.T) { var u UserAuth assert.True(t, u.IsGuest()) - u.ID = uuid.NewString() + u.Subject = uuid.NewString() assert.False(t, u.IsGuest()) } @@ -47,10 +47,10 @@ func TestOptionalAuthentication(t *testing.T) { h := &HttpServer{} req, err := http.NewRequest(http.MethodGet, "https://example.com/hello", nil) assert.NoError(t, err) - auth, err := h.internalAuthenticationHandler(req) + auth, err := h.internalAuthenticationHandler(nil, req) assert.NoError(t, err) assert.True(t, auth.IsGuest()) - auth.ID = "567" + auth.Subject = "567" } func TestPrepareRedirectUrl(t *testing.T) { diff --git a/server/db.go b/server/db.go index 9f5f5f8..104fa08 100644 --- a/server/db.go +++ b/server/db.go @@ -1,22 +1,33 @@ package server import ( + "errors" "github.com/1f349/tulip/database" "github.com/1f349/tulip/logger" "net/http" ) +var ErrDatabaseActionFailed = errors.New("database action failed") + // DbTx wraps a database transaction with http error messages and a simple action // function. If the action function returns an error the transaction will be // rolled back. If there is no error then the transaction is committed. -func (h *HttpServer) DbTx(rw http.ResponseWriter, action func(db *database.Queries) error) bool { - err := action(h.db) - if err != nil { +func (h *HttpServer) DbTx(rw http.ResponseWriter, action func(tx *database.Queries) error) bool { + logger.Logger.Helper() + if h.DbTxError(action) != nil { http.Error(rw, "Database error", http.StatusInternalServerError) - logger.Logger.Helper() - logger.Logger.Warn("Database action error", "err", err) return true } return false } + +func (h *HttpServer) DbTxError(action func(tx *database.Queries) error) error { + logger.Logger.Helper() + err := action(h.db) + if err != nil { + logger.Logger.Warn("Database action error", "err", err) + return ErrDatabaseActionFailed + } + return nil +} diff --git a/server/edit.go b/server/edit.go index 2b2ac63..663f208 100644 --- a/server/edit.go +++ b/server/edit.go @@ -16,7 +16,7 @@ func (h *HttpServer) EditGet(rw http.ResponseWriter, req *http.Request, _ httpro if h.DbTx(rw, func(tx *database.Queries) error { var err error - user, err = tx.GetUser(req.Context(), auth.ID) + user, err = tx.GetUser(req.Context(), auth.Subject) if err != nil { return fmt.Errorf("failed to read user data: %w", err) } @@ -73,7 +73,7 @@ func (h *HttpServer) EditPost(rw http.ResponseWriter, req *http.Request, _ httpr Zoneinfo: patch.ZoneInfo, Locale: patch.Locale, UpdatedAt: time.Now(), - Subject: auth.ID, + Subject: auth.Subject, } if h.DbTx(rw, func(tx *database.Queries) error { if err := tx.ModifyUser(req.Context(), m); err != nil { diff --git a/server/home.go b/server/home.go index a5ac337..ea2b406 100644 --- a/server/home.go +++ b/server/home.go @@ -34,15 +34,15 @@ func (h *HttpServer) Home(rw http.ResponseWriter, req *http.Request, _ httproute var userRole types.UserRole var hasTwoFactor bool if h.DbTx(rw, func(tx *database.Queries) (err error) { - userWithName, err = tx.GetUserDisplayName(req.Context(), auth.ID) + userWithName, err = tx.GetUserDisplayName(req.Context(), auth.Subject) if err != nil { return fmt.Errorf("failed to get user display name: %w", err) } - hasTwoFactor, err = tx.HasOtp(req.Context(), auth.ID) + hasTwoFactor, err = tx.HasOtp(req.Context(), auth.Subject) if err != nil { return fmt.Errorf("failed to get user two factor state: %w", err) } - userRole, err = tx.GetUserRole(req.Context(), auth.ID) + userRole, err = tx.GetUserRole(req.Context(), auth.Subject) if err != nil { return fmt.Errorf("failed to get user role: %w", err) } @@ -53,7 +53,7 @@ func (h *HttpServer) Home(rw http.ResponseWriter, req *http.Request, _ httproute pages.RenderPageTemplate(rw, "index", map[string]any{ "ServiceName": h.conf.ServiceName, "Auth": auth, - "User": database.User{Subject: auth.ID, Name: userWithName, Role: userRole}, + "User": database.User{Subject: auth.Subject, Name: userWithName, Role: userRole}, "Nonce": lNonce, "OtpEnabled": hasTwoFactor, "IsAdmin": userRole == types.RoleAdmin, diff --git a/server/login.go b/server/login.go index 7fb5451..0d15629 100644 --- a/server/login.go +++ b/server/login.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "fmt" + "github.com/1f349/mjwt" "github.com/1f349/mjwt/auth" "github.com/1f349/mjwt/claims" "github.com/1f349/tulip/database" @@ -35,8 +36,8 @@ func getUserLoginName(req *http.Request) string { return originUrl.Query().Get("login_name") } -func (h *HttpServer) LoginGet(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, auth UserAuth) { - if !auth.IsGuest() { +func (h *HttpServer) LoginGet(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, userAuth UserAuth) { + if !userAuth.IsGuest() { h.SafeRedirect(rw, req) return } @@ -53,7 +54,7 @@ func (h *HttpServer) LoginGet(rw http.ResponseWriter, req *http.Request, _ httpr }) } -func (h *HttpServer) LoginPost(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, auth UserAuth) { +func (h *HttpServer) LoginPost(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, userAuth UserAuth) { un := req.FormValue("username") pw := req.FormValue("password") @@ -121,12 +122,13 @@ func (h *HttpServer) LoginPost(rw http.ResponseWriter, req *http.Request, _ http } // only continues if the above tx succeeds - auth = UserAuth{ - ID: userInfo.Subject, + userAuth = UserAuth{ + Subject: userInfo.Subject, NeedOtp: hasOtp, } - if h.setLoginDataCookie(rw, auth) { + if h.setLoginDataCookie(rw, userAuth) { + http.Error(rw, "Failed to save login cookie", http.StatusInternalServerError) return } @@ -144,29 +146,87 @@ func (h *HttpServer) LoginPost(rw http.ResponseWriter, req *http.Request, _ http h.SafeRedirect(rw, req) } -const oneYear = 365 * 24 * time.Hour +const twelveHours = 12 * time.Hour +const oneMonth = 30 * 24 * time.Hour func (h *HttpServer) setLoginDataCookie(rw http.ResponseWriter, authData UserAuth) bool { ps := claims.NewPermStorage() if authData.NeedOtp { ps.Set("needs-otp") } - gen, err := h.signingKey.GenerateJwt(authData.ID, uuid.NewString(), jwt.ClaimStrings{h.conf.BaseUrl}, oneYear, auth.AccessTokenClaims{Perms: ps}) + accId := uuid.NewString() + gen, err := h.signingKey.GenerateJwt(authData.Subject, accId, jwt.ClaimStrings{h.conf.BaseUrl}, twelveHours, auth.AccessTokenClaims{Perms: ps}) + if err != nil { + http.Error(rw, "Failed to generate cookie token", http.StatusInternalServerError) + return true + } + ref, err := h.signingKey.GenerateJwt(authData.Subject, uuid.NewString(), jwt.ClaimStrings{h.conf.BaseUrl}, oneMonth, auth.RefreshTokenClaims{AccessTokenId: accId}) if err != nil { http.Error(rw, "Failed to generate cookie token", http.StatusInternalServerError) return true } http.SetCookie(rw, &http.Cookie{ - Name: "tulip-login-data", + Name: "tulip-login-access", Value: gen, Path: "/", - Expires: time.Now().AddDate(1, 0, 0), Secure: true, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) + http.SetCookie(rw, &http.Cookie{ + Name: "tulip-login-refresh", + Value: ref, + Path: "/", + Expires: time.Now().AddDate(0, 0, 32), + Secure: true, + HttpOnly: true, SameSite: http.SameSiteLaxMode, }) return false } +func readJwtCookie[T mjwt.Claims](req *http.Request, cookieName string, signingKey mjwt.Verifier) (mjwt.BaseTypeClaims[T], error) { + loginCookie, err := req.Cookie(cookieName) + if err != nil { + return mjwt.BaseTypeClaims[T]{}, err + } + _, b, err := mjwt.ExtractClaims[T](signingKey, loginCookie.Value) + if err != nil { + return mjwt.BaseTypeClaims[T]{}, err + } + return b, nil +} + +func (h *HttpServer) readLoginAccessCookie(rw http.ResponseWriter, req *http.Request, u *UserAuth) error { + loginData, err := readJwtCookie[auth.AccessTokenClaims](req, "tulip-login-access", h.signingKey) + if err != nil { + return h.readLoginRefreshCookie(rw, req, u) + } + *u = UserAuth{ + Subject: loginData.Subject, + NeedOtp: loginData.Claims.Perms.Has("needs-otp"), + } + return nil +} + +func (h *HttpServer) readLoginRefreshCookie(rw http.ResponseWriter, req *http.Request, userAuth *UserAuth) error { + refreshData, err := readJwtCookie[auth.RefreshTokenClaims](req, "tulip-login-refresh", h.signingKey) + if err != nil { + return err + } + + *userAuth = UserAuth{ + Subject: refreshData.Subject, + NeedOtp: false, + } + + if h.setLoginDataCookie(rw, *userAuth) { + http.Error(rw, "Failed to save login cookie", http.StatusInternalServerError) + return fmt.Errorf("failed to save login cookie: %w", ErrAuthHttpError) + } + return nil +} + func (h *HttpServer) LoginResetPasswordPost(rw http.ResponseWriter, req *http.Request, _ httprouter.Params) { email := req.PostFormValue("email") address, err := mail.ParseAddress(email) diff --git a/server/manage-apps.go b/server/manage-apps.go index 42ec0eb..8584c7c 100644 --- a/server/manage-apps.go +++ b/server/manage-apps.go @@ -27,12 +27,12 @@ func (h *HttpServer) ManageAppsGet(rw http.ResponseWriter, req *http.Request, _ var role types.UserRole var appList []database.GetAppListRow if h.DbTx(rw, func(tx *database.Queries) (err error) { - role, err = tx.GetUserRole(req.Context(), auth.ID) + role, err = tx.GetUserRole(req.Context(), auth.Subject) if err != nil { return } appList, err = tx.GetAppList(req.Context(), database.GetAppListParams{ - Owner: auth.ID, + Owner: auth.Subject, Column2: role == types.RoleAdmin, Offset: int64(offset), }) @@ -84,7 +84,7 @@ func (h *HttpServer) ManageAppsPost(rw http.ResponseWriter, req *http.Request, _ if sso { var role types.UserRole if h.DbTx(rw, func(tx *database.Queries) (err error) { - role, err = tx.GetUserRole(req.Context(), auth.ID) + role, err = tx.GetUserRole(req.Context(), auth.Subject) return }) { return @@ -107,7 +107,7 @@ func (h *HttpServer) ManageAppsPost(rw http.ResponseWriter, req *http.Request, _ Name: name, Secret: secret, Domain: domain, - Owner: auth.ID, + Owner: auth.Subject, Public: public, Sso: sso, Active: active, @@ -124,7 +124,7 @@ func (h *HttpServer) ManageAppsPost(rw http.ResponseWriter, req *http.Request, _ Sso: sso, Active: active, Subject: req.FormValue("subject"), - Owner: auth.ID, + Owner: auth.Subject, }) }) { return @@ -145,7 +145,7 @@ func (h *HttpServer) ManageAppsPost(rw http.ResponseWriter, req *http.Request, _ err = tx.ResetClientAppSecret(req.Context(), database.ResetClientAppSecretParams{ Secret: secret, Subject: sub, - Owner: auth.ID, + Owner: auth.Subject, }) return err }) { diff --git a/server/manage-users.go b/server/manage-users.go index cf23745..719e832 100644 --- a/server/manage-users.go +++ b/server/manage-users.go @@ -31,7 +31,7 @@ func (h *HttpServer) ManageUsersGet(rw http.ResponseWriter, req *http.Request, _ var role types.UserRole var userList []database.GetUserListRow if h.DbTx(rw, func(tx *database.Queries) (err error) { - role, err = tx.GetUserRole(req.Context(), auth.ID) + role, err = tx.GetUserRole(req.Context(), auth.Subject) if err != nil { return } @@ -51,7 +51,7 @@ func (h *HttpServer) ManageUsersGet(rw http.ResponseWriter, req *http.Request, _ "Users": userList, "Offset": offset, "EmailShow": req.URL.Query().Has("show-email"), - "CurrentAdmin": auth.ID, + "CurrentAdmin": auth.Subject, "Namespace": h.conf.Namespace, } if q.Has("edit") { @@ -80,7 +80,7 @@ func (h *HttpServer) ManageUsersPost(rw http.ResponseWriter, req *http.Request, var role types.UserRole if h.DbTx(rw, func(tx *database.Queries) (err error) { - role, err = tx.GetUserRole(req.Context(), auth.ID) + role, err = tx.GetUserRole(req.Context(), auth.Subject) return }) { return diff --git a/server/oauth.go b/server/oauth.go index c69bcbc..371996f 100644 --- a/server/oauth.go +++ b/server/oauth.go @@ -80,11 +80,11 @@ func (h *HttpServer) authorizeEndpoint(rw http.ResponseWriter, req *http.Request var user string var hasOtp bool if h.DbTx(rw, func(tx *database.Queries) (err error) { - user, err = tx.GetUserDisplayName(req.Context(), auth.ID) + user, err = tx.GetUserDisplayName(req.Context(), auth.Subject) if err != nil { return } - hasOtp, err = tx.HasOtp(req.Context(), auth.ID) + hasOtp, err = tx.HasOtp(req.Context(), auth.Subject) return }) { return @@ -117,7 +117,7 @@ func (h *HttpServer) authorizeEndpoint(rw http.ResponseWriter, req *http.Request if !isSSO { otpInput := req.FormValue("code") - if h.fetchAndValidateOtp(rw, auth.ID, otpInput) { + if h.fetchAndValidateOtp(rw, auth.Subject, otpInput) { return } } @@ -147,7 +147,7 @@ func (h *HttpServer) oauthUserAuthorization(rw http.ResponseWriter, req *http.Re return "", err } - auth, err := h.internalAuthenticationHandler(req) + auth, err := h.internalAuthenticationHandler(rw, req) if err != nil { return "", err } @@ -169,5 +169,5 @@ func (h *HttpServer) oauthUserAuthorization(rw http.ResponseWriter, req *http.Re http.Redirect(rw, req, redirectUrl.String(), http.StatusFound) return "", nil } - return auth.ID, nil + return auth.Subject, nil } diff --git a/server/otp.go b/server/otp.go index 7340b20..6622d70 100644 --- a/server/otp.go +++ b/server/otp.go @@ -34,7 +34,7 @@ func (h *HttpServer) LoginOtpPost(rw http.ResponseWriter, req *http.Request, _ h } otpInput := req.FormValue("code") - if h.fetchAndValidateOtp(rw, auth.ID, otpInput) { + if h.fetchAndValidateOtp(rw, auth.Subject, otpInput) { return } @@ -86,13 +86,13 @@ func (h *HttpServer) EditOtpPost(rw http.ResponseWriter, req *http.Request, _ ht } otpInput := req.Form.Get("code") - if h.fetchAndValidateOtp(rw, auth.ID, otpInput) { + if h.fetchAndValidateOtp(rw, auth.Subject, otpInput) { return } if h.DbTx(rw, func(tx *database.Queries) error { return tx.SetOtp(req.Context(), database.SetOtpParams{ - Subject: auth.ID, + Subject: auth.Subject, Secret: "", Digits: 0, }) @@ -128,7 +128,7 @@ func (h *HttpServer) EditOtpPost(rw http.ResponseWriter, req *http.Request, _ ht var email string if h.DbTx(rw, func(tx *database.Queries) error { var err error - email, err = tx.GetUserEmail(req.Context(), auth.ID) + email, err = tx.GetUserEmail(req.Context(), auth.Subject) return err }) { return @@ -177,7 +177,7 @@ func (h *HttpServer) EditOtpPost(rw http.ResponseWriter, req *http.Request, _ ht if h.DbTx(rw, func(tx *database.Queries) error { return tx.SetOtp(req.Context(), database.SetOtpParams{ - Subject: auth.ID, + Subject: auth.Subject, Secret: secret, Digits: int64(digits), }) diff --git a/server/server.go b/server/server.go index 689fdff..b3ce915 100644 --- a/server/server.go +++ b/server/server.go @@ -124,7 +124,14 @@ func NewHttpServer(conf Conf, db *database.Queries, signingKey mjwt.Signer) *htt } if subtle.ConstantTimeCompare([]byte(cookie.Value), []byte(req.PostFormValue("nonce"))) == 1 { http.SetCookie(rw, &http.Cookie{ - Name: "tulip-login-data", + Name: "tulip-login-access", + Path: "/", + MaxAge: -1, + Secure: true, + SameSite: http.SameSiteLaxMode, + }) + http.SetCookie(rw, &http.Cookie{ + Name: "tulip-login-refresh", Path: "/", MaxAge: -1, Secure: true,