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

Autocreate autojoin rooms #3975

Merged
merged 16 commits into from
Oct 25, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
1 change: 1 addition & 0 deletions changelog.d/3975.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
First user should autocreate autojoin rooms
Copy link
Member

Choose a reason for hiding this comment

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

could you expand on this a bit so that it makes sense for users, please?

4 changes: 4 additions & 0 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def read_config(self, config):
)

self.auto_join_rooms = config.get("auto_join_rooms", [])
self.autocreate_auto_join_rooms = config.get("autocreate_auto_join_rooms", True)

def default_config(self, **kwargs):
registration_shared_secret = random_string_with_symbols(50)
Expand Down Expand Up @@ -98,6 +99,9 @@ def default_config(self, **kwargs):
# to these rooms
#auto_join_rooms:
# - "#example:example.com"

# Have first user on server autocreate autojoin rooms
Copy link
Member

Choose a reason for hiding this comment

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

maybe explain what will happen if it is false, and conversely what happens if it is true?

autocreate_auto_join_rooms: true
""" % locals()

def add_arguments(self, parser):
Expand Down
28 changes: 28 additions & 0 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,34 @@ def get_or_register_3pid_guest(self, medium, address, inviter_user_id):

@defer.inlineCallbacks
def _join_user_to_room(self, requester, room_identifier):

Copy link
Member

Choose a reason for hiding this comment

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

please no

Copy link
Contributor

Choose a reason for hiding this comment

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

it was oversight I promise!

# try to create the room if we're the first user on the server
if self.hs.config.autocreate_auto_join_rooms:
count = yield self.store.count_all_users()
if count == 1 and RoomAlias.is_valid(room_identifier):
Copy link
Member

Choose a reason for hiding this comment

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

it feels very random to be doing this at this point. (Which is when the first user is registered? when they log in?). what if two users register at once? what if the first user didn't qualify for autojoining for some reason?

Can we not do this when the HS first starts, somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

the only way to do it at start would be if we also autocreated an admin user, which in turn then means shared-secret registering them and then setting up their profile and 3pid and passwords etc correctly. agreed this would be cleaner, but this is intended as a fast fix to avoid having to do that in the provisioning process, on the basis that the first user who signs in is 99% always going to be the server admin, and if it isn't, then you can always repoint the alias.

Copy link
Member

@richvdh richvdh Oct 1, 2018

Choose a reason for hiding this comment

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

ok, fair enough, but still, having _join_user_to_room magically do something different if there is now exactly one registered user feels very magical.

can we pull out the is this the first user logic to register (where we only have to do it once per user, rather than once per user per room)? and have a separate _create_auto_join_room method?

Copy link
Member

Choose a reason for hiding this comment

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

and have a separate _create_auto_join_room method

or just call room_creation_handler.create_room directly from the for r in self.hs.config.auto_join_rooms loop in register.

room_creation_handler = self.hs.get_room_creation_handler()
info = yield room_creation_handler.create_room(
requester,
config={
"preset": "public_chat",
},
ratelimit=False,
)
room_id = info["room_id"]

directory_handler = self.hs.get_handlers().directory_handler
Copy link
Member

Choose a reason for hiding this comment

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

can we not do the alias bits by passing room_alias_name to create_room ?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, possibly.

room_alias = RoomAlias.from_string(room_identifier)
yield directory_handler.create_association(
user_id=requester.user.to_string(),
room_alias=room_alias,
room_id=room_id,
servers=[self.hs.hostname],
)

yield directory_handler.send_room_alias_update_event(
requester, requester.user.to_string(), room_id
)

room_id = None
room_member_handler = self.hs.get_room_member_handler()
if RoomID.is_valid(room_identifier):
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def get_aliases_for_room(self, room_id):
class DirectoryStore(DirectoryWorkerStore):
@defer.inlineCallbacks
def create_room_alias_association(self, room_alias, room_id, servers, creator=None):
""" Creates an associatin between a room alias and room_id/servers
""" Creates an association between a room alias and room_id/servers

Args:
room_alias (RoomAlias)
Expand Down