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

Fix /refresh endpoint version #14364

Merged
merged 3 commits into from
Nov 4, 2022
Merged

Fix /refresh endpoint version #14364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b507c3d
Fix /refresh endpoint version
tulir Nov 4, 2022
59c854e
Add changelog
tulir Nov 4, 2022
bf74719
Fix refresh endpoint version in tests
tulir Nov 4, 2022
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/14364.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix refresh token endpoint to be under /r0 and /v3 instead of /v1. Contributed by Tulir @ Beeper.
2 changes: 1 addition & 1 deletion synapse/rest/client/login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ def _get_auth_flow_dict_for_idp(idp: SsoIdentityProvider) -> JsonDict:


class RefreshTokenServlet(RestServlet):
PATTERNS = (re.compile("^/_matrix/client/v1/refresh$"),)
PATTERNS = client_patterns("/refresh$")
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this valid for r0? Should this be:

Suggested change
PATTERNS = client_patterns("/refresh$")
PATTERNS = client_patterns("/refresh$", releases=("v3",), unstable=False)

Copy link
Member Author

Choose a reason for hiding this comment

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

It hasn't been in the spec as r0, but implementing it as r0 was technically allowed (implementing stable prefixes is allowed immediately after a MSC is merged, and the MSC was merged before everything was bumped to v3).

Copy link
Contributor

Choose a reason for hiding this comment

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

Regardless, I think including it as unstable is not wanted. We've been trying to reduce the number of unstable endpoints we expose.


def __init__(self, hs: "HomeServer"):
self._auth_handler = hs.get_auth_handler()
Expand Down
16 changes: 8 additions & 8 deletions tests/rest/client/test_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def use_refresh_token(self, refresh_token: str) -> FakeChannel:
"""
return self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": refresh_token},
)

Expand Down Expand Up @@ -724,7 +724,7 @@ def test_token_refresh(self) -> None:

refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(refresh_response.code, HTTPStatus.OK, refresh_response.result)
Expand Down Expand Up @@ -765,7 +765,7 @@ def test_refreshable_access_token_expiration(self) -> None:

refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(refresh_response.code, HTTPStatus.OK, refresh_response.result)
Expand Down Expand Up @@ -1002,7 +1002,7 @@ def test_refresh_token_invalidation(self) -> None:
# This first refresh should work properly
first_refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -1012,7 +1012,7 @@ def test_refresh_token_invalidation(self) -> None:
# This one as well, since the token in the first one was never used
second_refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -1022,7 +1022,7 @@ def test_refresh_token_invalidation(self) -> None:
# This one should not, since the token from the first refresh is not valid anymore
third_refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": first_refresh_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand Down Expand Up @@ -1056,7 +1056,7 @@ def test_refresh_token_invalidation(self) -> None:
# Now that the access token from the last valid refresh was used once, refreshing with the N-1 token should fail
fourth_refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -1068,7 +1068,7 @@ def test_refresh_token_invalidation(self) -> None:
# But refreshing from the last valid refresh token still works
fifth_refresh_response = self.make_request(
"POST",
"/_matrix/client/v1/refresh",
"/_matrix/client/v3/refresh",
{"refresh_token": second_refresh_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand Down