diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 637d5e442a..49bf3f6c69 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -283,7 +283,7 @@ jobs: - name: Build upgrade-tests run: go build ./cmd/dendrite-upgrade-tests - name: Test upgrade (PostgreSQL) - run: ./dendrite-upgrade-tests --head . + run: ./dendrite-upgrade-tests . # run database upgrade tests, skipping over one version upgrade_test_direct: @@ -301,7 +301,7 @@ jobs: - name: Build upgrade-tests run: go build ./cmd/dendrite-upgrade-tests - name: Test upgrade (PostgreSQL) - run: ./dendrite-upgrade-tests -direct -from HEAD-2 --head . + run: ./dendrite-upgrade-tests -direct -from HEAD-2 . # run Sytest in different variations sytest: diff --git a/clientapi/clientapi_test.go b/clientapi/clientapi_test.go index 3a4ae4ff9e..7202fd62fc 100644 --- a/clientapi/clientapi_test.go +++ b/clientapi/clientapi_test.go @@ -255,7 +255,6 @@ func TestDeleteDevice(t *testing.T) { }) } -// Deleting devices requires the UIA dance, so do this in a different test func TestDeleteDevices(t *testing.T) { alice := test.NewUser(t) localpart, serverName, _ := gomatrixserverlib.SplitID('@', alice.ID) @@ -300,34 +299,15 @@ func TestDeleteDevices(t *testing.T) { devices = append(devices, devRes.Device.ID) } - // initiate UIA rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/delete_devices", strings.NewReader("")) - req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) - routers.Client.ServeHTTP(rec, req) - if rec.Code != http.StatusUnauthorized { - t.Fatalf("expected HTTP 401, got %d: %s", rec.Code, rec.Body.String()) - } - // get the session ID - sessionID := gjson.GetBytes(rec.Body.Bytes(), "session").Str - - // prepare UIA request body + // prepare request body reqBody := bytes.Buffer{} if err := json.NewEncoder(&reqBody).Encode(map[string]interface{}{ - "auth": map[string]string{ - "session": sessionID, - "type": authtypes.LoginTypePassword, - "user": alice.ID, - "password": accessTokens[alice].password, - }, "devices": devices[5:], }); err != nil { t.Fatal(err) } - - // do the same request again, this time with our UIA, - rec = httptest.NewRecorder() - req = httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/delete_devices", &reqBody) + req := httptest.NewRequest(http.MethodPost, "/_matrix/client/v3/delete_devices", strings.NewReader(reqBody.String())) req.Header.Set("Authorization", "Bearer "+accessTokens[alice].accessToken) routers.Client.ServeHTTP(rec, req) if rec.Code != http.StatusOK { diff --git a/clientapi/routing/device.go b/clientapi/routing/device.go index f57b8957fb..6c59475e3a 100644 --- a/clientapi/routing/device.go +++ b/clientapi/routing/device.go @@ -15,7 +15,6 @@ package routing import ( - "encoding/json" "io" "net" "net/http" @@ -162,6 +161,12 @@ func UpdateDeviceByID( JSON: spec.Forbidden("device not owned by current user"), } } + if performRes.Forbidden { + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: spec.Forbidden("device not owned by current user"), + } + } return util.JSONResponse{ Code: http.StatusOK, @@ -258,39 +263,13 @@ 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: spec.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: spec.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") - return util.JSONResponse{ - Code: http.StatusInternalServerError, - JSON: spec.InternalServerError{}, - } + + if resErr := httputil.UnmarshalJSONRequest(req, &payload); resErr != nil { + return *resErr } defer req.Body.Close() // nolint: errcheck diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index f34eec17f5..5af421fd70 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -1175,7 +1175,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)