Create user with expiry #741
Conversation
negzi
commented
Apr 21, 2016
negzi
commented
- Add unit tests for client, api and handler
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Does this need to use an AS? AS's require a fair bit of config to set up, which isn't ideal if the only thing the other server needs to do is to create users. On the other hand, using an AS means that you can ensure the other server only creates users with a certain prefix. Could the other server register the user once, and then store a token? As the other server could simply add the time constraint to the macaroon/token itself each time it served the token to whatever used it. There is also the registration_shared_secret config option that might be another way of doing this; instead of requiring the other server be a registered AS, auth using the registration shared secret. |
| duration_seconds = 0 | ||
| try: | ||
| duration_seconds = int(user_json["duration_seconds"]) | ||
| except: |
There was a problem hiding this comment.
I prefer to never use plain except: and to always specify the exception type. Otherwise, its very easy for other exceptions to get swallowed and incorrectly handled, leading to a lot of confusion while debugging.
In this case I think it should be except ValueError:
No worries, sorry for the delay in review. Enjoy your trip! @matrixbot ok to test |
| # Sets the expiry for the short term user creation in | ||
| # milliseconds. For instance the bellow duration is two weeks | ||
| # in milliseconds. | ||
| user_creation_max_duration: 1209600000 |
There was a problem hiding this comment.
Might be worth calling the short_term_user_max_duration, just to spell out it only affects short term users?
There was a problem hiding this comment.
Oh, but it affects all users if expire_access_token is on?
|
This looks good now! Just need you to sign off the PR: https://github.com/matrix-org/synapse/blob/master/CONTRIBUTING.rst#sign-off. Feel free to add yourself to It might also be nice to separate out expiration of users created via |
- Add unittests for client, api and handler Signed-off-by: Negar Fazeli <[email protected]>
|
Thanks for the review Erik, I really appreciate it :) |
|
I couldn't find anything in the changelog. Did this add a functionality to automatically desable a user after a certain period? Or did it add a timeout only on user creation, if the user is not created within the timeout? |
