Improve session management

- Implement session validation and verification before OAuth flow
- Remove redundant session reset functionality
- Refactor session validation logic into dedicated handler

commit: 5fc37eaf68142a92e0fa30aff629a83b0fa9f76d
author: bnema · 2y ago · 2023-11-04 09:36 UTC
parent: 0a33911
signature: unsigned

5 files · +124 -103

M internal/httpserve/handler/adminroutes.go +1 -2


@@ -5,7 +5,6 @@ import (
5	5
6	6		"net/http"
7	7
8		-	"github.com/bnema/gordon/internal/httpserve/middleware"
9	8		"github.com/bnema/gordon/internal/server"
10	9		"github.com/bnema/gordon/internal/templating/render"
11	10		"github.com/labstack/echo/v4"

@@ -18,7 +17,7 @@ var (
18	17
19	18		// GetLocalizedData returns the data for the localization
20	19		func GetLocalizedData(c echo.Context, a *server.App) (map[string]interface{}, error) {
21		-	lang := c.Get(middleware.LangKey)
	20	+	lang := c.Get("LangKey")
22	21
23	22		if lang == nil {
24	23		return nil, fmt.Errorf("LangKey not found in context")

 	"net/http"
-	"github.com/bnema/gordon/internal/httpserve/middleware"
 	"github.com/bnema/gordon/internal/server"
 	"github.com/bnema/gordon/internal/templating/render"
 	"github.com/labstack/echo/v4"
 // GetLocalizedData returns the data for the localization
 func GetLocalizedData(c echo.Context, a *server.App) (map[string]interface{}, error) {
-	lang := c.Get(middleware.LangKey)
+	lang := c.Get("LangKey")
 	if lang == nil {
 		return nil, fmt.Errorf("LangKey not found in context")

M internal/httpserve/handler/oauth.go +17 -3


@@ -7,6 +7,7 @@ import (
7	7		"io"
8	8		"net/http"
9	9		"os"
	10	+	"time"
10	11
11	12		"github.com/bnema/gordon/internal/db"
12	13		"github.com/bnema/gordon/internal/db/queries"

@@ -65,9 +66,22 @@ func StartOAuthGithub(c echo.Context, a *server.App) error {
65	66		}
66	67
67	68		// Check if the session values are valid
68		-	accountID, ok := sess.Values["accountID"].(string)
69		-	if ok && accountID != "" {
70		-	return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path) // StatusSeeOther is HTTP 303
	69	+	sessionID, accountID, ok := sess.Values["accountID"].(string), sess.Values["sessionID"].(string), sess.Values["authenticated"].(bool)
	70	+	if ok && accountID != "" && sessionID != "" {
	71	+	// Check if the session is expired
	72	+	sessionExpired, err := IsSessionExpiredInDB(a, accountID, sessionID)
	73	+	if err != nil {
	74	+	return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
	75	+	}
	76	+	if !sessionExpired {
	77	+	sessionExpiryTime := 30 * time.Minute
	78	+	// Extend session expiration conditionally, e.g., if it's close to expiring
	79	+	err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
	80	+	if err != nil {
	81	+	return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
	82	+	}
	83	+	return c.Redirect(http.StatusFound, a.Config.Admin.Path)
	84	+	}
71	85		}
72	86
73	87		//Initiate the Github OAuth flow

 	"io"
 	"net/http"
 	"os"
+	"time"
 	"github.com/bnema/gordon/internal/db"
 	"github.com/bnema/gordon/internal/db/queries"
 	}
 	// Check if the session values are valid
-	accountID, ok := sess.Values["accountID"].(string)
+	sessionID, accountID, ok := sess.Values["accountID"].(string), sess.Values["sessionID"].(string), sess.Values["authenticated"].(bool)
-	if ok && accountID != "" {
+	if ok && accountID != "" && sessionID != "" {
-		return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path) // StatusSeeOther is HTTP 303
+		// Check if the session is expired
+		sessionExpired, err := IsSessionExpiredInDB(a, accountID, sessionID)
+		if err != nil {
+			return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
+		}
+		if !sessionExpired {
+			sessionExpiryTime := 30 * time.Minute
+			// Extend session expiration conditionally, e.g., if it's close to expiring
+			err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
+			if err != nil {
+				return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
+			}
+			return c.Redirect(http.StatusFound, a.Config.Admin.Path)
+		}
 	}
 	//Initiate the Github OAuth flow

D internal/httpserve/handler/session.go +0 -26

@@ -1,26 +0,0 @@
1	-	package handler
2	-
3	-	// ResetSession resets the session cookie and session data
4	-	// func ResetSession(c echo.Context) error {
5	-	// // Manually expire the cookie
6	-	// cookie := new(http.Cookie)
7	-	// cookie.Name = "session"
8	-	// cookie.Value = ""
9	-	// cookie.Path = "/"
10	-	// cookie.Expires = time.Unix(0, 0)
11	-	// cookie.MaxAge = -1 // Clear immediately
12	-	// c.SetCookie(cookie)
13	-	// // Invalidate the existing session
14	-	// sess, err := session.Get("session", c)
15	-	// if err != nil {
16	-	// log.Println(err)
17	-	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not get session")
18	-	// }
19	-	// sess.Options.MaxAge = -1
20	-	// if err := sess.Save(c.Request(), c.Response()); err != nil {
21	-	// log.Println("Could not delete session:", err)
22	-	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not delete session")
23	-	// }
24	-
25	-	// return nil
26	-	// }

@@ -1,26 +0,0 @@
1	-	package handler
2	-
3	-	// ResetSession resets the session cookie and session data
4	-	// func ResetSession(c echo.Context) error {
5	-	// // Manually expire the cookie
6	-	// cookie := new(http.Cookie)
7	-	// cookie.Name = "session"
8	-	// cookie.Value = ""
9	-	// cookie.Path = "/"
10	-	// cookie.Expires = time.Unix(0, 0)
11	-	// cookie.MaxAge = -1 // Clear immediately
12	-	// c.SetCookie(cookie)
13	-	// // Invalidate the existing session
14	-	// sess, err := session.Get("session", c)
15	-	// if err != nil {
16	-	// log.Println(err)
17	-	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not get session")
18	-	// }
19	-	// sess.Options.MaxAge = -1
20	-	// if err := sess.Save(c.Request(), c.Response()); err != nil {
21	-	// log.Println("Could not delete session:", err)
22	-	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not delete session")
23	-	// }
24	-
25	-	// return nil
26	-	// }

A internal/httpserve/handler/sessiondb.go +92 -0

@@ -0,0 +1,92 @@
	1	+	package handler
	2	+
	3	+	import (
	4	+	"fmt"
	5	+	"time"
	6	+
	7	+	"github.com/bnema/gordon/internal/db/queries"
	8	+	"github.com/bnema/gordon/internal/server"
	9	+	"github.com/labstack/echo-contrib/session"
	10	+	"github.com/labstack/echo/v4"
	11	+	)
	12	+
	13	+	func ValidateSessionAndUser(c echo.Context, a *server.App) error {
	14	+	sessionExpiryTime := 30 * time.Minute
	15	+
	16	+	sess, err := session.Get("session", c)
	17	+	if err != nil {
	18	+	return err
	19	+	}
	20	+
	21	+	accountID, ok := sess.Values["accountID"].(string)
	22	+	if !ok {
	23	+	return fmt.Errorf("accountID not found or invalid type")
	24	+	}
	25	+
	26	+	sessionID, ok := sess.Values["sessionID"].(string)
	27	+	if !ok {
	28	+	return fmt.Errorf("sessionID not found or invalid type")
	29	+	}
	30	+
	31	+	accountExists, err := IsAccountIDInDB(a, accountID)
	32	+	if err != nil \|\| !accountExists {
	33	+	return fmt.Errorf("account does not exist")
	34	+	}
	35	+
	36	+	sessionExpired, err := IsSessionExpiredInDB(a, accountID, sessionID)
	37	+	if err != nil \|\| sessionExpired {
	38	+	return fmt.Errorf("session is expired")
	39	+	}
	40	+
	41	+	// Extend session expiration conditionally, e.g., if it's close to expiring
	42	+	err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
	43	+	if err != nil {
	44	+	return fmt.Errorf("failed to extend session: %v", err)
	45	+	}
	46	+
	47	+	return nil
	48	+	}
	49	+
	50	+	func IsSessionExpiredInDB(a *server.App, userID string, sessionID string) (bool, error) {
	51	+	currentTime := time.Now()
	52	+	expirationTime, err := queries.GetSessionExpiration(a, userID, sessionID, currentTime)
	53	+	if err != nil {
	54	+	return false, err
	55	+	}
	56	+
	57	+	return currentTime.After(expirationTime), nil
	58	+	}
	59	+
	60	+	func IsAccountIDInDB(a *server.App, accountID string) (bool, error) {
	61	+	accountExists, err := queries.CheckDBAccountExists(a, accountID)
	62	+	if err != nil {
	63	+	return false, err
	64	+	}
	65	+
	66	+	return accountExists, nil
	67	+	}
	68	+
	69	+	// ResetSession resets the session cookie and session data
	70	+	// func ResetSession(c echo.Context) error {
	71	+	// // Manually expire the cookie
	72	+	// cookie := new(http.Cookie)
	73	+	// cookie.Name = "session"
	74	+	// cookie.Value = ""
	75	+	// cookie.Path = "/"
	76	+	// cookie.Expires = time.Unix(0, 0)
	77	+	// cookie.MaxAge = -1 // Clear immediately
	78	+	// c.SetCookie(cookie)
	79	+	// // Invalidate the existing session
	80	+	// sess, err := session.Get("session", c)
	81	+	// if err != nil {
	82	+	// log.Println(err)
	83	+	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not get session")
	84	+	// }
	85	+	// sess.Options.MaxAge = -1
	86	+	// if err := sess.Save(c.Request(), c.Response()); err != nil {
	87	+	// log.Println("Could not delete session:", err)
	88	+	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not delete session")
	89	+	// }
	90	+
	91	+	// return nil
	92	+	// }

@@ -0,0 +1,92 @@
	1	+	package handler
	2	+
	3	+	import (
	4	+	"fmt"
	5	+	"time"
	6	+
	7	+	"github.com/bnema/gordon/internal/db/queries"
	8	+	"github.com/bnema/gordon/internal/server"
	9	+	"github.com/labstack/echo-contrib/session"
	10	+	"github.com/labstack/echo/v4"
	11	+	)
	12	+
	13	+	func ValidateSessionAndUser(c echo.Context, a *server.App) error {
	14	+	sessionExpiryTime := 30 * time.Minute
	15	+
	16	+	sess, err := session.Get("session", c)
	17	+	if err != nil {
	18	+	return err
	19	+	}
	20	+
	21	+	accountID, ok := sess.Values["accountID"].(string)
	22	+	if !ok {
	23	+	return fmt.Errorf("accountID not found or invalid type")
	24	+	}
	25	+
	26	+	sessionID, ok := sess.Values["sessionID"].(string)
	27	+	if !ok {
	28	+	return fmt.Errorf("sessionID not found or invalid type")
	29	+	}
	30	+
	31	+	accountExists, err := IsAccountIDInDB(a, accountID)
	32	+	if err != nil \|\| !accountExists {
	33	+	return fmt.Errorf("account does not exist")
	34	+	}
	35	+
	36	+	sessionExpired, err := IsSessionExpiredInDB(a, accountID, sessionID)
	37	+	if err != nil \|\| sessionExpired {
	38	+	return fmt.Errorf("session is expired")
	39	+	}
	40	+
	41	+	// Extend session expiration conditionally, e.g., if it's close to expiring
	42	+	err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
	43	+	if err != nil {
	44	+	return fmt.Errorf("failed to extend session: %v", err)
	45	+	}
	46	+
	47	+	return nil
	48	+	}
	49	+
	50	+	func IsSessionExpiredInDB(a *server.App, userID string, sessionID string) (bool, error) {
	51	+	currentTime := time.Now()
	52	+	expirationTime, err := queries.GetSessionExpiration(a, userID, sessionID, currentTime)
	53	+	if err != nil {
	54	+	return false, err
	55	+	}
	56	+
	57	+	return currentTime.After(expirationTime), nil
	58	+	}
	59	+
	60	+	func IsAccountIDInDB(a *server.App, accountID string) (bool, error) {
	61	+	accountExists, err := queries.CheckDBAccountExists(a, accountID)
	62	+	if err != nil {
	63	+	return false, err
	64	+	}
	65	+
	66	+	return accountExists, nil
	67	+	}
	68	+
	69	+	// ResetSession resets the session cookie and session data
	70	+	// func ResetSession(c echo.Context) error {
	71	+	// // Manually expire the cookie
	72	+	// cookie := new(http.Cookie)
	73	+	// cookie.Name = "session"
	74	+	// cookie.Value = ""
	75	+	// cookie.Path = "/"
	76	+	// cookie.Expires = time.Unix(0, 0)
	77	+	// cookie.MaxAge = -1 // Clear immediately
	78	+	// c.SetCookie(cookie)
	79	+	// // Invalidate the existing session
	80	+	// sess, err := session.Get("session", c)
	81	+	// if err != nil {
	82	+	// log.Println(err)
	83	+	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not get session")
	84	+	// }
	85	+	// sess.Options.MaxAge = -1
	86	+	// if err := sess.Save(c.Request(), c.Response()); err != nil {
	87	+	// log.Println("Could not delete session:", err)
	88	+	// return echo.NewHTTPError(http.StatusInternalServerError, "Could not delete session")
	89	+	// }
	90	+
	91	+	// return nil
	92	+	// }

M internal/httpserve/middleware/requirelogin.go +14 -72

@@ -1,14 +1,11 @@
1	1		package middleware
2	2
3	3		import (
4		-	"fmt"
5		-	"net/http"
6	4		"time"
7	5
8		-	"github.com/bnema/gordon/internal/db/queries"
	6	+	"github.com/bnema/gordon/internal/httpserve/handler"
9	7		"github.com/bnema/gordon/internal/server"
10	8
11		-	"github.com/labstack/echo-contrib/session"
12	9		"github.com/labstack/echo/v4"
13	10		)
14	11

@@ -18,7 +15,7 @@ func RequireLogin(a *server.App) echo.MiddlewareFunc {
18	15		const maxRetries = 3
19	16		var err error
20	17		for i := 0; i < maxRetries; i++ {
21		-	err = validateSessionAndUser(c, a)
	18	+	err = handler.ValidateSessionAndUser(c, a)
22	19		if err == nil {
23	20		return next(c)
24	21		}

@@ -30,73 +27,18 @@ func RequireLogin(a *server.App) echo.MiddlewareFunc {
30	27		}
31	28		}
32	29		}
33		-	func validateSessionAndUser(c echo.Context, a *server.App) error {
34		-	sessionExpiryTime := 30 * time.Minute
35	30
36		-	sess, err := session.Get("session", c)
37		-	if err != nil {
38		-	return err
39		-	}
40		-
41		-	accountID, ok := sess.Values["accountID"].(string)
42		-	if !ok {
43		-	return fmt.Errorf("accountID not found or invalid type")
44		-	}
45		-
46		-	sessionID, ok := sess.Values["sessionID"].(string)
47		-	if !ok {
48		-	return fmt.Errorf("sessionID not found or invalid type")
49		-	}
50		-
51		-	accountExists, err := isAccountIDInDB(a, accountID)
52		-	if err != nil \|\| !accountExists {
53		-	return fmt.Errorf("account does not exist")
54		-	}
55		-
56		-	sessionExpired, err := isSessionExpiredInDB(a, accountID, sessionID)
57		-	if err != nil \|\| sessionExpired {
58		-	return fmt.Errorf("session is expired")
59		-	}
60		-
61		-	// Extend session expiration conditionally, e.g., if it's close to expiring
62		-	err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
63		-	if err != nil {
64		-	return fmt.Errorf("failed to extend session: %v", err)
65		-	}
66		-
67		-	return nil
68		-	}
69		-
70		-	func redirectToLogin(c echo.Context, a *server.App) error {
71		-	return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path+"/login")
72		-	}
73		-
74		-	func isSessionExpiredInDB(a *server.App, userID string, sessionID string) (bool, error) {
75		-	currentTime := time.Now()
76		-	expirationTime, err := queries.GetSessionExpiration(a, userID, sessionID, currentTime)
77		-	if err != nil {
78		-	return false, err
79		-	}
80		-
81		-	return currentTime.After(expirationTime), nil
82		-	}
83		-
84		-	func isAccountIDInDB(a *server.App, accountID string) (bool, error) {
85		-	accountExists, err := queries.CheckDBAccountExists(a, accountID)
86		-	if err != nil {
87		-	return false, err
88		-	}
	31	+	// func redirectToLogin(c echo.Context, a *server.App) error {
	32	+	// return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path+"/login")
	33	+	// }
89	34
90		-	return accountExists, nil
91		-	}
92		-
93		-	func DeleteCookieAndRedirectToLogin(c echo.Context, a *server.App) error {
94		-	// Delete session cookie
95		-	cookie := new(http.Cookie)
96		-	cookie.Name = "session"
97		-	cookie.Value = ""
98		-	cookie.Expires = time.Now().Add(-1 * time.Hour)
99		-	c.SetCookie(cookie)
	35	+	// func DeleteCookieAndRedirectToLogin(c echo.Context, a *server.App) error {
	36	+	// // Delete session cookie
	37	+	// cookie := new(http.Cookie)
	38	+	// cookie.Name = "session"
	39	+	// cookie.Value = ""
	40	+	// cookie.Expires = time.Now().Add(-1 * time.Hour)
	41	+	// c.SetCookie(cookie)
100	42
101		-	return redirectToLogin(c, a)
102		-	}
	43	+	// return redirectToLogin(c, a)
	44	+	// }

@@ -1,14 +1,11 @@
1		package middleware	1		package middleware
2			2
3		import (	3		import (
4	-	"fmt"
5	-	"net/http"
6		"time"	4		"time"
7			5
8	-	"github.com/bnema/gordon/internal/db/queries"	6	+	"github.com/bnema/gordon/internal/httpserve/handler"
9		"github.com/bnema/gordon/internal/server"	7		"github.com/bnema/gordon/internal/server"
10			8
11	-	"github.com/labstack/echo-contrib/session"
12		"github.com/labstack/echo/v4"	9		"github.com/labstack/echo/v4"
13		)	10		)
14			11

@@ -18,7 +15,7 @@ func RequireLogin(a *server.App) echo.MiddlewareFunc {
18		const maxRetries = 3	15		const maxRetries = 3
19		var err error	16		var err error
20		for i := 0; i < maxRetries; i++ {	17		for i := 0; i < maxRetries; i++ {
21	-	err = validateSessionAndUser(c, a)	18	+	err = handler.ValidateSessionAndUser(c, a)
22		if err == nil {	19		if err == nil {
23		return next(c)	20		return next(c)
24		}	21		}

@@ -30,73 +27,18 @@ func RequireLogin(a *server.App) echo.MiddlewareFunc {
30		}	27		}
31		}	28		}
32		}	29		}
33	-	func validateSessionAndUser(c echo.Context, a *server.App) error {
34	-	sessionExpiryTime := 30 * time.Minute
35			30
36	-	sess, err := session.Get("session", c)	31	+	// func redirectToLogin(c echo.Context, a *server.App) error {
37	-	if err != nil {	32	+	// return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path+"/login")
38	-	return err	33	+	// }
39	-	}
40	-
41	-	accountID, ok := sess.Values["accountID"].(string)
42	-	if !ok {
43	-	return fmt.Errorf("accountID not found or invalid type")
44	-	}
45	-
46	-	sessionID, ok := sess.Values["sessionID"].(string)
47	-	if !ok {
48	-	return fmt.Errorf("sessionID not found or invalid type")
49	-	}
50	-
51	-	accountExists, err := isAccountIDInDB(a, accountID)
52	-	if err != nil \|\| !accountExists {
53	-	return fmt.Errorf("account does not exist")
54	-	}
55	-
56	-	sessionExpired, err := isSessionExpiredInDB(a, accountID, sessionID)
57	-	if err != nil \|\| sessionExpired {
58	-	return fmt.Errorf("session is expired")
59	-	}
60	-
61	-	// Extend session expiration conditionally, e.g., if it's close to expiring
62	-	err = queries.ExtendSessionExpiration(a, accountID, sessionID, time.Now().Add(sessionExpiryTime))
63	-	if err != nil {
64	-	return fmt.Errorf("failed to extend session: %v", err)
65	-	}
66	-
67	-	return nil
68	-	}
69	-
70	-	func redirectToLogin(c echo.Context, a *server.App) error {
71	-	return c.Redirect(http.StatusSeeOther, a.Config.Admin.Path+"/login")
72	-	}
73	-
74	-	func isSessionExpiredInDB(a *server.App, userID string, sessionID string) (bool, error) {
75	-	currentTime := time.Now()
76	-	expirationTime, err := queries.GetSessionExpiration(a, userID, sessionID, currentTime)
77	-	if err != nil {
78	-	return false, err
79	-	}
80	-
81	-	return currentTime.After(expirationTime), nil
82	-	}
83	-
84	-	func isAccountIDInDB(a *server.App, accountID string) (bool, error) {
85	-	accountExists, err := queries.CheckDBAccountExists(a, accountID)
86	-	if err != nil {
87	-	return false, err
88	-	}
89			34
90	-	return accountExists, nil	35	+	// func DeleteCookieAndRedirectToLogin(c echo.Context, a *server.App) error {
91	-	}	36	+	// // Delete session cookie
92	-		37	+	// cookie := new(http.Cookie)
93	-	func DeleteCookieAndRedirectToLogin(c echo.Context, a *server.App) error {	38	+	// cookie.Name = "session"
94	-	// Delete session cookie	39	+	// cookie.Value = ""
95	-	cookie := new(http.Cookie)	40	+	// cookie.Expires = time.Now().Add(-1 * time.Hour)
96	-	cookie.Name = "session"	41	+	// c.SetCookie(cookie)
97	-	cookie.Value = ""
98	-	cookie.Expires = time.Now().Add(-1 * time.Hour)
99	-	c.SetCookie(cookie)
100			42
101	-	return redirectToLogin(c, a)	43	+	// return redirectToLogin(c, a)
102	-	}	44	+	// }