-
-
Notifications
You must be signed in to change notification settings - Fork 33
doveauth: add invite_tokens to restrict address creation #600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Hm -.- right now, when I run but when I copy the very same URL into Delta Chat's "create new profile > use other server > scan invitation code", it tries to login with a passphrase which doesn't contain |
So server-side everything works, just client-side it doesn't. |
With 2.x.x it works :) So we can merge this now. |
Awesome, can't wait for the merge! Good job! |
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.
-
Could you add docs to the README to tell how to use this invite token? I am not sure yet i understand how this is overall supposed to be used from admins and users.
-
I am dubious about nocreate override -- i think nocreate should remain absolute and block all account creation (if your server is overwhelmed with creation requests, you first want to stop it, and then analyze what happens -- might be a leaked invite token).
chatmaild/src/chatmaild/doveauth.py
Outdated
|
||
if len(cleartext_password) < config.password_min_length: | ||
if ( | ||
len(cleartext_password.replace(config.invite_token, "")) |
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.
this can randomly fail depending on the invite_token (might be short)
I think there should be a fixed "{invite_token}:password" format, and then do a more specific check.
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.
I'd leave out the :
, as it could be part of the password; but yes, the invite_token needs to be at the beginning of the password now, that is what newemail.py generates anyway.
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.
if someone makes eg a 4-char invite code there might still be random passwords that match it, i am afraid.
00afbb4
to
887b647
Compare
Do we want it to be easy to find? Personally I'd leave it out of the README.md and point anyone who really asks for it to this PR description instead. The chatmail ecosystem will be much more resilient and anonymous if people can generally sign up on relays of people they don't know. I get why some people need a fence, so I built a fence; but we want to have a public park, not a Kleingartenverein. |
if invite codes are set, then normal QR codes without an invite token (like the one on the web page) don't work anymore, right? Also, the invite code maybe needs have a minimum length of 9 or so because otherwise someone can brute force account creation (invite tokens with 4 characters for example are probably easy to crack in a few days, even without showing up in monitoring, it's just ~1M combinations, can be tried with one password per second, and in ten days you have the invite token). Given that the recommended UX flow is the QR code / link, it doesn't matter that it becomes harder to type manually by requiring long invite codes. Besides, chatmail.ini so far contains no secrets and can be public in some repo even -- with the invite codes it's more sensitive. Lastly, i am still grappling with the overall idea and the implications of invite links. Not 100% convinced yet. |
1547e0d
to
fe81f0b
Compare
…ginning of password
2123d27
to
848ecd1
Compare
848ecd1
to
4a7b425
Compare
Done :)
True, I added a recommendation on token length to the README and the PR description. Enforcing it would add complexity for showing error messages; let's hope admins can read and choose risks.
True, but I don't think it will become a problem. So far I haven't seen a chatmail.ini in a public repository - and if you have public sysadmin docs, you anyway need to think about how to avoid committing secrets. I'd say, if someone asks, we can allow setting invite tokens per environment variable.
Same tbh. This PR is fun to implement though :D and I think the compromise of implementing it but hiding the documentation in a GitHub PR is good enough, as some people seem to need this feature. Then again - once it's in the world, it's hard to put back in the box :/ |
fix #594
To restrict address creation for anyone who doesn't have the invite link/QR code:
invite_token
option to add one or more tokens of your choice tochatmail.ini
:invite_token = s3cr3t privil3g3
scripts/cmdeploy run
dcaccount
invite link/QR code (like the one on your web page) with one of your invite tokens added at the end, for example:dcaccount:https://example.org/new?s3cr3t
To try it out:
dcaccount:https://c17.testrun.org/new?asdf
(Right now broken client-side because of chatmail/core#6988)