Threepid tests and fixes

clientapi/auth/authtypes/threepid.go

@@ -16,6 +16,8 @@ package authtypes
 
 // ThreePID represents a third-party identifier
 type ThreePID struct {
-	Address string `json:"address"`
+	Address     string `json:"address"`
-	Medium  string `json:"medium"`
+	Medium      string `json:"medium"`
+	AddedAt     int64  `json:"added_at"`
+	ValidatedAt int64  `json:"validated_at"`
 }

clientapi/clientapi_test.go

@@ -4,12 +4,15 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/matrix-org/dendrite/clientapi/routing"
+	"github.com/matrix-org/dendrite/clientapi/threepid"
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/internal/httputil"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
@@ -192,3 +195,171 @@ func TestTurnserver(t *testing.T) {
 		})
 	}
 }
+
+func Test3PID(t *testing.T) {
+	alice := test.NewUser(t)
+	ctx := context.Background()
+
+	test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) {
+		cfg, processCtx, close := testrig.CreateConfig(t, dbType)
+		cfg.ClientAPI.RateLimiting.Enabled = false
+		cfg.FederationAPI.DisableTLSValidation = true // needed to be able to connect to our identityServer below
+		defer close()
+		natsInstance := jetstream.NATSInstance{}
+
+		routers := httputil.NewRouters()
+		cm := sqlutil.NewConnectionManager(processCtx, cfg.Global.DatabaseOptions)
+
+		// Needed to create accounts
+		rsAPI := roomserver.NewInternalAPI(processCtx, cfg, cm, &natsInstance, nil, caching.DisableMetrics)
+		userAPI := userapi.NewInternalAPI(processCtx, cfg, cm, &natsInstance, rsAPI, nil)
+		// We mostly need the rsAPI/userAPI for this test, so nil for other APIs etc.
+		AddPublicRoutes(processCtx, routers, cfg, &natsInstance, nil, rsAPI, nil, nil, nil, userAPI, nil, nil, caching.DisableMetrics)
+
+		// Create the users in the userapi and login
+		accessTokens := map[*test.User]string{
+			alice: "",
+		}
+		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
+
+		identityServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			switch {
+			case strings.Contains(r.URL.String(), "getValidated3pid"):
+				resp := threepid.GetValidatedResponse{}
+				switch r.URL.Query().Get("client_secret") {
+				case "fail":
+					resp.ErrCode = "M_SESSION_NOT_VALIDATED"
+				case "fail2":
+					resp.ErrCode = "some other error"
+				case "fail3":
+					_, _ = w.Write([]byte("{invalidJson"))
+					return
+				case "success":
+					resp.Medium = "email"
+				case "success2":
+					resp.Medium = "email"
+					resp.Address = "somerandom@address.com"
+				}
+				_ = json.NewEncoder(w).Encode(resp)
+			case strings.Contains(r.URL.String(), "requestToken"):
+				resp := threepid.SID{SID: "randomSID"}
+				_ = json.NewEncoder(w).Encode(resp)
+			}
+		}))
+		defer identityServer.Close()
+
+		identityServerBase := strings.TrimPrefix(identityServer.URL, "https://")
+
+		testCases := []struct {
+			name             string
+			request          *http.Request
+			wantOK           bool
+			setTrustedServer bool
+			wantLen3PIDs     int
+		}{
+			{
+				name:    "can get associated threepid info",
+				request: httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/account/3pid", strings.NewReader("")),
+				wantOK:  true,
+			},
+			{
+				name:    "can not set threepid info with invalid JSON",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader("")),
+			},
+			{
+				name:    "can not set threepid info with untrusted server",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader("{}")),
+			},
+			{
+				name:             "can check threepid info with trusted server, but unverified",
+				request:          httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader(fmt.Sprintf(`{"three_pid_creds":{"id_server":"%s","client_secret":"fail"}}`, identityServerBase))),
+				setTrustedServer: true,
+				wantOK:           false,
+			},
+			{
+				name:             "can check threepid info with trusted server, but fails for some other reason",
+				request:          httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader(fmt.Sprintf(`{"three_pid_creds":{"id_server":"%s","client_secret":"fail2"}}`, identityServerBase))),
+				setTrustedServer: true,
+				wantOK:           false,
+			},
+			{
+				name:             "can check threepid info with trusted server, but fails because of invalid json",
+				request:          httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader(fmt.Sprintf(`{"three_pid_creds":{"id_server":"%s","client_secret":"fail3"}}`, identityServerBase))),
+				setTrustedServer: true,
+				wantOK:           false,
+			},
+			{
+				name:             "can save threepid info with trusted server",
+				request:          httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader(fmt.Sprintf(`{"three_pid_creds":{"id_server":"%s","client_secret":"success"}}`, identityServerBase))),
+				setTrustedServer: true,
+				wantOK:           true,
+			},
+			{
+				name:             "can save threepid info with trusted server using bind=true",
+				request:          httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid", strings.NewReader(fmt.Sprintf(`{"three_pid_creds":{"id_server":"%s","client_secret":"success2"},"bind":true}`, identityServerBase))),
+				setTrustedServer: true,
+				wantOK:           true,
+			},
+			{
+				name:         "can get associated threepid info again",
+				request:      httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/account/3pid", strings.NewReader("")),
+				wantOK:       true,
+				wantLen3PIDs: 2,
+			},
+			{
+				name:    "can delete associated threepid info",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid/delete", strings.NewReader(`{"medium":"email","address":"somerandom@address.com"}`)),
+				wantOK:  true,
+			},
+			{
+				name:         "can get associated threepid after deleting association",
+				request:      httptest.NewRequest(http.MethodGet, "/_matrix/client/v3/account/3pid", strings.NewReader("")),
+				wantOK:       true,
+				wantLen3PIDs: 1,
+			},
+			{
+				name:    "can not request emailToken with invalid request body",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid/email/requestToken", strings.NewReader("")),
+			},
+			{
+				name:    "can not request emailToken for in use address",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid/email/requestToken", strings.NewReader(fmt.Sprintf(`{"client_secret":"somesecret","email":"","send_attempt":1,"id_server":"%s"}`, identityServerBase))),
+			},
+			{
+				name:    "can request emailToken",
+				request: httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/account/3pid/email/requestToken", strings.NewReader(fmt.Sprintf(`{"client_secret":"somesecret","email":"somerandom@address.com","send_attempt":1,"id_server":"%s"}`, identityServerBase))),
+				wantOK:  true,
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+
+				if tc.setTrustedServer {
+					cfg.Global.TrustedIDServers = []string{identityServerBase}
+				}
+
+				rec := httptest.NewRecorder()
+				tc.request.Header.Set("Authorization", "Bearer "+accessTokens[alice])
+
+				routers.Client.ServeHTTP(rec, tc.request)
+				t.Logf("Response: %s", rec.Body.String())
+				if tc.wantOK && rec.Code != http.StatusOK {
+					t.Fatalf("expected HTTP 200, got %d: %s", rec.Code, rec.Body.String())
+				}
+				if !tc.wantOK && rec.Code == http.StatusOK {
+					t.Fatalf("expected request to fail, but didn't: %s", rec.Body.String())
+				}
+				if tc.wantLen3PIDs > 0 {
+					var resp routing.ThreePIDsResponse
+					if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil {
+						t.Fatal(err)
+					}
+					if len(resp.ThreePIDs) != tc.wantLen3PIDs {
+						t.Fatalf("expected %d threepids, got %d", tc.wantLen3PIDs, len(resp.ThreePIDs))
+					}
+				}
+			})
+		}
+	})
+}

