diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 36d7a2ffe1..83fb445a23 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -29,6 +29,8 @@ import ( cache "github.com/go-pkgz/lcw/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/umputun/remark42/backend/app/notify" ) const ( @@ -1491,3 +1493,44 @@ func TestServerCommand_servesAudience(t *testing.T) { cmd.Store.Type = "rpc" assert.True(t, cmd.servesAudience([]string{"Remark"}), "a remote store decides which sites exist") } + +// TestServerApp_UnsubscribeTokenIsNotALogin covers the token in unsubscribe links: it names the user +// and their email, is signed with the shared secret and is valid for a century, so the email login +// must not accept it in place of a confirmation token it issued itself. +func TestServerApp_UnsubscribeTokenIsNotALogin(t *testing.T) { + port := chooseUnusedPort(t) + app, ctx, cancel := prepServerApp(t, func(o ServerCommand) ServerCommand { + o.Port = port + o.emailMsgTemplatePath, o.emailVerificationTemplatePath = "", "" // the embedded templates, as in production + return o + }) + defer cancel() + + go func() { _ = app.run(ctx) }() + waitForHTTPServerStart(t, port) + + destinations, err := app.makeNotifyDestinations(app.authenticator) + require.NoError(t, err) + var tkn string + for _, d := range destinations { + if e, ok := d.(*notify.Email); ok { + tkn, err = e.TokenGenFn("email_user1", "user1@example.org", "remark") + require.NoError(t, err) + } + } + require.NotEmpty(t, tkn, "email notifications are configured, so unsubscribe links carry a token") + + client := http.Client{Timeout: 10 * time.Second} + defer client.CloseIdleConnections() + resp := getRetryThrottled(t, &client, fmt.Sprintf("http://localhost:%d/auth/email/login?token=%s", port, tkn)) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + assert.Equal(t, http.StatusForbidden, resp.StatusCode, "an unsubscribe token is not a login, %s", body) + for _, c := range resp.Cookies() { + assert.NotEqual(t, "JWT", c.Name, "no session from an unsubscribe token") + } + + cancel() + app.Wait() +} diff --git a/backend/go.mod b/backend/go.mod index 2ba4ad15b5..2aa910683b 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -7,7 +7,7 @@ require ( github.com/PuerkitoBio/goquery v1.13.0 github.com/alecthomas/chroma/v2 v2.27.0 github.com/didip/tollbooth/v8 v8.0.1 - github.com/go-pkgz/auth/v2 v2.3.0 + github.com/go-pkgz/auth/v2 v2.3.1 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 diff --git a/backend/go.sum b/backend/go.sum index c29f0ee418..0612cbbdc4 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.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/auth/v2 v2.3.1 h1:ReJat7qXaaqSjsTrZGsWyaM5dvXuGF65qG7lfELlh7s= +github.com/go-pkgz/auth/v2 v2.3.1/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= diff --git a/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go b/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go index 0cee5a7d42..d5880092fb 100644 --- a/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go +++ b/backend/vendor/github.com/go-pkgz/auth/v2/provider/verify.go @@ -222,12 +222,23 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { // GET /login?token=confirmation-jwt&sess=1 confClaims, err := e.TokenService.Parse(tkn) if err != nil { - rest.SendErrorJSON(w, r, e.L, http.StatusForbidden, err, "failed to verify confirmation token") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusForbidden, err, "failed to verify confirmation token") return } if e.TokenService.IsExpired(confClaims) { - rest.SendErrorJSON(w, r, e.L, http.StatusForbidden, fmt.Errorf("expired"), "failed to verify confirmation token") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusForbidden, fmt.Errorf("expired"), "failed to verify confirmation token") + return + } + + // only a handshake token carrying this provider's name may sign someone in. Other tokens are + // signed with the same secret, including handshake tokens of the same user::address shape that + // an application mails for its own purposes, and none of them prove the holder received this + // provider's confirmation. The token is still a live credential elsewhere, so it is kept out of + // the logged URL. + if confClaims.Handshake == nil || confClaims.AuthProvider == nil || confClaims.AuthProvider.Name != e.ProviderName { + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusForbidden, fmt.Errorf("not a %s confirmation token", e.ProviderName), + "failed to verify confirmation token") return } @@ -263,7 +274,7 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { elems := strings.Split(confClaims.Handshake.ID, "::") if len(elems) != 2 { - rest.SendErrorJSON(w, r, e.L, http.StatusBadRequest, fmt.Errorf("%s", confClaims.Handshake.ID), "invalid handshake token") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusBadRequest, fmt.Errorf("%s", confClaims.Handshake.ID), "invalid handshake token") return } user, address := elems[0], elems[1] @@ -281,13 +292,13 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { } if u, err = setAvatar(e.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second}); err != nil { - rest.SendErrorJSON(w, r, e.L, http.StatusInternalServerError, err, "failed to save avatar to proxy") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusInternalServerError, err, "failed to save avatar to proxy") return } cid, err := randToken() if err != nil { - rest.SendErrorJSON(w, r, e.L, http.StatusInternalServerError, err, "can't make token id") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusInternalServerError, err, "can't make token id") return } @@ -305,7 +316,7 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) { } if _, err = e.TokenService.Set(w, claims); err != nil { - rest.SendErrorJSON(w, r, e.L, http.StatusInternalServerError, err, "failed to set token") + rest.SendErrorJSON(w, scrubTokenFromRequest(r), e.L, http.StatusInternalServerError, err, "failed to set token") return } if confClaims.Handshake != nil && confClaims.Handshake.From != "" { diff --git a/backend/vendor/modules.txt b/backend/vendor/modules.txt index 00c43c1f18..5e9f533680 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.3.0 +# github.com/go-pkgz/auth/v2 v2.3.1 ## explicit; go 1.25.0 github.com/go-pkgz/auth/v2 github.com/go-pkgz/auth/v2/avatar