From 1753fe9ea88cc11706488d980d65a9fd99271788 Mon Sep 17 00:00:00 2001
From: "Felipe M." <me@fmartingr.com>
Date: Tue, 31 Dec 2024 10:02:59 +0100
Subject: [PATCH] feat: Add secure destination URL handling for login redirect

---
 internal/http/response/shortcuts.go        |  2 +-
 internal/http/routes/bookmark_test.go      |  2 +-
 internal/view/assets/js/component/login.js | 33 ++++++++++++++++++++++
 internal/view/index.html                   | 12 ++++----
 4 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/internal/http/response/shortcuts.go b/internal/http/response/shortcuts.go
index dec01c9fa..ed614e543 100644
--- a/internal/http/response/shortcuts.go
+++ b/internal/http/response/shortcuts.go
@@ -36,7 +36,7 @@ func SendInternalServerError(ctx *gin.Context) {
 
 // SendNotFound directly sends a not found response
 func RedirectToLogin(ctx *gin.Context, dst string) {
-	ctx.Redirect(http.StatusFound, "/login?dst="+dst)
+	ctx.Redirect(http.StatusFound, "/?dst="+dst)
 }
 
 func NotFound(ctx *gin.Context) {
diff --git a/internal/http/routes/bookmark_test.go b/internal/http/routes/bookmark_test.go
index 400dd95b2..edaf13428 100644
--- a/internal/http/routes/bookmark_test.go
+++ b/internal/http/routes/bookmark_test.go
@@ -122,7 +122,7 @@ func TestBookmarkContentHandler(t *testing.T) {
 		req, _ := http.NewRequest("GET", path, nil)
 		g.ServeHTTP(w, req)
 		require.Equal(t, http.StatusFound, w.Code)
-		require.Equal(t, "/login?dst="+path, w.Header().Get("Location"))
+		require.Equal(t, "/?dst="+path, w.Header().Get("Location"))
 	})
 
 	t.Run("get existing bookmark content", func(t *testing.T) {
diff --git a/internal/view/assets/js/component/login.js b/internal/view/assets/js/component/login.js
index 4e74ba25b..e75925556 100644
--- a/internal/view/assets/js/component/login.js
+++ b/internal/view/assets/js/component/login.js
@@ -39,10 +39,35 @@ export default {
 			username: "",
 			password: "",
 			remember: false,
+			destination: "/", // Default destination
 		};
 	},
 	emits: ["login-success"],
 	methods: {
+		sanitizeDestination(dst) {
+			try {
+				// Remove any leading/trailing whitespace
+				dst = dst.trim();
+
+				// Decode the URL to handle any encoded characters
+				dst = decodeURIComponent(dst);
+
+				// Create a URL object to parse the destination
+				const url = new URL(dst, window.location.origin);
+
+				// Only allow paths from the same origin
+				if (url.origin !== window.location.origin) {
+					return "/";
+				}
+
+				// Only return the pathname and search params
+				return url.pathname + url.search + url.hash;
+			} catch (e) {
+				// If any error occurs during parsing, return root
+				return "/";
+			}
+		},
+
 		async getErrorMessage(err) {
 			switch (err.constructor) {
 				case Error:
@@ -119,6 +144,9 @@ export default {
 
 					this.visible = false;
 					this.$emit("login-success");
+
+					// Redirect to sanitized destination
+					if (this.destination !== "/") window.location.href = this.destination;
 				})
 				.catch((err) => {
 					this.loading = false;
@@ -129,6 +157,11 @@ export default {
 		},
 	},
 	async mounted() {
+		// Get and sanitize destination from URL parameters
+		const urlParams = new URLSearchParams(window.location.search);
+		const dst = urlParams.get("dst");
+		this.destination = dst ? this.sanitizeDestination(dst) : "/";
+
 		// Check if there's a valid session first
 		const token = localStorage.getItem("shiori-token");
 		if (token) {
diff --git a/internal/view/index.html b/internal/view/index.html
index 177cc542f..c23f677a0 100644
--- a/internal/view/index.html
+++ b/internal/view/index.html
@@ -105,6 +105,8 @@
 								document.cookie = `token=; Path=${new URL(document.baseURI).pathname}; Expires=Thu, 01 Jan 1970 00:00:00 GMT;`;
 								this.isLoggedIn = false;
 								this.loginRequired = true;
+								this.dialog.loading = false;
+								this.dialog.visible = false;
 							}).catch(err => {
 								this.dialog.loading = false;
 								this.getErrorMessage(err).then(msg => {
@@ -155,7 +157,7 @@
 						owner: owner,
 					};
 				},
-				
+
 				onLoginSuccess() {
 					this.loadSetting();
 					this.loadAccount();
@@ -165,7 +167,7 @@
 				async validateSession() {
 					const token = localStorage.getItem("shiori-token");
 					const account = localStorage.getItem("shiori-account");
-					
+
 					if (!(token && account)) {
 						return false;
 					}
@@ -176,11 +178,11 @@
 								"Authorization": `Bearer ${token}`
 							}
 						});
-						
+
 						if (!response.ok) {
 							throw new Error('Invalid session');
 						}
-						
+
 						return true;
 					} catch (err) {
 						// Clear invalid session data
@@ -195,7 +197,7 @@
 				async checkLoginStatus() {
 					const isValid = await this.validateSession();
 					this.isLoggedIn = isValid;
-					
+
 					if (isValid) {
 						this.loadSetting();
 						this.loadAccount();