clientapi/routing/routing.go

@@ -21,6 +21,7 @@ import (
 	"sync"
 
 	"github.com/gorilla/mux"
+	"github.com/matrix-org/dendrite/setup/base"
 	userapi "github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
@@ -861,6 +862,8 @@ func Setup(
 	// Browsers use the OPTIONS HTTP method to check if the CORS policy allows
 	// PUT requests, so we need to allow this method
 
+	threePIDClient := base.CreateClient(dendriteCfg, nil) // TODO: Move this somewhere else, e.g. pass in as parameter
+
 	v3mux.Handle("/account/3pid",
 		httputil.MakeAuthAPI("account_3pid", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
 			return GetAssociated3PIDs(req, userAPI, device)
@@ -869,11 +872,11 @@ func Setup(
 
 	v3mux.Handle("/account/3pid",
 		httputil.MakeAuthAPI("account_3pid", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
-			return CheckAndSave3PIDAssociation(req, userAPI, device, cfg)
+			return CheckAndSave3PIDAssociation(req, userAPI, device, cfg, threePIDClient)
 		}),
 	).Methods(http.MethodPost, http.MethodOptions)
 
-	unstableMux.Handle("/account/3pid/delete",
+	v3mux.Handle("/account/3pid/delete",
 		httputil.MakeAuthAPI("account_3pid", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
 			return Forget3PID(req, userAPI)
 		}),
@@ -881,7 +884,7 @@ func Setup(
 
 	v3mux.Handle("/{path:(?:account/3pid|register)}/email/requestToken",
 		httputil.MakeExternalAPI("account_3pid_request_token", func(req *http.Request) util.JSONResponse {
-			return RequestEmailToken(req, userAPI, cfg)
+			return RequestEmailToken(req, userAPI, cfg, threePIDClient)
 		}),
 	).Methods(http.MethodPost, http.MethodOptions)
 

clientapi/routing/threepid.go

@@ -33,7 +33,7 @@ type reqTokenResponse struct {
 	SID string `json:"sid"`
 }
 
-type threePIDsResponse struct {
+type ThreePIDsResponse struct {
 	ThreePIDs []authtypes.ThreePID `json:"threepids"`
 }
 
@@ -41,7 +41,7 @@ type threePIDsResponse struct {
 //
 //	POST /account/3pid/email/requestToken
 //	POST /register/email/requestToken
-func RequestEmailToken(req *http.Request, threePIDAPI api.ClientUserAPI, cfg *config.ClientAPI) util.JSONResponse {
+func RequestEmailToken(req *http.Request, threePIDAPI api.ClientUserAPI, cfg *config.ClientAPI, client *gomatrixserverlib.Client) util.JSONResponse {
 	var body threepid.EmailAssociationRequest
 	if reqErr := httputil.UnmarshalJSONRequest(req, &body); reqErr != nil {
 		return *reqErr
@@ -72,7 +72,7 @@ func RequestEmailToken(req *http.Request, threePIDAPI api.ClientUserAPI, cfg *co
 		}
 	}
 
-	resp.SID, err = threepid.CreateSession(req.Context(), body, cfg)
+	resp.SID, err = threepid.CreateSession(req.Context(), body, cfg, client)
 	if err == threepid.ErrNotTrusted {
 		return util.JSONResponse{
 			Code: http.StatusBadRequest,
@@ -92,7 +92,7 @@ func RequestEmailToken(req *http.Request, threePIDAPI api.ClientUserAPI, cfg *co
 // CheckAndSave3PIDAssociation implements POST /account/3pid
 func CheckAndSave3PIDAssociation(
 	req *http.Request, threePIDAPI api.ClientUserAPI, device *api.Device,
-	cfg *config.ClientAPI,
+	cfg *config.ClientAPI, client *gomatrixserverlib.Client,
 ) util.JSONResponse {
 	var body threepid.EmailAssociationCheckRequest
 	if reqErr := httputil.UnmarshalJSONRequest(req, &body); reqErr != nil {
@@ -100,7 +100,7 @@ func CheckAndSave3PIDAssociation(
 	}
 
 	// Check if the association has been validated
-	verified, address, medium, err := threepid.CheckAssociation(req.Context(), body.Creds, cfg)
+	verified, address, medium, err := threepid.CheckAssociation(req.Context(), body.Creds, cfg, client)
 	if err == threepid.ErrNotTrusted {
 		return util.JSONResponse{
 			Code: http.StatusBadRequest,
@@ -123,13 +123,8 @@ func CheckAndSave3PIDAssociation(
 
 	if body.Bind {
 		// Publish the association on the identity server if requested
-		err = threepid.PublishAssociation(body.Creds, device.UserID, cfg)
+		err = threepid.PublishAssociation(req.Context(), body.Creds, device.UserID, cfg, client)
-		if err == threepid.ErrNotTrusted {
+		if err != nil {
-			return util.JSONResponse{
-				Code: http.StatusBadRequest,
-				JSON: jsonerror.NotTrusted(body.Creds.IDServer),
-			}
-		} else if err != nil {
 			util.GetLogger(req.Context()).WithError(err).Error("threepid.PublishAssociation failed")
 			return jsonerror.InternalServerError()
 		}
@@ -180,7 +175,7 @@ func GetAssociated3PIDs(
 
 	return util.JSONResponse{
 		Code: http.StatusOK,
-		JSON: threePIDsResponse{res.ThreePIDs},
+		JSON: ThreePIDsResponse{res.ThreePIDs},
 	}
 }
 
@@ -191,7 +186,10 @@ func Forget3PID(req *http.Request, threepidAPI api.ClientUserAPI) util.JSONRespo
 		return *reqErr
 	}
 
-	if err := threepidAPI.PerformForgetThreePID(req.Context(), &api.PerformForgetThreePIDRequest{}, &struct{}{}); err != nil {
+	if err := threepidAPI.PerformForgetThreePID(req.Context(), &api.PerformForgetThreePIDRequest{
+		ThreePID: body.Address,
+		Medium:   body.Medium,
+	}, &struct{}{}); err != nil {
 		util.GetLogger(req.Context()).WithError(err).Error("threepidAPI.PerformForgetThreePID failed")
 		return jsonerror.InternalServerError()
 	}

clientapi/threepid/threepid.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 
 	"github.com/matrix-org/dendrite/setup/config"
+	"github.com/matrix-org/gomatrixserverlib"
 )
 
 // EmailAssociationRequest represents the request defined at https://matrix.org/docs/spec/client_server/r0.2.0.html#post-matrix-client-r0-register-email-requesttoken
@@ -37,7 +38,7 @@ type EmailAssociationRequest struct {
 
 // EmailAssociationCheckRequest represents the request defined at https://matrix.org/docs/spec/client_server/r0.2.0.html#post-matrix-client-r0-account-3pid
 type EmailAssociationCheckRequest struct {
-	Creds Credentials `json:"threePidCreds"`
+	Creds Credentials `json:"three_pid_creds"`
 	Bind  bool        `json:"bind"`
 }
 
@@ -48,12 +49,16 @@ type Credentials struct {
 	Secret   string `json:"client_secret"`
 }
 
+type SID struct {
+	SID string `json:"sid"`
+}
+
 // CreateSession creates a session on an identity server.
 // Returns the session's ID.
 // Returns an error if there was a problem sending the request or decoding the
 // response, or if the identity server responded with a non-OK status.
 func CreateSession(
-	ctx context.Context, req EmailAssociationRequest, cfg *config.ClientAPI,
+	ctx context.Context, req EmailAssociationRequest, cfg *config.ClientAPI, client *gomatrixserverlib.Client,
 ) (string, error) {
 	if err := isTrusted(req.IDServer, cfg); err != nil {
 		return "", err
@@ -73,8 +78,7 @@ func CreateSession(
 	}
 	request.Header.Add("Content-Type", "application/x-www-form-urlencoded")
 
-	client := http.Client{}
+	resp, err := client.DoHTTPRequest(ctx, request)
-	resp, err := client.Do(request.WithContext(ctx))
 	if err != nil {
 		return "", err
 	}
@@ -85,14 +89,20 @@ func CreateSession(
 	}
 
 	// Extract the SID from the response and return it
-	var sid struct {
+	var sid SID
-		SID string `json:"sid"`
-	}
 	err = json.NewDecoder(resp.Body).Decode(&sid)
 
 	return sid.SID, err
 }
 
+type GetValidatedResponse struct {
+	Medium      string `json:"medium"`
+	ValidatedAt int64  `json:"validated_at"`
+	Address     string `json:"address"`
+	ErrCode     string `json:"errcode"`
+	Error       string `json:"error"`
+}
+
 // CheckAssociation checks the status of an ongoing association validation on an
 // identity server.
 // Returns a boolean set to true if the association has been validated, false if not.
@@ -102,6 +112,7 @@ func CreateSession(
 // response, or if the identity server responded with a non-OK status.
 func CheckAssociation(
 	ctx context.Context, creds Credentials, cfg *config.ClientAPI,
+	client *gomatrixserverlib.Client,
 ) (bool, string, string, error) {
 	if err := isTrusted(creds.IDServer, cfg); err != nil {
 		return false, "", "", err
@@ -112,19 +123,12 @@ func CheckAssociation(
 	if err != nil {
 		return false, "", "", err
 	}
-	resp, err := http.DefaultClient.Do(req.WithContext(ctx))
+	resp, err := client.DoHTTPRequest(ctx, req)
 	if err != nil {
 		return false, "", "", err
 	}
 
-	var respBody struct {
+	var respBody GetValidatedResponse
-		Medium      string `json:"medium"`
-		ValidatedAt int64  `json:"validated_at"`
-		Address     string `json:"address"`
-		ErrCode     string `json:"errcode"`
-		Error       string `json:"error"`
-	}
-
 	if err = json.NewDecoder(resp.Body).Decode(&respBody); err != nil {
 		return false, "", "", err
 	}
@@ -142,7 +146,7 @@ func CheckAssociation(
 // identifier and a Matrix ID.
 // Returns an error if there was a problem sending the request or decoding the
 // response, or if the identity server responded with a non-OK status.
-func PublishAssociation(creds Credentials, userID string, cfg *config.ClientAPI) error {
+func PublishAssociation(ctx context.Context, creds Credentials, userID string, cfg *config.ClientAPI, client *gomatrixserverlib.Client) error {
 	if err := isTrusted(creds.IDServer, cfg); err != nil {
 		return err
 	}
@@ -160,8 +164,7 @@ func PublishAssociation(creds Credentials, userID string, cfg *config.ClientAPI)
 	}
 	request.Header.Add("Content-Type", "application/x-www-form-urlencoded")
 
-	client := http.Client{}
+	resp, err := client.DoHTTPRequest(ctx, request)
-	resp, err := client.Do(request)
 	if err != nil {
 		return err
 	}