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

Commit

Permalink
Warn, instead of erroring, if the client dict changes UI Auth.
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep committed May 12, 2020
1 parent fa4af2c commit 55a271a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
1 change: 1 addition & 0 deletions changelog.d/7483.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure that a user inteactive authentication session is tied to a single request.
27 changes: 16 additions & 11 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,25 +346,30 @@ async def check_auth(

# Ensure that the queried operation does not vary between stages of
# the UI authentication session. This is done by generating a stable
# comparator based on the URI, method, and client dict (minus the
# auth dict) and storing it during the initial query. Subsequent
# comparator and storing it during the initial query. Subsequent
# queries ensure that this comparator has not changed.
if validate_clientdict:
session_comparator = (session.uri, session.method, session.clientdict)
comparator = (uri, method, clientdict)
else:
session_comparator = (session.uri, session.method) # type: ignore
comparator = (uri, method) # type: ignore

if session_comparator != comparator:
#
# The comparator is based on the requested URI and HTTP method. The
# client dict (minus the auth dict) should also be checked, but some
# clients are not spec compliant, just warn for now if the client
# dict changes.
if (session.uri, session.method) != (uri, method):
raise SynapseError(
403,
"Requested operation has changed during the UI authentication session.",
)

if validate_clientdict:
if session.clientdict != clientdict:
logger.warning(
"Requested operation has changed during the UI "
"authentication session. A future version of Synapse "
"will remove this capability."
)

# For backwards compatibility the registration endpoint persists
# changes to the client dict instead of validating them.
if not validate_clientdict:
else:
await self.store.set_ui_auth_clientdict(sid, clientdict)

if not authdict:
Expand Down
3 changes: 3 additions & 0 deletions tests/rest/client/v2_alpha/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ def test_cannot_change_body(self):
},
)

# This test is currently skipped since it is allowed for non-spec compliant clients.
test_cannot_change_body.skip = "Skipped to allow for non-compliant clients" # type: ignore

def test_cannot_change_uri(self):
"""
The initial requested URI cannot be modified during the user interactive authentication session.
Expand Down

0 comments on commit 55a271a

Please sign in to comment.