diff --git a/README.md b/README.md index 5c78547..f09ceaf 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,8 @@ Apple Reminders, forward mail into it, and add it to Claude as a connector. | Web interface: sign-in, views, quick add, ⌘K, shortcuts | Done | | Task detail: description, sub-tasks, comments, files, reminders | Done | | Settings: account, notifications, integrations, AI, tokens, data | Done | +| Device list: where you are signed in, and signing one out | Done, tested | +| Invite and password-reset links that open the right page | Done, smoke-tested | | Drag to reorder, in both the list and the board | Done | | Drag a task to another project or another day | Done, smoke-tested | | Foldable project groups in the sidebar | Done, tested | diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 5964bba..44205e8 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -351,6 +351,44 @@ paths: "204": { description: Off. } "422": { $ref: "#/components/responses/Validation" } + /api/v1/auth/sessions: + get: + tags: [auth] + summary: Where you are signed in. + description: | + Most recently used first. `id` is the stored hash of the cookie and never + the cookie itself — it cannot be pasted into a browser, and it is what the + delete below takes. + + Session only: an API token cannot list the devices of the person it was + stolen from. + responses: + "200": + description: OK + content: + application/json: + schema: + type: object + properties: + sessions: + type: array + items: { $ref: "#/components/schemas/Session" } + "403": { $ref: "#/components/responses/Forbidden" } + + /api/v1/auth/sessions/{sessionID}: + parameters: + - { name: sessionID, in: path, required: true, schema: { type: string } } + delete: + tags: [auth] + summary: Sign one device out. + description: | + Ending the current session is allowed and is simply logging out — + somebody clearing every device wants this one gone too. + responses: + "204": { description: Ended. } + "403": { $ref: "#/components/responses/Forbidden" } + "404": { $ref: "#/components/responses/NotFound" } + /api/v1/auth/recovery-codes: get: tags: [auth] @@ -2232,6 +2270,27 @@ components: type: array items: { type: string } + Session: + type: object + properties: + id: { type: string } + current: + type: boolean + description: The one making this request. + device: + type: string + description: | + A summary of the user agent — "Chrome på macOS". A handful of + substring tests rather than a parsing library: the question it + answers is "is one of these me?", and the raw string is sent beside + it so anything the summary gets wrong is still visible. + example: Chrome på macOS + user_agent: { type: string } + ip: { type: string } + created_at: { type: string, format: date-time } + last_seen_at: { type: string, format: date-time } + expires_at: { type: string, format: date-time } + Task: type: object properties: diff --git a/internal/httpapi/auth_test.go b/internal/httpapi/auth_test.go index 5a1e73f..633510c 100644 --- a/internal/httpapi/auth_test.go +++ b/internal/httpapi/auth_test.go @@ -761,3 +761,137 @@ func TestUnknownSessionTokenIsUnauthorized(t *testing.T) { t.Errorf("status %d, want 401", resp.StatusCode) } } + +// last_seen_at has been written on every request since sessions existed, for +// exactly this list, and nothing read it. A session you cannot see is a session +// you cannot end. +func TestSessionsCanBeListedAndEnded(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + // A second sign-in from "another device": same account, its own cookie jar. + other := &testServer{Server: ts.Server, db: ts.db, client: newJarClient(t)} + resp, _ := other.do(t, "POST", "/api/v1/auth/login", map[string]string{ + "email": "kristian@example.dk", "password": "et langt kodeord", + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("second login: status %d", resp.StatusCode) + } + + resp, body := ts.do(t, "GET", "/api/v1/auth/sessions", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("list sessions: %d %v", resp.StatusCode, body) + } + sessions, _ := body["sessions"].([]any) + if len(sessions) != 2 { + t.Fatalf("want two sessions, got %v", sessions) + } + + // Exactly one is this one, or the interface cannot tell somebody which row is + // the browser they are reading it in. + current := "" + for _, raw := range sessions { + s := raw.(map[string]any) + if s["current"] == true { + if current != "" { + t.Fatal("two sessions both claim to be the current one") + } + current = s["id"].(string) + } + if s["device"] == "" { + t.Errorf("no device summary: %v", s) + } + if s["last_seen_at"] == nil { + t.Errorf("no last_seen_at, which is the whole reason the column is written: %v", s) + } + } + if current == "" { + t.Fatal("none of the sessions is marked as the current one") + } + + // End the other one. + var otherID string + for _, raw := range sessions { + if s := raw.(map[string]any); s["id"] != current { + otherID = s["id"].(string) + } + } + if resp, _ := ts.do(t, "DELETE", "/api/v1/auth/sessions/"+otherID, nil); resp.StatusCode != http.StatusNoContent { + t.Fatalf("delete session: status %d", resp.StatusCode) + } + + // That device is signed out, and this one is not. + if r, _ := other.do(t, "GET", "/api/v1/auth/me", nil); r.StatusCode != http.StatusUnauthorized { + t.Errorf("the ended session still works: status %d", r.StatusCode) + } + if r, _ := ts.do(t, "GET", "/api/v1/auth/me", nil); r.StatusCode != http.StatusOK { + t.Errorf("ending another session logged this one out: status %d", r.StatusCode) + } +} + +// Somebody else's session is not yours to end, even with its id in hand. +func TestASessionCanOnlyBeEndedByItsOwner(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + other := ts.newUser(t, "anden@example.dk", "Anden") + + _, body := ts.do(t, "GET", "/api/v1/auth/sessions", nil) + mine := body["sessions"].([]any)[0].(map[string]any)["id"].(string) + + if resp, _ := other.do(t, "DELETE", "/api/v1/auth/sessions/"+mine, nil); resp.StatusCode != http.StatusNotFound { + t.Errorf("status %d, want 404", resp.StatusCode) + } + // And it still works. + if r, _ := ts.do(t, "GET", "/api/v1/auth/me", nil); r.StatusCode != http.StatusOK { + t.Errorf("somebody else ended my session: status %d", r.StatusCode) + } +} + +// An API token is accepted almost everywhere, and must not be accepted here: a +// leaked token that could end its owner's sessions, or even list their devices, +// turns a theft into a lockout. +func TestSessionsAreNotReachableWithAToken(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + user, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + token := ts.apiToken(t, user.ID) + + req, _ := http.NewRequest("GET", ts.URL+"/api/v1/auth/sessions", nil) + req.Header.Set("Authorization", "Bearer "+token) + resp, err := (&http.Client{}).Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("status %d, want 403", resp.StatusCode) + } +} + +func TestUserAgentsAreSummarisedForAPerson(t *testing.T) { + cases := []struct { + ua string + want string + }{ + {"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0 Safari/537.36", "Chrome på macOS"}, + // An iPhone's user agent says Mac OS X too, so the order of the tests is + // the whole correctness of this function. + {"Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1", "Safari på iPhone"}, + // And Edge claims to be Chrome, which claims to be Safari. + {"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 Chrome/131.0 Safari/537.36 Edg/131.0", "Edge på Windows"}, + {"Mozilla/5.0 (X11; Linux x86_64; rv:133.0) Gecko/20100101 Firefox/133.0", "Firefox på Linux"}, + {"", "Ukendt enhed"}, + // Not a browser at all. Its own name beats a word that says the server + // gave up: a CalDAV client is a real session. + {"curl/8.4.0", "curl/8.4.0"}, + } + for _, c := range cases { + if got := describeUserAgent(c.ua); got != c.want { + t.Errorf("describeUserAgent(%q) = %q, want %q", c.ua, got, c.want) + } + } +} diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 007b78c..aaea91a 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -151,6 +151,15 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server r.Post("/totp/disable", s.handleTOTPDisable) r.Get("/recovery-codes", s.handleRecoveryCodesCount) r.Post("/recovery-codes", s.handleRecoveryCodesRegenerate) + + // Behind requireSession, like the API tokens and for the same + // reason: a leaked token must not be able to end the sessions of + // the person it was stolen from, or list their devices. + r.Group(func(r chi.Router) { + r.Use(s.requireSession) + r.Get("/sessions", s.handleListSessions) + r.Delete("/sessions/{sessionID}", s.handleDeleteSession) + }) }) }) diff --git a/internal/httpapi/sessions_handlers.go b/internal/httpapi/sessions_handlers.go new file mode 100644 index 0000000..9891236 --- /dev/null +++ b/internal/httpapi/sessions_handlers.go @@ -0,0 +1,147 @@ +package httpapi + +import ( + "net/http" + "strings" + "time" + + "github.com/go-chi/chi/v5" +) + +// Where a person is signed in, and how to end one of them. +// +// `last_seen_at` has been written on every request since sessions existed — +// throttled to once a minute, precisely so this list could say "this device, 2 +// minutes ago" — and nothing has ever read it. A session you cannot see is a +// session you cannot end. + +type sessionJSON struct { + ID string `json:"id"` + // Current marks the one making the request, so the interface can say "denne + // enhed" rather than making somebody work it out from a user agent. + Current bool `json:"current"` + // Device is a summary; UserAgent is what it was summarised from. Both, because + // the summary is what you read and the original is what settles the question + // when the summary is not enough. + Device string `json:"device"` + UserAgent string `json:"user_agent"` + IP string `json:"ip"` + CreatedAt string `json:"created_at"` + LastSeenAt string `json:"last_seen_at"` + ExpiresAt string `json:"expires_at"` +} + +// handleListSessions lists the caller's live sessions, most recently used first. +// +// The id here is the stored hash of the cookie, never the cookie itself: it cannot +// be pasted into a browser, and it is what the delete below needs. +func (s *Server) handleListSessions(w http.ResponseWriter, r *http.Request) { + user := userFrom(r.Context()) + current := sessionFrom(r.Context()) + + sessions, err := s.db.ListSessions(r.Context(), user.ID) + if err != nil { + s.internal(w, "list sessions", err) + return + } + + out := make([]sessionJSON, 0, len(sessions)) + for _, sess := range sessions { + out = append(out, sessionJSON{ + ID: sess.ID, + Current: current != nil && sess.ID == current.ID, + Device: describeUserAgent(sess.UA), + UserAgent: sess.UA, + IP: sess.IP, + CreatedAt: sess.CreatedAt.Format(time.RFC3339), + LastSeenAt: sess.LastSeenAt.Format(time.RFC3339), + ExpiresAt: sess.ExpiresAt.Format(time.RFC3339), + }) + } + writeJSON(w, http.StatusOK, map[string]any{"sessions": out}) +} + +// handleDeleteSession signs one device out. Ending the current one is allowed and +// is simply logging out — refusing it would be a rule with no reason behind it, +// and somebody clearing every device wants this one gone too. +func (s *Server) handleDeleteSession(w http.ResponseWriter, r *http.Request) { + user := userFrom(r.Context()) + sessionID := chi.URLParam(r, "sessionID") + + if err := s.db.DeleteUserSession(r.Context(), sessionID, user.ID); err != nil { + s.storeError(w, "delete session", err) + return + } + if current := sessionFrom(r.Context()); current != nil && current.ID == sessionID { + s.clearSessionCookie(w) + } + w.WriteHeader(http.StatusNoContent) +} + +// describeUserAgent turns a user-agent string into something a person recognises. +// +// Deliberately a handful of substring tests rather than a parsing library. The +// question this answers is "is one of these me?", and for that a wrong guess +// between two Chromium forks costs nothing while a dependency that has to be kept +// current costs something every year. The raw string is sent alongside, so +// anything this gets wrong is still visible. +// +// Order matters: every browser's user agent claims to be several other browsers, +// so the most specific name has to be tested first. +func describeUserAgent(ua string) string { + if strings.TrimSpace(ua) == "" { + return "Ukendt enhed" + } + + browser := "" + switch { + case strings.Contains(ua, "Edg/"): + browser = "Edge" + case strings.Contains(ua, "OPR/"), strings.Contains(ua, "Opera"): + browser = "Opera" + case strings.Contains(ua, "Firefox/"): + browser = "Firefox" + case strings.Contains(ua, "Chrome/"): + browser = "Chrome" + case strings.Contains(ua, "Safari/"): + browser = "Safari" + } + + system := "" + switch { + // Before "Mac OS X": an iPhone's user agent says both. + case strings.Contains(ua, "iPhone"): + system = "iPhone" + case strings.Contains(ua, "iPad"): + system = "iPad" + case strings.Contains(ua, "Android"): + system = "Android" + case strings.Contains(ua, "Mac OS X"), strings.Contains(ua, "Macintosh"): + system = "macOS" + case strings.Contains(ua, "Windows"): + system = "Windows" + case strings.Contains(ua, "Linux"): + system = "Linux" + } + + switch { + case browser != "" && system != "": + return browser + " på " + system + case browser != "": + return browser + case system != "": + return system + } + // Nothing recognised: show the string itself rather than "Ukendt". A CalDAV + // client or a script is a real session, and its own name is more useful than a + // word that says the server gave up. + return truncateRunes(ua, 60) +} + +func truncateRunes(s string, max int) string { + runes := []rune(s) + if len(runes) <= max { + return s + } + return string(runes[:max]) + "…" +} diff --git a/internal/store/sessions.go b/internal/store/sessions.go index 0f88fd3..c8b0fe5 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -123,6 +123,23 @@ func (db *DB) DeleteSession(ctx context.Context, sessionID string) error { return err } +// DeleteUserSession ends one session, and only if it belongs to the caller. +// +// Scoped in the statement rather than checked first: an id read and then deleted +// in two steps is a window, and the whole point of this call is that somebody is +// using it because they think another person is in their account. +func (db *DB) DeleteUserSession(ctx context.Context, sessionID, userID string) error { + res, err := db.ExecContext(ctx, + `DELETE FROM sessions WHERE id = ? AND user_id = ?`, sessionID, userID) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return ErrNotFound + } + return nil +} + func (db *DB) DeleteUserSessions(ctx context.Context, userID string) error { _, err := db.ExecContext(ctx, `DELETE FROM sessions WHERE user_id = ?`, userID) return err diff --git a/web/e2e/auth.setup.js b/web/e2e/auth.setup.js index 6d3bfc8..81f6d26 100644 --- a/web/e2e/auth.setup.js +++ b/web/e2e/auth.setup.js @@ -7,11 +7,7 @@ import { test as setup, expect } from '@playwright/test'; * or the app shell is broken, nothing below runs and the failure names this file. */ -export const USER = { - name: 'Kristian', - email: 'test@example.dk', - password: 'et langt kodeord til test' -}; +import { USER } from './user.js'; const AUTH_FILE = 'e2e/.auth/user.json'; diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index 9b46b08..eb22939 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -1,4 +1,5 @@ import { test, expect } from '@playwright/test'; +import { USER } from './user.js'; /** * The four flows a broken build must not survive: add a task, tick it off, reach @@ -472,6 +473,47 @@ test('nulstillingslinket viser et felt til den nye adgangskode', async ({ browse await context.close(); }); +/** + * The device list, and ending one of them. + * + * `last_seen_at` has been written on every request since sessions existed — once + * a minute, deliberately, so this list could say "lige nu" — and until now + * nothing read it. A session you cannot see is a session you cannot end, which + * matters most in the one situation it exists for: somebody else has your cookie. + */ +test('enhedslisten viser denne enhed, og en anden kan logges ud', async ({ browser, page }) => { + const trouble = watchForTrouble(page); + + // A second sign-in as the same person: a real second session, not a second tab + // sharing this one's cookie. + const context = await browser.newContext({ storageState: { cookies: [], origins: [] } }); + const elsewhere = await context.newPage(); + await elsewhere.goto('/'); + await elsewhere.getByLabel('E-mail').fill(USER.email); + await elsewhere.getByLabel('Adgangskode').fill(USER.password); + await elsewhere.getByRole('button', { name: 'Log ind' }).click(); + await expect(elsewhere.getByRole('navigation', { name: 'Hovedmenu' })).toBeVisible(); + + await page.goto('/indstillinger'); + const panel = page.locator('section.panel').filter({ hasText: 'Enheder' }); + await expect(panel.getByText('denne enhed')).toBeVisible(); + await expect(panel.locator('li')).toHaveCount(2); + + // End the one that is not this browser — the row without the badge. + await panel.locator('li').filter({ hasNot: page.getByText('denne enhed') }) + .getByRole('button', { name: 'Log ud' }) + .click(); + await expect(panel.locator('li')).toHaveCount(1); + + // That session is really gone, and this one is untouched. + await elsewhere.reload(); + await expect(elsewhere.getByText('Log ind for at fortsætte')).toBeVisible(); + await context.close(); + + await expect(panel.getByText('denne enhed')).toBeVisible(); + expect(trouble).toEqual([]); +}); + /** * The MCP connector address must not fall through to the app shell. * diff --git a/web/e2e/user.js b/web/e2e/user.js new file mode 100644 index 0000000..9f9c5c8 --- /dev/null +++ b/web/e2e/user.js @@ -0,0 +1,13 @@ +/** + * The account every test runs as. + * + * In its own file rather than exported from auth.setup.js, because Playwright + * refuses to let one test file import another — and both the setup that creates + * this account and the device-list test that signs in as it a second time need + * the same credentials. + */ +export const USER = { + name: 'Kristian', + email: 'test@example.dk', + password: 'et langt kodeord til test' +}; diff --git a/web/src/lib/api.js b/web/src/lib/api.js index 1b1eba5..cd78a2d 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -167,6 +167,9 @@ export const api = { totpSetup: () => post('/auth/totp/setup'), totpConfirm: (code) => post('/auth/totp/confirm', { code }), totpDisable: (password) => post('/auth/totp/disable', { password }), + listSessions: () => get('/auth/sessions'), + endSession: (id) => del(`/auth/sessions/${id}`), + recoveryCodes: () => get('/auth/recovery-codes'), regenerateRecoveryCodes: (password) => post('/auth/recovery-codes', { password }), diff --git a/web/src/routes/indstillinger/+page.svelte b/web/src/routes/indstillinger/+page.svelte index ce6fbaa..9cee9b8 100644 --- a/web/src/routes/indstillinger/+page.svelte +++ b/web/src/routes/indstillinger/+page.svelte @@ -144,6 +144,54 @@ if (!Object.keys(totpErrors).length) app.toast(humanMessage(e)); } } + + // --- sessions -------------------------------------------------------------- + + let sessions = $state([]); + + $effect(() => { + api.listSessions().then((r) => (sessions = r.sessions)).catch(() => {}); + }); + + async function endSession(session) { + if (session.current) { + if (!confirm('Det er denne enhed. Du bliver logget ud.')) return; + await api.endSession(session.id); + // A full reload rather than clearing the store by hand: the session is + // gone, so every open socket and every cached answer in this tab is now + // about an account nobody is signed in to. + location.href = '/'; + return; + } + + const previous = sessions; + sessions = sessions.filter((s) => s.id !== session.id); + try { + await api.endSession(session.id); + } catch (e) { + sessions = previous; + app.toast(humanMessage(e)); + } + } + + /** + * "for 2 minutter siden". + * + * Relative rather than a timestamp, because the question is "was that me, just + * now?" and an absolute time makes the reader do the subtraction. Anything + * older than a week gets its date instead: "for 23 dage siden" is a number + * nobody can place. + */ + function ago(iso) { + const then = new Date(iso); + const seconds = Math.round((Date.now() - then) / 1000); + + if (seconds < 60) return 'lige nu'; + if (seconds < 3600) return `for ${Math.floor(seconds / 60)} min. siden`; + if (seconds < 86400) return `for ${Math.floor(seconds / 3600)} timer siden`; + if (seconds < 604800) return `for ${Math.floor(seconds / 86400)} dage siden`; + return then.toLocaleDateString('da-DK', { day: 'numeric', month: 'short', year: 'numeric' }); + }
@@ -367,6 +415,35 @@ {/if}
+
+
+

Enheder

+

+ Hvor du er logget ind. Genkender du ikke en af dem, så log den ud og skift + adgangskode. +

+
+ + +
+