Repository: syncthing/syncthing (language: go)

Issue:
Display error in 1.26 with login screen
I reverted back to http-basic-auth in 1.26. Essential information about the authentication with the new login screen is not displayed to me. There is full access after authorization because I can, for example, reactivate Basic Auth via the GUI. Browser: Chrome 118. In Firefox the GUI doesn't even build completely, some JS restarts the build every few seconds. After activating http-basic-auth, the GUI works again as usual in Firefox.
![2023-11-09_09-17](https://github.com/syncthing/syncthing/assets/33317545/eef74df0-6edc-4c59-b4ee-0f8b631142c7)
![2023-11-09_09-18](https://github.com/syncthing/syncthing/assets/33317545/43729a65-9367-4b0b-82b1-14dd7a4a7037)



Discussion / hints:
We're aware of the issue but weren't able reproduce it yet. Your screenshot is pretty interesting though, as half of the UI appears to work. This almost looks like some kind of race condition.
I can now add the following observation. After I was able to observe the error pattern on two hosts with syncthing increased from 1.25 to 1.26, I switched back to basic-auth as written and the error disappeared. Now I have switched two more hosts and the error didn't even occur on them in the same browser session with the new login mask, which surprised me. Then I switched the two first migrated hosts back to the new login mask as a cross-check, and lo and behold, the error disappeared, as if Chrome had to get used to it... If I can do anything to help find the cause, let me do so let me know.
Doesn't work for me in Chrome Version 119.0.6045.124 (Official Build) (64-bit).
But it works in Firefox and Edge.
![Screenshot 2023-11-10 003926](https://github.com/syncthing/syncthing/assets/150396699/5f7087fa-b086-45be-bc5a-966342c7f6ef)


![image](https://github.com/syncthing/syncthing/assets/150396699/e6254f24-a62f-4d1e-ae4a-7040365edf14)
Firefox 119.0.1 (64-bit)

@yuri-273 can you open the developer console in Chrome by pressing the F12 key? I'd be interested to see which cookies are present. Redacting the value column is enough.

https://developer.chrome.com/docs/devtools/application/cookies/


I just experienced the same issue, and I fixed it by clearing cookies. It seems there were 2 cookies named `sessionid-xxxx`






Deleted cookies. The problem is not solved.
![image](https://github.com/syncthing/syncthing/assets/150396699/715aa9e2-ec72-41e9-913d-5360705b8582)

I see this too (Chrome 119.0.6045.105; syncthing 1.26 built from source using Go 1.20.5). Not tried clearing cookies yet, will do so. Backed down to 1.24, which fortunately worked fine. The only extension I'm using is uBlock Origin, which is not blocking anything on this page.
I use MS Edge, met this issue also.
And I found why: I use Nginx proxy manager, and enabled `cache asset`. After disable it, syncthing works well.
I had the "endless reload loop" here (Firefox 118.0.2) on multiple installations of 1.26. Clearing cookies has solved the problem.

These were the cookies before I did that:

| Name             | Path         | Expires | HttpOnly | Secure |
| ---------------- | ------------ | ------- | -------- | ------ |
| CSRF-Token-XXXXX | /            | Session | false    | false  |
| sessionid-XXXXX  | /            | Session | false    | true   |
| sessionid-XXXXX  | /rest        | Session | false    | true   |
| sessionid-XXXXX  | /rest/svc    | Session | false    | true   |
| sessionid-XXXXX  | /rest/system | Session | false    | true   |

After deleting the cookies and logging in again, only CSRF-Token-XXXXX and the sessionid-XXXXX for path "/" were recreated.
Had to revert to Basic Auth as well (Chrome here). Clearing cache and cookies did not help. It logged but the interface was broken. Could only login through an anonymous tab.
So, my interpretation of all this:

- Previously, we did not set `Path: /` on the cookie. Sometimes it ended up on that path, but we also sometimes polluted the cookie space with `/rest` etc cookies as seen above.
- Currently, we set the cookie on `/` but when *getting* the cookie we sometimes get a random cookie for another path that we didn't set. This is an auth failure so we loop.

We can solve this now by either looking at *all* cookies and verifying that we check the one for `/`, or we can use a new cookie name and the old ones should be irrelevant.

Fix patch:
diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go
index 1dfaeee0ff0..7f96100683c 100644
--- a/lib/api/api_auth.go
+++ b/lib/api/api_auth.go
@@ -85,14 +85,19 @@ func basicAuthAndSessionMiddleware(cookieName string, guiCfg config.GUIConfigura
 			return
 		}
 
-		cookie, err := r.Cookie(cookieName)
-		if err == nil && cookie != nil {
-			sessionsMut.Lock()
-			_, ok := sessions[cookie.Value]
-			sessionsMut.Unlock()
-			if ok {
-				next.ServeHTTP(w, r)
-				return
+		for _, cookie := range r.Cookies() {
+			// We iterate here since there may, historically, be multiple
+			// cookies with the same name but different path. Any "old" ones
+			// won't match an existing session and will be ignored, then
+			// later removed on logout or when timing out.
+			if cookie.Name == cookieName {
+				sessionsMut.Lock()
+				_, ok := sessions[cookie.Value]
+				sessionsMut.Unlock()
+				if ok {
+					next.ServeHTTP(w, r)
+					return
+				}
 			}
 		}
 
@@ -198,21 +203,26 @@ func createSession(cookieName string, username string, guiCfg config.GUIConfigur
 
 func handleLogout(cookieName string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		cookie, err := r.Cookie(cookieName)
-		if err == nil && cookie != nil {
-			sessionsMut.Lock()
-			delete(sessions, cookie.Value)
-			sessionsMut.Unlock()
+		for _, cookie := range r.Cookies() {
+			// We iterate here since there may, historically, be multiple
+			// cookies with the same name but different path. We drop them
+			// all.
+			if cookie.Name == cookieName {
+				sessionsMut.Lock()
+				delete(sessions, cookie.Value)
+				sessionsMut.Unlock()
+
+				// Delete the cookie
+				http.SetCookie(w, &http.Cookie{
+					Name:   cookieName,
+					Value:  "",
+					MaxAge: -1,
+					Secure: cookie.Secure,
+					Path:   cookie.Path,
+				})
+			}
 		}
-		// else: If there is no session cookie, that's also a successful logout in terms of user experience.
-
-		http.SetCookie(w, &http.Cookie{
-			Name:   cookieName,
-			Value:  "",
-			MaxAge: -1,
-			Secure: true,
-			Path:   "/",
-		})
+
 		w.WriteHeader(http.StatusNoContent)
 	})
 }


Test patch: