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

Allow for identifier dicts in User Interactive Auth dicts #7438

Closed
wants to merge 17 commits into from
Closed

Allow for identifier dicts in User Interactive Auth dicts #7438

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b674bb8
Move utility methods from login handler to auth handler
anoadragon453 Jun 12, 2020
7044c1f
Factor out identifier -> username conversion into its own method
anoadragon453 Jun 12, 2020
f240a8d
Reconfigure m.login.password authdict checker to process identifiers
anoadragon453 Jun 12, 2020
cb64c95
Comment cleanups, log on KeyError during login
anoadragon453 Jun 12, 2020
1807115
Remove placeholders/dummy classes for supporting identifiers in exist…
anoadragon453 Jun 12, 2020
358e51b
Add some tests for m.id.phone and m.id.thirdparty
anoadragon453 Jun 12, 2020
699904c
Changelog
anoadragon453 Jun 12, 2020
7184c16
Change login_id_phone_to_thirdparty to return a dict again
anoadragon453 Jun 16, 2020
1876235
pop() instead of pull then del
anoadragon453 Jun 16, 2020
b8f4b0c
Use assert_param_in_dict
anoadragon453 Jun 16, 2020
efb5670
Update synapse/handlers/auth.py
anoadragon453 Jun 16, 2020
53981c3
Change SynapseError comment
anoadragon453 Jun 16, 2020
b1c0eb3
Docstring spacing
anoadragon453 Jun 16, 2020
d9277e9
Don't lowercase medium in this PR
anoadragon453 Jun 16, 2020
cb272bc
Explain why we rate-limit using a threepid
anoadragon453 Jun 25, 2020
af21fbb
Simplify medium and address assignment
anoadragon453 Jun 25, 2020
7affcd0
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/use…
anoadragon453 Aug 26, 2020
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
4 changes: 1 addition & 3 deletions tests/rest/client/v1/test_login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,7 @@ def _delete_device(self, access_token, user_id, password, device_id):

auth = {
"type": "m.login.password",
# https://github.com/matrix-org/synapse/issues/5665
# "identifier": {"type": "m.id.user", "user": user_id},
"user": user_id,
"identifier": {"type": "m.id.user", "user": user_id},
Copy link
Member

Choose a reason for hiding this comment

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

We likely still want a test for the legacy login type.

Copy link
Member Author

@anoadragon453 anoadragon453 Jun 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, so the login unittest function still uses the old method:

synapse/tests/unittest.py

Lines 512 to 518 in c2e1a21

def login(self, username, password, device_id=None):
"""
Log in a user, and get an access token. Requires the Login API be
registered.
"""
body = {"type": "m.login.password", "user": username, "password": password}
So it's definitely still tested.

But it may be a better idea to update this to the new format and just have a specific test for the old behaviour?

"password": password,
"session": channel.json_body["session"],
}
Expand Down
8 changes: 0 additions & 8 deletions tests/rest/client/v2_alpha/test_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ def check_auth(self, authdict, clientip):
return succeed(True)


class DummyPasswordChecker(UserInteractiveAuthChecker):
def check_auth(self, authdict, clientip):
return succeed(authdict["identifier"]["user"])
Comment on lines -41 to -43
Copy link
Member

Choose a reason for hiding this comment

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

Is this just not used?

Copy link
Member Author

@anoadragon453 anoadragon453 Jun 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It was used by 1807115#diff-5fc780094ae3ec668024efd505af6a3dL170

This checker was a patch to allow for identifiers being passed to AuthHandler.check_auth, but this PR removes the need for it as it providers that functionality.



class FallbackAuthTests(unittest.HomeserverTestCase):

servlets = [
Expand Down Expand Up @@ -166,9 +161,6 @@ class UIAuthTests(unittest.HomeserverTestCase):
]

def prepare(self, reactor, clock, hs):
auth_handler = hs.get_auth_handler()
auth_handler.checkers[LoginType.PASSWORD] = DummyPasswordChecker(hs)

self.user_pass = "pass"
self.user = self.register_user("test", self.user_pass)
self.user_tok = self.login("test", self.user_pass)
Expand Down