-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Implement MSC3231: Token authenticated registration #10142
Implement MSC3231: Token authenticated registration #10142
Conversation
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking excellent so far, nice work! I've dropped a myriad of comments all over the place, and I realise the PR is still in a draft state. In any case, let me know if any of them are unclear or don't make sense at this point 🙂
synapse/storage/schema/main/delta/59/999create_registration_tokens.sql
Outdated
Show resolved
Hide resolved
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
aa8444e
to
1debc22
Compare
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
rather than using storage methods directly. Also use UIAuthSessionDataConstants. Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
So error fields are merged into UIA response. (Failures are due to authentication) Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for taking a while to get back to this. I left a review mid-way and only now realised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broadly this is looking excellent. I've got just a few notes below, but overall this looks solid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, note that 63
is actually the latest schema delta, rather than 62. It was raised in this commit. It didn't involve a migration though, which could be confusing if one is just looking at the list of schema delta directories.
I'll need to take another overarching look at this tomorrow - but it's looking very close! I'd also at least try using it from a worker setup manually, by creating a generic worker alongside the main process that handles client traffic. Then, attempt to register through it using a token.
I need to try it again, but during my last review I was unable to call the verify endpoint on the worker (I got back an "Unrecognised request" error). So it looks like the endpoint wasn't mounted on the worker, but didn't look at deeper than that yet.
Signed-off-by: Callum Brown <callum@calcuode.com>
Check if token already exists before trying to create it Signed-off-by: Callum Brown <callum@calcuode.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few remaining bits and pieces, but otherwise this is looking ready to go!
Signed-off-by: Callum Brown <callum@calcuode.com> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
To get the flattened tests/rest/client directory of matrix-org#10667
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm other than the outstanding comments!
Signed-off-by: Callum Brown <callum@calcuode.com>
Signed-off-by: Callum Brown <callum@calcuode.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice method of testing token generation collision.
This LGTM!
Signed-off-by: Callum Brown callum@calcuode.com
This is part of my GSoC project implementing MSC3231.
Status: