diff --git a/modules/activitypub/client.go b/modules/activitypub/client.go index 3d5275d379..2fffa017ff 100644 --- a/modules/activitypub/client.go +++ b/modules/activitypub/client.go @@ -72,6 +72,16 @@ func NewClientFactory() (c *ClientFactory, err error) { return NewClientFactoryWithTimeout(5 * time.Second) } +func checkRedirect(req *http.Request, via []*http.Request) error { + // NOTE: we don't want to allow any redirects for the ActivityPub client. + // For our use-case there is no legitimate context for a redirect, + // e.g. fetching keys, posting mailbox messages, etc. + // + // At some point in the future, we may want to support limited redirects, + // possibly configurable through a settings option. + return errors.New("activitypub: client: redirects are not allowed") +} + // NewClient function func NewClientFactoryWithTimeout(timeout time.Duration) (c *ClientFactory, err error) { if err = containsRequiredHTTPHeaders(http.MethodGet, setting.Federation.GetHeaders); err != nil { @@ -85,7 +95,8 @@ func NewClientFactoryWithTimeout(timeout time.Duration) (c *ClientFactory, err e Transport: &http.Transport{ Proxy: proxy.Proxy(), }, - Timeout: timeout, + Timeout: timeout, + CheckRedirect: checkRedirect, }, algs: setting.HttpsigAlgs, digestAlg: httpsig.DigestAlgorithm(setting.Federation.DigestAlgorithm), diff --git a/modules/activitypub/client_test.go b/modules/activitypub/client_test.go index e36594e065..0509dcc80e 100644 --- a/modules/activitypub/client_test.go +++ b/modules/activitypub/client_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "testing" "time" @@ -168,3 +169,30 @@ func TestActivityPubSignedPost(t *testing.T) { require.NoError(t, err) assert.Equal(t, expected, string(body)) } + +func TestActivityPubRedirect(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + pubID := "https://example.com/pubID" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + statusCode, _ := strconv.ParseInt(r.URL.Query()["code"][0], 10, 32) + w.WriteHeader(int(statusCode)) + w.Header().Add("Location", "/evil-redirect") + w.Write([]byte("/evil-redirect")) + })) + + cf, err := activitypub.NewClientFactory() + require.NoError(t, err) + srvURL, err := url.Parse(srv.URL) + require.NoError(t, err) + c, err := cf.WithKeys(db.DefaultContext, user, pubID, []*url.URL{srvURL}) + require.NoError(t, err) + + // try each HTTP status code (encoded in a query param for convenience) + // to ensure all redirects result in an error + for _, code := range []int{http.StatusMovedPermanently, http.StatusFound, http.StatusTemporaryRedirect, http.StatusPermanentRedirect, http.StatusMultipleChoices, http.StatusNotModified} { + _, err = c.Get(fmt.Sprintf("%s?code=%d", srv.URL, code)) + require.Error(t, err) + } +}