Revert "Add CS API device tests (#3029)"

This reverts commit 2854ffeb7d.

clientapi/admin_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/federationapi"
 	"github.com/matrix-org/dendrite/internal/caching"
 	"github.com/matrix-org/dendrite/internal/httputil"
@@ -54,10 +55,10 @@ func TestAdminResetPassword(t *testing.T) {
 		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]userDevice{
+		accessTokens := map[*test.User]string{
-			aliceAdmin: {},
+			aliceAdmin: "",
-			bob:        {},
+			bob:        "",
-			vhUser:     {},
+			vhUser:     "",
 		}
 		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
 
@@ -103,7 +104,7 @@ func TestAdminResetPassword(t *testing.T) {
 				}
 
 				if tc.withHeader {
-					req.Header.Set("Authorization", "Bearer "+accessTokens[tc.requestingUser].accessToken)
+					req.Header.Set("Authorization", "Bearer "+accessTokens[tc.requestingUser])
 				}
 
 				rec := httptest.NewRecorder()
@@ -154,8 +155,8 @@ func TestPurgeRoom(t *testing.T) {
 		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]userDevice{
+		accessTokens := map[*test.User]string{
-			aliceAdmin: {},
+			aliceAdmin: "",
 		}
 		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
 
@@ -174,7 +175,7 @@ func TestPurgeRoom(t *testing.T) {
 			t.Run(tc.name, func(t *testing.T) {
 				req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/purgeRoom/"+tc.roomID)
 
-				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken)
+				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin])
 
 				rec := httptest.NewRecorder()
 				routers.DendriteAdmin.ServeHTTP(rec, req)
@@ -224,8 +225,8 @@ func TestAdminEvacuateRoom(t *testing.T) {
 		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]userDevice{
+		accessTokens := map[*test.User]string{
-			aliceAdmin: {},
+			aliceAdmin: "",
 		}
 		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
 
@@ -243,7 +244,7 @@ func TestAdminEvacuateRoom(t *testing.T) {
 			t.Run(tc.name, func(t *testing.T) {
 				req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateRoom/"+tc.roomID)
 
-				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken)
+				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin])
 
 				rec := httptest.NewRecorder()
 				routers.DendriteAdmin.ServeHTTP(rec, req)
@@ -327,8 +328,8 @@ func TestAdminEvacuateUser(t *testing.T) {
 		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]userDevice{
+		accessTokens := map[*test.User]string{
-			aliceAdmin: {},
+			aliceAdmin: "",
 		}
 		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
 
@@ -348,7 +349,7 @@ func TestAdminEvacuateUser(t *testing.T) {
 			t.Run(tc.name, func(t *testing.T) {
 				req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/evacuateUser/"+tc.userID)
 
-				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken)
+				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin])
 
 				rec := httptest.NewRecorder()
 				routers.DendriteAdmin.ServeHTTP(rec, req)
@@ -409,8 +410,8 @@ func TestAdminMarkAsStale(t *testing.T) {
 		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]userDevice{
+		accessTokens := map[*test.User]string{
-			aliceAdmin: {},
+			aliceAdmin: "",
 		}
 		createAccessTokens(t, accessTokens, userAPI, ctx, routers)
 
@@ -428,7 +429,7 @@ func TestAdminMarkAsStale(t *testing.T) {
 			t.Run(tc.name, func(t *testing.T) {
 				req := test.NewRequest(t, http.MethodPost, "/_dendrite/admin/refreshDevices/"+tc.userID)
 
-				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin].accessToken)
+				req.Header.Set("Authorization", "Bearer "+accessTokens[aliceAdmin])
 
 				rec := httptest.NewRecorder()
 				routers.DendriteAdmin.ServeHTTP(rec, req)
@@ -440,3 +441,35 @@ func TestAdminMarkAsStale(t *testing.T) {
 		}
 	})
 }
+
+func createAccessTokens(t *testing.T, accessTokens map[*test.User]string, userAPI uapi.UserInternalAPI, ctx context.Context, routers httputil.Routers) {
+	t.Helper()
+	for u := range accessTokens {
+		localpart, serverName, _ := gomatrixserverlib.SplitID('@', u.ID)
+		userRes := &uapi.PerformAccountCreationResponse{}
+		password := util.RandomString(8)
+		if err := userAPI.PerformAccountCreation(ctx, &uapi.PerformAccountCreationRequest{
+			AccountType: u.AccountType,
+			Localpart:   localpart,
+			ServerName:  serverName,
+			Password:    password,
+		}, userRes); err != nil {
+			t.Errorf("failed to create account: %s", err)
+		}
+
+		req := test.NewRequest(t, http.MethodPost, "/_matrix/client/v3/login", test.WithJSONBody(t, map[string]interface{}{
+			"type": authtypes.LoginTypePassword,
+			"identifier": map[string]interface{}{
+				"type": "m.id.user",
+				"user": u.ID,
+			},
+			"password": password,
+		}))
+		rec := httptest.NewRecorder()
+		routers.Client.ServeHTTP(rec, req)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("failed to login: %s", rec.Body.String())
+		}
+		accessTokens[u] = gjson.GetBytes(rec.Body.Bytes(), "access_token").String()
+	}
+}

clientapi/routing/deactivate.go

@@ -38,7 +38,7 @@ func Deactivate(
 		userId = deviceAPI.UserID
 	}
 
-	localpart, serverName, err := gomatrixserverlib.SplitID('@', userId)
+	localpart, _, err := gomatrixserverlib.SplitID('@', userId)
 	if err != nil {
 		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
 		return jsonerror.InternalServerError()
@@ -46,8 +46,7 @@ func Deactivate(
 
 	var res api.PerformAccountDeactivationResponse
 	err = accountAPI.PerformAccountDeactivation(ctx, &api.PerformAccountDeactivationRequest{
-		Localpart:  localpart,
+		Localpart: localpart,
-		ServerName: serverName,
 	}, &res)
 	if err != nil {
 		util.GetLogger(ctx).WithError(err).Error("userAPI.PerformAccountDeactivation failed")

clientapi/routing/device.go

@@ -15,7 +15,6 @@
 package routing
 
 import (
-	"encoding/json"
 	"io"
 	"net"
 	"net/http"
@@ -147,6 +146,12 @@ func UpdateDeviceByID(
 			JSON: jsonerror.Forbidden("device does not exist"),
 		}
 	}
+	if performRes.Forbidden {
+		return util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("device not owned by current user"),
+		}
+	}
 
 	return util.JSONResponse{
 		Code: http.StatusOK,
@@ -184,7 +189,7 @@ func DeleteDeviceById(
 		if dev != deviceID {
 			return util.JSONResponse{
 				Code: http.StatusForbidden,
-				JSON: jsonerror.Forbidden("session and device mismatch"),
+				JSON: jsonerror.Forbidden("session & device mismatch"),
 			}
 		}
 	}
@@ -237,38 +242,17 @@ func DeleteDeviceById(
 
 // DeleteDevices handles POST requests to /delete_devices
 func DeleteDevices(
-	req *http.Request, userInteractiveAuth *auth.UserInteractive, userAPI api.ClientUserAPI, device *api.Device,
+	req *http.Request, userAPI api.ClientUserAPI, device *api.Device,
 ) util.JSONResponse {
 	ctx := req.Context()
-
-	bodyBytes, err := io.ReadAll(req.Body)
-	if err != nil {
-		return util.JSONResponse{
-			Code: http.StatusBadRequest,
-			JSON: jsonerror.BadJSON("The request body could not be read: " + err.Error()),
-		}
-	}
-	defer req.Body.Close() // nolint:errcheck
-
-	// initiate UIA
-	login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device)
-	if errRes != nil {
-		return *errRes
-	}
-
-	if login.Username() != device.UserID {
-		return util.JSONResponse{
-			Code: http.StatusForbidden,
-			JSON: jsonerror.Forbidden("unable to delete devices for other user"),
-		}
-	}
-
 	payload := devicesDeleteJSON{}
-	if err = json.Unmarshal(bodyBytes, &payload); err != nil {
+
-		util.GetLogger(ctx).WithError(err).Error("unable to unmarshal device deletion request")
+	if resErr := httputil.UnmarshalJSONRequest(req, &payload); resErr != nil {
-		return jsonerror.InternalServerError()
+		return *resErr
 	}
 
+	defer req.Body.Close() // nolint: errcheck
+
 	var res api.PerformDeviceDeletionResponse
 	if err := userAPI.PerformDeviceDeletion(ctx, &api.PerformDeviceDeletionRequest{
 		UserID:    device.UserID,

clientapi/routing/routing.go

@@ -1120,7 +1120,7 @@ func Setup(
 
 	v3mux.Handle("/delete_devices",
 		httputil.MakeAuthAPI("delete_devices", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse {
-			return DeleteDevices(req, userInteractiveAuth, userAPI, device)
+			return DeleteDevices(req, userAPI, device)
 		}),
 	).Methods(http.MethodPost, http.MethodOptions)
 

userapi/api/api.go

@@ -232,6 +232,7 @@ type PerformDeviceUpdateRequest struct {
 }
 type PerformDeviceUpdateResponse struct {
 	DeviceExists bool
+	Forbidden    bool
 }
 
 type PerformDeviceDeletionRequest struct {

userapi/internal/user_api.go

@@ -388,6 +388,11 @@ func (a *UserInternalAPI) PerformDeviceUpdate(ctx context.Context, req *api.Perf
 	}
 	res.DeviceExists = true
 
+	if dev.UserID != req.RequestingUserID {
+		res.Forbidden = true
+		return nil
+	}
+
 	err = a.DB.UpdateDevice(ctx, localpart, domain, req.DeviceID, req.DisplayName)
 	if err != nil {
 		util.GetLogger(ctx).WithError(err).Error("deviceDB.UpdateDevice failed")