Implement MSC3231: Token authenticated registration #10142

govynnus · 2021-06-08T12:49:04Z

Signed-off-by: Callum Brown callum@calcuode.com

This is part of my GSoC project implementing MSC3231.

Status:

Works with a list of hard-coded tokens
Checks against tokens stored in the database
Validity checking endpoint
Fallback
Admin API for managing tokens

Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453

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/config/registration.py

synapse/handlers/ui_auth/checkers.py

synapse/storage/databases/main/registration.py

tests/rest/client/v2_alpha/test_register.py

synapse/handlers/ui_auth/checkers.py

synapse/storage/schema/main/delta/59/999create_registration_tokens.sql

Signed-off-by: Callum Brown <callum@calcuode.com>

synapse/storage/databases/main/registration.py

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>

synapse/rest/client/v2_alpha/register.py

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>

synapse/rest/client/v2_alpha/auth.py

Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453

Apologies for taking a while to get back to this. I left a review mid-way and only now realised.

synapse/rest/client/v2_alpha/auth.py

docs/SUMMARY.md

anoadragon453

Broadly this is looking excellent. I've got just a few notes below, but overall this looks solid.

synapse/rest/admin/__init__.py

synapse/rest/admin/registration_tokens.py

synapse/storage/databases/main/registration.py

synapse/rest/admin/registration_tokens.py

tests/rest/admin/test_registration_tokens.py

synapse/storage/databases/main/registration.py

anoadragon453

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.

synapse/storage/databases/main/registration.py

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>

anoadragon453

A few remaining bits and pieces, but otherwise this is looking ready to go!

docs/usage/administration/admin_api/registration_tokens.md

synapse/rest/admin/registration_tokens.py

docs/usage/administration/admin_api/registration_tokens.md

synapse/rest/admin/registration_tokens.py

synapse/rest/client/register.py

synapse/handlers/ui_auth/__init__.py

synapse/handlers/ui_auth/checkers.py

synapse/rest/admin/registration_tokens.py

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>

To get the flattened tests/rest/client directory of matrix-org#10667

anoadragon453

lgtm other than the outstanding comments!

docs/usage/administration/admin_api/registration_tokens.md

synapse/rest/admin/registration_tokens.py

Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453

Nice method of testing token generation collision.

This LGTM!

synapse/storage/databases/main/registration.py

Hard-coded token authenticated registration

5856f81

Signed-off-by: Callum Brown <callum@calcuode.com>

govynnus mentioned this pull request Jun 8, 2021

MSC3231: Token authenticated registration matrix-org/matrix-spec-proposals#3231

Merged

Create registration_tokens table

5f21580

Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453 reviewed Jun 16, 2021

View reviewed changes

govynnus added 9 commits June 17, 2021 10:38

Check in database to validate registration token

2b8726c

Signed-off-by: Callum Brown <callum@calcuode.com>

Increment completed when registration token used

5b1ec0b

Signed-off-by: Callum Brown <callum@calcuode.com>

Rename total_uses to uses_allowed

15e5769

Signed-off-by: Callum Brown <callum@calcuode.com>

Improve unit tests

9c502b0

Signed-off-by: Callum Brown <callum@calcuode.com>

Increment pending while registration in progress

e7754a9

Signed-off-by: Callum Brown <callum@calcuode.com>

Add unit test for registration token expiry

ef05a6d

Signed-off-by: Callum Brown <callum@calcuode.com>

Fix config file related bits

53f0e05

Signed-off-by: Callum Brown <callum@calcuode.com>

Run connected database ops in same transaction

7883191

Signed-off-by: Callum Brown <callum@calcuode.com>

Fix some formatting problems

1debc22

Signed-off-by: Callum Brown <callum@calcuode.com>

govynnus force-pushed the token-registration branch from aa8444e to 1debc22 Compare June 17, 2021 10:17

govynnus commented Jun 17, 2021

View reviewed changes

synapse/storage/databases/main/registration.py Outdated Show resolved Hide resolved

govynnus added 5 commits June 17, 2021 11:46

Test completed is empty when auth should fail

6ac376d

Signed-off-by: Callum Brown <callum@calcuode.com>

Override type of simple_select_one_txn

dfa8fec

Signed-off-by: Callum Brown <callum@calcuode.com>

Raise error if token changes during UIA

c89d786

Signed-off-by: Callum Brown <callum@calcuode.com>

Add validity checking endpoint

e7bd00a

Signed-off-by: Callum Brown <callum@calcuode.com>

Use AuthHandler methods for accessing UIA session

d6704fd

