Limit the number of users on an instance #30236
Conversation
techknowlogick
added
the
type/feature
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/S
github-actions
bot
added
modifies/go
| @@ -729,6 +734,15 @@ func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefa | |||
| return committer.Commit() | |||
| } | |||
|
|
|||
| func HitCreationLimit(ctx context.Context) bool { | |||
delvh
changed the title
Co-authored-by: delvh <dev.lh@web.de>
pull-request-size
bot
added
size/M
I mean, what more do you want to document about it? But the tests would be nice, to ensure it doesn't just break at one point. |
| func HitCreationLimit(ctx context.Context) bool { | ||
| // don't bother calling DB if limit not set | ||
| if setting.Service.MaxUserCreationLimit < 0 || | ||
| int64(setting.Service.MaxUserCreationLimit) < CountUsers(ctx, &CountUserFilter{}) { |
There was a problem hiding this comment.
Shouldn't it be a concurrent limit? Deactivated users should not be counted.
There was a problem hiding this comment.
However, doing this means we also need to keep the user count in mind when re-activating a user, and the CountUsers must ignore deactivated users.
There was a problem hiding this comment.
I think this feature should be similar to "you have 10 user licenses". Usually if someone leaves, you just deactivate the account and get 1 license back. This would not be possible with the current code.
| func HitCreationLimit(ctx context.Context) bool { | ||
| // don't bother calling DB if limit not set | ||
| if setting.Service.MaxUserCreationLimit < 0 || | ||
| int64(setting.Service.MaxUserCreationLimit) < CountUsers(ctx, &CountUserFilter{}) { |
There was a problem hiding this comment.
I think this feature should be similar to "you have 10 user licenses". Usually if someone leaves, you just deactivate the account and get 1 license back. This would not be possible with the current code.
| @@ -717,6 +717,13 @@ func ActivatePost(ctx *context.Context) { | |||
| return | |||
| } | |||
|
|
|||
| // Check the number of users already in the system, and if there are too many forbid any new ones. | |||
| if user_model.HitCreationLimit(ctx) { | |||
There was a problem hiding this comment.
If the user is created auto-active there is no limit?
Fix #29975
WIP as it should have tests & am unsure if there is enough documentation