From e528d04b43e09ff7b9379bec6308300db64f5715 Mon Sep 17 00:00:00 2001 From: ponzischeme89 Date: Sat, 15 Aug 2026 21:11:43 +1200 Subject: [PATCH] 0.2.65 - End Credits wiring / Gateway: 0.1.45 - Credits --- .env.example | 14 ++ CLAUDE.md | 19 ++ app/build.gradle.kts | 2 +- server/README.md | 20 +- server/cmd/memby-credits/main.go | 4 + server/cmd/memby-server/main.go | 8 +- server/internal/api/admin.go | 2 +- server/internal/api/admin_access_test.go | 192 ++++++++++++++++++ server/internal/api/admin_spa.go | 4 +- server/internal/api/admin_test.go | 60 +++++- server/internal/api/credits_detection.go | 20 -- server/internal/api/features.go | 7 +- server/internal/api/installer_auth.go | 101 +++++++-- server/internal/api/release_test.go | 3 +- server/internal/buildinfo/VERSION | 2 +- server/internal/config/config.go | 20 ++ server/internal/credits/README.md | 22 ++ server/internal/emby/client.go | 65 +++++- server/internal/emby/client_media_url_test.go | 74 +++++++ 19 files changed, 574 insertions(+), 65 deletions(-) create mode 100644 server/internal/api/admin_access_test.go create mode 100644 server/internal/emby/client_media_url_test.go diff --git a/.env.example b/.env.example index 24c6c39..bf9f5a3 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,20 @@ MEMBY_EMBY_URL=https://molise.bounceme.net # above — video goes device -> Emby directly, never through the gateway. MEMBY_EMBY_PUBLIC_URL=https://molise.bounceme.net +# Where the gateway reads media *bytes* from, which is a different question from the two +# addresses above and matters for exactly one subsystem. +# +# Credits detection is the only server-side thing that opens a media file, and it does so as +# ranged reads over the last few minutes of it. The gateway runs on the NAS (10.0.0.213) and +# the media and Emby live on the HTPC (10.0.0.2), so with this unset every one of those +# ranged reads takes MEMBY_EMBY_URL: out to the DDNS name, through TLS and the reverse proxy, +# and back in — and a proxy that buffers turns the ranged read into a whole-file read, which +# is the one thing the scan design exists to avoid. +# +# Unset, it falls back to MEMBY_EMBY_URL and behaves exactly as before. Confirm the port your +# Emby listens on over plain HTTP on the LAN before uncommenting. +#MEMBY_EMBY_MEDIA_URL=http://10.0.0.2:8096 + # Postgres password for the memby role. Generate one, e.g. # openssl rand -base64 24 POSTGRES_PASSWORD=7dfc3eb07108013c7bea9787457397551b5eb9059adac51aea4741f4ec649be2 diff --git a/CLAUDE.md b/CLAUDE.md index b489772..600a728 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -660,6 +660,25 @@ console is still the whole point, and serving a page is still a write of a `[]by Guarded by `MEMBY_ADMIN_TOKEN`; unset means every `/admin` route 404s. It polls `/admin/api/status` every 30s and stops entirely on a hidden tab. +**Who may open it is Emby's answer, not the gateway's.** The sign-in behind the console is +the same discreet Emby password check the public installer uses, so on its own it admitted +anybody in the household — and opening `/admin/` then handed them the admin token cookie. +`embyAdministrator` requires `Policy.IsAdministrator`, which Emby returns *in the +authentication response*, so the check costs no request; a response carrying no policy at +all is asked again directly and an Emby that will not answer refuses the sign-in, because +silence is not permission and reading it as "no" would lock an operator out of their own +console. Three things to preserve: the refusal is worded exactly as a wrong password — +somebody who may not administer the server has no business learning that their password +was right — and the reason goes to the log instead; the *installer* is deliberately not +gated this way, since a viewer setting up a new television is who it is for; and the two +sign-ins issue **one cookie**, so they are separated by being signed with their own +purpose (`installerSessionPurpose` / `adminSessionPurpose`) rather than by a claim inside +the payload. An admin session satisfies the installer's gate; an installer session can +never satisfy the console's. `adminSessionTTL` is 90 days of idle time, matching +`MEMBY_SESSION_IDLE_EXPIRY` so a browser and a television are forgotten on the same +schedule, and only a request an operator actually made slides it forward — see +`operatorPresent`. + It was one HTML file holding every screen at once, all but one of them hidden, which is why opening `/admin/logs` also sent the accounts settings editor and the feature grid, and why each page read as a pile of unrelated controls. Things to preserve: diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 83a035f..7687b45 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -46,7 +46,7 @@ val projectNoticeText = // A release workflow can derive the app version from its Git tag without editing the // source tree. Local builds keep using the checked-in default. -val defaultVersionName = "0.2.64" +val defaultVersionName = "0.2.65" val membyVersionName: String = (project.findProperty("memby.versionName") as String?) ?.trim() diff --git a/server/README.md b/server/README.md index b16da80..010a4ca 100644 --- a/server/README.md +++ b/server/README.md @@ -331,15 +331,27 @@ leaves the server. `MEMBY_RADARR_TTL` controls the shared calendar cache lifetim `https://mserver.sublogue.com/admin/` — a self-contained console for library imports, maintenance, engagement and journeys. Set `MEMBY_ADMIN_TOKEN` to enable it; unset, every `/admin` route 404s so it cannot be left exposed by accident. The page first uses the same -discreet Emby login gate as the private installer. After successful verification it +discreet Emby login gate as the private installer, and additionally requires that the +account **administers Emby** — `Policy.IsAdministrator`, which Emby returns with the +authentication itself, so the check costs no extra request. Knowing a household password +is not the qualification for reaching the console: `/install` is public by design, and any +viewer may sign in there. A refusal is worded exactly as a wrong password, so nothing +discloses that the account was right but the person is not an administrator; the operator +sees the reason in the log. An Emby that returns no policy at all is asked directly rather +than read as a refusal, and one that will not answer refuses the sign-in — silence is +never taken as permission. + +The two sign-ins issue the *same* cookie, so they are separated by being signed with their +own purpose: an installer session cannot satisfy the console's gate, while an +administrator's session satisfies the installer's. After successful verification the page establishes the HttpOnly admin cookie, but browser API requests require both that cookie -and the current 12-hour Emby-verified session. The old admin cookie therefore cannot +and the current 90-day Emby-verified admin session. The old admin cookie therefore cannot bypass the gate after the browser session expires. Scripts may continue to use `Authorization: Bearer ` without a browser session. -Those 12 hours are idle time, not a hard limit: opening an admin page, making any change, +Those 90 days are idle time, not a hard limit: opening an admin page, making any change, or reading one while interacting with it slides the expiry forward once it is inside the -last six hours. What deliberately does **not** extend it is the page's own status +last 45 days. What deliberately does **not** extend it is the page's own status poll — a console left open on a second monitor still times out, which is the whole point of the TTL. The page marks its own requests with `X-Memby-Admin-Active` when there has been interaction in the last five minutes, and on a 401 it reloads, so an expiry lands as diff --git a/server/cmd/memby-credits/main.go b/server/cmd/memby-credits/main.go index 3408225..e114672 100644 --- a/server/cmd/memby-credits/main.go +++ b/server/cmd/memby-credits/main.go @@ -51,6 +51,10 @@ func run(itemID string) error { cfg.EmbyURL, cfg.EmbyPublicURL, cfg.ClientName, cfg.GatewayClientName, cfg.UpstreamTimeout, ) + // The bench measures the path the service actually takes, so it must read bytes from the + // same address the service reads them from. Benchmarking the public route and deploying + // the internal one would report a number belonging to neither. + embyClient.SetMediaURL(cfg.EmbyMediaURL) resolver := credits.NewEmbyResolver(embyClient, emby.Credentials{ UserID: cfg.SyncUserID, Token: cfg.SyncAPIKey, DeviceID: "memby-credits-bench", DeviceName: "MbyGateway Credits", Gateway: true, diff --git a/server/cmd/memby-server/main.go b/server/cmd/memby-server/main.go index b8ef8fc..d55812b 100644 --- a/server/cmd/memby-server/main.go +++ b/server/cmd/memby-server/main.go @@ -116,6 +116,7 @@ func run(log *slog.Logger, events *logging.Buffer) error { cfg.EmbyURL, cfg.EmbyPublicURL, cfg.ClientName, cfg.GatewayClientName, cfg.UpstreamTimeout, ) + embyClient.SetMediaURL(cfg.EmbyMediaURL) mdblistClient := mdblist.New(mdblist.DefaultBaseURL, cfg.UpstreamTimeout) var sonarrClient *sonarr.Client if cfg.SonarrURL != "" { @@ -217,10 +218,15 @@ func run(log *slog.Logger, events *logging.Buffer) error { Config: creditsConfig, }) go creditsService.Run(ctx) + // media_url is on this line rather than the ready line because credits detection is + // the only thing that uses it, and because reading it back is the only way an + // operator can tell that a scan is taking the short path. Where it equals EmbyURL + // the scans go out the public way, which on a split host is the whole cost. log.Info("credits detection enabled", "prefetch", creditsConfig.PrefetchEpisodes, "queue_limit", creditsConfig.QueueLimit, - "visual", detector != nil) + "visual", detector != nil, + "media_url", cfg.EmbyMediaURL) } // The administrative event bus, the integration dispatcher that subscribes to it and diff --git a/server/internal/api/admin.go b/server/internal/api/admin.go index 6726860..227d4af 100644 --- a/server/internal/api/admin.go +++ b/server/internal/api/admin.go @@ -185,7 +185,7 @@ func (s *Server) adminAuth(h http.HandlerFunc) http.Handler { } } if subtle.ConstantTimeCompare([]byte(presented), []byte(s.cfg.AdminToken)) != 1 || - (browser && !s.validInstallerSession(r)) { + (browser && !s.validAdminSession(r)) { writeError(w, http.StatusUnauthorized, "invalid admin token") return } diff --git a/server/internal/api/admin_access_test.go b/server/internal/api/admin_access_test.go new file mode 100644 index 0000000..cf91662 --- /dev/null +++ b/server/internal/api/admin_access_test.go @@ -0,0 +1,192 @@ +package api + +import ( + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/ponzischeme89/memby/server/internal/config" + "github.com/ponzischeme89/memby/server/internal/emby" +) + +// embyAccessLevel stands in for an Emby whose answer about a person's access level is +// under the test's control. userPolicy is the policy the authentication response carries +// ("" for a server that sends none, which is what sends the gate to ask again), and +// lookupPolicy is what a direct read of the account then answers ("" to fail that read). +func embyAccessLevel(t *testing.T, userPolicy, lookupPolicy string) *Server { + t.Helper() + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/Users/AuthenticateByName": + policy := "" + if userPolicy != "" { + policy = `,"Policy":` + userPolicy + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{ + "User":{"Id":"emby-user","Name":"Viewer"`+policy+`}, + "AccessToken":"temporary-emby-token", + "ServerId":"emby-server" + }`) + case r.URL.Path == "/Users/emby-user": + if lookupPolicy == "" { + http.Error(w, "no", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, + `{"Id":"emby-user","Name":"Viewer","Policy":`+lookupPolicy+`}`) + case r.URL.Path == "/Sessions/Logout": + w.WriteHeader(http.StatusNoContent) + case r.URL.Path == "/Devices": + if r.Method == http.MethodGet { + _, _ = io.WriteString(w, `{"Items":[]}`) + return + } + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(upstream.Close) + + return &Server{ + cfg: config.Config{ + AdminToken: "secret", + ReleasePublishToken: "release-secret", + SyncUserID: "service-user", SyncAPIKey: "service-token", + }, + emby: emby.New(upstream.URL, upstream.URL, "MbyATV", "MbyGateway", 2*time.Second), + log: slog.New(slog.NewTextHandler(io.Discard, nil)), + } +} + +func signIn(s *Server, next string) *httptest.ResponseRecorder { + form := url.Values{ + "username": {"Viewer"}, "password": {"correct horse"}, "next": {next}, + } + req := httptest.NewRequest(http.MethodPost, "/install/login", + strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + s.handleInstallLogin(rec, req) + return rec +} + +func sessionCookie(rec *httptest.ResponseRecorder) *http.Cookie { + for _, cookie := range rec.Result().Cookies() { + if cookie.Name == installerCookieName { + return cookie + } + } + return nil +} + +// The console administers the server, so knowing a household password is not the +// qualification for reaching it. Emby's own user policy is the authority on who may, and +// it is the one an operator already manages. +func TestAdminSignInRequiresAnEmbyAdministrator(t *testing.T) { + s := embyAccessLevel(t, `{"IsAdministrator":false}`, "") + + rec := signIn(s, "/admin/") + + if rec.Code != http.StatusForbidden { + t.Fatalf("non-administrator sign-in = %d, want 403", rec.Code) + } + if sessionCookie(rec) != nil { + t.Fatal("a refused admin sign-in issued a session anyway") + } + // The wording must not separate "wrong password" from "right password, wrong person": + // somebody who may not administer the server has no business learning either. + if !strings.Contains(rec.Body.String(), "Sign-in failed.") { + t.Fatalf("refusal discloses more than a failed sign-in: %s", rec.Body.String()) + } +} + +// The installer is the other half of the same gate and is public by design: a viewer +// putting Memby on a new television signs in there and must still be able to. +func TestNonAdministratorMayStillUseTheInstaller(t *testing.T) { + s := embyAccessLevel(t, `{"IsAdministrator":false}`, "") + + rec := signIn(s, "/install") + + if rec.Code != http.StatusSeeOther { + t.Fatalf("installer sign-in = %d, want 303", rec.Code) + } + cookie := sessionCookie(rec) + if cookie == nil { + t.Fatal("installer sign-in issued no session") + } + req := httptest.NewRequest(http.MethodGet, "/install", nil) + req.AddCookie(cookie) + if !s.validInstallerSession(req) { + t.Fatal("a viewer's installer session was not accepted by the installer") + } + if s.validAdminSession(req) { + t.Fatal("a viewer's installer session was accepted as an admin session") + } +} + +func TestAdministratorSignInMintsAnAdminSession(t *testing.T) { + s := embyAccessLevel(t, `{"IsAdministrator":true}`, "") + + rec := signIn(s, "/admin/overview") + + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/admin/overview" { + t.Fatalf("admin sign-in = %d %q", rec.Code, rec.Header().Get("Location")) + } + cookie := sessionCookie(rec) + if cookie == nil { + t.Fatal("admin sign-in issued no session") + } + if cookie.MaxAge != int(adminSessionTTL/time.Second) { + t.Fatalf("admin cookie MaxAge = %d, want %d", + cookie.MaxAge, int(adminSessionTTL/time.Second)) + } + req := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) + req.AddCookie(cookie) + if !s.validAdminSession(req) { + t.Fatal("an administrator's session was not accepted by the console") + } +} + +// An Emby that returns no policy at all is a question, not a refusal — reading its silence +// as "not an administrator" would lock an operator out of their own console. +func TestAdminSignInAsksEmbyWhenTheAuthResponseCarriesNoPolicy(t *testing.T) { + s := embyAccessLevel(t, "", `{"IsAdministrator":true}`) + + rec := signIn(s, "/admin/") + + if rec.Code != http.StatusSeeOther { + t.Fatalf("sign-in = %d, want the policy to be read directly and accepted", rec.Code) + } + cookie := sessionCookie(rec) + if cookie == nil { + t.Fatal("no session issued after the access level was read directly") + } + req := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) + req.AddCookie(cookie) + if !s.validAdminSession(req) { + t.Fatal("the session issued was not an admin session") + } +} + +// And an Emby that will not answer either way must not be guessed at in the permissive +// direction: no answer means no session. +func TestAdminSignInIsRefusedWhenTheAccessLevelCannotBeRead(t *testing.T) { + s := embyAccessLevel(t, "", "") + + rec := signIn(s, "/admin/") + + if rec.Code != http.StatusBadGateway { + t.Fatalf("unreadable access level = %d, want 502", rec.Code) + } + if sessionCookie(rec) != nil { + t.Fatal("a session was issued without knowing the access level") + } +} diff --git a/server/internal/api/admin_spa.go b/server/internal/api/admin_spa.go index 533f8db..36322d4 100644 --- a/server/internal/api/admin_spa.go +++ b/server/internal/api/admin_spa.go @@ -100,7 +100,7 @@ func (s *Server) handleAdminConsole(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(adminUnavailablePage)) return } - if !s.validInstallerSession(r) { + if !s.validAdminSession(r) { // The sign-in form is still the gateway's, server-rendered, and returns to // wherever the operator was trying to go. It is deliberately not part of the SPA: // a login page that has to be downloaded from the thing it guards is one more @@ -109,7 +109,7 @@ func (s *Server) handleAdminConsole(w http.ResponseWriter, r *http.Request) { s.renderAccessLogin(w, r, "", http.StatusOK, r.URL.Path) return } - // Opening a page is somebody at the keyboard, so it starts the twelve-hour clock again. + // Opening a page is somebody at the keyboard, so it starts the clock again. s.renewAdminSession(w, r) s.setAdminTokenCookie(w, r) preventDiscovery(w) diff --git a/server/internal/api/admin_test.go b/server/internal/api/admin_test.go index ea3c5a6..898f9c7 100644 --- a/server/internal/api/admin_test.go +++ b/server/internal/api/admin_test.go @@ -123,13 +123,15 @@ func TestHealthAndAdminStayReachableDuringMaintenance(t *testing.T) { } } -// installerSessionExpiring mints a session with a chosen life left, which is the only way -// to reach the renewal window without waiting six hours in a test. -func installerSessionExpiring(t *testing.T, s *Server, remaining time.Duration) *http.Cookie { +// browserSessionExpiring mints a session of one purpose with a chosen life left, which is +// the only way to reach the renewal window without waiting weeks in a test. +func browserSessionExpiring( + t *testing.T, s *Server, purpose string, remaining time.Duration, +) *http.Cookie { t.Helper() payload := make([]byte, 8+16) binary.BigEndian.PutUint64(payload[:8], uint64(time.Now().Add(remaining).Unix())) - signature := s.signInstallerValue("session", payload) + signature := s.signInstallerValue(purpose, payload) return &http.Cookie{ Name: installerCookieName, Value: base64.RawURLEncoding.EncodeToString(payload) + "." + @@ -137,10 +139,15 @@ func installerSessionExpiring(t *testing.T, s *Server, remaining time.Duration) } } +func adminSessionExpiring(t *testing.T, s *Server, remaining time.Duration) *http.Cookie { + t.Helper() + return browserSessionExpiring(t, s, adminSessionPurpose, remaining) +} + func adminRequest(s *Server, remaining time.Duration, t *testing.T) *http.Request { req := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) req.AddCookie(&http.Cookie{Name: adminCookieName, Value: s.cfg.AdminToken}) - req.AddCookie(installerSessionExpiring(t, s, remaining)) + req.AddCookie(adminSessionExpiring(t, s, remaining)) return req } @@ -182,7 +189,7 @@ func TestAdminSessionIsExtendedWhileTheOperatorIsWorking(t *testing.T) { } follow := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) follow.AddCookie(cookie) - expires, ok := server.installerSessionExpiry(follow) + expires, ok := server.browserSessionExpiry(follow, adminSessionPurpose) if !ok || time.Until(expires) < adminSessionTTL-time.Minute { t.Fatalf("renewed session should carry a full TTL, has %v (ok=%v)", time.Until(expires), ok) @@ -242,7 +249,7 @@ func TestAdminMutationExtendsTheSessionWithoutTheActivityHeader(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/admin/api/sync", nil) req.AddCookie(&http.Cookie{Name: adminCookieName, Value: server.cfg.AdminToken}) - req.AddCookie(installerSessionExpiring(t, server, 2*time.Minute)) + req.AddCookie(adminSessionExpiring(t, server, 2*time.Minute)) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) @@ -550,7 +557,7 @@ func TestAdminAuthAcceptsPersistentCookie(t *testing.T) { }) req := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) req.AddCookie(&http.Cookie{Name: adminCookieName, Value: "secret"}) - addInstallerSession(t, server, req) + req.AddCookie(adminSessionExpiring(t, server, adminSessionTTL)) rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) @@ -560,6 +567,43 @@ func TestAdminAuthAcceptsPersistentCookie(t *testing.T) { } } +// The installer is public: any member of the household signs in there to put Memby on a +// new television. One cookie serves both gates, so the console has to be able to tell the +// two sign-ins apart — otherwise a viewer's installer session reached /admin/, which then +// handed them the admin token cookie. +func TestInstallerSessionCannotReachTheAdminConsole(t *testing.T) { + server := testServer(config.Config{ + AdminToken: "secret", ReleasePublishToken: "release-secret", + }) + handler := server.adminAuth(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + req := httptest.NewRequest(http.MethodGet, "/admin/api/status", nil) + req.AddCookie(&http.Cookie{Name: adminCookieName, Value: "secret"}) + addInstallerSession(t, server, req) + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("an installer session reached the admin API: %d", rec.Code) + } +} + +// The other direction is deliberately allowed: somebody who may administer the server may +// certainly download the app. +func TestAdminSessionIsAcceptedByTheInstaller(t *testing.T) { + server := testServer(config.Config{ + AdminToken: "secret", ReleasePublishToken: "release-secret", + }) + req := httptest.NewRequest(http.MethodGet, "/install", nil) + req.AddCookie(adminSessionExpiring(t, server, adminSessionTTL)) + + if !server.validInstallerSession(req) { + t.Fatal("an administrator's session should satisfy the installer gate") + } +} + func TestAdminPageRequiresDiscreetEmbyGate(t *testing.T) { server := testServer(config.Config{ AdminToken: "secret", ReleasePublishToken: "release-secret", diff --git a/server/internal/api/credits_detection.go b/server/internal/api/credits_detection.go index 4e15ce4..5439cd0 100644 --- a/server/internal/api/credits_detection.go +++ b/server/internal/api/credits_detection.go @@ -2,7 +2,6 @@ package api import ( "context" - "fmt" "time" "github.com/ponzischeme89/memby/server/internal/scheduler" @@ -98,18 +97,6 @@ func (s *Server) RegisterCreditsTasks(sched *scheduler.Scheduler) { }) } -// creditsQueueDetail is the one line the admin console prints about the queue. -func (s *Server) creditsQueueDetail() string { - if s.credits == nil { - return "" - } - depth := s.credits.QueueDepth() - if depth == 0 { - return "" - } - return fmt.Sprintf("%d episode%s awaiting credits detection", depth, plural(depth)) -} - // playbackSessionKey identifies one stream for the load gauge. // // The play session where Emby issued one, because that is what distinguishes two @@ -122,10 +109,3 @@ func playbackSessionKey(deviceID, playSessionID string) string { } return deviceID } - -func plural(count int) string { - if count == 1 { - return "" - } - return "s" -} diff --git a/server/internal/api/features.go b/server/internal/api/features.go index eca5fe1..8e0ef81 100644 --- a/server/internal/api/features.go +++ b/server/internal/api/features.go @@ -88,9 +88,10 @@ var featureCatalogue = []featureDefinition{ { Key: featureEndCredits, Name: "Speed through the credits", Area: "Playback", Description: "Shrink the picture and run the closing credits at double speed with " + - "the next episode beside them, from the credits marker Emby writes. It is read " + - "from the same chapter list as the title sequence, so turning this off saves no " + - "request unless that is off too.", + "the next episode beside them. Emby's own marker is preferred where it has one, " + + "and where it has none the position discovered for the episodes the household " + + "is about to watch is used instead. It is read from the same chapter list as the " + + "title sequence, so turning this off saves no request unless that is off too.", DefaultEnabled: true, MinimumProtocol: 1, Capability: "end_credits_v1", Recovery: "Takes effect the next time playback starts; the credits simply play out full size.", }, diff --git a/server/internal/api/installer_auth.go b/server/internal/api/installer_auth.go index 563eba6..3624af2 100644 --- a/server/internal/api/installer_auth.go +++ b/server/internal/api/installer_auth.go @@ -1,6 +1,7 @@ package api import ( + "context" "crypto/hmac" "crypto/rand" "crypto/sha256" @@ -17,12 +18,29 @@ import ( const ( installerCookieName = "memby_installer" installerSessionTTL = 30 * time.Minute - adminSessionTTL = 12 * time.Hour - installerDeviceID = "memby-web-installer" + // adminSessionTTL is deliberately long. The console is reached from the household's + // own machines, the sign-in behind it is an Emby password check, and an operator who + // opens it once a month was being asked for that password every single visit — which + // is the shape of a gate people work around rather than one that protects anything. + // Ninety days of idle time matches MEMBY_SESSION_IDLE_EXPIRY, so a browser and a + // television are forgotten on the same schedule. + adminSessionTTL = 90 * 24 * time.Hour + installerDeviceID = "memby-web-installer" // adminRenewWithin is how close to expiry a session must be before an operator's own // request re-issues it. Half the TTL avoids rewriting the cookie on every request. adminRenewWithin = adminSessionTTL / 2 + + // A browser session says what it may be used for, and it says so by being signed with + // its own purpose rather than by carrying a claim the holder could edit. The two gates + // are not one gate: /install is public by design — any member of the household signs + // in there to install Memby on a new television — while the console administers the + // server. One cookie serves both, so without this separation an ordinary viewer's + // installer sign-in satisfied the console's gate, and opening /admin/ then handed them + // the admin token cookie. An admin session is accepted at /install as well, since + // somebody who may administer the server may certainly download the app. + installerSessionPurpose = "session" + adminSessionPurpose = "admin session" ) // gatewayDeviceName is what Emby records for a device row the gateway creates for itself. @@ -62,23 +80,24 @@ func (s *Server) signInstallerValue(purpose string, payload []byte) []byte { } func (s *Server) newInstallerSession() (string, error) { - return s.newBrowserSession(installerSessionTTL) + return s.newBrowserSession(installerSessionPurpose, installerSessionTTL) } -func (s *Server) newBrowserSession(ttl time.Duration) (string, error) { +func (s *Server) newBrowserSession(purpose string, ttl time.Duration) (string, error) { payload := make([]byte, 8+16) binary.BigEndian.PutUint64(payload[:8], uint64(time.Now().Add(ttl).Unix())) if _, err := rand.Read(payload[8:]); err != nil { return "", err } - signature := s.signInstallerValue("session", payload) + signature := s.signInstallerValue(purpose, payload) return base64.RawURLEncoding.EncodeToString(payload) + "." + base64.RawURLEncoding.EncodeToString(signature), nil } -// installerSessionExpiry reports when the request's session runs out. A cookie that is -// missing, malformed, forged or already expired is reported the same way: no session. -func (s *Server) installerSessionExpiry(r *http.Request) (time.Time, bool) { +// browserSessionExpiry reports when the request's session of this purpose runs out. A +// cookie that is missing, malformed, forged, signed for a different purpose or already +// expired is reported the same way: no session. +func (s *Server) browserSessionExpiry(r *http.Request, purpose string) (time.Time, bool) { if len(s.installerSecret()) == 0 { return time.Time{}, false } @@ -95,7 +114,7 @@ func (s *Server) installerSessionExpiry(r *http.Request) (time.Time, bool) { return time.Time{}, false } signature, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err != nil || !hmac.Equal(signature, s.signInstallerValue("session", payload)) { + if err != nil || !hmac.Equal(signature, s.signInstallerValue(purpose, payload)) { return time.Time{}, false } expires := int64(binary.BigEndian.Uint64(payload[:8])) @@ -106,8 +125,20 @@ func (s *Server) installerSessionExpiry(r *http.Request) (time.Time, bool) { return time.Unix(expires, 0), true } +// validInstallerSession gates the public installer, which an administrator's own session +// satisfies too. func (s *Server) validInstallerSession(r *http.Request) bool { - _, ok := s.installerSessionExpiry(r) + if s.validAdminSession(r) { + return true + } + _, ok := s.browserSessionExpiry(r, installerSessionPurpose) + return ok +} + +// validAdminSession gates the console. Only a sign-in Emby confirmed as an administrator +// mints one of these, so a household member's installer cookie cannot reach it. +func (s *Server) validAdminSession(r *http.Request) bool { + _, ok := s.browserSessionExpiry(r, adminSessionPurpose) return ok } @@ -118,11 +149,11 @@ func (s *Server) validInstallerSession(r *http.Request) bool { // an operator actually made — see operatorPresent — or an abandoned tab's own polling // would keep the session alive indefinitely, which is what the TTL exists to stop. func (s *Server) renewAdminSession(w http.ResponseWriter, r *http.Request) { - expires, ok := s.installerSessionExpiry(r) + expires, ok := s.browserSessionExpiry(r, adminSessionPurpose) if !ok || time.Until(expires) > adminRenewWithin { return } - session, err := s.newBrowserSession(adminSessionTTL) + session, err := s.newBrowserSession(adminSessionPurpose, adminSessionTTL) if err != nil { s.loggerFor(r.Context()).Error("installer session renewal failed", "error", err) return @@ -183,6 +214,26 @@ func (s *Server) allowedReleaseDownload(r *http.Request, filename string) bool { return expected != "" && hmac.Equal([]byte(presented), []byte(expected)) } +// embyAdministrator asks Emby whether the account that just signed in administers the +// server — the access level in its own user policy, which is the only authority on the +// question and the one an operator already manages. Emby answers it in the authentication +// response, so this normally costs nothing; a response carrying no policy at all is asked +// again directly rather than read as a refusal, because reading silence as "no" would lock +// an operator out of their own console with no way back in. +func (s *Server) embyAdministrator(ctx context.Context, auth *emby.AuthResult) (bool, error) { + if auth.User.Policy.IsAdministrator != nil { + return *auth.User.Policy.IsAdministrator, nil + } + user, err := s.emby.UserByID(ctx, emby.Credentials{ + UserID: auth.User.ID, Token: auth.AccessToken, + DeviceID: installerDeviceID, DeviceName: s.installerDeviceName(), Gateway: true, + }, auth.User.ID) + if err != nil { + return false, err + } + return user.Policy.IsAdministrator, nil +} + func (s *Server) handleInstallLogin(w http.ResponseWriter, r *http.Request) { if len(s.installerSecret()) == 0 { http.NotFound(w, r) @@ -222,6 +273,10 @@ func (s *Server) handleInstallLogin(w http.ResponseWriter, r *http.Request) { s.renderAccessLogin(w, r, "Sign-in failed.", http.StatusUnauthorized, next) return } + // Ask before the token is retired below: the fallback lookup needs it. Whether the + // answer is wanted depends on where the sign-in was headed, but it is asked either way + // so that the cleanup underneath runs on one path rather than two. + administrator, adminErr := s.embyAdministrator(r.Context(), auth) // Authentication creates an Emby access token. The installer needs only proof that // it succeeded, so retire the upstream session immediately and never persist it. if err := s.emby.Logout(r.Context(), emby.Credentials{ @@ -242,11 +297,27 @@ func (s *Server) handleInstallLogin(w http.ResponseWriter, r *http.Request) { return } - ttl := installerSessionTTL + purpose, ttl := installerSessionPurpose, installerSessionTTL if strings.HasPrefix(next, "/admin/") { - ttl = adminSessionTTL + if adminErr != nil { + s.loggerFor(r.Context()).Error("admin sign-in could not read Emby access level", + "user", username, "error", adminErr) + s.renderAccessLogin(w, r, "Sign-in is temporarily unavailable.", + http.StatusBadGateway, next) + return + } + if !administrator { + // Deliberately the same wording an unknown password gets. Somebody who is not + // an administrator has no business learning that the console exists and that + // their password was right; the operator can see the refusal in the log. + s.loggerFor(r.Context()).Warn("admin sign-in refused: not an Emby administrator", + "user", username) + s.renderAccessLogin(w, r, "Sign-in failed.", http.StatusForbidden, next) + return + } + purpose, ttl = adminSessionPurpose, adminSessionTTL } - session, err := s.newBrowserSession(ttl) + session, err := s.newBrowserSession(purpose, ttl) if err != nil { s.loggerFor(r.Context()).Error("installer session generation failed", "error", err) writeError(w, http.StatusInternalServerError, "could not start installer session") diff --git a/server/internal/api/release_test.go b/server/internal/api/release_test.go index 1395b45..e6f883d 100644 --- a/server/internal/api/release_test.go +++ b/server/internal/api/release_test.go @@ -252,7 +252,8 @@ func TestInstallerLoginUsesEmbyWithoutCreatingTVSession(t *testing.T) { case "/Users/AuthenticateByName": w.Header().Set("Content-Type", "application/json") _, _ = io.WriteString(w, `{ - "User":{"Id":"emby-user","Name":"Matt"}, + "User":{"Id":"emby-user","Name":"Matt", + "Policy":{"IsAdministrator":true}}, "AccessToken":"temporary-emby-token", "ServerId":"emby-server" }`) diff --git a/server/internal/buildinfo/VERSION b/server/internal/buildinfo/VERSION index 4a3e97d..6ee33ba 100644 --- a/server/internal/buildinfo/VERSION +++ b/server/internal/buildinfo/VERSION @@ -1 +1 @@ -0.1.42 +0.1.45 diff --git a/server/internal/config/config.go b/server/internal/config/config.go index 396e682..2ac3baf 100644 --- a/server/internal/config/config.go +++ b/server/internal/config/config.go @@ -22,6 +22,22 @@ type Config struct { // Defaults to EmbyURL; set it when the gateway talks to Emby over a network the // TVs cannot reach. EmbyPublicURL string + // EmbyMediaURL is the address the gateway reads *media bytes* from, which is a + // different question from how it reads metadata and is the only place the difference + // costs anything. + // + // Credits detection is the one thing on the server side that opens a media file, and it + // does so with ranged reads over HTTP. A metadata call is a few kilobytes and does not + // care which way it is routed; a scan is megabytes, and where EmbyURL is a public + // hostname — which it legitimately may be, since the gateway and Emby need not share a + // network — every one of those ranged reads leaves the host for the internet-facing + // edge, pays TLS, and comes back in through a reverse proxy that is free to buffer the + // response and discard the range economy entirely. + // + // So this is stated separately rather than inferred: there is no way to look at a URL + // and tell whether it happens to resolve locally. Unset, it falls back to EmbyURL and + // nothing changes. + EmbyMediaURL string DatabaseURL string RedisURL string @@ -191,6 +207,7 @@ func Load() (Config, error) { ListenAddr: env("MEMBY_LISTEN_ADDR", ":8080"), EmbyURL: strings.TrimRight(os.Getenv("MEMBY_EMBY_URL"), "/"), EmbyPublicURL: strings.TrimRight(os.Getenv("MEMBY_EMBY_PUBLIC_URL"), "/"), + EmbyMediaURL: strings.TrimRight(os.Getenv("MEMBY_EMBY_MEDIA_URL"), "/"), DatabaseURL: os.Getenv("MEMBY_DATABASE_URL"), RedisURL: env("MEMBY_REDIS_URL", "redis://localhost:6379/0"), ClientName: env("MEMBY_CLIENT_NAME", "MbyATV"), @@ -262,6 +279,9 @@ func Load() (Config, error) { if c.EmbyPublicURL == "" { c.EmbyPublicURL = c.EmbyURL } + if c.EmbyMediaURL == "" { + c.EmbyMediaURL = c.EmbyURL + } if c.ReleasePublishToken != "" && c.PublicURL == "" { return c, fmt.Errorf("MEMBY_PUBLIC_URL is required when release publishing is enabled") } diff --git a/server/internal/credits/README.md b/server/internal/credits/README.md index 6c7e0a1..aa1f327 100644 --- a/server/internal/credits/README.md +++ b/server/internal/credits/README.md @@ -77,6 +77,28 @@ costs a 0.3 penalty, which usually means storing nothing. **Prefer no marker to marker** is the governing rule throughout: a missing Skip Credits button is an absence nobody notices, a button during the final scene is a fault they remember. +## Where the bytes come from + +The gateway has no filesystem access to the media, so a scan reads Emby's own stream route +over HTTP and `-ss` ahead of `-i` is what makes that a ranged request rather than a download. +That optimisation is only worth anything if the range survives the trip, which is why the +address is configurable separately from every other way the gateway reaches Emby. + +`MEMBY_EMBY_MEDIA_URL` is the address media bytes are read from; unset it falls back to +`MEMBY_EMBY_URL` and nothing changes. The distinction is not cosmetic in this deployment: the +stack runs on the NAS, Emby and the media live on the HTPC, and `MEMBY_EMBY_URL` is a public +DDNS name. Left to fall back, every ranged read leaves the host for the internet-facing edge, +pays TLS and returns through a reverse proxy — and a proxy that buffers its upstream turns +the ranged read into a whole-file read, which is the single thing this package is built not to +do. Naming the LAN address instead keeps the read on the wire between the two machines. + +It is **stated rather than inferred** for the usual reason: there is no way to look at a URL +and tell whether it happens to resolve locally, and guessing wrong either sends scans the long +way round or points them at a host that is not there. `media_url` is on the `credits detection +enabled` start-up line so an operator can read back which path is in force, and the benchmark +below sets it too — measuring the public route and deploying the internal one would report a +number belonging to neither. + ## Cost, and what is still unmeasured **Derived arithmetic** (checkable without hardware): diff --git a/server/internal/emby/client.go b/server/internal/emby/client.go index c286197..20d2dc4 100644 --- a/server/internal/emby/client.go +++ b/server/internal/emby/client.go @@ -24,6 +24,7 @@ import ( type Client struct { baseURL string publicURL string + mediaURL string clientName string // gatewayClientName identifies the requests the gateway makes on its own behalf, so // Emby's device list separates a television from the server standing behind it. The @@ -72,8 +73,17 @@ type ItemsResult struct { type AuthResult struct { User struct { - ID string `json:"Id"` - Name string `json:"Name"` + ID string `json:"Id"` + Name string `json:"Name"` + Policy struct { + // IsAdministrator is a pointer because "Emby says this person is not an + // administrator" and "Emby did not answer the question" are different facts + // that must be acted on differently: the first refuses the console, the second + // is worth asking again rather than locking an operator out of their own + // server. Emby returns the policy in the authentication response itself, so + // the ordinary path costs no extra request. + IsAdministrator *bool `json:"IsAdministrator"` + } `json:"Policy"` } `json:"User"` AccessToken string `json:"AccessToken"` ServerID string `json:"ServerId"` @@ -83,7 +93,8 @@ type User struct { ID string `json:"Id"` Name string `json:"Name"` Policy struct { - IsDisabled bool `json:"IsDisabled"` + IsDisabled bool `json:"IsDisabled"` + IsAdministrator bool `json:"IsAdministrator"` } `json:"Policy"` } @@ -174,6 +185,18 @@ func New(baseURL, publicURL, clientName, gatewayClientName string, timeout time. } } +// SetMediaURL names the address server-side media reads go to, which is only ever credits +// detection. Unset — or set to nothing — media reads use the same address as metadata. +// +// A setter rather than a constructor argument because it is one deployment's answer to one +// subsystem's problem, and threading it through four test call sites that will never use it +// would be paying for it everywhere to spend it in one place. Call it during start-up, +// before anything is serving: nothing here is synchronised, for the same reason the client's +// other addresses are not. +func (c *Client) SetMediaURL(mediaURL string) { + c.mediaURL = strings.TrimRight(strings.TrimSpace(mediaURL), "/") +} + // Authenticate signs a television in. cred carries the device the record is created for // and that set's ClientVersion — Emby stamps it on the record, so an empty one leaves the // entry claiming the gateway's own build. cred.Gateway marks a sign-in the gateway is @@ -247,6 +270,22 @@ func (c *Client) DeleteDevice( return nil } +// UserByID reads one account with that account's own token. It exists for the admin gate +// and is asked only when an authentication response carried no policy at all, which is +// why it takes the credentials it does: the question is "who did I just authenticate", +// and an Emby user may always read themselves. +func (c *Client) UserByID(ctx context.Context, cred Credentials, userID string) (*User, error) { + req, err := c.newRequest(ctx, http.MethodGet, "/Users/"+url.PathEscape(userID), nil, cred, nil) + if err != nil { + return nil, err + } + var user User + if err := c.do(req, &user); err != nil { + return nil, err + } + return &user, nil +} + // Users returns the household accounts visible to an administrative/service token. // It is used only by the background For You builder, never on a television request. func (c *Client) Users(ctx context.Context, cred Credentials) ([]User, error) { @@ -593,19 +632,29 @@ func (c *Client) StreamURL(cred Credentials, itemID string) string { return fmt.Sprintf("%s/Videos/%s/stream?%s", c.publicURL, url.PathEscape(itemID), params.Encode()) } -// InternalStreamURL is the same file, addressed the way the *gateway* reaches Emby rather +// InternalStreamURL is the same file, addressed the way the *gateway* reads bytes rather // than the way a television does. // // It exists for credits detection, which is the only thing that reads media bytes on the -// server side. Using StreamURL there would send a ranged read out to the public address and -// back in through the reverse proxy — the two containers are on the same network, and a scan -// has no business leaving it. Nothing here is ever handed to a client. +// server side, and it reads them as ranged requests over the tail of a file. Using StreamURL +// there would send every one of those out to the public address and back in through the +// reverse proxy — and where a proxy buffers the response, the ranged read stops being a +// ranged read and the whole economy of the scan is gone. Nothing here is ever handed to a +// client. +// +// mediaURL is preferred and baseURL is the fallback, because how the gateway reaches Emby +// for *metadata* is not evidence about the best path for megabytes: a deployment whose +// MEMBY_EMBY_URL is a public hostname is an ordinary one, not a misconfiguration. func (c *Client) InternalStreamURL(cred Credentials, itemID string) string { + host := c.mediaURL + if host == "" { + host = c.baseURL + } params := url.Values{} params.Set("static", "true") params.Set("api_key", cred.Token) params.Set("DeviceId", cred.DeviceID) - return fmt.Sprintf("%s/Videos/%s/stream?%s", c.baseURL, url.PathEscape(itemID), params.Encode()) + return fmt.Sprintf("%s/Videos/%s/stream?%s", host, url.PathEscape(itemID), params.Encode()) } // DeliveryURL converts a PlaybackInfo URL into a TV-reachable, authenticated URL. diff --git a/server/internal/emby/client_media_url_test.go b/server/internal/emby/client_media_url_test.go new file mode 100644 index 0000000..14eac27 --- /dev/null +++ b/server/internal/emby/client_media_url_test.go @@ -0,0 +1,74 @@ +package emby + +import ( + "strings" + "testing" + "time" +) + +// Where the gateway reads media bytes from is the one address that is not interchangeable +// with the others, so all three are pinned against each other here: a television must never +// be handed the internal address, and a scan must never be sent out the public way when an +// internal one has been named. + +func TestInternalStreamURLPrefersTheMediaAddress(t *testing.T) { + client := New( + "https://molise.bounceme.net", "https://molise.bounceme.net", + "MbyATV", "MbyGateway", time.Second, + ) + client.SetMediaURL("http://10.0.0.2:8096") + + stream := client.InternalStreamURL(Credentials{Token: "tok", DeviceID: "dev"}, "item-1") + if !strings.HasPrefix(stream, "http://10.0.0.2:8096/Videos/item-1/stream?") { + t.Fatalf("scan should read from the media address, got %q", stream) + } +} + +// The fallback is what makes the setting optional. A deployment that never names a media +// address must behave exactly as it did before there was one, or adding the option would be +// a breaking change to every install that ignores it. +func TestInternalStreamURLFallsBackToTheGatewayAddress(t *testing.T) { + client := New("http://emby:8096", "https://public.example", "MbyATV", "MbyGateway", time.Second) + + stream := client.InternalStreamURL(Credentials{Token: "tok", DeviceID: "dev"}, "item-1") + if !strings.HasPrefix(stream, "http://emby:8096/Videos/item-1/stream?") { + t.Fatalf("with no media address the scan should use the gateway address, got %q", stream) + } +} + +// A blank or whitespace value is the same as unset. It arrives from an environment variable +// somebody left empty, which is the ordinary way to turn the setting off again, and treating +// it as an address would build "/Videos/…" and read from the gateway itself. +func TestBlankMediaURLIsTreatedAsUnset(t *testing.T) { + client := New("http://emby:8096", "https://public.example", "MbyATV", "MbyGateway", time.Second) + client.SetMediaURL(" ") + + stream := client.InternalStreamURL(Credentials{Token: "tok", DeviceID: "dev"}, "item-1") + if !strings.HasPrefix(stream, "http://emby:8096/Videos/") { + t.Fatalf("blank media address should fall back, got %q", stream) + } +} + +// A trailing slash is what an operator actually types, and it must not reach the URL as a +// double slash — some reverse proxies answer that with a redirect ffmpeg will not follow. +func TestMediaURLTolerantOfATrailingSlash(t *testing.T) { + client := New("http://emby:8096", "https://public.example", "MbyATV", "MbyGateway", time.Second) + client.SetMediaURL("http://10.0.0.2:8096/") + + stream := client.InternalStreamURL(Credentials{Token: "tok", DeviceID: "dev"}, "item-1") + if !strings.HasPrefix(stream, "http://10.0.0.2:8096/Videos/") { + t.Fatalf("media address should lose its trailing slash, got %q", stream) + } +} + +// The television's URL is the public address and is not affected by any of this. Nothing +// reachable only from the NAS may ever be handed to a set in the house. +func TestStreamURLForTelevisionsIgnoresTheMediaAddress(t *testing.T) { + client := New("http://emby:8096", "https://public.example", "MbyATV", "MbyGateway", time.Second) + client.SetMediaURL("http://10.0.0.2:8096") + + stream := client.StreamURL(Credentials{Token: "tok", DeviceID: "dev"}, "item-1") + if !strings.HasPrefix(stream, "https://public.example/Videos/item-1/stream?") { + t.Fatalf("televisions must keep the public address, got %q", stream) + } +}