diff --git a/.github/workflows/ci-backend.yml b/.github/workflows/ci-backend.yml index 5517456447..b789f38012 100644 --- a/.github/workflows/ci-backend.yml +++ b/.github/workflows/ci-backend.yml @@ -66,13 +66,13 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v9 with: - version: "v2.13.2" + version: "v2.14.0" working-directory: backend/app - name: golangci-lint on example directory uses: golangci/golangci-lint-action@v9 with: - version: "v2.13.2" + version: "v2.14.0" args: --config ../../.golangci.yml working-directory: backend/_example/memory_store diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index d2e25363df..8892449bcd 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -1469,6 +1469,7 @@ func (s *ServerCommand) getAuthenticator(ds *service.DataStore, avas avatar.Stor SecretReader: token.SecretFunc(func(aud string) (string, error) { // get secret per site return admns.Key(aud) }), + AudienceReader: s.tokenAudiences(), ClaimsUpd: token.ClaimsUpdFunc(func(c token.Claims) token.Claims { // set attributes, on new token or refresh if c.User == nil { return c @@ -1509,6 +1510,14 @@ func (s *ServerCommand) getAuthenticator(ds *service.DataStore, avas avatar.Stor if claims.User.Audience == "" { // reject empty aud, made with old (pre 0.8.x) version of auth package return false } + // a delete-me token names the user and is signed like a session, but it is handed to the + // admin, so it must only ever work as the deletion request it is + if claims.User.BoolAttr("delete_me") { + return false + } + if !s.servesAudience(claims.Audience) { + return false + } return !claims.User.BoolAttr("blocked") }), JWTQuery: "jwt", // change default from "token" as it used for deleteme @@ -1522,6 +1531,33 @@ func (s *ServerCommand) getAuthenticator(ds *service.DataStore, avas avatar.Stor }) } +// tokenAudiences limits the audience of issued and accepted tokens to the configured sites when +// remark42 keeps the data itself. The audience is the site a user names at login, and it is echoed +// back in responses and error messages, so a value nothing serves must not be accepted at all. A +// remote store decides which sites exist on its side, so with one any audience is let through. +func (s *ServerCommand) tokenAudiences() token.Audience { + if s.Store.Type != "bolt" { + return nil + } + sites := slices.Clone(s.Sites) + return token.AudienceFunc(func() ([]string, error) { return sites, nil }) +} + +// servesAudience reports whether every audience in a token is a configured site, compared exactly. +// tokenAudiences restricts the auth package's own check, which ignores case, while the bolt store +// keys sites by the exact name +func (s *ServerCommand) servesAudience(auds []string) bool { + if s.Store.Type != "bolt" { + return true + } + for _, aud := range auds { + if !slices.Contains(s.Sites, aud) { + return false + } + } + return true +} + func (s *ServerCommand) parseSameSite(ss string) http.SameSite { switch strings.ToLower(ss) { case "default": diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 7128fd5ba4..36d7a2ffe1 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "strconv" "strings" @@ -901,10 +902,12 @@ func TestServerAuthHooks(t *testing.T) { assert.Equal(t, http.StatusCreated, resp.StatusCode, "non-blocked user able to post") // try to add comment with no-aud claim + // the token service refuses to issue it, so sign it the way a holder of the secret could badClaimsNoAud := claims badClaimsNoAud.Audience = jwt.ClaimStrings{""} - tkNoAud, err := tkService.Token(badClaimsNoAud) - require.NoError(t, err) + _, err = tkService.Token(badClaimsNoAud) + require.Error(t, err, "token service must not issue a token with an empty audience") + tkNoAud := signRawToken(t, badClaimsNoAud) t.Logf("no-aud claims: %s", tkNoAud) req, err = http.NewRequest("POST", fmt.Sprintf("http://localhost:%d/api/v1/comment?site=remark", port), strings.NewReader(`{"text": "test 123", "locator":{"url": "https://radio-t.com/p/2018/12/29/podcast-631/", @@ -918,6 +921,36 @@ func TestServerAuthHooks(t *testing.T) { require.NoError(t, resp.Body.Close()) assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "user without aud claim rejected, \n"+tkNoAud+"\n"+string(body)) + // a token for a site this instance does not serve is neither issued nor accepted + badClaimsOtherSite := claims + badClaimsOtherSite.Audience = jwt.ClaimStrings{"other"} + _, err = tkService.Token(badClaimsOtherSite) + require.Error(t, err, "token service must not issue a token for a site it does not serve") + tkOtherSite := signRawToken(t, badClaimsOtherSite) + req, err = http.NewRequest("GET", fmt.Sprintf("http://localhost:%d/api/v1/user?site=%s", port, url.QueryEscape("other")), http.NoBody) + require.NoError(t, err) + req.Header.Set("X-JWT", tkOtherSite) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "token for a site not served rejected, %s", body) + + // the auth package matches audiences ignoring case, so it issues this token, but the bolt store + // keys sites by the exact name and the session must not be accepted + tkOtherCase, err := tkService.Token(func() token.Claims { c := claims; c.Audience = jwt.ClaimStrings{"REMARK"}; return c }()) + require.NoError(t, err) + req, err = http.NewRequest("GET", fmt.Sprintf("http://localhost:%d/api/v1/user?site=REMARK", port), http.NoBody) + require.NoError(t, err) + req.Header.Set("X-JWT", tkOtherCase) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "token for a differently cased site rejected, %s", body) + // try to add comment with multiple auds badClaimsMultipleAud := claims badClaimsMultipleAud.Audience = jwt.ClaimStrings{"remark", "second_aud"} @@ -955,6 +988,37 @@ func TestServerAuthHooks(t *testing.T) { require.NoError(t, resp.Body.Close()) assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "user without user information rejected, \n"+tkNoUser+"\n"+string(body)) + // a delete-me token, as the endpoint issues it, is a deletion request for the admin and not a session + req, err = http.NewRequest(http.MethodPost, fmt.Sprintf("http://localhost:%d/api/v1/deleteme?site=remark", port), http.NoBody) + require.NoError(t, err) + req.Header.Set("X-JWT", tk) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + deleteMe := struct { + Token string `json:"token"` + }{} + require.NoError(t, json.Unmarshal(body, &deleteMe)) + require.NotEmpty(t, deleteMe.Token) + for _, probe := range []struct{ method, path, body string }{ + {http.MethodGet, "/api/v1/user?site=remark", ""}, + {http.MethodPost, "/api/v1/comment?site=remark", + `{"text": "test 123", "locator":{"url": "https://radio-t.com/p/2018/12/29/podcast-632/", "site": "remark"}}`}, + } { + req, err = http.NewRequest(probe.method, fmt.Sprintf("http://localhost:%d%s", port, probe.path), strings.NewReader(probe.body)) + require.NoError(t, err) + req.Header.Set("X-JWT", deleteMe.Token) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "delete-me token rejected on %s %s, %s", probe.method, probe.path, body) + } + // block user github_dev as admin req, err = http.NewRequest(http.MethodPut, fmt.Sprintf("http://localhost:%d/api/v1/admin/user/github_dev?site=remark&block=1&ttl=10d", port), http.NoBody) @@ -1204,15 +1268,15 @@ func waitForServerStart(t *testing.T, ports ...int) { // getRetryThrottled issues a GET and retries while the auth routes answer 429, since the /auth/ // group is limited to 2 req/s and this test logs in more often than that. a transport error is // retried a couple of times and then reported as itself, so a dead server is not read as throttling -func getRetryThrottled(t *testing.T, client *http.Client, url string) *http.Response { +func getRetryThrottled(t *testing.T, client *http.Client, reqURL string) *http.Response { t.Helper() const transportRetries = 2 errCount := 0 for deadline := time.Now().Add(serverStartTimeout); time.Now().Before(deadline); time.Sleep(authRetryPoll) { - r, err := client.Get(url) + r, err := client.Get(reqURL) if err != nil { errCount++ - require.LessOrEqual(t, errCount, transportRetries, "request to %s failed: %v", url, err) + require.LessOrEqual(t, errCount, transportRetries, "request to %s failed: %v", reqURL, err) continue } if r.StatusCode == http.StatusTooManyRequests { @@ -1221,7 +1285,7 @@ func getRetryThrottled(t *testing.T, client *http.Client, url string) *http.Resp } return r } - t.Fatalf("request to %s kept being rate limited", url) + t.Fatalf("request to %s kept being rate limited", reqURL) return nil } @@ -1240,6 +1304,52 @@ func waitForServerPort(port int, timeout time.Duration) bool { return false } +// signRawToken signs claims with the shared secret prepServerApp configures, bypassing the token +// service and the checks it makes before issuing +func signRawToken(t *testing.T, claims token.Claims) string { + t.Helper() + tkn, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte("secret")) + require.NoError(t, err) + return tkn +} + +// TestServerApp_LoginRejectsUnknownSite covers the audience a user names at login: it ends up in +// the session token and is echoed back in responses, so a site the instance does not serve gets +// no session at all. +func TestServerApp_LoginRejectsUnknownSite(t *testing.T) { + port := chooseUnusedPort(t) + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { + o.Port = port + o.Auth.Anonymous = true + return o + }) + defer cancel() + + go func() { _ = app.run(ctx) }() + waitForHTTPServerStart(t, port) + + client := http.Client{Timeout: 10 * time.Second} + defer client.CloseIdleConnections() + + resp := getRetryThrottled(t, &client, fmt.Sprintf("http://localhost:%d/auth/anonymous/login?user=blah123&aud=%s", + port, url.QueryEscape("other"))) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.NotEqual(t, http.StatusOK, resp.StatusCode, "login to a site not served must fail, %s", body) + for _, c := range resp.Cookies() { + assert.NotEqual(t, "JWT", c.Name, "no session cookie for a site not served") + } + + // the configured site still signs in + resp = getRetryThrottled(t, &client, fmt.Sprintf("http://localhost:%d/auth/anonymous/login?user=blah123&aud=remark", port)) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + cancel() + app.Wait() +} + func prepServerApp(t *testing.T, fn func(o ServerCommand) ServerCommand) (*serverApp, context.Context, context.CancelFunc) { cmd := ServerCommand{} cmd.SetCommon(CommonOpts{RemarkURL: "https://demo.remark42.com", SharedSecret: "secret"}) @@ -1351,3 +1461,33 @@ func TestServerApp_MakeCacheKeepsLargeValuesByDefault(t *testing.T) { } assert.Equal(t, 1, loads, "a 300 KB value must be served from cache with default limits") } + +func TestServerCommand_tokenAudiences(t *testing.T) { + cmd := ServerCommand{Sites: []string{"remark", "other"}} + cmd.Store.Type = "bolt" + auds := cmd.tokenAudiences() + require.NotNil(t, auds, "the bolt store knows its sites, so the audience is limited to them") + got, err := auds.Get() + require.NoError(t, err) + assert.Equal(t, []string{"remark", "other"}, got) + + cmd.Sites[0] = "changed" + got, err = auds.Get() + require.NoError(t, err) + assert.Equal(t, []string{"remark", "other"}, got, "the list is fixed when the authenticator is built") + + cmd.Store.Type = "rpc" + assert.Nil(t, cmd.tokenAudiences(), "a remote store decides which sites exist, so any audience passes") +} + +func TestServerCommand_servesAudience(t *testing.T) { + cmd := ServerCommand{Sites: []string{"remark", "other"}} + cmd.Store.Type = "bolt" + assert.True(t, cmd.servesAudience([]string{"remark"})) + assert.True(t, cmd.servesAudience([]string{"remark", "other"})) + assert.False(t, cmd.servesAudience([]string{"Remark"}), "sites are compared exactly") + assert.False(t, cmd.servesAudience([]string{"remark", "third"}), "every audience must be served") + + cmd.Store.Type = "rpc" + assert.True(t, cmd.servesAudience([]string{"Remark"}), "a remote store decides which sites exist") +} diff --git a/backend/app/rest/api/admin.go b/backend/app/rest/api/admin.go index 10a21f5bab..c376dd0857 100644 --- a/backend/app/rest/api/admin.go +++ b/backend/app/rest/api/admin.go @@ -113,6 +113,14 @@ func (a *admin) deleteMeRequestCtrl(w http.ResponseWriter, r *http.Request) { audience := claims.Audience[0] + // admin rights hold for the site the session belongs to, which matchSiteID has already matched + // against this query value. Only the basic-auth admin, who administers every site, may omit it + if site := r.URL.Query().Get("site"); site != "" && site != audience { + rest.SendErrorJSON(w, r, http.StatusForbidden, fmt.Errorf("token site %q, request site %q", audience, site), + "can't use provided token for this site", rest.ErrNoAccess) + return + } + if err = a.dataService.DeleteUserDetail(audience, claims.User.ID, engine.AllUserDetails); err != nil { code := parseError(err, rest.ErrInternal) rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't delete user details for user", code) diff --git a/backend/app/rest/api/admin_test.go b/backend/app/rest/api/admin_test.go index ceac3f3c90..2571f10f33 100644 --- a/backend/app/rest/api/admin_test.go +++ b/backend/app/rest/api/admin_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "strings" "testing" "time" @@ -19,8 +20,11 @@ import ( "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + bolt "go.etcd.io/bbolt" "github.com/umputun/remark42/backend/app/store" + adminstore "github.com/umputun/remark42/backend/app/store/admin" + "github.com/umputun/remark42/backend/app/store/engine" "github.com/umputun/remark42/backend/app/store/service" ) @@ -782,6 +786,63 @@ func TestAdmin_DeleteMeRequest(t *testing.T) { assert.NoFileExists(t, os.TempDir()+"/ava-remark42/42/pic.image", "user's avatar should be removed on deleteme") } +// an admin is an admin of the site the session belongs to, so a deletion token for another site +// served by the same instance must not be processed under that session +func TestAdmin_DeleteMeRequestRejectsOtherSite(t *testing.T) { + ts, srv, teardown := startupT(t, func(srv *Rest) { + require.NoError(t, srv.DataService.Engine.Close()) + b, err := engine.NewBoltDB(bolt.Options{}, + engine.BoltSite{FileName: filepath.Join(t.TempDir(), "remark42.db"), SiteID: "remark42"}, + engine.BoltSite{FileName: filepath.Join(t.TempDir(), "other.db"), SiteID: "other"}) + require.NoError(t, err) + srv.DataService.Engine = b + srv.DataService.AdminStore = adminstore.NewStaticStore("123456", []string{"remark42", "other"}, []string{"a1", "a2"}, "admin@remark-42.com") + }) + defer teardown() + + c := store.Comment{Text: "test test #1", Locator: store.Locator{SiteID: "other", URL: "https://radio-t.com/blah"}, + User: store.User{Name: "user1 name", ID: "user1"}} + _, err := srv.DataService.Create(c) + require.NoError(t, err) + + tkn, err := srv.Authenticator.TokenService().Token(token.Claims{ + SessionOnly: true, + RegisteredClaims: jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"other"}, + ID: "3456789", + Issuer: "remark42", + NotBefore: jwt.NewNumericDate(time.Now().Add(-1 * time.Minute)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(30 * time.Minute)), + }, + User: &token.User{ID: "user1", Attributes: map[string]any{"delete_me": true}}, + }) + require.NoError(t, err) + + // adminUmputunToken is a session for remark42 + req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/admin/deleteme?site=remark42&token=%s", ts.URL, tkn), http.NoBody) + require.NoError(t, err) + resp, err := sendReq(req, adminUmputunToken) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + comments, err := srv.DataService.User("other", "user1", 0, 0, store.User{}) + require.NoError(t, err) + assert.Len(t, comments, 1, "the other site's data is untouched") + + // the basic-auth admin administers every site and processes it when naming that site + client := http.Client{} + defer client.CloseIdleConnections() + req, err = http.NewRequest(http.MethodGet, fmt.Sprintf("%s/api/v1/admin/deleteme?site=other&token=%s", ts.URL, tkn), http.NoBody) + require.NoError(t, err) + req.SetBasicAuth("admin", "password") + resp, err = client.Do(req) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusOK, resp.StatusCode) + _, err = srv.DataService.User("other", "user1", 0, 0, store.User{}) + assert.EqualError(t, err, "no comments for user user1 in store") +} + // a delete_me request whose token carries a picture must still succeed when the avatar is // already gone from the store: the user data is deleted and a missing avatar is tolerated func TestAdmin_DeleteMeRequestMissingAvatar(t *testing.T) { @@ -949,7 +1010,7 @@ func TestAdmin_DeleteMeRequestFailed(t *testing.T) { resp, err = client.Do(req) assert.NoError(t, err) assert.NoError(t, resp.Body.Close()) - assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "the delete-me token in the query is not a session") // unknown user: deletion is idempotent, so a valid (signed) delete_me token for a user with // no stored data is a no-op success rather than an error diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 2590797d28..5f7d50f7b0 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -12,6 +12,7 @@ import ( "io" "net" "net/http" + "net/url" "strings" "time" @@ -720,7 +721,8 @@ func (s *private) deleteMeCtrl(w http.ResponseWriter, r *http.Request) { return } - link := fmt.Sprintf("%s/web/deleteme.html?token=%s", s.remarkURL, tokenStr) + // the page signs the admin's requests for the site it is given, so the link names the user's site + link := fmt.Sprintf("%s/web/deleteme.html?site_id=%s&token=%s", s.remarkURL, url.QueryEscape(siteID), tokenStr) R.RenderJSON(w, R.JSON{"site": siteID, "user_id": user.ID, "token": tokenStr, "link": link}) } diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 5e8ea4c9cc..3d3fef2205 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -1493,7 +1493,7 @@ func TestRest_DeleteMe(t *testing.T) { assert.Equal(t, "provider1_dev", claims.User.ID) assert.Equal(t, "http://example.com/pic.png", claims.User.Picture, "delete_me token must carry the user's picture so the avatar can be removed when the request is processed") - assert.Equal(t, "https://demo.remark42.com/web/deleteme.html?token="+tkn, m["link"]) + assert.Equal(t, "https://demo.remark42.com/web/deleteme.html?site_id=remark42&token="+tkn, m["link"]) req, err = http.NewRequest(http.MethodPost, fmt.Sprintf("%s/api/v1/deleteme?site=remark42", ts.URL), http.NoBody) assert.NoError(t, err) diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index b7db438730..d347b2b301 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -627,6 +627,7 @@ func startupT(t *testing.T, srvHook ...func(srv *Rest)) (ts *httptest.Server, sr AdminPasswd: "password", SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }), AvatarStore: avatar.NewLocalFS(tmp + "/ava-remark42"), + JWTQuery: "jwt", // as in the server, "token" carries the delete-me token }), Cache: memCache, WebRoot: tmp, diff --git a/frontend/apps/remark42/app/deleteme.test.ts b/frontend/apps/remark42/app/deleteme.test.ts new file mode 100644 index 0000000000..2c24035aeb --- /dev/null +++ b/frontend/apps/remark42/app/deleteme.test.ts @@ -0,0 +1,71 @@ +import type * as Api from 'common/api'; + +jest.mock('common/api', () => ({ + getUser: jest.fn(), + approveDeleteMe: jest.fn(), +})); + +const payload = ''; + +/** loads the entry script against a fresh root node and waits for it to finish rendering */ +async function runPage(): Promise { + process.env.REMARK_NODE = 'remark42'; + document.body.innerHTML = '
'; + const root = document.getElementById('remark42')!; + + await jest.isolateModulesAsync(async () => { + await import('./deleteme'); + }); + // getUser and approveDeleteMe resolve on separate turns before the page renders + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + + return root; +} + +function mockedApi(): jest.Mocked { + return jest.requireMock('common/api'); +} + +describe('deleteme page', () => { + beforeEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + jest.spyOn(console, 'error').mockImplementation(() => {}); + window.history.replaceState(null, '', '/web/deleteme.html?token=t'); + mockedApi().getUser.mockResolvedValue({ admin: true } as Awaited>); + }); + + it('shows an error from the server as text', async () => { + // the shape the backend returns when the token names a site it does not serve + mockedApi().approveDeleteMe.mockRejectedValue({ code: 0, error: `site "${payload}" not found` }); + + const root = await runPage(); + + expect(root.querySelector('img')).toBeNull(); + expect(root.querySelector('pre')?.textContent).toBe(`site "${payload}" not found`); + }); + + it('asks a signed-out admin to sign in to the site the request came from', async () => { + mockedApi().getUser.mockResolvedValue(null); + window.history.replaceState(null, '', `/web/deleteme.html?site_id=${encodeURIComponent(payload)}&token=t`); + + const root = await runPage(); + + expect(root.querySelector('img')).toBeNull(); + expect(root.querySelector('h3')?.textContent).toBe('You are not logged in'); + expect(root.querySelector('pre')?.textContent).toContain(`site "${payload}"`); + expect(mockedApi().approveDeleteMe).not.toHaveBeenCalled(); + }); + + it('shows the deletion result as text', async () => { + mockedApi().approveDeleteMe.mockResolvedValue({ user_id: 'dev_user', site_id: [payload] } as never); + + const root = await runPage(); + + expect(root.querySelector('img')).toBeNull(); + expect(root.querySelector('h3')?.textContent).toBe('User deleted successfully'); + expect(root.querySelector('pre')?.textContent).toContain(payload); + }); +}); diff --git a/frontend/apps/remark42/app/deleteme.ts b/frontend/apps/remark42/app/deleteme.ts index 2a0e90e006..cb15816e1a 100644 --- a/frontend/apps/remark42/app/deleteme.ts +++ b/frontend/apps/remark42/app/deleteme.ts @@ -1,6 +1,6 @@ import { NODE_ID } from 'common/constants'; import { approveDeleteMe, getUser } from 'common/api'; -import { token } from 'common/settings'; +import { siteId, token } from 'common/settings'; import type { ApiError } from 'common/types'; if (document.readyState === 'loading') { @@ -23,29 +23,38 @@ async function init(): Promise { return; } + // everything below reaches the page as text. the token is supplied by whoever sent the link, + // and both the response and the error echo values it carries, its site id among them approveDeleteMe(token).then( (data) => { - node.innerHTML = ` -

User deleted successfully

-
${JSON.stringify(data, null, 4)}
- `; + render(node, 'User deleted successfully', JSON.stringify(data, null, 4)); }, (err: Error | ApiError | string) => { const message = err instanceof Error ? err.message : typeof err === 'object' && err !== null && err.error ? err.error : err; console.error(err); - node.innerHTML = ` -

Something went wrong

-
${message}
- `; + render(node, 'Something went wrong', String(message)); } ); }); } +function render(node: HTMLElement, heading: string, details: string): void { + const title = document.createElement('h3'); + title.textContent = heading; + + const body = document.createElement('pre'); + body.textContent = details; + + node.replaceChildren(title, body); +} + function handleNotAuthorizedError(node: HTMLElement): void { - node.innerHTML = ` -

You are not logged in

-

Sign in as admin to delete user information

- `; + // the session has to belong to the site the request came from, so the admin signs in through the + // comments on that site rather than the demo page, which is fixed to a site of its own + render( + node, + 'You are not logged in', + `Sign in as admin to the comments on site "${siteId}", then open this link again to delete user information.` + ); } diff --git a/site/content/docs/contributing/api/index.md b/site/content/docs/contributing/api/index.md index f11083109b..53b005c21c 100644 --- a/site/content/docs/contributing/api/index.md +++ b/site/content/docs/contributing/api/index.md @@ -231,6 +231,6 @@ http://oldsite.com/from-old-page/1 https://newsite.com/to-new-page/1 - `DELETE /api/v1/admin/user/{userid}?site=site-id` - delete the user's comments and stored details; succeeds even if the user has no comments or is already absent - `PUT /api/v1/admin/readonly?site=site-id&url=post-url&ro=1` - set read-only status - `PUT /api/v1/admin/verify/{userid}?site=site-id&verified=1` - set verified status -- `GET /api/v1/admin/deleteme?token=token` - process a user's deleteme request; already-deleted or dataless users return success (idempotent) +- `GET /api/v1/admin/deleteme?site=site-id&token=token` - process a user's deleteme request; already-deleted or dataless users return success (idempotent) _all admin calls require auth and admin privilege_