move user name validation to Users model

This commit is contained in:
Daniel Friesel 2021-06-13 08:48:08 +02:00
parent bccdefbb7d
commit 1803a1723c
2 changed files with 34 additions and 29 deletions

View file

@ -85,8 +85,8 @@ sub register {
return; return;
} }
if ( not length($user) ) { if ( my $error = $self->users->is_name_invalid( name => $user ) ) {
$self->render( 'register', invalid => 'user_empty' ); $self->render( 'register', invalid => $error );
return; return;
} }
@ -95,16 +95,6 @@ sub register {
return; return;
} }
if ( $user !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) {
$self->render( 'register', invalid => 'user_format' );
return;
}
if ( $self->users->user_name_exists( name => $user ) ) {
$self->render( 'register', invalid => 'user_collision' );
return;
}
if ( $self->users->mail_is_blacklisted( email => $email ) ) { if ( $self->users->mail_is_blacklisted( email => $email ) ) {
$self->render( 'register', invalid => 'mail_blacklisted' ); $self->render( 'register', invalid => 'mail_blacklisted' );
return; return;
@ -485,13 +475,8 @@ sub change_name {
return; return;
} }
if ( not length($new_name) ) { if ( my $error = $self->users->is_name_invalid( name => $new_name ) ) {
$self->render( 'change_name', invalid => 'user_empty' ); $self->render( 'change_name', invalid => $error );
return;
}
if ( $new_name !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) {
$self->render( 'change_name', invalid => 'user_format' );
return; return;
} }
@ -500,16 +485,10 @@ sub change_name {
return; return;
} }
# This call is technically superfluous. The users table has a unique # The users table has a unique constraint on the "name" column, so having
# constraint on the "name" column, so having two users with the same name # two users with the same name is not possible. The race condition
# is not possible. However, to minimize the number of failed SQL # between the user_name_exists check in is_name_invalid and this
# queries, we first do a select check here and only attempt an update # change_name call is harmless.
# if it succeeded.
if ( $self->users->user_name_exists( name => $new_name ) ) {
$self->render( 'change_name', invalid => 'user_collision' );
return;
}
my $success = $self->users->change_name( my $success = $self->users->change_name(
uid => $self->current_user->{id}, uid => $self->current_user->{id},
name => $new_name name => $new_name

View file

@ -201,6 +201,32 @@ sub change_mail_with_token {
return; return;
} }
sub is_name_invalid {
my ( $self, %opt ) = @_;
my $db = $opt{db} // $self->{pg}->db;
my $name = $opt{name};
if ( not length($name) ) {
return 'user_empty';
}
if ( $name !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) {
return 'user_format';
}
if (
$self->user_name_exists(
db => $db,
name => $name
)
)
{
return 'user_collision';
}
return;
}
sub change_name { sub change_name {
my ( $self, %opt ) = @_; my ( $self, %opt ) = @_;
my $db = $opt{db} // $self->{pg}->db; my $db = $opt{db} // $self->{pg}->db;