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

[WIP] device dehydration #7955

Closed
wants to merge 7 commits into from
Closed

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Jul 24, 2020

changelog.d/7955.feature Outdated Show resolved Hide resolved
return (
200,
await self.device_handler.rehydrate_device(
submission.get("dehydration_token")
Copy link
Member

Choose a reason for hiding this comment

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

We probably want to use assert_params_in_dict for dehydration_token since it is required -- this will raise a sane error message. (This is true for any parameters that are required in the added endpoints.)

@@ -401,6 +423,52 @@ async def _do_jwt_login(self, login_submission: JsonDict) -> Dict[str, str]:
return result


class RestoreDeviceServlet(RestServlet):
PATTERNS = client_patterns("/org.matrix.msc2697/restore_device")
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is supposed to show up only in unstable or also r0 (the current code would do both).

"dehydration_token": token,
}

# FIXME: call callback?
Copy link
Member

@clokep clokep Aug 14, 2020

Choose a reason for hiding this comment

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

The documentation about what this callback might be (see the check_auth info in there) is less than ideal. It comes from password auth modules and might be None or a Callable that is "called with the result from the /login call (including access_token, device_id, etc.)"

I do not know of any auth providers which use this frankly, but I think calling it like below is the reasonable thing to do. Is there a particular reason you think it should not be called?

Copy link
Member

Choose a reason for hiding this comment

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

We talked a bit more about this and the reason we can't just call it here is that we don't yet have the access token (the client hasn't acknowledged that they've been able to use the dehydrated device).

It seems like we would ideally want to call this at the end of the call to /restore_device, but we no longer have the callback at that point. Will need to think about what the best option is here.

@uhoreg uhoreg mentioned this pull request Sep 22, 2020
@clokep
Copy link
Member

clokep commented Oct 1, 2020

I think we've decided the implementation in #8380 is a bit less invasive. Shall we close this?

@uhoreg
Copy link
Member Author

uhoreg commented Oct 1, 2020

replaced by 8380

@uhoreg uhoreg closed this Oct 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants