Fix the /web demo on instances that do not serve site remark

backend/app/cmd/server.go

@@ -741,6 +741,7 @@ func (s *ServerCommand) newServerApp(ctx context.Context) (*serverApp, error) {
 		WebRoot:                    s.WebRoot,
 		WebFS:                      webFS,
 		RemarkURL:                  s.RemarkURL,
+		DemoSiteID:                 s.demoSiteID(),
 		ImageProxy:                 imgProxy,
 		CommentFormatter:           commentFormatter,
 		Migrator:                   migr,
@@ -1558,6 +1559,18 @@ func (s *ServerCommand) servesAudience(auds []string) bool {
 	return true
 }
 
+// demoSiteID is the site the demo pages under /web show: the first configured one, as the docker
+// image picks it from SITE. It is returned exactly as configured, since the stores register it
+// that way and look it up by exact match
+func (s *ServerCommand) demoSiteID() string {
+	for _, site := range s.Sites {
+		if strings.TrimSpace(site) != "" {
+			return site
+		}
+	}
+	return ""
+}
+
 func (s *ServerCommand) parseSameSite(ss string) http.SameSite {
 	switch strings.ToLower(ss) {
 	case "default":

backend/app/cmd/server_test.go

@@ -1494,6 +1494,24 @@ func TestServerCommand_servesAudience(t *testing.T) {
 	assert.True(t, cmd.servesAudience([]string{"Remark"}), "a remote store decides which sites exist")
 }
 
+func TestServerCommand_demoSiteID(t *testing.T) {
+	tbl := []struct {
+		sites []string
+		want  string
+	}{
+		{[]string{"remark"}, "remark"},
+		{[]string{"myblog", "other"}, "myblog"},
+		// the stores register a site exactly as given, so the demo pages must name it the same way
+		{[]string{" myblog "}, " myblog "},
+		{[]string{"  ", "second"}, "second"},
+		{[]string{"", "second"}, "second"},
+		{nil, ""},
+	}
+	for _, tt := range tbl {
+		assert.Equal(t, tt.want, (&ServerCommand{Sites: tt.sites}).demoSiteID(), "sites %q", tt.sites)
+	}
+}
+
 // TestServerApp_UnsubscribeTokenIsNotALogin covers the token in unsubscribe links: it names the user
 // and their email, is signed with the shared secret and is valid for a century, so the email login
 // must not accept it in place of a confirmation token it issued itself.

backend/app/rest/api/rest.go

@@ -47,6 +47,7 @@ type Rest struct {
 	WebRoot         string
 	WebFS           fs.FS
 	RemarkURL       string
+	DemoSiteID      string // site the demo pages under /web show, the first configured one
 	ReadOnlyAge     int
 	SharedSecret    string
 	TrustedProxies  []*net.IPNet // reverse-proxy networks whose forwarding headers (X-Real-IP, X-Forwarded-For, ...) are trusted
@@ -398,7 +399,7 @@ func (s *Rest) routes() http.Handler {
 		log.Printf("[WARN] no embedded frontend, serving built-in assets only: %v", err)
 		embeddedFrontend = emptyFS{}
 	}
-	addFileServer(router, embeddedFrontend, s.WebRoot, s.Version, s.RemarkURL)
+	addFileServer(router, embeddedFrontend, s.WebRoot, s.Version, s.RemarkURL, s.DemoSiteID)
 	return router
 }
 
@@ -514,7 +515,7 @@ func (s *Rest) configCtrl(w http.ResponseWriter, r *http.Request) {
 // serves /web from the frontend build, falling back to the assets embedded in the binary for
 // names the build does not produce. the frontend build is read from webRoot on disk, or from the
 // copy embedded at app/cmd/web when that directory is absent.
-func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, version, remarkURL string) {
+func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, version, remarkURL, siteID string) {
 	frontendFS := embeddedFrontend
 
 	if _, err := os.Stat(webRoot); err == nil {
@@ -529,17 +530,18 @@ func addFileServer(r *routegroup.Bundle, embeddedFrontend fs.FS, webRoot, versio
 	sources := templatedFS{
 		fs:        webFiles{frontend: frontendFS, embedded: webassets.FS},
 		remarkURL: remarkURL,
+		siteID:    siteID,
 	}
 	webFS := http.StripPrefix("/web", http.FileServer(http.FS(sources)))
 	r.HandleFunc("GET /web", http.RedirectHandler("/web/", http.StatusMovedPermanently).ServeHTTP)
 
 	r.With(rateLimiter(20),
 		R.Timeout(10*time.Second),
-		// the served body now depends on remarkURL, so it has to be part of the validator. Without
-		// it an operator who corrects a wrong REMARK_URL and restarts the same binary keeps getting
-		// 304 on revalidation, and the client keeps a bundle addressed to the old host for good,
+		// the served body now depends on remarkURL and the demo site, so both have to be part of the
+		// validator. Without them an operator who corrects REMARK_URL or SITE and restarts the same
+		// binary keeps getting 304 on revalidation, and the client keeps the old page for good,
 		// since no-cache means it revalidates rather than aging out
-		cacheControl(time.Hour, version+":"+remarkURL),
+		cacheControl(time.Hour, version+":"+remarkURL+":"+siteID),
 	).HandleFunc("GET /web/", func(w http.ResponseWriter, r *http.Request) {
 		// don't show dirs, just serve files
 		if strings.HasSuffix(r.URL.Path, "/") && len(r.URL.Path) > 1 && r.URL.Path != ("/web/") {

backend/app/rest/api/rest_test.go

@@ -189,7 +189,7 @@ func TestRest_FileServerBackendAssets(t *testing.T) {
 func TestRest_FileServerEmbeddedFrontend(t *testing.T) {
 	frontend := fstest.MapFS{"index.html": {Data: []byte("embedded frontend index")}}
 	router := routegroup.New(http.NewServeMux())
-	addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com")
+	addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com", "")
 
 	ts := httptest.NewServer(router)
 	defer ts.Close()

backend/app/rest/api/webfiles.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"io"
 	"io/fs"
@@ -77,12 +78,19 @@ func (emptyFS) Open(name string) (fs.File, error) {
 // serves the build embedded in itself and has nothing to rewrite, does it here.
 const remarkURLPlaceholder = "{% REMARK_URL %}"
 
+// demoSiteLiteral is how the built demo, counter and last-comments pages name their site. The
+// docker image replaces it with the first configured site at container start; the binary does it
+// here, since a site the instance does not serve makes those pages fail and, signed in or not, ask
+// the API about a site the reader's session does not belong to.
+const demoSiteLiteral = `site_id:"remark"`
+
 // templatedFS fills the instance URL into the files carrying the placeholder. Without it the
 // binary serves whatever the build baked in, which is a host no visitor can reach, and the widget
 // falls back to it whenever a page omits remark_config.host.
 type templatedFS struct {
 	fs        fs.FS
 	remarkURL string
+	siteID    string // first configured site, filled into the demo pages; empty leaves them as built
 }
 
 // Open substitutes in the file types the frontend templates, and hands everything else through
@@ -107,9 +115,22 @@ func (t templatedFS) Open(name string) (fs.File, error) {
 	}
 
 	body = bytes.ReplaceAll(body, []byte(remarkURLPlaceholder), []byte(t.remarkURL))
+	if t.siteID != "" && filepath.Ext(name) == ".html" {
+		body = bytes.ReplaceAll(body, []byte(demoSiteLiteral), demoSiteValue(t.siteID))
+	}
 	return &memFile{Reader: bytes.NewReader(body), info: sizedInfo{FileInfo: info, size: int64(len(body))}}, nil
 }
 
+// demoSiteValue renders the site as the demo pages' literal does. json.Marshal quotes it as a
+// script string and escapes <, > and &, so no site id can end the string or the script block
+func demoSiteValue(siteID string) []byte {
+	quoted, err := json.Marshal(siteID)
+	if err != nil { // a string always marshals; keep the build's own literal if that ever changes
+		return []byte(demoSiteLiteral)
+	}
+	return append([]byte("site_id:"), quoted...)
+}
+
 // templatedName reports whether the frontend templates this file type. It mirrors the set the
 // docker image rewrites, so both distributions substitute in the same files
 func templatedName(name string) bool {

backend/app/rest/api/webfiles_test.go

@@ -266,6 +266,52 @@ func TestTemplatedFS_SubstitutesTheInstanceURL(t *testing.T) {
 	}
 }
 
+// TestTemplatedFS_FillsTheDemoSite covers the demo, counter and last-comments pages, which the build
+// emits for site "remark". The docker image rewrites that literal in html files at container start,
+// and the binary has to do the same, or an instance serving another site answers those pages with
+// "site not found" and their widget asks the API about a site the reader is not signed in to.
+func TestTemplatedFS_FillsTheDemoSite(t *testing.T) {
+	const built = `var remark_config={site_id:"remark",host:"` + remarkURLPlaceholder + `"}`
+	source := fstest.MapFS{
+		"index.html":  {Data: []byte(built)},
+		"counter.mjs": {Data: []byte(built)},
+		"custom.html": {Data: []byte(`var remark_config={site_id:"mine"}`)},
+	}
+
+	tbl := []struct {
+		name, file, siteID, want string
+	}{
+		{"html gets the site", "index.html", "myblog",
+			`var remark_config={site_id:"myblog",host:"https://remark.example.com"}`},
+		{"no site leaves the build's", "index.html", "",
+			`var remark_config={site_id:"remark",host:"https://remark.example.com"}`},
+		{"the site cannot end the string or the script", "index.html", `x"</script><script>alert(1)//`,
+			`var remark_config={site_id:"x\"\u003c/script\u003e\u003cscript\u003ealert(1)//",host:"https://remark.example.com"}`},
+		// the docker image rewrites the site in html only, so a script keeps the literal here too
+		{"scripts are left alone", "counter.mjs", "myblog",
+			`var remark_config={site_id:"remark",host:"https://remark.example.com"}`},
+		// only the build's own default is replaced, never a site an operator wrote into a page
+		{"another site is left alone", "custom.html", "myblog", `var remark_config={site_id:"mine"}`},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			tfs := templatedFS{fs: source, remarkURL: "https://remark.example.com", siteID: tt.siteID}
+			f, err := tfs.Open(tt.file)
+			require.NoError(t, err)
+			defer func() { assert.NoError(t, f.Close()) }()
+
+			body, err := io.ReadAll(f)
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, string(body))
+
+			info, err := f.Stat()
+			require.NoError(t, err)
+			assert.Equal(t, int64(len(tt.want)), info.Size())
+		})
+	}
+}
+
 func TestTemplatedFS_PassesErrorsThrough(t *testing.T) {
 	tfs := templatedFS{fs: fstest.MapFS{}, remarkURL: "https://remark.example.com"}
 
@@ -282,7 +328,7 @@ func TestRest_FileServerFillsInTheInstanceURL(t *testing.T) {
 		"plain.html": {Data: []byte("nothing to fill in")},
 	}
 	router := routegroup.New(http.NewServeMux())
-	addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com")
+	addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com", "")
 
 	ts := httptest.NewServer(router)
 	defer ts.Close()
@@ -322,7 +368,7 @@ func TestRest_FileServerEtagVariesWithTheInstanceURL(t *testing.T) {
 
 	etagFor := func(remarkURL string) string {
 		router := routegroup.New(http.NewServeMux())
-		addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", remarkURL)
+		addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", remarkURL, "")
 		ts := httptest.NewServer(router)
 		defer ts.Close()
 
@@ -341,3 +387,26 @@ func TestRest_FileServerEtagVariesWithTheInstanceURL(t *testing.T) {
 		"same version and same path, different instance url: the validator has to change or the "+
 			"client keeps a bundle addressed to the old host")
 }
+
+// TestRest_FileServerEtagVariesWithTheDemoSite is the same case for SITE: the demo pages carry the
+// first configured site, so an operator who changes it must not keep getting the old page on 304
+func TestRest_FileServerEtagVariesWithTheDemoSite(t *testing.T) {
+	frontend := fstest.MapFS{"index.html": {Data: []byte(`site_id:"remark"`)}}
+
+	etagFor := func(siteID string) string {
+		router := routegroup.New(http.NewServeMux())
+		addFileServer(router, frontend, filepath.Join(t.TempDir(), "absent"), "test-version", "https://remark.example.com", siteID)
+		ts := httptest.NewServer(router)
+		defer ts.Close()
+
+		resp, err := http.Get(ts.URL + "/web/index.html")
+		require.NoError(t, err)
+		defer resp.Body.Close()
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		return resp.Header.Get("Etag")
+	}
+
+	first := etagFor("old")
+	require.NotEmpty(t, first)
+	assert.NotEqual(t, first, etagFor("new"), "a different demo site has to change the validator")
+}

frontend/apps/remark42/app/common/fetcher.test.ts

@@ -246,7 +246,7 @@ describe('fetcher', () => {
       consoleErrorSpy.mockRestore();
     });
 
-    it('should reset activeJwtToken and clear cookies on 401/403 responses', async () => {
+    it('should reset activeJwtToken and clear cookies on 401 responses', async () => {
       // Mock clearAuthCookie
       jest.spyOn(cookies, 'clearAuthCookie').mockImplementationOnce(jest.fn());
 
@@ -277,6 +277,31 @@ describe('fetcher', () => {
 
       expect(window.fetch).toHaveBeenCalled();
     });
+
+    it('should keep the session on 403 responses', async () => {
+      const clear = jest.spyOn(cookies, 'clearAuthCookie').mockImplementation(jest.fn());
+      jest.spyOn(window, 'atob').mockReturnValueOnce(JSON.stringify({ jti: 'test-jti-id' }));
+
+      mockFetch({ headers: { [JWT_HEADER]: 'kept.jwt.token', ...headers } });
+      await apiFetcher.get(apiUri);
+
+      // a session for another site asking about this one, or an anonymous user, is refused, not signed out
+      mockFetch({ status: 403 });
+      await expect(apiFetcher.get(apiUri)).rejects.toEqual(new RequestError('Forbidden.', 403));
+
+      expect(clear).not.toHaveBeenCalled();
+
+      mockFetch({ headers });
+      await apiFetcher.get(apiUri);
+      const { calls } = (window.fetch as jest.Mock).mock;
+      const [, init] = calls[calls.length - 1];
+      expect(init.headers[JWT_HEADER]).toBe('kept.jwt.token');
+
+      // the token lives at module level, so drop it the way the fetcher does before the next test
+      mockFetch({ status: 401 });
+      await expect(apiFetcher.get(apiUri)).rejects.toEqual(new RequestError('Not authorized.', 401));
+      clear.mockRestore();
+    });
   });
 
   describe('send data', () => {

frontend/apps/remark42/app/common/fetcher.ts

@@ -89,10 +89,11 @@ const createFetcher = (baseUrl: string = ''): Methods => {
         }
       }
 
-      if ([401, 403].includes(res.status)) {
-        activeJwtToken = undefined;
-        clearAuthCookie(JWT_COOKIE_NAME);
-        clearAuthCookie(XSRF_COOKIE);
+      // only 401 says the session itself is gone. A 403 comes with a valid session that may not do
+      // this, such as one belonging to another site served from the same remark42 host, and
+      // clearing on it signs the reader out of every site sharing these cookies
+      if (res.status === 401) {
+        dropSession();
       }
 
       if (res.status >= 400) {
@@ -135,6 +136,13 @@ const createFetcher = (baseUrl: string = ''): Methods => {
   } as Methods;
 };
 
+/** Forgets the session this document holds: the in-memory JWT and the auth cookies it can reach */
+export function dropSession(): void {
+  activeJwtToken = undefined;
+  clearAuthCookie(JWT_COOKIE_NAME);
+  clearAuthCookie(XSRF_COOKIE);
+}
+
 export const apiFetcher = createFetcher(`${BASE_URL}${API_BASE}`);
 export const authFetcher = createFetcher(`${BASE_URL}/auth`);
 export const adminFetcher = createFetcher(`${BASE_URL}${API_BASE}/admin`);

frontend/apps/remark42/app/components/auth/auth.api.spec.ts

@@ -1,4 +1,6 @@
-import { oauthSignin } from './auth.api';
+import { apiFetcher, JWT_HEADER } from 'common/fetcher';
+
+import { logout, oauthSignin } from './auth.api';
 
 /**
  * The OAuth flow has no message from the popup to listen for: the popup closes itself and the
@@ -116,3 +118,32 @@ describe('oauthSignin', () => {
     await assertion;
   });
 });
+
+describe('logout', () => {
+  beforeEach(() => {
+    fetchMock.resetMocks();
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('forgets a header-held session even when the server refuses the logout', async () => {
+    // a token delivered as a header is what the fetcher keeps in memory and sends back
+    jest.spyOn(window, 'atob').mockReturnValueOnce(JSON.stringify({ jti: 'jti' }));
+    fetchMock.mockResponseOnce('{}', {
+      headers: { [JWT_HEADER]: 'held.jwt.token', 'Content-Type': 'application/json' },
+    });
+    await apiFetcher.get('/anything');
+
+    // an expired header token is refused with 403 by the logout handler
+    fetchMock.mockResponseOnce(JSON.stringify({ error: 'logout not allowed' }), { status: 403 });
+    // resolving lets the caller clear the signed-in state as well
+    await expect(logout()).resolves.toBeUndefined();
+
+    fetchMock.mockResponseOnce('{}', { headers: { 'Content-Type': 'application/json' } });
+    await apiFetcher.get('/anything');
+    const [, init] = fetchMock.mock.calls[fetchMock.mock.calls.length - 1];
+    expect((init?.headers as Record<string, string>)[JWT_HEADER]).toBeUndefined();
+  });
+});

frontend/apps/remark42/app/components/auth/auth.api.ts

@@ -1,6 +1,6 @@
 import type { User } from 'common/types';
 
-import { authFetcher } from 'common/fetcher';
+import { authFetcher, dropSession } from 'common/fetcher';
 import { siteId } from 'common/settings';
 import { getUser } from 'common/api';
 
@@ -121,6 +121,15 @@ export function verifyTelegramSignin(token: string): Promise<User> {
   return authFetcher.get(TELEGRAM_SIGNIN_ENDPOINT, { token });
 }
 
+/**
+ * Signs out, and forgets the session locally whatever the server answers. With the JWT sent as a
+ * header, an expired token is refused with 403, and a fetcher that kept it would sign the reader
+ * back in on the next page load. The refusal is not passed on either: callers clear the signed-in
+ * state only once this resolves, and the credentials are gone by then
+ */
 export function logout(): Promise<void> {
-  return authFetcher.get('/logout');
+  return authFetcher
+    .get<void>('/logout')
+    .catch(() => undefined)
+    .finally(dropSession);
 }