rather than using storage methods directly. Also use UIAuthSessionDataConstants. Signed-off-by: Callum Brown <callum@calcuode.com>

govynnus commented Jun 20, 2021

View reviewed changes

synapse/rest/client/v2_alpha/register.py Outdated Show resolved Hide resolved

govynnus added 3 commits June 29, 2021 14:34

Rate limit validity checking endpoint

003e67d

Signed-off-by: Callum Brown <callum@calcuode.com>

Use LoginError rather than SynapseError in checker

3c51680

So error fields are merged into UIA response. (Failures are due to authentication) Signed-off-by: Callum Brown <callum@calcuode.com>

Add fallback

af90be7

Signed-off-by: Callum Brown <callum@calcuode.com>

govynnus commented Jun 30, 2021

View reviewed changes

synapse/rest/client/v2_alpha/auth.py Outdated Show resolved Hide resolved

govynnus commented Jun 30, 2021

View reviewed changes

synapse/rest/client/v2_alpha/auth.py Outdated Show resolved Hide resolved

Docs for currently non-existent admin API

1552b70

Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453 reviewed Jul 12, 2021

View reviewed changes

synapse/rest/client/v2_alpha/auth.py Outdated Show resolved Hide resolved

synapse/rest/client/v2_alpha/auth.py Outdated Show resolved Hide resolved

dklimpel reviewed Jul 16, 2021

View reviewed changes

docs/SUMMARY.md Outdated Show resolved Hide resolved

anoadragon453 suggested changes Jul 19, 2021

View reviewed changes

govynnus marked this pull request as ready for review August 19, 2021 16:36

govynnus requested a review from anoadragon453 August 19, 2021 16:37

anoadragon453 suggested changes Aug 19, 2021

View reviewed changes

synapse/storage/databases/main/registration.py Outdated Show resolved Hide resolved

govynnus added 2 commits August 20, 2021 12:40

Move table creation sql to actual newest delta

c6bcae2

Signed-off-by: Callum Brown <callum@calcuode.com>

Avoid integrity error when creating tokens

01a74da

Check if token already exists before trying to create it Signed-off-by: Callum Brown <callum@calcuode.com>

anoadragon453 suggested changes Aug 20, 2021

View reviewed changes

govynnus and others added 5 commits August 20, 2021 13:27

Fix docs, comments and variable names

5bfc707

Signed-off-by: Callum Brown <callum@calcuode.com> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

Try again if generated token already exists

b5608c3

Signed-off-by: Callum Brown <callum@calcuode.com>

Let validity checking endpoint be used by workers

bf28876

Signed-off-by: Callum Brown <callum@calcuode.com>

Document usage of null when updating tokens

2e59dda

Signed-off-by: Callum Brown <callum@calcuode.com>

Merge remote-tracking branch 'upstream/develop' into token-registration

df0077d

To get the flattened tests/rest/client directory of matrix-org#10667

anoadragon453 reviewed Aug 20, 2021

View reviewed changes

docs/usage/administration/admin_api/registration_tokens.md Outdated Show resolved Hide resolved

synapse/rest/admin/registration_tokens.py Show resolved Hide resolved

govynnus added 2 commits August 21, 2021 12:00

Simplify retrying of token generation

54867ef

Signed-off-by: Callum Brown <callum@calcuode.com>

Small additions to admin api documentation

20b566c

Signed-off-by: Callum Brown <callum@calcuode.com>

govynnus mentioned this pull request Aug 21, 2021

Add support for registration token management JOJ0/synadm#42

Merged

anoadragon453 approved these changes Aug 21, 2021

View reviewed changes

synapse/storage/databases/main/registration.py Outdated Show resolved Hide resolved

Update synapse/storage/databases/main/registration.py

04b237a

anoadragon453 merged commit 947dbbd into matrix-org:develop Aug 21, 2021

govynnus deleted the token-registration branch August 22, 2021 13:18

Sleuth56 mentioned this pull request Sep 23, 2021

Feature: Add token based registration creation and management Awesome-Technologies/synapse-admin#194

Closed

DMRobertson mentioned this pull request Oct 13, 2021

Registration Token requirement can be bypassed by logging in with SSO #11067

Open

ShadowJonathan mentioned this pull request Jan 31, 2022

Stabilise MSC3231 (Token Based Registration) #11867

Merged

4 tasks

Uh oh!

Implement MSC3231: Token authenticated registration #10142

Implement MSC3231: Token authenticated registration #10142

Uh oh!

Conversation

govynnus commented Jun 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

govynnus commented Jun 8, 2021 •

edited

Loading