Make remaining sytest pass

clientapi/auth/user_interactive.go

@@ -172,23 +172,48 @@ func (u *UserInteractive) NewSession() *util.JSONResponse {
 	return u.Challenge(sessionID)
 }
 
+// ResponseWithChallenge mixes together a JSON body (e.g an error with errcode/message) with the
+// standard challenge response.
+func (u *UserInteractive) ResponseWithChallenge(sessionID string, response interface{}) *util.JSONResponse {
+	mixedObjects := make(map[string]interface{})
+	b, err := json.Marshal(response)
+	if err != nil {
+		ise := jsonerror.InternalServerError()
+		return &ise
+	}
+	_ = json.Unmarshal(b, &mixedObjects)
+	challenge := u.Challenge(sessionID)
+	b, err = json.Marshal(challenge.JSON)
+	if err != nil {
+		ise := jsonerror.InternalServerError()
+		return &ise
+	}
+	_ = json.Unmarshal(b, &mixedObjects)
+
+	return &util.JSONResponse{
+		Code: 401,
+		JSON: mixedObjects,
+	}
+}
+
 // Verify returns an error/challenge response to send to the client, or nil if the user is authenticated.
 // `bodyBytes` is the HTTP request body which must contain an `auth` key.
-func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *api.Device) *util.JSONResponse {
+// Returns the login that was verified for additional checks if required.
+func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *api.Device) (*Login, *util.JSONResponse) {
 	// TODO: rate limit
 
 	// "A client should first make a request with no auth parameter. The homeserver returns an HTTP 401 response, with a JSON body"
 	// https://matrix.org/docs/spec/client_server/r0.6.1#user-interactive-api-in-the-rest-api
 	hasResponse := gjson.GetBytes(bodyBytes, "auth").Exists()
 	if !hasResponse {
-		return u.NewSession()
+		return nil, u.NewSession()
 	}
 
 	// extract the type so we know which login type to use
 	authType := gjson.GetBytes(bodyBytes, "auth.type").Str
 	loginType, ok := u.Types[authType]
 	if !ok {
-		return &util.JSONResponse{
+		return nil, &util.JSONResponse{
 			Code: http.StatusBadRequest,
 			JSON: jsonerror.BadJSON("unknown auth.type: " + authType),
 		}
@@ -199,7 +224,7 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *
 	if _, ok = u.Sessions[sessionID]; !ok {
 		// if the login type is part of a single stage flow then allow them to omit the session ID
 		if !u.IsSingleStageFlow(authType) {
-			return &util.JSONResponse{
+			return nil, &util.JSONResponse{
 				Code: http.StatusBadRequest,
 				JSON: jsonerror.Unknown("missing or unknown auth.session"),
 			}
@@ -208,15 +233,16 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *
 
 	r := loginType.Request()
 	if err := json.Unmarshal([]byte(gjson.GetBytes(bodyBytes, "auth").Raw), r); err != nil {
-		return &util.JSONResponse{
+		return nil, &util.JSONResponse{
 			Code: http.StatusBadRequest,
 			JSON: jsonerror.BadJSON("The request body could not be decoded into valid JSON. " + err.Error()),
 		}
 	}
-	_, resErr := loginType.Login(ctx, r)
+	login, resErr := loginType.Login(ctx, r)
 	if resErr == nil {
 		u.AddCompletedStage(sessionID, authType)
 		// TODO: Check if there's more stages to go and return an error
+		return login, nil
 	}
-	return resErr
+	return nil, u.ResponseWithChallenge(sessionID, resErr.JSON)
 }

clientapi/auth/user_interactive_test.go

@@ -41,7 +41,7 @@ func setup() *UserInteractive {
 func TestUserInteractiveChallenge(t *testing.T) {
 	uia := setup()
 	// no auth key results in a challenge
-	errRes := uia.Verify(ctx, []byte(`{}`), device)
+	_, errRes := uia.Verify(ctx, []byte(`{}`), device)
 	if errRes == nil {
 		t.Fatalf("Verify succeeded with {} but expected failure")
 	}
@@ -81,7 +81,7 @@ func TestUserInteractivePasswordLogin(t *testing.T) {
 		}`),
 	}
 	for _, tc := range testCases {
-		errRes := uia.Verify(ctx, tc, device)
+		_, errRes := uia.Verify(ctx, tc, device)
 		if errRes != nil {
 			t.Errorf("Verify failed but expected success for request: %s - got %+v", string(tc), errRes)
 		}
@@ -162,7 +162,7 @@ func TestUserInteractivePasswordBadLogin(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		errRes := uia.Verify(ctx, tc.body, device)
+		_, errRes := uia.Verify(ctx, tc.body, device)
 		if errRes == nil {
 			t.Errorf("Verify succeeded but expected failure for request: %s", string(tc.body))
 			continue

clientapi/routing/device.go

@@ -177,7 +177,7 @@ func DeleteDeviceById(
 			JSON: jsonerror.BadJSON("The request body could not be read: " + err.Error()),
 		}
 	}
-	errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device)
+	login, errRes := userInteractiveAuth.Verify(ctx, bodyBytes, device)
 	if errRes != nil {
 		return *errRes
 	}
@@ -188,7 +188,14 @@ func DeleteDeviceById(
 		return jsonerror.InternalServerError()
 	}
 
-	defer req.Body.Close() // nolint: errcheck
+	// make sure that the access token being used matches the login creds used for user interactive auth, else
+	// 1 compromised access token could be used to logout all devices.
+	if login.Username() != localpart && login.Username() != device.UserID {
+		return util.JSONResponse{
+			Code: 403,
+			JSON: jsonerror.Forbidden("Cannot delete another user's device"),
+		}
+	}
 
 	if err := deviceDB.RemoveDevice(ctx, deviceID, localpart); err != nil {
 		util.GetLogger(ctx).WithError(err).Error("deviceDB.RemoveDevice failed")

sytest-whitelist

@@ -34,6 +34,9 @@ PUT /device/{deviceId} gives a 404 for unknown devices
 GET /device/{deviceId}
 GET /devices
 PUT /device/{deviceId} updates device fields
+DELETE /device/{deviceId}
+DELETE /device/{deviceId} requires UI auth user to match device owner
+DELETE /device/{deviceId} with no body gives a 401
 POST /createRoom makes a public room
 POST /createRoom makes a private room
 POST /createRoom makes a private room with invites