Merge pull request #73 from globekeeper/daniel/remove-delete_devices-UIA

Daniel/remove delete devices UIA
This commit is contained in:
Daniel Aloni 2023-06-19 12:20:53 +03:00 committed by GitHub
commit 303a9599b8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 72 additions and 1117 deletions

View file

@ -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{
aliceAdmin: {},
bob: {},
vhUser: {},
accessTokens := map[*test.User]string{
aliceAdmin: "",
bob: "",
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{
aliceAdmin: {},
accessTokens := map[*test.User]string{
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{
aliceAdmin: {},
accessTokens := map[*test.User]string{
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{
aliceAdmin: {},
accessTokens := map[*test.User]string{
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{
aliceAdmin: {},
accessTokens := map[*test.User]string{
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()
}
}

File diff suppressed because it is too large Load diff

View file

@ -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()
@ -47,7 +47,6 @@ func Deactivate(
var res api.PerformAccountDeactivationResponse
err = accountAPI.PerformAccountDeactivation(ctx, &api.PerformAccountDeactivationRequest{
Localpart: localpart,
ServerName: serverName,
}, &res)
if err != nil {
util.GetLogger(ctx).WithError(err).Error("userAPI.PerformAccountDeactivation failed")

View file

@ -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")
return jsonerror.InternalServerError()
if resErr := httputil.UnmarshalJSONRequest(req, &payload); resErr != nil {
return *resErr
}
defer req.Body.Close() // nolint: errcheck
var res api.PerformDeviceDeletionResponse
if err := userAPI.PerformDeviceDeletion(ctx, &api.PerformDeviceDeletionRequest{
UserID: device.UserID,

View file

@ -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)

View file

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

View file

@ -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")