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

Commit 9dc6f30

Browse files
authored
Hash passwords earlier in the password reset process (#7538)
This now matches the logic of the registration process as modified in 56db0b1 / #7523.
1 parent 4fa74c7 commit 9dc6f30

File tree

5 files changed

+33
-11
lines changed

5 files changed

+33
-11
lines changed

changelog.d/7538.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Hash passwords as early as possible during password reset.

synapse/handlers/set_password.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,13 @@ def __init__(self, hs):
3535
async def set_password(
3636
self,
3737
user_id: str,
38-
new_password: str,
38+
password_hash: str,
3939
logout_devices: bool,
4040
requester: Optional[Requester] = None,
4141
):
4242
if not self.hs.config.password_localdb_enabled:
4343
raise SynapseError(403, "Password change disabled", errcode=Codes.FORBIDDEN)
4444

45-
self._password_policy_handler.validate_password(new_password)
46-
password_hash = await self._auth_handler.hash(new_password)
47-
4845
try:
4946
await self.store.user_set_password_hash(user_id, password_hash)
5047
except StoreError as e:

synapse/rest/admin/users.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,14 @@ async def on_PUT(self, request, user_id):
222222
else:
223223
new_password = body["password"]
224224
logout_devices = True
225+
226+
new_password_hash = await self.auth_handler.hash(new_password)
227+
225228
await self.set_password_handler.set_password(
226-
target_user.to_string(), new_password, logout_devices, requester
229+
target_user.to_string(),
230+
new_password_hash,
231+
logout_devices,
232+
requester,
227233
)
228234

229235
if "deactivated" in body:
@@ -523,6 +529,7 @@ def __init__(self, hs):
523529
self.store = hs.get_datastore()
524530
self.hs = hs
525531
self.auth = hs.get_auth()
532+
self.auth_handler = hs.get_auth_handler()
526533
self._set_password_handler = hs.get_set_password_handler()
527534

528535
async def on_POST(self, request, target_user_id):
@@ -539,8 +546,10 @@ async def on_POST(self, request, target_user_id):
539546
new_password = params["new_password"]
540547
logout_devices = params.get("logout_devices", True)
541548

549+
new_password_hash = await self.auth_handler.hash(new_password)
550+
542551
await self._set_password_handler.set_password(
543-
target_user_id, new_password, logout_devices, requester
552+
target_user_id, new_password_hash, logout_devices, requester
544553
)
545554
return 200, {}
546555

synapse/rest/client/v2_alpha/account.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,27 @@ def __init__(self, hs):
220220
self.auth = hs.get_auth()
221221
self.auth_handler = hs.get_auth_handler()
222222
self.datastore = self.hs.get_datastore()
223+
self.password_policy_handler = hs.get_password_policy_handler()
223224
self._set_password_handler = hs.get_set_password_handler()
224225

225226
@interactive_auth_handler
226227
async def on_POST(self, request):
227228
body = parse_json_object_from_request(request)
228229

230+
# we do basic sanity checks here because the auth layer will store these
231+
# in sessions. Pull out the new password provided to us.
232+
if "new_password" in body:
233+
new_password = body.pop("new_password")
234+
if not isinstance(new_password, str) or len(new_password) > 512:
235+
raise SynapseError(400, "Invalid password")
236+
self.password_policy_handler.validate_password(new_password)
237+
238+
# If the password is valid, hash it and store it back on the body.
239+
# This ensures that only the hashed password is handled everywhere.
240+
if "new_password_hash" in body:
241+
raise SynapseError(400, "Unexpected property: new_password_hash")
242+
body["new_password_hash"] = await self.auth_handler.hash(new_password)
243+
229244
# there are two possibilities here. Either the user does not have an
230245
# access token, and needs to do a password reset; or they have one and
231246
# need to validate their identity.
@@ -276,12 +291,12 @@ async def on_POST(self, request):
276291
logger.error("Auth succeeded but no known type! %r", result.keys())
277292
raise SynapseError(500, "", Codes.UNKNOWN)
278293

279-
assert_params_in_dict(params, ["new_password"])
280-
new_password = params["new_password"]
294+
assert_params_in_dict(params, ["new_password_hash"])
295+
new_password_hash = params["new_password_hash"]
281296
logout_devices = params.get("logout_devices", True)
282297

283298
await self._set_password_handler.set_password(
284-
user_id, new_password, logout_devices, requester
299+
user_id, new_password_hash, logout_devices, requester
285300
)
286301

287302
return 200, {}

synapse/rest/client/v2_alpha/register.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,8 +431,8 @@ async def on_POST(self, request):
431431
raise SynapseError(400, "Invalid password")
432432
self.password_policy_handler.validate_password(password)
433433

434-
# If the password is valid, hash it and store it back on the request.
435-
# This ensures the hashed password is handled everywhere.
434+
# If the password is valid, hash it and store it back on the body.
435+
# This ensures that only the hashed password is handled everywhere.
436436
if "password_hash" in body:
437437
raise SynapseError(400, "Unexpected property: password_hash")
438438
body["password_hash"] = await self.auth_handler.hash(password)

0 commit comments

Comments
 (0)