Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Cross-signing [1/4] -- hidden devices #5759

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 47 additions & 16 deletions synapse/storage/devices.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# -*- coding: utf-8 -*-
# Copyright 2016 OpenMarket Ltd
# Copyright 2019 New Vector Ltd
# Copyright 2019 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -20,7 +22,7 @@

from twisted.internet import defer

from synapse.api.errors import StoreError
from synapse.api.errors import Codes, StoreError
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.storage._base import Cache, SQLBaseStore, db_to_json
from synapse.storage.background_updates import BackgroundUpdateStore
Expand All @@ -35,6 +37,7 @@


class DeviceWorkerStore(SQLBaseStore):
@defer.inlineCallbacks
def get_device(self, user_id, device_id):
"""Retrieve a device.

Expand All @@ -46,12 +49,15 @@ def get_device(self, user_id, device_id):
Raises:
StoreError: if the device is not found
"""
return self._simple_select_one(
ret = yield self._simple_select_one(
table="devices",
keyvalues={"user_id": user_id, "device_id": device_id},
retcols=("user_id", "device_id", "display_name"),
retcols=("user_id", "device_id", "display_name", "hidden"),
desc="get_device",
)
if ret["hidden"]:
raise StoreError(404, "No row found (devices)")
return ret

@defer.inlineCallbacks
def get_devices_by_user(self, user_id):
Expand All @@ -67,11 +73,11 @@ def get_devices_by_user(self, user_id):
devices = yield self._simple_select_list(
table="devices",
keyvalues={"user_id": user_id},
retcols=("user_id", "device_id", "display_name"),
retcols=("user_id", "device_id", "display_name", "hidden"),
desc="get_devices_by_user",
)

defer.returnValue({d["device_id"]: d for d in devices})
defer.returnValue({d["device_id"]: d for d in devices if not d["hidden"]})

@defer.inlineCallbacks
def get_devices_by_remote(self, destination, from_stream_id, limit):
Expand Down Expand Up @@ -540,6 +546,8 @@ def store_device(self, user_id, device_id, initial_device_display_name):
Returns:
defer.Deferred: boolean whether the device was inserted or an
existing device existed with that ID.
Raises:
StoreError: if the device is already in use
"""
key = (user_id, device_id)
if self.device_id_exists_cache.get(key, None):
Expand All @@ -552,12 +560,25 @@ def store_device(self, user_id, device_id, initial_device_display_name):
"user_id": user_id,
"device_id": device_id,
"display_name": initial_device_display_name,
"hidden": False,
},
desc="store_device",
or_ignore=True,
)
if not inserted:
# if the device already exists, check if it's a real device, or
# if the device ID is reserved by something else
hidden = yield self._simple_select_one_onecol(
"devices",
keyvalues={"user_id": user_id, "device_id": device_id},
retcol="hidden",
)
if hidden:
raise StoreError(400, "The device ID is in use", Codes.FORBIDDEN)
self.device_id_exists_cache.prefill(key, True)
defer.returnValue(inserted)
except StoreError:
raise
except Exception as e:
logger.error(
"store_device with device_id=%s(%r) user_id=%s(%r)"
Expand All @@ -582,11 +603,11 @@ def delete_device(self, user_id, device_id):
Returns:
defer.Deferred
"""
yield self._simple_delete_one(
table="devices",
keyvalues={"user_id": user_id, "device_id": device_id},
desc="delete_device",
)
sql = """
DELETE FROM devices
WHERE user_id = ? AND device_id = ? AND NOT COALESCE(hidden, ?)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having looked at how this works out, I'm really sorry but I think I'm going to revise my original comments here; it's just too awkward. I think the 30s or so it's going to take to add a NOT NULL column to the table will be worth it for not jumping through these hoops and having to manage a tristate.

Really sorry for vacillating on this.

"""
yield self._execute("delete_device", None, sql, user_id, device_id, False)

self.device_id_exists_cache.invalidate((user_id, device_id))

Expand All @@ -600,13 +621,21 @@ def delete_devices(self, user_id, device_ids):
Returns:
defer.Deferred
"""
yield self._simple_delete_many(
table="devices",
column="device_id",
iterable=device_ids,
keyvalues={"user_id": user_id},
desc="delete_devices",

if not device_ids or len(device_ids) == 0:
return
sql = """
DELETE FROM devices
WHERE user_id = ? AND device_id IN (%s) AND NOT COALESCE(hidden, ?)
""" % (
",".join("?" for _ in device_ids)
)
values = [user_id]
values.extend(device_ids)
values.append(False)

yield self._execute("delete_devices", None, sql, *values)

for device_id in device_ids:
self.device_id_exists_cache.invalidate((user_id, device_id))

Expand All @@ -628,6 +657,8 @@ def update_device(self, user_id, device_id, new_display_name=None):
updates["display_name"] = new_display_name
if not updates:
return defer.succeed(None)
# FIXME: should only update if hidden is not True. But updating the
# display name of a hidden device should be harmless
return self._simple_update_one(
table="devices",
keyvalues={"user_id": user_id, "device_id": device_id},
Expand Down
18 changes: 18 additions & 0 deletions synapse/storage/schema/delta/56/signing_keys.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright 2019 New Vector Ltd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you give this delta file a different name?

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

-- device list needs to know which ones are "real" devices, and which ones are
-- just used to avoid collisions
ALTER TABLE devices ADD COLUMN hidden BOOLEAN NULLABLE;