allow users to change their name

lib/Travelynx.pm

@@ -2548,6 +2548,7 @@ sub startup {
 	$authed_r->get('/fgr')->to('passengerrights#list_candidates');
 	$authed_r->get('/account/password')->to('account#password_form');
 	$authed_r->get('/account/mail')->to('account#change_mail');
+	$authed_r->get('/account/name')->to('account#change_name');
 	$authed_r->get('/export.json')->to('account#json_export');
 	$authed_r->get('/history.json')->to('traveling#json_history');
 	$authed_r->get('/history.csv')->to('traveling#csv_history');
@@ -2572,6 +2573,7 @@ sub startup {
 	  ->to('passengerrights#generate');
 	$authed_r->post('/account/password')->to('account#change_password');
 	$authed_r->post('/account/mail')->to('account#change_mail');
+	$authed_r->post('/account/name')->to('account#change_name');
 	$authed_r->post('/delete')->to('account#delete');
 	$authed_r->post('/logout')->to('account#do_logout');
 	$authed_r->post('/set_token')->to('api#set_token');

lib/Travelynx/Controller/Account.pm

@@ -468,6 +468,93 @@ sub change_mail {
 	}
 }
 
+sub change_name {
+	my ($self) = @_;
+
+	my $action   = $self->req->param('action');
+	my $password = $self->req->param('password');
+	my $old_name = $self->current_user->{name};
+	my $new_name = $self->req->param('name');
+
+	if ( $action and $action eq 'update_name' ) {
+		if ( $self->validation->csrf_protect->has_error('csrf_token') ) {
+			$self->render(
+				'change_name',
+				invalid => 'csrf',
+			);
+			return;
+		}
+
+		if ( not length($new_name) ) {
+			$self->render( 'change_name', invalid => 'user_empty' );
+			return;
+		}
+
+		if ( $new_name !~ m{ ^ [0-9a-zA-Z_-]+ $ }x ) {
+			$self->render( 'change_name', invalid => 'user_format' );
+			return;
+		}
+
+		if ( not $self->authenticate( $old_name, $self->param('password') ) ) {
+			$self->render( 'change_name', invalid => 'password' );
+			return;
+		}
+
+       # This call is technically superfluous. The users table has a unique
+       # constraint on the "name" column, so having two users with the same name
+       # is not possible. However, to minimize the number of failed SQL
+       # queries, we first do a select check here and only attempt an update
+       # if it succeeded.
+		if ( $self->users->check_if_user_name_exists( name => $new_name ) ) {
+			$self->render( 'change_name', invalid => 'user_collision' );
+			return;
+		}
+
+		my $success = $self->users->change_name(
+			uid  => $self->current_user->{id},
+			name => $new_name
+		);
+
+		if ( not $success ) {
+			$self->render( 'change_name', invalid => 'user_collision' );
+			return;
+		}
+
+		$self->flash( success => 'name' );
+		$self->redirect_to('account');
+
+		my $ip   = $self->req->headers->header('X-Forwarded-For');
+		my $ua   = $self->req->headers->user_agent;
+		my $date = DateTime->now( time_zone => 'Europe/Berlin' )
+		  ->strftime('%d.%m.%Y %H:%M:%S %z');
+
+		# In case Mojolicious is not running behind a reverse proxy
+		$ip
+		  //= sprintf( '%s:%s', $self->tx->remote_address,
+			$self->tx->remote_port );
+		my $confirm_url
+		  = $self->url_for('confirm_mail')->to_abs->scheme('https');
+		my $imprint_url = $self->url_for('impressum')->to_abs->scheme('https');
+
+		my $body = "Hallo ${new_name},\n\n";
+		$body
+		  .= "Der Name deines Travelynx-Accounts wurde erfolgreich geändert.\n";
+		$body .= "Alter Name: ${old_name}\n";
+		$body .= "Neue Name: ${new_name}\n\n";
+		$body .= "Daten zur Anfrage:\n";
+		$body .= " * Datum: ${date}\n";
+		$body .= " * Client: ${ip}\n";
+		$body .= " * UserAgent: ${ua}\n\n\n";
+		$body .= "Impressum: ${imprint_url}\n";
+
+		$self->sendmail->custom( $self->current_user->{email},
+			'travelynx: Name geändert', $body );
+	}
+	else {
+		$self->render('change_name');
+	}
+}
+
 sub password_form {
 	my ($self) = @_;
 

lib/Travelynx/Model/Users.pm

@@ -1,4 +1,5 @@
 package Travelynx::Model::Users;
+
 # Copyright (C) 2020 Daniel Friesel
 #
 # SPDX-License-Identifier: AGPL-3.0-or-later
@@ -16,9 +17,9 @@ sub new {
 }
 
 sub mark_seen {
-	my ($self, %opt) = @_;
+	my ( $self, %opt ) = @_;
 	my $uid = $opt{uid};
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db  = $opt{db} // $self->{pg}->db;
 
 	$db->update(
 		'users',
@@ -29,9 +30,9 @@ sub mark_seen {
 
 sub verify_registration_token {
 	my ( $self, %opt ) = @_;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $token = $opt{token};
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
 
 	my $tx = $db->begin;
 
@@ -55,8 +56,8 @@ sub verify_registration_token {
 
 sub get_uid_by_name_and_mail {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $name = $opt{name};
+	my $name  = $opt{name};
 	my $email = $opt{email};
 
 	my $res = $db->select(
@@ -77,7 +78,7 @@ sub get_uid_by_name_and_mail {
 
 sub get_privacy_by_name {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db   = $opt{db} // $self->{pg}->db;
 	my $name = $opt{name};
 
 	my $res = $db->select(
@@ -97,21 +98,17 @@ sub get_privacy_by_name {
 
 sub set_privacy {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db           = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid          = $opt{uid};
 	my $public_level = $opt{level};
 
-	$db->update(
+	$db->update( 'users', { public_level => $public_level }, { id => $uid } );
-		'users',
-		{ public_level => $public_level },
-		{ id           => $uid }
-	);
 }
 
 sub mark_for_password_reset {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $token = $opt{token};
 
 	my $res = $db->select(
@@ -126,10 +123,9 @@ sub mark_for_password_reset {
 	$db->insert(
 		'pending_passwords',
 		{
-			user_id => $uid,
+			user_id      => $uid,
-			token   => $token,
+			token        => $token,
-			requested_at =>
+			requested_at => DateTime->now( time_zone => 'Europe/Berlin' )
-				DateTime->now( time_zone => 'Europe/Berlin' )
 		}
 	);
 
@@ -138,8 +134,8 @@ sub mark_for_password_reset {
 
 sub verify_password_token {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $token = $opt{token};
 
 	my $res = $db->select(
@@ -159,19 +155,18 @@ sub verify_password_token {
 
 sub mark_for_mail_change {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $email = $opt{email};
 	my $token = $opt{token};
 
 	$db->insert(
 		'pending_mails',
 		{
-			user_id => $uid,
+			user_id      => $uid,
-			email   => $email,
+			email        => $email,
-			token   => $token,
+			token        => $token,
-			requested_at =>
+			requested_at => DateTime->now( time_zone => 'Europe/Berlin' )
-				DateTime->now( time_zone => 'Europe/Berlin' )
 		},
 		{
 			on_conflict => \
@@ -182,8 +177,8 @@ sub mark_for_mail_change {
 
 sub change_mail_with_token {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $token = $opt{token};
 
 	my $tx = $db->begin;
@@ -198,11 +193,7 @@ sub change_mail_with_token {
 	)->hash;
 
 	if ($res_h) {
-		$db->update(
+		$db->update( 'users', { email => $res_h->{email} }, { id => $uid } );
-			'users',
-			{ email => $res_h->{email} },
-			{ id    => $uid }
-		);
 		$db->delete( 'pending_mails', { user_id => $uid } );
 		$tx->commit;
 		return 1;
@@ -210,10 +201,24 @@ sub change_mail_with_token {
 	return;
 }
 
+sub change_name {
+	my ( $self, %opt ) = @_;
+	my $db  = $opt{db} // $self->{pg}->db;
+	my $uid = $opt{uid};
+
+	eval { $db->update( 'users', { name => $opt{name} }, { id => $uid } ); };
+
+	if ($@) {
+		return 0;
+	}
+
+	return 1;
+}
+
 sub remove_password_token {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $token = $opt{token};
 
 	$db->delete(
@@ -226,16 +231,16 @@ sub remove_password_token {
 }
 
 sub get_data {
-	my ($self, %opt) = @_;
+	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db  = $opt{db} // $self->{pg}->db;
 	my $uid = $opt{uid};
 
 	my $user = $db->select(
 		'users',
 		'id, name, status, public_level, email, '
-			. 'extract(epoch from registered_at) as registered_at_ts, '
+		  . 'extract(epoch from registered_at) as registered_at_ts, '
-			. 'extract(epoch from last_seen) as last_seen_ts, '
+		  . 'extract(epoch from last_seen) as last_seen_ts, '
-			. 'extract(epoch from deletion_requested) as deletion_requested_ts',
+		  . 'extract(epoch from deletion_requested) as deletion_requested_ts',
 		{ id => $uid }
 	)->hash;
 	if ($user) {
@@ -257,7 +262,7 @@ sub get_data {
 			? DateTime->from_epoch(
 				epoch     => $user->{deletion_requested_ts},
 				time_zone => 'Europe/Berlin'
-				)
+			  )
 			: undef,
 		};
 	}
@@ -266,7 +271,7 @@ sub get_data {
 
 sub get_login_data {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db   = $opt{db} // $self->{pg}->db;
 	my $name = $opt{name};
 
 	my $res_h = $db->select(
@@ -280,11 +285,11 @@ sub get_login_data {
 
 sub add_user {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db        = $opt{db} // $self->{pg}->db;
 	my $user_name = $opt{name};
-	my $email = $opt{email};
+	my $email     = $opt{email};
-	my $token = $opt{token};
+	my $token     = $opt{token};
-	my $password = $opt{password_hash};
+	my $password  = $opt{password_hash};
 
 	# This helper must be called during a transaction, as user creation
 	# may fail even after the database entry has been generated, e.g.  if
@@ -322,7 +327,7 @@ sub add_user {
 
 sub flag_deletion {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db  = $opt{db} // $self->{pg}->db;
 	my $uid = $opt{uid};
 
 	my $now = DateTime->now( time_zone => 'Europe/Berlin' );
@@ -338,7 +343,7 @@ sub flag_deletion {
 
 sub unflag_deletion {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db  = $opt{db} // $self->{pg}->db;
 	my $uid = $opt{uid};
 
 	$db->update(
@@ -354,27 +359,21 @@ sub unflag_deletion {
 
 sub set_password_hash {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db       = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid      = $opt{uid};
 	my $password = $opt{password_hash};
 
-	$db->update(
+	$db->update( 'users', { password => $password }, { id => $uid } );
-		'users',
-		{ password => $password },
-		{ id       => $uid }
-	);
 }
 
 sub check_if_user_name_exists {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db        = $opt{db} // $self->{pg}->db;
 	my $user_name = $opt{name};
 
-	my $count = $db->select(
+	my $count
-		'users',
+	  = $db->select( 'users', 'count(*) as count', { name => $user_name } )
-		'count(*) as count',
+	  ->hash->{count};
-		{ name => $user_name }
-	)->hash->{count};
 
 	if ($count) {
 		return 1;
@@ -384,7 +383,7 @@ sub check_if_user_name_exists {
 
 sub check_if_mail_is_blacklisted {
 	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db   = $opt{db} // $self->{pg}->db;
 	my $mail = $opt{email};
 
 	my $count = $db->select(
@@ -416,21 +415,17 @@ sub check_if_mail_is_blacklisted {
 }
 
 sub use_history {
-	my ($self, %opt) = @_;
+	my ( $self, %opt ) = @_;
-	my $db = $opt{db} // $self->{pg}->db;
+	my $db    = $opt{db} // $self->{pg}->db;
-	my $uid = $opt{uid};
+	my $uid   = $opt{uid};
 	my $value = $opt{set};
 
 	if ($value) {
-		$db->update(
+		$db->update( 'users', { use_history => $value }, { id => $uid } );
-			'users',
-			{ use_history => $value },
-			{ id          => $uid }
-		);
 	}
 	else {
-		return $db->select( 'users', ['use_history'],
+		return $db->select( 'users', ['use_history'], { id => $uid } )
-			{ id => $uid } )->hash->{use_history};
+		  ->hash->{use_history};
 	}
 }
 

templates/account.html.ep

@@ -7,7 +7,10 @@
 		<div class="col s12">
 			<div class="card success-color">
 				<div class="card-content white-text">
-					% if ($success eq 'mail') {
+					% if ($success eq 'name') {
+						<span class="card-title">Name geändert</span>
+					% }
+					% elsif ($success eq 'mail') {
 						<span class="card-title">Mail-Adresse geändert</span>
 					% }
 					% elsif ($success eq 'password') {
@@ -41,7 +44,7 @@
 		<table class="striped">
 			<tr>
 				<th scope="row">Name</th>
-				<td><%= $acc->{name} %></td>
+				<td><a href="/account/name"><i class="material-icons">edit</i></a><%= $acc->{name} %></td>
 			</tr>
 			<tr>
 				<th scope="row">Mail</th>