- Notifications
You must be signed in to change notification settings - Fork1k
fix: add database constraint to enforce minimum username length#19453
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
length and regex are already enforced at the application layer, but wehave some code paths with database queries where we could optimize awaymany of the DB query calls if we could be sure at the database levelthat the username is never an empty string.Signed-off-by: Callum Styan <callumstyan@gmail.com>
The failing tests are actually catching this! |
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE users | |||
| ADD CONSTRAINT users_username_min_length | |||
| CHECK (length(username) >= 1); | |||
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 wonder if we should expand this constraint to take the other clauses into account:
https://github.com/coder/coder/blob/main/codersdk/name.go#L41-L60
In any case, enforcing the minimum solves our immediate problem so I'm approving, but we might be overfitting.
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 don't think we need to duplicate the entire logic here.
dannykoppingAug 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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'm not advocating for duplicating so much as replacing the logic; should've been more explicit.
bcdade7 intomainUh oh!
There was an error while loading.Please reload this page.
Username length and format, via regex, are already enforced at the application layer, but we have some code paths with database queries where we could optimize away many of the DB query calls if we could be sure at the database level that the username is never an empty string.
For example:#19395