feat: implements allow_new_users setting#2304
Conversation
HorlogeSkynet
force-pushed
the
feat/allow_new_users
branch
from
a17eef7 to
c7ab2fe
Compare
|
@HorlogeSkynet Thank you for the patch. I just tried running the tests and there is a failure. See comments above. |
HorlogeSkynet
force-pushed
the
feat/allow_new_users
branch
from
c7ab2fe to
40a5ca6
Compare
|
Any news @chenba ? 🙂 |
HorlogeSkynet
force-pushed
the
feat/allow_new_users
branch
from
40a5ca6 to
76e70ad
Compare
I'm sorry. I got distracted/busy. I should have time to take a look today. Thanks for the reminder. |
chenba
left a comment
chenba
left a comment
There was a problem hiding this comment.
Got a couple comments wrt error handling.
| kind, | ||
| status: StatusCode::NOT_FOUND, | ||
| backtrace: Box::new(Backtrace::new_unresolved()), | ||
| }, |
There was a problem hiding this comment.
When impl From<DbError> for TokenserverError converts an error of this type we'd end up with a HTTP 503. That's misleading for the clients. (We don't want them to retry at the very least.) Whatever error we add here for the feature should become a HTTP 403.
There was a problem hiding this comment.
By explicitly matching DbErrorKind::UserNotCreated kind in this context, I guess we prevent _ from converting it to a HTTP 503 by default ?
I've updated the pattern matching to throw a 403 FORBIDDEN as you proposed. Tell me whether it suits what you had in mind.
HorlogeSkynet
force-pushed
the
feat/allow_new_users
branch
from
76e70ad to
8b1da2e
Compare
2 participants
Description
New
allow_new_userssetting, useful for self-hosted nodes.Testing
Set
SYNC_TOKENSERVER__ALLOW_NEW_USERStofalseand no new users allocation should occur.Issue(s)
Closes #1429.