diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 93611c6c98..add6ac2e5e 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -246,6 +246,7 @@ type AdminGroup struct { type TelegramGroup struct { Token string `long:"token" env:"TOKEN" description:"telegram token (used for auth and telegram notifications)"` Timeout time.Duration `long:"timeout" env:"TIMEOUT" default:"5s" description:"telegram timeout"` + APIURL string `long:"api-url" env:"API_URL" description:"telegram bot api base url, for a proxy or a test double; the public api when unset"` } // SMTPGroup defines options for SMTP server connection, used in auth and notify modules @@ -437,8 +438,13 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { s.Telegram.Timeout = s.Notify.Telegram.Timeout result = append(result, DeprecatedFlag{Old: "notify.telegram.timeout", New: "telegram.timeout", Version: "1.9"}) } - if s.Notify.Telegram.API != "https://api.telegram.org/bot" { - result = append(result, DeprecatedFlag{Old: "notify.telegram.api", Version: "1.9"}) + if s.Notify.Telegram.API != "https://api.telegram.org/bot" && s.Telegram.APIURL == "" { + if base, ok := telegramAPIBaseFromDeprecated(s.Notify.Telegram.API); ok { + s.Telegram.APIURL = base + result = append(result, DeprecatedFlag{Old: "notify.telegram.api", New: "telegram.api-url", Version: "1.9"}) + } else { + result = append(result, DeprecatedFlag{Old: "notify.telegram.api", Version: "1.9"}) + } } if s.Auth.Twitter.CID != "" { result = append(result, DeprecatedFlag{Old: "auth.twitter.cid", Version: "1.14"}) @@ -481,9 +487,21 @@ func (s *ServerCommand) findDeprecatedFlagsCollisions() (result []DeprecatedFlag if s.Notify.Telegram.Timeout != telegramDefaultTimeout && s.Telegram.Timeout != telegramDefaultTimeout && s.Notify.Telegram.Timeout != s.Telegram.Timeout { result = append(result, DeprecatedFlag{Old: "notify.telegram.timeout", New: "telegram.timeout", Collision: true}) } + if base, ok := telegramAPIBaseFromDeprecated(s.Notify.Telegram.API); ok && + s.Notify.Telegram.API != "https://api.telegram.org/bot" && stringsSetAndDifferent(base, s.Telegram.APIURL) { + result = append(result, DeprecatedFlag{Old: "notify.telegram.api", New: "telegram.api-url", Collision: true}) + } return result } +func telegramAPIBaseFromDeprecated(prefix string) (string, bool) { + trimmed := strings.TrimSuffix(prefix, "/") + if !strings.HasSuffix(trimmed, "/bot") { + return "", false + } + return strings.TrimSuffix(trimmed, "/bot"), true +} + func (s *ServerCommand) handleDeprecatedNotifications() { for _, t := range s.Notify.Type { if t == "email" && !contains(t, s.Notify.Users) { @@ -656,8 +674,19 @@ func (s *ServerCommand) newServerApp(ctx context.Context) (*serverApp, error) { authRefreshCache := newAuthRefreshCache() authenticator := s.getAuthenticator(dataService, avatarStore, adminStore, authRefreshCache) - telegramAuth := s.makeTelegramAuth(authenticator) // telegram auth requires TelegramAPI listener which is constructed below - telegramService := s.startTelegramAuthAndNotify(ctx, telegramAuth) + // telegram auth requires the TelegramAPI listener started below + telegramAuth, err := s.makeTelegramAuth(authenticator) + if err != nil { + _ = dataService.Close() + _ = authRefreshCache.Close() + return nil, err + } + telegramService, err := s.startTelegramAuthAndNotify(ctx, telegramAuth) + if err != nil { + _ = dataService.Close() + _ = authRefreshCache.Close() + return nil, err + } err = s.addAuthProviders(authenticator) if err != nil { @@ -721,7 +750,6 @@ func (s *ServerCommand) newServerApp(ctx context.Context) (*serverApp, error) { Authenticator: authenticator, Cache: loadingCache, NotifyService: notifyService, - TelegramService: telegramService, SSLConfig: sslConfig, UpdateLimiter: s.UpdateLimit, ImageService: imageService, @@ -739,6 +767,14 @@ func (s *ServerCommand) newServerApp(ctx context.Context) (*serverApp, error) { ExternalImageProxy: s.ImageProxy.CacheExternal, } + // assigned only when there is one: telegramService is an interface field, so a nil + // *notify.Telegram put in it is a non-nil interface, and the handler's own `== nil` guard then + // lets a dereference through as a 500. Telegram auth without telegram notifications is an + // ordinary reachable configuration + if telegramService != nil && contains("telegram", s.Notify.Users) { + srv.TelegramService = telegramService + } + srv.ScoreThresholds.Low, srv.ScoreThresholds.Critical = s.LowScore, s.CriticalScore var devAuth *provider.DevAuthServer @@ -1230,20 +1266,36 @@ func (s *ServerCommand) addAuthProviders(authenticator *auth.Service) error { } // creates and registers telegram auth, which we need separately from other auth providers -func (s *ServerCommand) makeTelegramAuth(authenticator *auth.Service) providers.TGUpdatesReceiver { - if s.Auth.Telegram { - telegram := &provider.TelegramHandler{ - ProviderName: "telegram", - SuccessMsg: "✅ You have successfully authenticated, check the web!", - Telegram: provider.NewTelegramAPI(s.Telegram.Token, &http.Client{Timeout: s.Telegram.Timeout}), - L: log.Default(), - TokenService: authenticator.TokenService(), - AvatarSaver: authenticator.AvatarProxy(), +func (s *ServerCommand) makeTelegramAuth(authenticator *auth.Service) (*provider.TelegramHandler, error) { + if !s.Auth.Telegram { + return nil, nil + } + + client := &http.Client{Timeout: s.Telegram.Timeout} + + // the public api unless an operator points it elsewhere. A bad base url stops the server because + // every request carries the bot token in its path; a base that resolves somewhere unintended + // ships the token there, and an operator who set the option meant it to be used + tgAPI := provider.NewTelegramAPI(s.Telegram.Token, client) + if s.Telegram.APIURL != "" { + withBase, err := provider.NewTelegramAPIWithBaseURL(s.Telegram.Token, client, s.Telegram.APIURL) + if err != nil { + return nil, fmt.Errorf("failed to make telegram auth: %w", err) } - authenticator.AddCustomHandler(telegram) - return telegram + tgAPI = withBase } - return nil + + telegram := &provider.TelegramHandler{ + ProviderName: "telegram", + ErrorMsg: "Authentication request was not found or expired.", + SuccessMsg: "✅ You have successfully authenticated, check the web!", + Telegram: tgAPI, + L: log.Default(), + TokenService: authenticator.TokenService(), + AvatarSaver: authenticator.AvatarProxy(), + } + authenticator.AddCustomHandler(telegram) + return telegram, nil } func (s *ServerCommand) makeNotifyService(dataStore *service.DataStore, destinations []notify.Destination, telegram *notify.Telegram) *notify.Service { @@ -1355,7 +1407,11 @@ func (s *ServerCommand) makeTelegramNotify() (*notify.Telegram, error) { UserNotifications: contains("telegram", s.Notify.Users), Token: s.Telegram.Token, Timeout: s.Telegram.Timeout, - SuccessMsg: "✅ You have successfully subscribed for notifications, check the web!", + // the same base as auth: the update poll runs through this service, so leaving it on the + // public API would send the bot token there while everything else went where the operator + // pointed it + APIURL: s.Telegram.APIURL, + SuccessMsg: "✅ You have successfully subscribed for notifications, check the web!", } tg, err := notify.NewTelegram(telegramParams) if err != nil { @@ -1481,27 +1537,44 @@ func (s *ServerCommand) parseSameSite(ss string) http.SameSite { } } -// startTelegramAuthAndNotify initializes telegram notify and auth Telegram Bot listen loop. +// startTelegramAuthAndNotify initializes telegram notify and the shared Telegram Bot update loop. // Does nothing if telegram auth and notifications are disabled. -func (s *ServerCommand) startTelegramAuthAndNotify(ctx context.Context, telegramAuth providers.TGUpdatesReceiver) (tg *notify.Telegram) { - if !contains("telegram", s.Notify.Users) && !contains("telegram", s.Notify.Admins) && !s.Auth.Telegram { - return nil +func (s *ServerCommand) startTelegramAuthAndNotify( + ctx context.Context, telegramAuth *provider.TelegramHandler, +) (tg *notify.Telegram, err error) { + notifyWanted := contains("telegram", s.Notify.Users) || contains("telegram", s.Notify.Admins) + if !notifyWanted && telegramAuth == nil { + return nil, nil } - var err error if tg, err = s.makeTelegramNotify(); err != nil { + // Auth cannot serve a request without an update source. A caller-supplied base is also + // security-sensitive because the bot token travels in its path, so either configuration + // fails startup instead of leaving the requested feature silently disabled. + if telegramAuth != nil || s.Telegram.APIURL != "" { + return nil, fmt.Errorf("failed to make telegram update source: %w", err) + } log.Printf("[WARN] failed to make telegram notify service, %s", err) - return nil + return nil, nil } - telegramReceivers := []providers.TGUpdatesReceiver{tg} + telegramReceivers := make([]providers.TGUpdatesReceiver, 0, 2) + if notifyWanted { + telegramReceivers = append(telegramReceivers, tg) + } if telegramAuth != nil { + // ProcessUpdate owns the auth handler's request state when the shared dispatcher is used. + // Set it up before the HTTP server can accept a login: the dispatcher's first poll is + // five seconds away, and a login before that otherwise fails as if no listener existed. + if err := telegramAuth.ProcessUpdate(ctx, `{"ok":true,"result":[]}`); err != nil { + return nil, fmt.Errorf("failed to set up telegram auth update receiver: %w", err) + } telegramReceivers = append(telegramReceivers, telegramAuth) } // start bot messages receiver for both notify and auth services go providers.DispatchTelegramUpdates(ctx, tg, telegramReceivers, time.Second*5) - return tg + return tg, nil } // splitAtCommas split s at commas, ignoring commas in strings. diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 7f21662263..891aa9f7ff 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -3,10 +3,12 @@ package cmd import ( "context" "crypto/tls" + "encoding/json" "fmt" "io" "net" "net/http" + "net/http/httptest" "os" "strconv" "strings" @@ -14,8 +16,10 @@ import ( "testing" "time" + "github.com/go-pkgz/auth/v2" "github.com/go-pkgz/auth/v2/provider" "github.com/go-pkgz/auth/v2/token" + log "github.com/go-pkgz/lgr" "github.com/golang-jwt/jwt/v5" "github.com/jessevdk/go-flags" "go.uber.org/goleak" @@ -97,7 +101,7 @@ func TestServerApp_DevMode(t *testing.T) { waitForHTTPServerStart(t, port) providers := app.restSrv.Authenticator.Providers() - require.Equal(t, 11+1, len(providers), "extra auth provider") + require.Equal(t, 10+1, len(providers), "extra auth provider") assert.Equal(t, "dev", providers[len(providers)-2].Name(), "dev auth provider") // send ping resp, err := http.Get(fmt.Sprintf("http://localhost:%d/api/v1/ping", port)) @@ -130,7 +134,7 @@ func TestServerApp_CustomOAuthProvider(t *testing.T) { waitForHTTPServerStart(t, port) providers := app.restSrv.Authenticator.Providers() - require.Equal(t, 11+1, len(providers), "extra auth provider") + require.Equal(t, 10+1, len(providers), "extra auth provider") assert.Equal(t, "oidc", providers[len(providers)-2].Name(), "custom auth provider") cancel() @@ -149,7 +153,7 @@ func TestServerApp_AnonMode(t *testing.T) { waitForHTTPServerStart(t, port) providers := app.restSrv.Authenticator.Providers() - require.Equal(t, 11+1, len(providers), "extra auth provider for anon") + require.Equal(t, 10+1, len(providers), "extra auth provider for anon") assert.Equal(t, "anonymous", providers[len(providers)-1].Name(), "anon auth provider") client := http.Client{Timeout: 10 * time.Second} @@ -708,7 +712,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { "--auth.email.template=file.tmpl", "--notify.telegram.token=abcd", "--notify.telegram.timeout=3m", - "--notify.telegram.api=http://example.org", + "--notify.telegram.api=http://example.org/bot", "--auth.twitter.cid=123", "--auth.twitter.csec=456", } @@ -735,7 +739,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { {Old: "notify.type", New: "notify.(users|admins)", Version: "1.9"}, {Old: "notify.telegram.token", New: "telegram.token", Version: "1.9"}, {Old: "notify.telegram.timeout", New: "telegram.timeout", Version: "1.9"}, - {Old: "notify.telegram.api", Version: "1.9"}, + {Old: "notify.telegram.api", New: "telegram.api-url", Version: "1.9"}, {Old: "auth.twitter.cid", Version: "1.14"}, {Old: "auth.twitter.csec", Version: "1.14"}, }, @@ -746,6 +750,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { assert.Equal(t, "test_user", s.SMTP.Username) assert.Equal(t, "test_password", s.SMTP.Password) assert.Equal(t, 15*time.Second, s.SMTP.TimeOut) + assert.Equal(t, "http://example.org", s.Telegram.APIURL) } func TestServerApp_DeprecatedArgsCollisions(t *testing.T) { @@ -772,6 +777,8 @@ func TestServerApp_DeprecatedArgsCollisions(t *testing.T) { "--telegram.token=dcba", "--notify.telegram.timeout=3m", "--telegram.timeout=5m", + "--notify.telegram.api=http://old.example.org/bot", + "--telegram.api-url=http://new.example.org", } _, err := p.ParseArgs(args) require.NoError(t, err) @@ -786,6 +793,7 @@ func TestServerApp_DeprecatedArgsCollisions(t *testing.T) { {Old: "auth.email.timeout", New: "smtp.timeout", Collision: true}, {Old: "notify.telegram.token", New: "telegram.token", Collision: true}, {Old: "notify.telegram.timeout", New: "telegram.timeout", Collision: true}, + {Old: "notify.telegram.api", New: "telegram.api-url", Collision: true}, }, deprecatedFlagsCollisions) @@ -996,6 +1004,93 @@ func TestServerCommand_parseSameSite(t *testing.T) { } } +func TestServerCommand_TelegramAuthAcceptsLoginBeforeFirstSharedPoll(t *testing.T) { + telegramAPI := newTelegramAPITestServer(t) + + client := &http.Client{Timeout: time.Second} + defer client.CloseIdleConnections() + authAPI, err := provider.NewTelegramAPIWithBaseURL("stub-token", client, telegramAPI.URL) + require.NoError(t, err) + authHandler := &provider.TelegramHandler{ProviderName: "telegram", Telegram: authAPI, L: log.Default()} + + cmd := ServerCommand{} + cmd.Telegram.Token = "stub-token" + cmd.Telegram.Timeout = time.Second + cmd.Telegram.APIURL = telegramAPI.URL + + ctx := t.Context() + tg, err := cmd.startTelegramAuthAndNotify(ctx, authHandler) + require.NoError(t, err) + require.NotNil(t, tg, "telegram auth needs the shared update source even when notifications are off") + + recorder := httptest.NewRecorder() + authHandler.LoginHandler(recorder, httptest.NewRequest(http.MethodGet, "/auth/telegram/login?site=remark", http.NoBody)) + require.Equal(t, http.StatusOK, recorder.Code, recorder.Body.String()) + + var response struct { + Token string `json:"token"` + Bot string `json:"bot"` + } + require.NoError(t, json.NewDecoder(recorder.Body).Decode(&response)) + assert.NotEmpty(t, response.Token) + assert.Equal(t, "remark42_test_bot", response.Bot) +} + +func TestServerApp_TelegramAuthAloneDoesNotOfferNotifications(t *testing.T) { + telegramAPI := newTelegramAPITestServer(t) + port := chooseUnusedPort(t) + app, ctx, cancel := prepServerApp(t, func(cmd ServerCommand) ServerCommand { + cmd.Port = port + cmd.Auth.Telegram = true + cmd.Telegram.Token = "stub-token" + cmd.Telegram.Timeout = time.Second + cmd.Telegram.APIURL = telegramAPI.URL + return cmd + }) + + require.Nil(t, app.restSrv.TelegramService, + "telegram auth alone must not put a typed nil or an update-only client behind the subscription endpoint") + + go func() { _ = app.run(ctx) }() + waitForHTTPServerStart(t, port) + cancel() + app.Wait() +} + +func TestServerCommand_TelegramCustomAPIErrorStopsRequestedFeature(t *testing.T) { + t.Run("auth", func(t *testing.T) { + cmd := ServerCommand{} + cmd.Auth.Telegram = true + cmd.Telegram.APIURL = "://bad" + + _, err := cmd.makeTelegramAuth(&auth.Service{}) + require.ErrorContains(t, err, "failed to make telegram auth") + }) + + t.Run("notifications", func(t *testing.T) { + cmd := ServerCommand{} + cmd.Notify.Users = []string{"telegram"} + cmd.Telegram.APIURL = "://bad" + + _, err := cmd.startTelegramAuthAndNotify(context.Background(), nil) + require.ErrorContains(t, err, "failed to make telegram update source") + }) +} + +func newTelegramAPITestServer(t *testing.T) *httptest.Server { + t.Helper() + telegramAPI := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/getUpdates") { + _, _ = io.WriteString(w, `{"ok":true,"result":[]}`) + return + } + _, _ = io.WriteString(w, `{"ok":true,"result":{"id":1,"is_bot":true,"username":"remark42_test_bot"}}`) + })) + t.Cleanup(telegramAPI.Close) + return telegramAPI +} + func Test_splitAtCommas(t *testing.T) { tbl := []struct { inp string @@ -1163,8 +1258,6 @@ func prepServerApp(t *testing.T, fn func(o ServerCommand) ServerCommand) (*serve cmd.Auth.Twitter.CSEC, cmd.Auth.Twitter.CID = "csec", "cid" cmd.Auth.Patreon.CSEC, cmd.Auth.Patreon.CID = "csec", "cid" cmd.Auth.Discord.CSEC, cmd.Auth.Discord.CID = "csec", "cid" - cmd.Auth.Telegram = true - cmd.Telegram.Token = "token" cmd.Auth.Email.Enable = true cmd.Auth.Email.MsgTemplate = "testdata/email.tmpl" cmd.BackupLocation = "/tmp" diff --git a/backend/app/notify/telegram.go b/backend/app/notify/telegram.go index 43158cff43..e8fa8104c9 100644 --- a/backend/app/notify/telegram.go +++ b/backend/app/notify/telegram.go @@ -19,6 +19,7 @@ type TelegramParams struct { Timeout time.Duration // http client timeout UserNotifications bool // flag which enables user notifications ErrorMsg, SuccessMsg string // messages for successful and unsuccessful subscription requests to bot + APIURL string // bot API base url, for a proxy or a test double; the public API when empty } // Telegram implements notify.Destination for telegram @@ -36,6 +37,7 @@ func NewTelegram(params TelegramParams) (*Telegram, error) { Timeout: params.Timeout, ErrorMsg: params.ErrorMsg, SuccessMsg: params.SuccessMsg, + APIURL: params.APIURL, }) if err != nil { return nil, err diff --git a/backend/go.mod b/backend/go.mod index 22227b8815..b9a6b8a561 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -7,11 +7,11 @@ require ( github.com/PuerkitoBio/goquery v1.12.0 github.com/alecthomas/chroma/v2 v2.27.0 github.com/didip/tollbooth/v8 v8.0.1 - github.com/go-pkgz/auth/v2 v2.2.0 + github.com/go-pkgz/auth/v2 v2.3.0 github.com/go-pkgz/jrpc v0.4.2 github.com/go-pkgz/lcw/v2 v2.1.0 github.com/go-pkgz/lgr v0.12.4 - github.com/go-pkgz/notify v1.4.0 + github.com/go-pkgz/notify v1.5.0 github.com/go-pkgz/repeater/v2 v2.2.0 github.com/go-pkgz/rest v1.24.0 github.com/go-pkgz/routegroup v1.6.1 diff --git a/backend/go.sum b/backend/go.sum index 269beacdb0..e0cb29a8ed 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -40,8 +40,8 @@ github.com/gavv/httpexpect v2.0.0+incompatible h1:1X9kcRshkSKEjNJJxX9Y9mQ5BRfbxU github.com/gavv/httpexpect v2.0.0+incompatible/go.mod h1:x+9tiU1YnrOvnB725RkpoLv1M62hOWzwo5OXotisrKc= github.com/go-oauth2/oauth2/v4 v4.5.4 h1:YjI0tmGW8oxVhn9QSBIxlr641QugWrJY5UWa6XmLcW0= github.com/go-oauth2/oauth2/v4 v4.5.4/go.mod h1:BXiOY+QZtZy2ewbsGk2B5P8TWmtz/Rf7ES5ZttQFxfQ= -github.com/go-pkgz/auth/v2 v2.2.0 h1:vQO+GTFDjAaBNSdcLLLr3Xibka67GZPtkRRItVVS4ow= -github.com/go-pkgz/auth/v2 v2.2.0/go.mod h1:iZx2JiGZ8Aef+wM0BPLMQY8aur4fLE0uyPhFRO9dYQ4= +github.com/go-pkgz/auth/v2 v2.3.0 h1:Y/ZZaMfn0YrxNC+3wh+U/n0fH+Jwanfjur+nZ/9XO3Y= +github.com/go-pkgz/auth/v2 v2.3.0/go.mod h1:iZx2JiGZ8Aef+wM0BPLMQY8aur4fLE0uyPhFRO9dYQ4= github.com/go-pkgz/email v0.8.0 h1:6+Tgjfj7zFccFCPmURV2spKDXDb7aX/iWXBY2hBa9ww= github.com/go-pkgz/email v0.8.0/go.mod h1:+wgi4x7S33IuCzfcCM5euN0GwQG6XvO/PBLxrNffYLI= github.com/go-pkgz/expirable-cache/v3 v3.1.1 h1:ryHiSI5gBE8aJ0Jt90VFMKZxe/cosf2dZPDcUTMaHNg= @@ -52,8 +52,8 @@ github.com/go-pkgz/lcw/v2 v2.1.0 h1:JAGUHRQPon658XimxIUwaqPgOPfIKa850qO8jpFJchc= github.com/go-pkgz/lcw/v2 v2.1.0/go.mod h1:UUo4cgD6oTPooBuUslVaWqOYZAGnh/91SoaB5DBWC8s= github.com/go-pkgz/lgr v0.12.4 h1:lDeQ4BR28ldXrKau6BOjq7A8nHzcXz+MF4xUfV4l1Ok= github.com/go-pkgz/lgr v0.12.4/go.mod h1:Lw6DkNRnCPyX07mqkiUK/p+eA1opq4GKkWfWia64RA8= -github.com/go-pkgz/notify v1.4.0 h1:4pP7UGdYqFO7e7V3OsQStYF006CO0cCh1ahdawt6l18= -github.com/go-pkgz/notify v1.4.0/go.mod h1:UFpL9ZvCYnLBEjeay++3afh8GceZ6qT8wj5lqfSR9U4= +github.com/go-pkgz/notify v1.5.0 h1:y+/1jozruHk2qWXt3pUMsGcgI8mTFdxF/iCXS63WjVY= +github.com/go-pkgz/notify v1.5.0/go.mod h1:UFpL9ZvCYnLBEjeay++3afh8GceZ6qT8wj5lqfSR9U4= github.com/go-pkgz/repeater/v2 v2.2.0 h1:8nZR/NaknmLfx2YMHbr78u9OL4Xj+8+romm9dz4FpMg= github.com/go-pkgz/repeater/v2 v2.2.0/go.mod h1:RgX5vUbLKq7PV82QUDP5pFbQS1os4Z+U9XzKymK23A8= github.com/go-pkgz/rest v1.24.0 h1:GAUCgx7U8xCOC2OynLjhCRMhtnMQH4d1mTdKpQyX2yI= diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go index a23de201f0..795b78c16a 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/telegram.go @@ -27,6 +27,9 @@ import ( authtoken "github.com/go-pkgz/auth/v2/token" ) +// TelegramAPIBaseURL is the public Telegram bot API, used unless a caller supplies its own +const TelegramAPIBaseURL = "https://api.telegram.org" + // TelegramHandler implements login via telegram type TelegramHandler struct { logger.L @@ -360,8 +363,14 @@ func (th *TelegramHandler) LogoutHandler(w http.ResponseWriter, _ *http.Request) // tgAPI implements TelegramAPI type tgAPI struct { logger.L - token string - client *http.Client + token string + client *http.Client + baseURL string + // avatarClient is set only by NewTelegramAPIWithBaseURL. A caller who supplies a base URL is + // pointing the whole API elsewhere, and its TLS material, redirect policy and proxy settings + // live on the client it passed, so the avatar download has to use it too. Left nil by + // NewTelegramAPI so existing callers keep the transport they have always had there. + avatarClient *http.Client // identifier of the first update to be requested. // should be equal to LastSeenUpdateID + 1 @@ -371,10 +380,87 @@ type tgAPI struct { // NewTelegramAPI returns initialized TelegramAPI implementation func NewTelegramAPI(token string, client *http.Client) TelegramAPI { - return &tgAPI{ - client: client, - token: token, + // the default base is a constant and cannot fail validation; a nil client is what the + // caller already had before, so this keeps its behavior rather than changing it + return &tgAPI{client: client, token: token, baseURL: TelegramAPIBaseURL} +} + +// NewTelegramAPIWithBaseURL makes a Telegram API client talking to baseURL instead of the public +// API. Intended for a proxy in front of Telegram and for tests that need to answer as Telegram; +// every request the client makes goes through it, avatar downloads included, so the production +// path stays the code under test. Both forms are derived from it, as baseURL/bot/ +// and baseURL/file/bot/. +// +// An empty baseURL falls back to the public API. Anything else has to be an absolute http or +// https URL with a host and no userinfo, query, fragment or opaque part, since every request +// carries the bot token in its path and a malformed base sends it somewhere else. A path prefix +// is allowed, for a proxy mounted under one. +// +// Redirects are refused by default, because Go copies the previous URL into Referer on any hop +// that is not https-to-http and every URL here carries the token. A client arriving with its own +// CheckRedirect keeps it: that hook is this caller's policy about a base they chose, and following +// a redirect under it hands the token to the destination unless the hook strips the header or +// refuses the hop itself. +func NewTelegramAPIWithBaseURL(token string, client *http.Client, baseURL string) (TelegramAPI, error) { + if client == nil { + return nil, fmt.Errorf("nil http client") + } + + baseURL = strings.TrimRight(baseURL, "/") + if baseURL == "" { + baseURL = TelegramAPIBaseURL + } + + u, err := validateTelegramBaseURL(baseURL) + if err != nil { + return nil, err } + + // rebuild from what was validated, so the string checked is the string used. Parsing accepts + // shapes the checks below see as empty while fmt.Sprintf does not: a trailing "?" sets + // ForceQuery with RawQuery empty, and would push the whole "/bot/method" into the query + // string, which is the part logs and referrers capture most eagerly. A trailing "#" is the + // mirror image and would bury every request in a fragment that is never sent + u.ForceQuery = false + u.Path = strings.TrimRight(u.Path, "/") + + return &tgAPI{client: client, token: token, baseURL: u.String(), avatarClient: client}, nil +} + +// validateTelegramBaseURL rejects anything that would send the bot token somewhere other than the +// host the caller meant. "https://api.telegram.org@evil.tld" parses to host evil.tld, which is the +// shape this exists for. +func validateTelegramBaseURL(baseURL string) (*neturl.URL, error) { + u, err := neturl.Parse(baseURL) + if err != nil { + // the parse error is dropped rather than wrapped. *url.Error prints its URL field + // verbatim, so a base that is both credentialed and malformed, which never reaches the + // userinfo case below, would carry the credentials into the log; and the inner error is + // no safer, since "invalid port \":s3cr3t\" after host" and the IPv6 zone path both + // quote the input back + return nil, errors.New("telegram api base url is not a valid url") + } + // no rejection echoes the value, here or in the parse branch above: it is configuration that + // can carry credentials in its userinfo or a secret in its query or its port, and the error + // travels to whatever logs the constructor failure. Naming the property that was wrong tells + // the operator what to fix without that + switch { + case u.Opaque != "": + return nil, errors.New("telegram api base url must not be opaque") + case u.Scheme != "http" && u.Scheme != "https": + // the scheme is not a secret, and knowing which one was read is what makes this fixable + return nil, fmt.Errorf("telegram api base url must be http or https, got %q", u.Scheme) + case u.Hostname() == "": + // u.Host is non-empty for "http://:9000", which resolves to the local machine + return nil, errors.New("telegram api base url must have a host") + case u.User != nil: + return nil, errors.New("telegram api base url must not carry userinfo") + case u.RawQuery != "" || u.ForceQuery: + return nil, errors.New("telegram api base url must not carry a query") + case u.Fragment != "" || strings.HasSuffix(baseURL, "#"): + return nil, errors.New("telegram api base url must not carry a fragment") + } + return u, nil } // GetUpdates fetches incoming updates @@ -443,7 +529,7 @@ func (tg *tgAPI) Avatar(ctx context.Context, id int) (string, error) { return "", err } - avatarURL := fmt.Sprintf("https://api.telegram.org/file/bot%s/%s", tg.token, fileMetadata.Result.Path) + avatarURL := fmt.Sprintf("%s/file/bot%s/%s", tg.baseURL, tg.token, fileMetadata.Result.Path) return avatarURL, nil } @@ -466,27 +552,36 @@ func (tg *tgAPI) BotInfo(ctx context.Context) (*botInfo, error) { if resp.Result == nil { return nil, fmt.Errorf("received empty result") } + // LoginHandler returns this straight to an unauthenticated caller in its "bot" field, and with + // a caller-supplied base the answer comes from whatever host that points at. An upstream free + // to choose the string could put the request URI, and so the bot token, into it + if !telegramUsername.MatchString(resp.Result.Username) { + return nil, fmt.Errorf("telegram api returned an implausible bot username") + } return resp.Result, nil } func (tg *tgAPI) request(ctx context.Context, method string, data any) error { return repeater.NewFixed(3, time.Millisecond*50).Do(ctx, func() error { - url := fmt.Sprintf("https://api.telegram.org/bot%s/%s", tg.token, method) + url := fmt.Sprintf("%s/bot%s/%s", tg.baseURL, tg.token, method) req, err := http.NewRequestWithContext(ctx, "GET", url, http.NoBody) if err != nil { - return fmt.Errorf("failed to create request: %w", redactBotURLInErr(err)) + return fmt.Errorf("failed to create request: %w", tg.redactToken(redactBotURLInErr(err))) } - resp, err := tg.client.Do(req) + resp, err := noRedirect(tg.client).Do(req) if err != nil { - return fmt.Errorf("failed to send request: %w", redactBotURLInErr(err)) + return fmt.Errorf("failed to send request: %w", tg.redactToken(redactBotURLInErr(err))) } defer resp.Body.Close() //nolint gosec // we don't care about response body if resp.StatusCode != http.StatusOK { - return tg.parseError(resp.Body, resp.StatusCode) + // the upstream controls this text, and a proxy standing in for the API can echo the + // request URI straight back into it. Redaction by URL shape would not catch that, so + // the token itself is scrubbed from whatever comes out + return tg.redactToken(tg.parseError(resp.Body, resp.StatusCode)) } if err = json.NewDecoder(resp.Body).Decode(data); err != nil { @@ -497,6 +592,65 @@ func (tg *tgAPI) request(ctx context.Context, method string, data any) error { }) } +// redactToken removes the bot token from an error before it reaches a log. The upstream decides +// the text of an API error, and a proxy answering for the API can echo the request URI into it, +// so scrubbing the token is what holds rather than matching a URL shape. +func (tg *tgAPI) redactToken(err error) error { + if err == nil || tg.token == "" { + return err + } + msg := err.Error() + for _, form := range []string{tg.token, neturl.QueryEscape(tg.token), neturl.PathEscape(tg.token)} { + msg = strings.ReplaceAll(msg, form, "") + } + + // substitution only catches encodings we thought of, and the upstream picks the encoding. So + // the result is checked once more against a decoded, case-folded copy: if the token is still + // recoverable from the text by any of those routes, the text goes rather than the token stays. + // A proxy echoing "%2Fbot1234%3ASECRET%2FgetMe" defeats every ReplaceAll above and lands here + if tokenRecoverable(msg, tg.token) { + return fmt.Errorf("unexpected telegram API error, text withheld because it carried the bot token") + } + + if msg == err.Error() { + return err + } + return errors.New(msg) +} + +// tokenRecoverable reports whether token can still be read out of msg after undoing the encodings +// an upstream might have applied to it +func tokenRecoverable(msg, token string) bool { + lowered := strings.ToLower(token) + // decode repeatedly, because one pass turns a double-encoded "%253A" into "%3A" and leaves the + // token just as recoverable as it was. The cap stops a pathological input looping + seen := msg + for i := 0; i < 5; i++ { + if strings.Contains(strings.ToLower(seen), lowered) { + return true + } + next, err := neturl.QueryUnescape(seen) + if err != nil { + if next, err = neturl.PathUnescape(seen); err != nil { + // malformed escaping, so the text cannot be decoded and the token cannot be shown + // absent from it. One stray "%" in whatever the upstream echoed reaches here, and + // answering "not recoverable" would release a message that may still carry the + // token in an encoding this never got to undo + return true + } + } + if next == seen { + // decoding has converged and the loop has already examined this form, so the token is + // absent from every encoding of the text rather than merely undecided + return false + } + seen = next + } + // the cap ran out while the text was still changing, so the fully decoded form was never + // examined. Same reasoning as the decode failure above: undecided means withhold + return true +} + func (tg *tgAPI) parseError(r io.Reader, statusCode int) error { tgErr := struct { Description string `json:"description"` @@ -507,6 +661,15 @@ func (tg *tgAPI) parseError(r io.Reader, statusCode int) error { return fmt.Errorf("unexpected telegram API status code %d, error: %q", statusCode, tgErr.Description) } +// telegramAvatarClientProvider is the optional capability that lets the avatar download reuse the +// client the API was built with, rather than a default one that would drop custom CA, client +// certificates, redirect policy and proxy settings. +type telegramAvatarClientProvider interface { + avatarHTTPClient() *http.Client +} + +func (tg *tgAPI) avatarHTTPClient() *http.Client { return tg.avatarClient } + // avatarContentSaver matches the optional method on AvatarSaver implementations // that can store already-fetched bytes (avatar.Proxy provides one). Used by the // Telegram provider to avoid passing a bot-token-bearing URL through the @@ -549,7 +712,20 @@ func (th *TelegramHandler) saveTelegramAvatar(ctx context.Context, userID, avata return "" } client := &http.Client{Timeout: 5 * time.Second} - resp, err := client.Do(req) + // only an API pointed at a caller-supplied base carries a client for this. NewTelegramAPI + // leaves it nil and keeps the default above, which is the transport those callers have always + // had here; *tgAPI satisfies the interface either way, so the nil is the signal, not the + // assertion + if provider, ok := th.Telegram.(telegramAvatarClientProvider); ok { + if supplied := provider.avatarHTTPClient(); supplied != nil { + client = supplied + } + } + // the cap is applied through the context so the client's Transport, CheckRedirect and Jar + // survive, which building a fresh http.Client with a Timeout would discard + avatarCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + resp, err := noRedirect(client).Do(req.WithContext(avatarCtx)) if err != nil { th.Logf("[WARN] telegram avatar fetch failed: %v", redactBotURLInErr(err)) return "" @@ -587,6 +763,36 @@ const maxTelegramAvatarSize = 10 << 20 // the username "botFather" appearing elsewhere in a log line). Replacement // preserves the slashes via "/bot/" to keep surrounding URL // structure intact for diagnostics. +// noRedirect returns a client that refuses redirects, as the default for a caller who expressed no +// policy of their own. Every URL here carries the bot token in its path, and Go copies the previous +// URL into Referer on any hop that is not https-to-http, so following a redirect hands the +// destination the token, usually into its access log. Telegram's own API does not redirect; a proxy +// standing in for it can, so refusing is the right default. +// +// A client that already carries a CheckRedirect is returned untouched. That hook is the operator's +// explicit policy about a base URL they chose themselves, and replacing it would override a +// decision this package is in no position to second-guess. It is their responsibility from there: +// see NewTelegramAPIWithBaseURL on what a permitted redirect exposes. +// +// The refusing copy is shallow, so the caller's client is untouched and its Transport is shared: +// custom CA, client certificates and proxy settings all survive +func noRedirect(c *http.Client) *http.Client { + if c == nil { + return nil + } + if c.CheckRedirect != nil { + return c + } + cp := *c + cp.CheckRedirect = func(_ *http.Request, _ []*http.Request) error { + return errors.New("refusing to follow a telegram api redirect: the bot token travels in the URL") + } + return &cp +} + +// telegramUsername is the shape Telegram allows: 5 to 32 of [A-Za-z0-9_] +var telegramUsername = regexp.MustCompile(`^[A-Za-z0-9_]{5,32}$`) + var botTokenInURLPath = regexp.MustCompile(`/bot[A-Za-z0-9:_-]+/`) // redactBotURLInErr returns the error with any embedded Telegram bot-token diff --git a/backend/vendor/github.com/go-pkgz/notify/telegram.go b/backend/vendor/github.com/go-pkgz/notify/telegram.go index 62a8456e72..63a0a41fb5 100644 --- a/backend/vendor/github.com/go-pkgz/notify/telegram.go +++ b/backend/vendor/github.com/go-pkgz/notify/telegram.go @@ -26,7 +26,13 @@ type TelegramParams struct { Timeout time.Duration // http client timeout ErrorMsg, SuccessMsg string // messages for successful and unsuccessful subscription requests to bot - apiPrefix string // changed only in tests + // APIURL points the client at something other than the public bot API, for a proxy standing in + // front of Telegram or a test double answering as it. Empty means the public API. Every request + // carries the bot token in its path, so this is validated rather than trusted: see + // validateTelegramBaseURL for what is refused and why. + APIURL string + + apiPrefix string // derived from APIURL, or the public API } // Telegram notifications client @@ -78,8 +84,20 @@ const tgCleanupInterval = time.Minute * 5 func NewTelegram(params TelegramParams) (*Telegram, error) { res := Telegram{TelegramParams: params} - if res.apiPrefix == "" { + switch { + case res.APIURL != "": + base, err := validateTelegramBaseURL(res.APIURL) + if err != nil { + return nil, err + } + // the "bot" literal belongs to the API's own shape rather than to the operator's base, so + // it is appended here: a caller passes https://proxy.example.com and the URLs come out as + // https://proxy.example.com/bot/, which is what Telegram itself serves + res.apiPrefix = base + "/bot" + case res.apiPrefix == "": res.apiPrefix = telegramAPIPrefix + default: + // a prefix set directly, which the package's own tests do } if res.Timeout == 0 { res.Timeout = telegramTimeOut @@ -450,6 +468,45 @@ func (t *Telegram) botInfo(ctx context.Context) (*TelegramBotInfo, error) { return resp.Result, nil } +// validateTelegramBaseURL checks a caller-supplied API base and returns it without its trailing +// slash. Every request built from it carries the bot token in its path, so a base that resolves +// somewhere unintended ships the token there: "https://api.telegram.org@evil.tld" is a valid URL +// whose host is evil.tld, and a bare scheme or an opaque form silently produces a request to +// somewhere else again. A path prefix is allowed, for a proxy mounted under one. +// +// No rejection echoes the value. It is configuration that can carry credentials in its userinfo, a +// secret in its query or a password where the port belongs, and the error travels to whatever logs +// the constructor failure. The parse error is dropped for the same reason: *url.Error prints the +// URL it was given, and the inner error quotes the input back as well. +func validateTelegramBaseURL(baseURL string) (string, error) { + baseURL = strings.TrimRight(baseURL, "/") + + u, err := neturl.Parse(baseURL) + if err != nil { + return "", errors.New("telegram api url is not a valid url") + } + + switch { + case u.Opaque != "": + return "", errors.New("telegram api url must not be opaque") + case u.Scheme != "http" && u.Scheme != "https": + return "", fmt.Errorf("telegram api url must be http or https, got %q", u.Scheme) + case u.Hostname() == "": + // u.Host is non-empty for "http://:9000", which resolves to the local machine + return "", errors.New("telegram api url must have a host") + case u.User != nil: + return "", errors.New("telegram api url must not carry userinfo") + case u.RawQuery != "" || u.ForceQuery: + return "", errors.New("telegram api url must not carry a query") + case u.Fragment != "" || strings.HasSuffix(baseURL, "#"): + return "", errors.New("telegram api url must not carry a fragment") + } + + // rebuilt from what was checked, so the string validated is the string used + u.Path = strings.TrimRight(u.Path, "/") + return u.String(), nil +} + // Request makes a request to the Telegram API and return the result func (t *Telegram) Request(ctx context.Context, method string, b []byte, data any) error { return repeater.NewFixed(3, time.Millisecond*250).Do(ctx, func() error { @@ -469,7 +526,16 @@ func (t *Telegram) Request(ctx context.Context, method string, b []byte, data an req.Header.Set("Content-Type", "application/json; charset=utf-8") } - client := http.Client{Timeout: t.Timeout} + // refusing redirects: Go copies the previous URL into Referer on any hop that is not + // https-to-http, and every URL here carries the bot token in its path, so following one + // hands the destination the token, usually into its access log. Telegram does not redirect; + // something standing in for it can + client := http.Client{ + Timeout: t.Timeout, + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { + return errors.New("refusing to follow a telegram api redirect: the bot token travels in the URL") + }, + } resp, err := client.Do(req) if err != nil { return fmt.Errorf("failed to send request: %w", t.redactToken(err)) @@ -477,7 +543,7 @@ func (t *Telegram) Request(ctx context.Context, method string, b []byte, data an defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return t.parseError(resp.Body, resp.StatusCode) + return t.redactToken(t.parseError(resp.Body, resp.StatusCode)) } if err = json.NewDecoder(resp.Body).Decode(data); err != nil { @@ -488,14 +554,91 @@ func (t *Telegram) Request(ctx context.Context, method string, b []byte, data an }) } -// redactToken hides the bot token in the URL of *url.Error returned by the http client, -// as the token is a part of every API URL and otherwise leaks into the logs of the caller printing the error +// redactToken removes the bot token from an error before it reaches a caller's log. +// +// Two routes, and the second only exists once APIURL can point somewhere the operator chose. The +// token is part of every API URL, so a transport failure carries it in *url.Error's URL field. And +// the upstream decides the text of an API error: something standing in for Telegram can echo the +// request URI into its description, in whatever encoding it likes, so scrubbing the token itself is +// what holds rather than matching a URL shape. func (t *Telegram) redactToken(err error) error { + if err == nil || t.Token == "" { + return err + } + var urlErr *neturl.Error - if t.Token == "" || !errors.As(err, &urlErr) || !strings.Contains(urlErr.URL, t.Token) { + if errors.As(err, &urlErr) && strings.Contains(urlErr.URL, t.Token) { + return &neturl.Error{ + Op: urlErr.Op, URL: strings.ReplaceAll(urlErr.URL, t.Token, ""), Err: urlErr.Err, + } + } + + // the text scrub is for something that could actually be a bot token. Telegram issues them as + // ":", and blanking a short arbitrary string out of a diagnostic corrupts more + // than it protects: a token of "404" would turn every "status code 404" into "status code + // ". The URL-field redaction above stays unconditional, since there the token is + // whatever the caller configured and the field is nothing else + if !looksLikeBotToken(t.Token) { return err } - return &neturl.Error{Op: urlErr.Op, URL: strings.ReplaceAll(urlErr.URL, t.Token, ""), Err: urlErr.Err} + + msg := err.Error() + for _, form := range []string{t.Token, neturl.QueryEscape(t.Token), neturl.PathEscape(t.Token)} { + msg = strings.ReplaceAll(msg, form, "") + } + + // substitution only catches encodings we thought of, and the upstream picks the encoding, so the + // result is checked once more against a decoded copy. If the token is still recoverable by any + // of those routes the text goes rather than the token stays + if tokenRecoverable(msg, t.Token) { + return errors.New("unexpected telegram API error, text withheld: the bot token could not be ruled out of it") + } + + if msg == err.Error() { + return err + } + return errors.New(msg) +} + +// looksLikeBotToken reports whether token has the shape Telegram issues, ":". Used +// to keep the text scrub off values that cannot be one, where it would damage diagnostics for +// nothing +func looksLikeBotToken(token string) bool { + id, secret, found := strings.Cut(token, ":") + if !found || len(secret) < 8 { + return false + } + if _, err := strconv.Atoi(id); err != nil { + return false + } + return true +} + +// tokenRecoverable reports whether token can still be read out of msg after undoing the encodings +// an upstream might have applied to it. +// +// Fails closed: after a decode error, or after the cap runs out while the text is still changing, +// absence was never established, and withholding is the only answer that cannot leak. One stray "%" +// in whatever the upstream echoed is enough to reach that. +func tokenRecoverable(msg, token string) bool { + lowered := strings.ToLower(token) + seen := msg + for i := 0; i < 5; i++ { + if strings.Contains(strings.ToLower(seen), lowered) { + return true + } + next, err := neturl.QueryUnescape(seen) + if err != nil { + return true + } + if next == seen { + // decoding has converged and every form has been examined, so the token is absent + // rather than undecided + return false + } + seen = next + } + return true } func (t *Telegram) parseError(r io.Reader, statusCode int) error { diff --git a/backend/vendor/modules.txt b/backend/vendor/modules.txt index 8ca7393b0e..9af9832a26 100644 --- a/backend/vendor/modules.txt +++ b/backend/vendor/modules.txt @@ -44,7 +44,7 @@ github.com/dlclark/regexp2/v2/syntax github.com/go-oauth2/oauth2/v4 github.com/go-oauth2/oauth2/v4/errors github.com/go-oauth2/oauth2/v4/server -# github.com/go-pkgz/auth/v2 v2.2.0 +# github.com/go-pkgz/auth/v2 v2.3.0 ## explicit; go 1.25.0 github.com/go-pkgz/auth/v2 github.com/go-pkgz/auth/v2/avatar @@ -69,7 +69,7 @@ github.com/go-pkgz/lcw/v2/eventbus # github.com/go-pkgz/lgr v0.12.4 ## explicit; go 1.21 github.com/go-pkgz/lgr -# github.com/go-pkgz/notify v1.4.0 +# github.com/go-pkgz/notify v1.5.0 ## explicit; go 1.25.0 github.com/go-pkgz/notify # github.com/go-pkgz/repeater/v2 v2.2.0 diff --git a/compose-e2e-coverage.yml b/compose-e2e-coverage.yml index 0fe8a7b19f..25bd45975b 100644 --- a/compose-e2e-coverage.yml +++ b/compose-e2e-coverage.yml @@ -18,6 +18,12 @@ services: volumes: - ./e2e/coverage/remark42:/coverage + remark42-telegram: + environment: + - GOCOVERDIR=/coverage + volumes: + - ./e2e/coverage/remark42-telegram:/coverage + remark42-shortedit: environment: - GOCOVERDIR=/coverage diff --git a/compose-e2e-test.yml b/compose-e2e-test.yml index cf97f02b3a..bbd3ee40e1 100644 --- a/compose-e2e-test.yml +++ b/compose-e2e-test.yml @@ -176,6 +176,59 @@ services: timeout: 3s retries: 30 + # answers as the Telegram bot API, so the auth flow that ends inside Telegram becomes drivable. + # the provider's configurable base url directs both authentication and notifications here + telegram-stub: + build: + context: ./e2e/telegramstub + container_name: "remark42-e2e-telegram-stub" + ports: + - "127.0.0.1:8091:8091" + environment: + - STUB_ADDR=:8091 + - STUB_BOT_USERNAME=remark42_e2e_bot + # the token the stub demands in the path, and the one the instance below presents + - STUB_BOT_TOKEN=stub-token + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8091/botstub-token/getMe"] + interval: 2s + timeout: 3s + retries: 30 + + # telegram auth on its own instance: adding it to the main one would change the provider list + # every other case reads out of the auth panel + remark42-telegram: + image: ghcr.io/umputun/remark42:dev + container_name: "remark42-e2e-telegram" + pull_policy: never + depends_on: + remark42: + condition: service_started + telegram-stub: + condition: service_healthy + + ports: + - "127.0.0.1:8087:8080" + + environment: + - REMARK_URL=http://remark42-telegram:8087 + - SECRET=12345 + - AUTH_TELEGRAM=true + - TELEGRAM_TOKEN=stub-token + # the whole reason this instance exists: every bot call goes to the stub instead of Telegram + - TELEGRAM_API_URL=http://telegram-stub:8091 + # notifications through the same stub, which is what makes the telegram subscription flow + # drivable: the update poll runs through the notify service, so both halves have to agree + - NOTIFY_USERS=telegram + - UPDATE_LIMIT=100 + volumes: + - remark42-e2e-telegram-var:/srv/var + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/ping"] + interval: 2s + timeout: 3s + retries: 30 + # no auth provider at all, which #1456 reported rendering as a widget with nothing to say. it # cannot be a setting on another instance: the absence is the configuration remark42-noauth: @@ -335,6 +388,7 @@ volumes: remark42-e2e-var: remark42-e2e-shortedit-var: remark42-e2e-adminedit-var: + remark42-e2e-telegram-var: remark42-e2e-jwtheader-var: remark42-e2e-noauth-var: remark42-e2e-anonvote-var: diff --git a/e2e/README.md b/e2e/README.md index eea222c8f6..ce5c0a477a 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -24,7 +24,7 @@ make e2e make e2e-down ``` -The default run admits four top-level tests at once. Each browser context has isolated cookies, storage, a unique thread URL and a stable test-only client IP, so the backend's per-IP controls remain local to one reader. The last-comments case reads a site-wide feed, so it stays serial. +The default run admits four top-level tests at once. Each browser context has isolated cookies, storage, a unique thread URL and a stable test-only client IP, so the backend's per-IP controls remain local to one reader. The Telegram cases share one bot stub, and the last-comments case reads a site-wide feed, so those groups stay serial. Run from `e2e/`; the compose path is relative to it. A single test: @@ -106,13 +106,13 @@ A failing test also logs whatever the browser wrote to its console, which is whe The suite refuses a running stack that was not brought up from the sources under test. Every checkout builds the image tag the compose file names, so a stack from another worktree, or from this one before an edit, answers on these ports and passes every readiness probe while serving code nobody is looking at. -`e2e/stamp.sh` digests the content of `backend`, `frontend`, `Dockerfile` and `docker-init.sh`; compose puts it in the container's environment and the suite reads it back. It digests content and not `HEAD`, so a commit touching only the suite does not invalidate a stack. `make e2e-up` stamps the same way, so a stack started by hand is accepted. On a mismatch the failure says to run `make e2e-down`. +`e2e/stamp.sh` digests the content of `backend`, `frontend`, `Dockerfile`, `docker-init.sh` and `e2e/telegramstub`, which is the second image compose builds from sources here; compose puts it in the container's environment and the suite reads it back. It digests content and not `HEAD`, so a commit touching only the suite does not invalidate a stack. `make e2e-up` stamps the same way, so a stack started by hand is accepted. On a mismatch the failure says to run `make e2e-down`. Before pushing, `cd e2e && go vet -tags=e2e ./...` and `golangci-lint run --build-tags=e2e --config ../backend/.golangci.yml`. CI runs both, and neither is covered by a plain `go vet ./...` because of the build tag. ## The stack -`compose-e2e-test.yml` at the repository root runs ten services, each bound to the loopback interface since it holds a known secret and an admin shared id: +`compose-e2e-test.yml` at the repository root runs twelve services, each bound to the loopback interface since it holds a known secret and an admin shared id: - **remark42** on `:8080`, with the dev oauth2 provider on `:8084`, anonymous and email sign-in - **remark42-shortedit** on `:8081`, with `EDIT_TIME=15s` and anonymous sign-in only, since the dev oauth2 provider's port is fixed at 8084 and cannot be published twice. It exists so the expired-edit path is observable without holding a test open for the default five minutes @@ -123,13 +123,15 @@ Before pushing, `cd e2e && go vet -tags=e2e ./...` and `golangci-lint run --buil - **host-site** on `:8090`, an nginx serving `e2e/hostsite/`, which is a page on an origin the widget is not served from. Every other host page here is served by remark42 itself, so without it the separate-domain setup the manuals describe is never exercised. `post.html` embeds the main instance; `restricted.html` embeds the one whose `ALLOWED_HOSTS` names only itself, which is the refusal case - **remark42-https** on `:8443`, the widget over TLS with `AUTH_SAME_SITE=none` and `AUTH_SEND_JWT_HEADER=true`. The header mode is what makes the widget write its own cookies through `setAuthCookie`, so the attributes it chooses are observable at all. Anonymous and email sign-in are both enabled, since the third-party cases run each flow: the writer keys off the `X-JWT` header rather than off the provider, and that is an assumption worth measuring rather than asserting - **host-site-https** on `:8444`, an nginx serving the same `e2e/hostsite/` over TLS, so the embed is cross-site *and* secure. `post-https.html` is its page +- **remark42-telegram** on `:8087`, telegram auth pointed at the stub below through `TELEGRAM_API_URL`. It has its own instance because telegram is an auth provider, and adding it to the main one would change the provider list every other case reads out of the auth panel +- **telegram-stub** on `:8091`, which answers as the Telegram bot API for the four calls the auth flow makes, takes an injected update through `/control/send`, and exposes the bot's replies through `/control/sent`. Those endpoints represent the exchange inside Telegram that a browser cannot perform. Its source is `e2e/telegramstub`, the one file in this module without the `e2e` build tag, and compose builds it as its own image from a throwaway module so the suite's browser tooling stays out of it - **mailpit** on `:8025`, which catches the email-auth verification message and the subscription token for the suite to read back Both TLS services read a self-signed certificate from `e2e/tls/`, which `e2e/tls/generate.sh` writes and `.gitignore` keeps out of the tree. `make e2e-up`, the workflow and `ensureStack` all run it before compose, so bringing the stack up by hand with a bare `docker compose up` is the one path that needs it run first. Every browser context and the readiness client accept that certificate, and they talk to nothing else. The main instance enables the notify module (`NOTIFY_USERS=email`). Without it `email_notifications` is false in the config, the widget never renders the subscribe control, and the whole subscribe, confirm and unsubscribe flow is unreachable from a browser. -The remark42 instances beyond the first offer anonymous sign-in only, for the reason `remark42-shortedit` does: the dev oauth2 provider binds a port fixed at 8084 and cannot be published twice. `remark42-adminedit` enables email authentication and sets `ADMIN_SHARED_ID` to the SHA-1-derived ID of `adminedit@example.com`, which is the address its browser case uses. +The remark42 instances beyond the first and `remark42-telegram` offer anonymous sign-in only, for the reason `remark42-shortedit` does: the dev oauth2 provider binds a port fixed at 8084 and cannot be published twice. `remark42-adminedit` enables email authentication and sets `ADMIN_SHARED_ID` to the SHA-1-derived ID of `adminedit@example.com`, which is the address its browser case uses. Four settings exist for the tests and not for realism, and each is there for a reason: @@ -144,6 +146,8 @@ Four settings exist for the tests and not for realism, and each is there for a r The stack carries TLS on two services, so behaviour the browser gates on the page protocol is reachable: `Secure` cookies, `SameSite=None`, `Partitioned`, and anything keyed on `window.location.protocol`. `https_test.go` is where those cases live. What is still out of reach is a browser engine other than Chromium for them, since the resolver rules the hostnames need are a Chromium flag. +The Telegram fixture reaches both authentication and subscription. The stub answers `getMe`, `getUpdates`, `sendMessage` and `getUserProfilePhotos`, so it does not hold the widget against the public bot API's semantics, and the reader's handoff into the Telegram client is injected from the test process. The suite exercises both consumers of `TELEGRAM_API_URL`: auth signs a reader in through the bot, while notifications confirm, persist, remove and repeat a subscription through the same update poll. + The trap that remains is which cookie policy a run is under. Playwright's own default `--disable-features` argument carries `ThirdPartyStoragePartitioning`, and it beats both `--test-third-party-cookie-phaseout` and `--block-third-party-cookies` passed through `Args`. A run configured that way keeps an ordinary third-party cookie exactly as it would with no flags at all, so it proves nothing while looking like it proved something. The lever is `IgnoreDefaultArgs` on the launch options: drop that default entry and re-supply `--disable-features` without that one feature, which is what `TestHTTPS_SessionSurvivesThirdPartyCookieBlocking` does. Measured on a cross-site https embed: | | ordinary third-party cookie | `Partitioned` cookie | diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 92239d561f..1e781698d5 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -22,6 +22,8 @@ // - hostframe_test.go: sender checks on both sides of the iframe boundary // - profile_test.go: the reader's own-comment overlay // - webfiles_test.go: the published /web surface +// - telegram_test.go: Telegram authentication through the bot API stub +// - telegramsub_test.go: the Telegram notification subscription round trip // - widgets_test.go: last-comments, counter and the profile iframe package e2e @@ -63,6 +65,9 @@ const ( noAuthURL = "http://remark42-noauth:8085" anonVoteURL = "http://remark42-anonvote:8086" + // telegram auth against a stub standing in for the bot api, see compose-e2e-test.yml + telegramURL = "http://remark42-telegram:8087" + // a page on an origin the widget is not served from, see compose-e2e-test.yml hostSiteURL = "http://host-site:8090" @@ -77,6 +82,10 @@ const ( jwtHeaderProbeURL = "http://127.0.0.1:8083" noAuthProbeURL = "http://127.0.0.1:8085" anonVoteProbeURL = "http://127.0.0.1:8086" + telegramProbeURL = "http://127.0.0.1:8087" + // the stub is driven from this process because the step it stands in for happens inside + // Telegram, where no page can reach + telegramStubURL = "http://127.0.0.1:8091" hostSiteProbeURL = "http://127.0.0.1:8090" httpsProbeURL = "https://127.0.0.1:8443" httpsHostProbeURL = "https://127.0.0.1:8444" @@ -183,6 +192,7 @@ func TestMain(m *testing.M) { "--host-resolver-rules=MAP remark42 127.0.0.1, MAP remark42-shortedit 127.0.0.1, " + "MAP remark42-adminedit 127.0.0.1, MAP remark42-jwtheader 127.0.0.1, " + "MAP remark42-noauth 127.0.0.1, MAP remark42-anonvote 127.0.0.1, " + + "MAP remark42-telegram 127.0.0.1, " + "MAP host-site 127.0.0.1, " + "MAP remark42-https 127.0.0.1, MAP host-site-https 127.0.0.1", }, @@ -269,6 +279,8 @@ func stackReady(timeout time.Duration) bool { jwtHeaderProbeURL + "/ping", noAuthProbeURL + "/ping", anonVoteProbeURL + "/ping", + telegramProbeURL + "/ping", + telegramStubURL + "/botstub-token/getMe", hostSiteProbeURL + "/post.html", httpsProbeURL + "/ping", httpsHostProbeURL + "/post-https.html", @@ -444,6 +456,7 @@ var readerIPAuthorities = []string{ "remark42-jwtheader:8083", "remark42-noauth:8085", "remark42-anonvote:8086", + "remark42-telegram:8087", "remark42-https:8443", "127.0.0.1:8080", "127.0.0.1:8081", @@ -451,6 +464,7 @@ var readerIPAuthorities = []string{ "127.0.0.1:8083", "127.0.0.1:8085", "127.0.0.1:8086", + "127.0.0.1:8087", "127.0.0.1:8443", } diff --git a/e2e/stamp.sh b/e2e/stamp.sh index ffb69206f9..73fb5591d0 100755 --- a/e2e/stamp.sh +++ b/e2e/stamp.sh @@ -1,18 +1,22 @@ #!/bin/sh -# Digest of everything that ends up in the e2e image. +# Digest of everything that ends up in an image the stack runs. # # The compose stack tags its image ghcr.io/umputun/remark42:dev, which every checkout of this # repository shares, so a stack brought up from one worktree answers on the same ports as one # brought up from another. The suite stamps the image it builds with this value and refuses a # running stack carrying a different one, so it never tests code nobody is looking at. # +# e2e/telegramstub is in the list because compose builds it too: it is a second image made from +# sources in this repository, and leaving it out let an edited stub run behind a stack the guard +# had just accepted. +# # Tracked content is covered exactly; an untracked file changes the digest when it appears, # by name, but later edits to it do not. set -eu cd "$(dirname "$0")/.." -sources="backend frontend Dockerfile docker-init.sh" +sources="backend frontend Dockerfile docker-init.sh e2e/telegramstub" digest() { if command -v sha256sum >/dev/null 2>&1; then diff --git a/e2e/telegram_test.go b/e2e/telegram_test.go new file mode 100644 index 0000000000..f1e5294587 --- /dev/null +++ b/e2e/telegram_test.go @@ -0,0 +1,268 @@ +//go:build e2e + +package e2e + +import ( + "encoding/json" + "fmt" + "hash/crc32" + "io" + "net/http" + "net/url" + "os" + "strings" + "testing" + "time" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Telegram auth is the one provider whose flow leaves the browser: the reader is told to message a +// bot, and the widget waits for the backend to see that message. Component tests cover the widget's +// state machine with mocked fetches; these cases also exercise the backend and bot exchange. +// +// The stub in e2e/telegramstub answers as the bot API and takes an injected update through its +// control endpoint, which is the step no page can perform. The instance pointing at it is +// remark42-telegram, on its own service so the main instance's provider list stays what every other +// case reads out of the auth panel. +// +// These cases stay serial because the stub exposes one shared update queue and sent-message log. + +// tgSendToBot injects the message a reader would send the bot. This process drives it because the +// step happens inside Telegram, where the browser has no reach +func tgSendToBot(t *testing.T, text, id, firstName string) { + t.Helper() + + q := url.Values{"text": {text}, "id": {id}, "first_name": {firstName}} + resp, err := probeClient.Get(telegramStubURL + "/control/send?" + q.Encode()) + require.NoError(t, err, "the telegram stub did not accept the update") + defer func() { _ = resp.Body.Close() }() + require.Equal(t, http.StatusNoContent, resp.StatusCode, "the telegram stub refused the update") +} + +// tgConfirmSignIn presses the widget's check button until the reader is signed in. +// +// Calls are spaced because the provider polls the bot API every five seconds (apiPollInterval in +// go-pkgz/auth), so the first press is legitimately early, and everything under /auth/ is capped at +// two requests a second per reader. A tight retry loop manufactures the 429s it would then have to +// interpret, so this waits out one poll interval and then presses on the limiter's own spacing, a +// handful of times at most +func tgConfirmSignIn(t *testing.T, page playwright.Page, frame playwright.FrameLocator) { + t.Helper() + + signOut := frame.Locator(`[title="Sign Out"]`) + submit := frame.Locator(`.auth-submit`) + + // two ways in, and which one happens is a race the test has no business deciding: the panel + // polls while it is open, so the sign-in can complete on its own, and the button is there for + // the reader who comes back to the tab after it stopped. + // + // The presence check costs the limiter nothing, so it runs often; the presses are what have to + // be spaced, since everything under /auth/ is capped at two requests a second. Detecting on the + // press interval instead would report a sign-in that landed immediately five seconds late, and + // a genuine failure half a minute late + deadline := time.Now().Add(waitTimeout) + var lastPress time.Time + for time.Now().Before(deadline) { + if n, err := signOut.Count(); err == nil && n > 0 { + assertSignedIn(t, page, frame) + return + } + + if time.Since(lastPress) >= 5*time.Second { + // the button goes as soon as the panel confirms on its own, so losing the race to it is + // not a failure: the next pass reads the signed-in state instead + if n, err := submit.Count(); err == nil && n > 0 { + pauseForAuthLimit() + _ = submit.Click() + } + lastPress = time.Now() + } + + time.Sleep(250 * time.Millisecond) + } + + // whatever the panel is showing is the diagnosis: a refused token and a token the backend never + // saw fail identically from out here + shown := "" + if n, err := frame.Locator(`.auth-error`).Count(); err == nil && n > 0 { + shown, _ = frame.Locator(`.auth-error`).First().TextContent() + } + t.Fatalf("the widget never confirmed the telegram sign-in, panel error: %q", shown) +} + +// tgSentMessages returns the messages the provider has asked the bot to deliver. Matching the text +// gives each wait its own barrier even when another process shares the stub. +func tgSentMessages(t *testing.T) []string { + t.Helper() + + resp, err := probeClient.Get(telegramStubURL + "/control/sent") + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + var sent []string + require.NoError(t, json.NewDecoder(resp.Body).Decode(&sent)) + return sent +} + +func tgWaitForMessage(t *testing.T, before int, contains string) { + t.Helper() + eventually(t, waitTimeout, "the bot never sent a message containing "+contains, func() bool { + messages := tgSentMessages(t) + for _, message := range messages[min(before, len(messages)):] { + if strings.Contains(strings.ToLower(message), strings.ToLower(contains)) { + return true + } + } + return false + }) +} + +// tgReaderID is stable within a test and distinct across tests, runs and processes. +func tgReaderID(t *testing.T) string { + t.Helper() + key := fmt.Sprintf("%s|%s|%d", runID, t.Name(), os.Getpid()) + return fmt.Sprintf("%d", 100_000_000+uint64(crc32.ChecksumIEEE([]byte(key)))%900_000_000) +} + +func telegramStartToken(t *testing.T, href string) string { + t.Helper() + parsed, err := url.Parse(href) + require.NoError(t, err, "parse telegram link %q", href) + token := parsed.Query().Get("start") + require.NotEmpty(t, token, "no token in the telegram link: %s", href) + return token +} + +// tgStartSignIn opens the auth panel, picks telegram and returns the token the backend minted. +// Reading it from the rendered link ties the panel to the request behind it +func tgStartSignIn(t *testing.T, frame playwright.FrameLocator) string { + t.Helper() + + // the backend saying the handler registered, before anything is driven through the panel. /ping + // answers while telegram auth is dead -- a stub the instance cannot reach leaves the provider + // off the list -- and without this the case fails on a locator timeout that names nothing + resp, err := probeClient.Get(telegramProbeURL + "/api/v1/config?site=remark") + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + var cfg struct { + AuthProviders []string `json:"auth_providers"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&cfg)) + require.Contains(t, cfg.AuthProviders, "telegram", + "the instance does not offer telegram auth, so it never reached the stub: %v", cfg.AuthProviders) + + pauseForAuthLimit() + require.NoError(t, frame.Locator(".auth-button").Click()) + waitVisible(t, frame.Locator(".auth-dropdown")) + + // the attribute carries the provider's display name, capitalised, and this instance offers + // exactly one provider, so the assertion is what ties the click to telegram, not the selector + provider := frame.Locator(`.oauth-button`).First() + waitVisible(t, provider) + name, nameErr := provider.GetAttribute("data-provider-name") + require.NoError(t, nameErr) + require.Equal(t, "telegram", strings.ToLower(name), "the only provider offered is not telegram") + require.NoError(t, provider.Click()) + + link := frame.Locator(`.telegram a`).First() + waitVisible(t, link) + + href, hrefErr := link.GetAttribute("href") + require.NoError(t, hrefErr) + require.Contains(t, href, "remark42_e2e_bot", "the panel names a bot the stub never reported: %s", href) + + return telegramStartToken(t, href) +} + +// TestTelegram_SignInThroughTheBot drives the whole two-step flow the widget performs: ask the +// backend for a token, show the reader a link carrying it, and then verify once the reader has +// messaged the bot. Both halves are asserted, since a widget that renders the link and never +// verifies looks identical to one that works until the reader tries it. +func TestTelegram_SignInThroughTheBot(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + + token := tgStartSignIn(t, frame) + + // the reader messages the bot, and the shared update dispatcher picks it up + tgSendToBot(t, "/start "+token, tgReaderID(t), "E2E Reader") + + // the widget verifies on demand, so this is the reader pressing the button after switching back + // from Telegram. The poll interval is five seconds, so the first + // press can legitimately come too early: press until the panel changes or the wait gives up + tgConfirmSignIn(t, page, frame) + + // the identity came from the update, not from a placeholder: the name is what the stub sent + waitVisible(t, frame.Locator(`text=E2E Reader`).First()) +} + +// TestTelegram_CommentSurvivesAReload is the other half of an auth provider being usable: a reader +// who signed in through Telegram can post, and is still that reader after a reload. The token lives +// in a cookie the backend set during verification, so a flow that signs in without persisting looks +// identical until the page reloads +func TestTelegram_CommentSurvivesAReload(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + + token := tgStartSignIn(t, frame) + + // a distinct id per run, so the comments of one run cannot be read as another's. Digits only, + // the way anonName builds a name: E2E_RUN_ID is "-" on CI, so anything slicing it + // raw hands the stub something strconv cannot read + tgSendToBot(t, "/start "+token, tgReaderID(t), "Reload Reader") + + tgConfirmSignIn(t, page, frame) + + text := "posted after telegram sign-in " + runID + postComment(t, frame, text) + + frame = reload(t, page) + + assertSignedIn(t, page, frame) + waitVisible(t, comment(frame, text)) +} + +// TestTelegram_UnknownTokenIsRefused pins that a login token is only ever honored for the request +// that minted it. The interesting negative is not an invented token, which fails whether or not the +// bot was ever involved: it is a real token from a real request, against a bot message carrying a +// different one. That fails the moment processUpdates stops matching what the reader sent against +// the requests it is holding, and nothing else here would notice. +func TestTelegram_UnknownTokenIsRefused(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + + minted := tgStartSignIn(t, frame) + before := len(tgSentMessages(t)) + + // the reader messages the bot with something else entirely + tgSendToBot(t, "/start deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", "9999", "Nobody") + + // the provider answers an unmatched token by messaging the reader, and the stub records every + // send. That condition marks the observable moment it + // has drained the update and decided + tgWaitForMessage(t, before, "authentication request was not found or expired") + + pauseForAuthLimit() + resp, err := probeClient.Get(telegramProbeURL + "/auth/telegram/login?site=remark&token=" + minted) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + // an exact status, not "anything but 200": a 429 from the limiter, a 500, or the route being + // gone altogether would all satisfy a looser assertion while proving nothing + assert.Equal(t, http.StatusNotFound, resp.StatusCode, + "a token nobody confirmed was answered with %d: %s", resp.StatusCode, body) + assert.Contains(t, string(body), "not verified", + "the refusal does not say the request was unverified: %s", body) + + // and nothing was handed out on the way past + for _, c := range resp.Cookies() { + assert.NotEqual(t, "JWT", c.Name, "an unverified request was given a token cookie") + } +} diff --git a/e2e/telegramstub/Dockerfile b/e2e/telegramstub/Dockerfile new file mode 100644 index 0000000000..82ed039ddc --- /dev/null +++ b/e2e/telegramstub/Dockerfile @@ -0,0 +1,17 @@ +# The stub is stdlib only, so it is compiled straight from the one source file with a throwaway +# module. Building it as part of the e2e module instead would pull playwright-go and its browser +# tooling into an image that needs none of it. +FROM golang:1.25-alpine AS build + +WORKDIR /src +COPY main.go . +RUN go mod init telegramstub >/dev/null && \ + CGO_ENABLED=0 go build -trimpath -o /telegramstub . + +FROM alpine:3.22 + +RUN apk add --no-cache curl +COPY --from=build /telegramstub /telegramstub + +EXPOSE 8091 +CMD ["/telegramstub"] diff --git a/e2e/telegramstub/main.go b/e2e/telegramstub/main.go new file mode 100644 index 0000000000..273ad68c88 --- /dev/null +++ b/e2e/telegramstub/main.go @@ -0,0 +1,198 @@ +// Command telegramstub answers as the Telegram bot API for the e2e suite. +// +// The provider talks to whatever base url it is given, so pointing remark42 at this instead of +// api.telegram.org is the whole of what makes the Telegram auth flow reachable from a browser test. +// It implements the four calls that flow uses and nothing else: getMe once at startup, getUpdates +// on a poll, sendMessage when a login is confirmed, and getUserProfilePhotos for the avatar. +// +// The test drives it through /control/send, which enqueues an update as if a reader had sent the +// bot a message, and reads /control/sent to match the bot's replies. Those are the steps no browser +// can perform, since they happen inside Telegram. +package main + +import ( + "encoding/json" + "log" + "net/http" + "os" + "strconv" + "strings" + "sync" +) + +// update is the subset of Telegram's Update the provider reads +type update struct { + UpdateID int `json:"update_id"` + Message struct { + Text string `json:"text"` + From struct { + ID int `json:"id"` + Username string `json:"username"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + LanguageCode string `json:"language_code"` + } `json:"from"` + // the provider skips any update whose chat is not private, and takes the reader's + // display name from the chat, not the sender + Chat struct { + ID int `json:"id"` + Name string `json:"first_name"` + Type string `json:"type"` + } `json:"chat"` + } `json:"message"` +} + +type stub struct { + mu sync.Mutex + queued []update + nextID int + botName string + token string + + // every message the provider asked to send, so a test can assert the reader was told + sent []string +} + +func main() { + addr := os.Getenv("STUB_ADDR") + if addr == "" { + addr = ":8091" + } + botName := os.Getenv("STUB_BOT_USERNAME") + if botName == "" { + botName = "remark42_e2e_bot" + } + // the token the caller has to present. Checking it is what makes these tests witness the + // /bot/ construction: without it a build that dropped, truncated or + // double-escaped the token would be served exactly as a correct one and every case would pass + token := os.Getenv("STUB_BOT_TOKEN") + if token == "" { + token = "stub-token" + } + + s := &stub{nextID: 1, botName: botName, token: token} + + mux := http.NewServeMux() + // the provider builds every call as /bot/, so the token segment is part + // of the path and the method is its last element + mux.HandleFunc("/", s.route) + mux.HandleFunc("/control/send", s.send) + mux.HandleFunc("/control/sent", s.listSent) + + log.Print("[INFO] telegram stub started") + srv := &http.Server{Addr: addr, Handler: mux} //nolint:gosec // test double, no timeouts wanted + if err := srv.ListenAndServe(); err != nil { + log.Fatalf("[ERROR] %v", err) + } +} + +// route dispatches a bot call. The provider builds every one as /bot/, so both +// halves of the path are checked: a request whose token segment is not the one this stub was given +// is refused with 401, matching Telegram's response +func (s *stub) route(w http.ResponseWriter, r *http.Request) { + segments := strings.Split(strings.Trim(r.URL.Path, "/"), "/") + if len(segments) > 1 && segments[0] == "file" { + segments = segments[1:] + } + if len(segments) < 2 || segments[0] != "bot"+s.token { + w.WriteHeader(http.StatusUnauthorized) + _ = json.NewEncoder(w).Encode(map[string]any{ + "ok": false, "error_code": 401, "description": "Unauthorized: bot token is wrong or missing", + }) + return + } + + method := segments[len(segments)-1] + switch method { + case "getMe": + s.reply(w, map[string]any{"id": 1, "is_bot": true, "username": s.botName, "first_name": "remark42 e2e"}) + case "getUpdates": + s.reply(w, s.drain()) + case "sendMessage": + text := r.URL.Query().Get("text") + if text == "" && r.Body != nil { + var message telegramMsg + if err := json.NewDecoder(r.Body).Decode(&message); err != nil { + http.Error(w, `{"ok":false,"description":"invalid message"}`, http.StatusBadRequest) + return + } + text = message.Text + } + s.record(text) + s.reply(w, map[string]any{"message_id": 1}) + case "getUserProfilePhotos": + // no avatar: the flow has to work for a reader who never set one, and an empty list is + // what Telegram returns for that + s.reply(w, map[string]any{"total_count": 0, "photos": []any{}}) + default: + http.Error(w, `{"ok":false,"description":"unsupported method"}`, http.StatusNotFound) + } +} + +type telegramMsg struct { + Text string `json:"text"` +} + +// send enqueues an update as if the reader had sent the bot a message. text and id are the two +// things a test decides; the rest is filled in so the provider has a complete user to build from +func (s *stub) send(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + id, err := strconv.Atoi(q.Get("id")) + if err != nil { + // never defaulted: a substituted id signs the test in as somebody else and says nothing + // about it, which is how a case asserting "a distinct user per run" can be neither + http.Error(w, "id must be a number", http.StatusBadRequest) + return + } + + var u update + u.Message.Text = q.Get("text") + u.Message.From.ID = id + u.Message.From.Username = q.Get("username") + u.Message.From.FirstName = q.Get("first_name") + u.Message.Chat.ID = id + u.Message.Chat.Name = q.Get("first_name") + u.Message.Chat.Type = "private" + + s.mu.Lock() + u.UpdateID = s.nextID + s.nextID++ + s.queued = append(s.queued, u) + s.mu.Unlock() + + w.WriteHeader(http.StatusNoContent) +} + +func (s *stub) listSent(w http.ResponseWriter, _ *http.Request) { + s.mu.Lock() + defer s.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(s.sent) +} + +// drain returns what is queued and clears it. The provider polls in a loop and tracks its own +// offset, so handing the same update back twice would have it processed twice +func (s *stub) drain() []update { + s.mu.Lock() + defer s.mu.Unlock() + + out := s.queued + s.queued = nil + if out == nil { + out = []update{} + } + return out +} + +func (s *stub) record(text string) { + s.mu.Lock() + defer s.mu.Unlock() + s.sent = append(s.sent, text) +} + +func (s *stub) reply(w http.ResponseWriter, result any) { + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": result}); err != nil { + log.Printf("[WARN] %v", err) + } +} diff --git a/e2e/telegramsub_test.go b/e2e/telegramsub_test.go new file mode 100644 index 0000000000..f2c747e9eb --- /dev/null +++ b/e2e/telegramsub_test.go @@ -0,0 +1,156 @@ +//go:build e2e + +package e2e + +import ( + "net/http" + "strings" + "testing" + + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Telegram subscription also leaves the browser: the notify service polls for updates through its +// configurable API host. Component tests cover the panel's state machine with mocked fetches; +// these cases also exercise persistence, the backend and the bot exchange. +// +// Everything here runs on remark42-telegram, whose notify service and auth provider both answer +// through e2e/telegramstub. +// These cases stay serial because the stub exposes one shared update queue and sent-message log. + +// TestTelegramSub_NotOfferedToAReaderWithoutAnAccount pins the control's precondition. A +// subscription is keyed to a user the backend can recognize later, so the widget offers none at all +// until the reader has an identity. The signed-in leg proves the same deployment offers the control. +func TestTelegramSub_NotOfferedToAReaderWithoutAnAccount(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + + // the form is there, so the widget rendered + waitVisible(t, frame.Locator(commentFormSel).First()) + + control := frame.Locator(`[title="Subscribe by Telegram"]`) + n, err := control.Count() + require.NoError(t, err) + assert.Zero(t, n, "Telegram subscription was offered to a reader with no account") + + token := tgStartSignIn(t, frame) + tgSendToBot(t, "/start "+token, tgReaderID(t), "Control Reader") + tgConfirmSignIn(t, page, frame) + waitVisible(t, control) +} + +// TestTelegramSub_RoundTrip drives the complete subscription lifecycle: the backend mints a +// one-time token, the reader sends it to the bot, the notify poll confirms it, and the widget +// persists and removes the subscription. It then starts again with a new token, since reusing the +// consumed token leaves the resubscribe control present but permanently unable to finish. +func TestTelegramSub_RoundTrip(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + readerID := tgReaderID(t) + + loginToken := tgStartSignIn(t, frame) + tgSendToBot(t, "/start "+loginToken, readerID, "Subscription Reader") + tgConfirmSignIn(t, page, frame) + + // a unique reader keeps runs isolated, and clearing here also makes a rerun against a kept + // stack deterministic if the same E2E_RUN_ID and process id are reused + status, body := pageFetch(t, page, http.MethodDelete, telegramURL+"/api/v1/telegram?site=remark", nil) + require.Equal(t, http.StatusOK, status, "could not clear a telegram subscription left by an earlier run: %s", body) + clearTelegramSubscriptionSession(t, page) + + frame = reload(t, page) + firstToken := openTelegramSubscription(t, frame) + confirmTelegramSubscription(t, page, frame, firstToken, readerID) + + // remove the panel state and reload, so the subscribed view comes from the backend's 409 and + // not from the step the component kept in session storage + clearTelegramSubscriptionSession(t, page) + frame = reload(t, page) + + require.NoError(t, frame.Locator(`[title="Subscribe by Telegram"]`).Click()) + unsubscribe := frame.Locator(`button:text-is("Unsubscribe")`) + waitVisible(t, unsubscribe) + + resp, err := page.ExpectResponse("**/api/v1/telegram**", func() error { + return unsubscribe.Click() + }, playwright.PageExpectResponseOptions{Timeout: playwright.Float(float64(waitTimeout.Milliseconds()))}) + require.NoError(t, err, "clicking Unsubscribe asked the server nothing") + assert.Equal(t, http.MethodDelete, resp.Request().Method()) + assert.Equal(t, http.StatusOK, resp.Status(), "the server refused the telegram unsubscribe") + waitVisible(t, frame.Locator(`text=You have been unsubscribed by telegram to updates`)) + + require.NoError(t, frame.Locator(`button:text-is("Resubscribe")`).Click()) + secondToken := telegramSubscriptionToken(t, frame) + require.NotEqual(t, firstToken, secondToken, "resubscribe reused the one-time token already consumed") + confirmTelegramSubscription(t, page, frame, secondToken, readerID) +} + +// TestTelegramSub_StartFailureIsShown covers the initial request, where no TelegramLink exists to +// carry its error. The dropdown must leave a diagnosis instead of becoming an empty panel. +func TestTelegramSub_StartFailureIsShown(t *testing.T) { + page := newPage(t) + frame := openURL(t, page, threadURLOn(t, telegramURL)) + + loginToken := tgStartSignIn(t, frame) + tgSendToBot(t, "/start "+loginToken, tgReaderID(t), "Error Reader") + tgConfirmSignIn(t, page, frame) + + require.NoError(t, page.Route("**/api/v1/telegram/subscribe**", func(route playwright.Route) { + if err := route.Fulfill(playwright.RouteFulfillOptions{ + Status: playwright.Int(http.StatusInternalServerError), + ContentType: playwright.String("application/json"), + Body: playwright.String(`{"error":"failed"}`), + }); err != nil { + t.Errorf("fulfill Telegram subscription failure: %v", err) + } + })) + + require.NoError(t, frame.Locator(`[title="Subscribe by Telegram"]`).Click()) + errorMessage := frame.Locator(`.auth-error`) + waitVisible(t, errorMessage) + text, err := errorMessage.TextContent() + require.NoError(t, err) + assert.Contains(t, strings.ToLower(text), "something went wrong") +} + +func clearTelegramSubscriptionSession(t *testing.T, page playwright.Page) { + t.Helper() + _, err := page.Evaluate(`() => { + sessionStorage.removeItem('telegram-subscription-step'); + sessionStorage.removeItem('telegram-subscription-telegram'); + }`) + require.NoError(t, err) +} + +func openTelegramSubscription(t *testing.T, frame playwright.FrameLocator) string { + t.Helper() + require.NoError(t, frame.Locator(`[title="Subscribe by Telegram"]`).Click()) + return telegramSubscriptionToken(t, frame) +} + +func telegramSubscriptionToken(t *testing.T, frame playwright.FrameLocator) string { + t.Helper() + link := frame.Locator(`.telegram a`).First() + waitVisible(t, link) + href, err := link.GetAttribute("href") + require.NoError(t, err) + return telegramStartToken(t, href) +} + +func confirmTelegramSubscription( + t *testing.T, page playwright.Page, frame playwright.FrameLocator, token, readerID string, +) { + t.Helper() + before := len(tgSentMessages(t)) + tgSendToBot(t, "/start "+token, readerID, "Subscription Reader") + tgWaitForMessage(t, before, "successfully subscribed") + + resp, err := page.ExpectResponse("**/api/v1/telegram/subscribe**", func() error { + return frame.Locator(`button:text-is("Check")`).Click() + }, playwright.PageExpectResponseOptions{Timeout: playwright.Float(float64(waitTimeout.Milliseconds()))}) + require.NoError(t, err, "clicking Check asked the server nothing") + require.Equal(t, http.StatusOK, resp.Status(), "the server refused a subscription token the bot confirmed") + waitVisible(t, frame.Locator(`text=You have been subscribed on updates by telegram`)) +} diff --git a/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.test.tsx b/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.test.tsx index a8936dd139..9b926cfdc5 100644 --- a/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.test.tsx +++ b/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.test.tsx @@ -97,6 +97,23 @@ describe('', () => { expect(screen.getByText(/You have been subscribed/)).toBeInTheDocument(); }); + it('should clear a failed check when a later check succeeds', async () => { + jest + .spyOn(api, 'telegramCurrentSubscribtion') + .mockRejectedValueOnce(new RequestError('failed', 500)) + .mockResolvedValueOnce({ address: '223211010', updated: true }); + + createWrapper(); + fireEvent.click(screen.getByTitle('Subscribe by Telegram')); + + fireEvent.click(await screen.findByText('Check')); + expect(await screen.findByText('Something went wrong.')).toHaveClass('auth-error'); + + fireEvent.click(screen.getByText('Check')); + expect(await screen.findByText(/You have been subscribed/)).toBeInTheDocument(); + expect(screen.queryByText('Something went wrong.')).not.toBeInTheDocument(); + }); + it('should subscribe and then unsubscribe', async () => { createWrapper(); const button = screen.getByTitle('Subscribe by Telegram'); @@ -116,6 +133,24 @@ describe('', () => { expect(screen.getByText(/You have been unsubscribed/)).toBeInTheDocument(); }); + it('should clear a failed unsubscribe when a later unsubscribe succeeds', async () => { + jest + .spyOn(api, 'telegramUnsubcribe') + .mockRejectedValueOnce(new RequestError('failed', 500)) + .mockResolvedValueOnce({ deleted: true }); + + createWrapper(); + fireEvent.click(screen.getByTitle('Subscribe by Telegram')); + fireEvent.click(await screen.findByText('Check')); + + fireEvent.click(await screen.findByText('Unsubscribe')); + expect(await screen.findByText('Something went wrong.')).toHaveClass('auth-error'); + + fireEvent.click(screen.getByText('Unsubscribe')); + expect(await screen.findByText(/You have been unsubscribed/)).toBeInTheDocument(); + expect(screen.queryByText('Something went wrong.')).not.toBeInTheDocument(); + }); + it('should subscribe, close window and then unsubscribe', async () => { createWrapper(); const button = screen.getByTitle('Subscribe by Telegram'); @@ -154,11 +189,38 @@ describe('', () => { await waitForElementToBeRemoved(() => screen.queryByLabelText('Loading...')); fireEvent.click(await screen.findByText('Resubscribe')); + expect(api.telegramSubscribe).toHaveBeenCalledTimes(2); fireEvent.click(await screen.findByText('Check')); expect(api.telegramCurrentSubscribtion).toHaveBeenCalledTimes(2); expect(await screen.findByText(/You have been subscribed/)).toBeInTheDocument(); }); + it('should resubscribe after loading an existing subscription', async () => { + jest + .spyOn(api, 'telegramSubscribe') + .mockRejectedValueOnce(new RequestError('Conflict.', 409)) + .mockResolvedValueOnce({ bot: 'foo_bot', token: 'fresh_token' }); + + createWrapper(); + fireEvent.click(screen.getByTitle('Subscribe by Telegram')); + fireEvent.click(await screen.findByText('Unsubscribe')); + await screen.findByText(/You have been unsubscribed/); + + fireEvent.click(screen.getByText('Resubscribe')); + + expect(await screen.findByText(/by the link/)).toHaveAttribute('href', 'https://t.me/foo_bot/?start=fresh_token'); + expect(api.telegramSubscribe).toHaveBeenCalledTimes(2); + }); + + it('should show an error when starting the subscription fails', async () => { + jest.spyOn(api, 'telegramSubscribe').mockRejectedValueOnce(new RequestError('not enabled', 17)); + + createWrapper(); + fireEvent.click(screen.getByTitle('Subscribe by Telegram')); + + expect(await screen.findByText('Action rejected. Please try again a bit later.')).toHaveClass('auth-error'); + }); + it('should show subscribed interface if user is already subscribed', async () => { jest.spyOn(api, 'telegramSubscribe').mockImplementation(async () => { await sleep(10); diff --git a/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.tsx b/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.tsx index 5bbd97abd1..cf257d979a 100644 --- a/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.tsx +++ b/frontend/apps/remark42/app/components/comment-form/__subscribe-by-telegram/comment-form__subscribe-by-telegram.tsx @@ -59,32 +59,42 @@ export const SubscribeByTelegramForm: FunctionComponent = () => { const [loading, setLoading] = useState(false); const [error, setError] = useState(null); - const [telegram, setTelegram] = useSessionStorage<{ token: string; bot: string }>('telegram-subscription-telegram'); + const [telegram, setTelegram] = useSessionStorage<{ token: string; bot: string } | null>( + 'telegram-subscription-telegram', + null + ); - useEffect(() => { - const fetchTelegram = async () => { + const fetchTelegram = async (showLoading = true): Promise => { + if (showLoading) { setLoading(true); - try { - const { token, bot } = await telegramSubscribe(); - setTelegram({ token, bot }); - } catch (e) { - if ((e as RequestError).error === 'already subscribed') { - setStep('subscribed'); - return; - } - if ((e as RequestError).code === 409) { - setStep('subscribed'); - return; - } - setError(extractErrorMessageFromResponse(e as FetcherError, intl)); - } finally { + } + try { + const { token, bot } = await telegramSubscribe(); + setTelegram({ token, bot }); + return true; + } catch (e) { + if ((e as RequestError).error === 'already subscribed') { + setStep('subscribed'); + return false; + } + if ((e as RequestError).code === 409) { + setStep('subscribed'); + return false; + } + setError(extractErrorMessageFromResponse(e as FetcherError, intl)); + return false; + } finally { + if (showLoading) { setLoading(false); } - }; + } + }; + + useEffect(() => { if (telegram) { return; } - fetchTelegram(); + void fetchTelegram(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [telegram]); @@ -95,6 +105,7 @@ export const SubscribeByTelegramForm: FunctionComponent = () => { setLoading(true); }, 0); const { updated } = await telegramCurrentSubscribtion({ token }); + setError(null); if (!updated) { return; } @@ -120,6 +131,7 @@ export const SubscribeByTelegramForm: FunctionComponent = () => { setLoading(true); }, 0); await telegramUnsubcribe(); + setError(null); setStep('unsubscribed'); } catch (err) { setError(extractErrorMessageFromResponse(err, intl)); @@ -129,11 +141,15 @@ export const SubscribeByTelegramForm: FunctionComponent = () => { }; const handleTelegramResubscribe = async () => { - setStep('initial'); + setError(null); + if (await fetchTelegram(false)) { + setStep('initial'); + } }; return (
{loading && } + {!loading && error && (!telegram || step !== 'initial') &&
{error}
} {!loading && telegram && step === 'initial' && ( )} diff --git a/site/content/docs/configuration/parameters/index.md b/site/content/docs/configuration/parameters/index.md index bdadc25054..69722587aa 100644 --- a/site/content/docs/configuration/parameters/index.md +++ b/site/content/docs/configuration/parameters/index.md @@ -132,6 +132,7 @@ services: | notify.email.verification_subj | NOTIFY_EMAIL_VERIFICATION_SUBJ | `Email verification` | verification message subject | | telegram.token | TELEGRAM_TOKEN | | Telegram token (used for auth and Telegram notifications) | | telegram.timeout | TELEGRAM_TIMEOUT | `5s` | Telegram connection timeout | +| telegram.api-url | TELEGRAM_API_URL | | Telegram Bot API origin/base path for auth and notifications, without `/bot`; the bot token is appended to request paths, and the public API is used when unset | | smtp.host | SMTP_HOST | | SMTP host | | smtp.port | SMTP_PORT | | SMTP port | | smtp.helo_host | SMTP_HELO_HOST | | SMTP HELO/EHLO hostname | diff --git a/site/content/docs/configuration/telegram/index.md b/site/content/docs/configuration/telegram/index.md index 75d6740ee4..ee2c006fa4 100644 --- a/site/content/docs/configuration/telegram/index.md +++ b/site/content/docs/configuration/telegram/index.md @@ -6,6 +6,8 @@ You can enable Telegram for a user or admin [notifications](https://remark42.com To set up notifications or auth with Telegram, first, you need to create a bot and write its access token to the remark42 configuration. +`TELEGRAM_API_URL` can point both features at a compatible proxy or Bot API server. Set it to an HTTP or HTTPS origin with an optional base path, without the trailing `/bot`; for example, `https://telegram-proxy.example.com/api`. Remark42 appends `/bot/`, so the token is present in every API request path. Use an endpoint and request logs that protect it as a secret. An invalid custom URL stops startup when Telegram authentication or notifications request it. + ## Getting bot token for Telegram To get a token, talk to [BotFather](https://core.telegram.org/bots#6-botfather). All you need is to send `/newbot` command and choose the name for your bot (it must end in `bot`). That is it, and you got a token which you'll need to write down into remark42 configuration as `TELEGRAM_TOKEN`.