Add global error handler, and begin support for display names

bin/migrate6.pl

@@ -0,0 +1,21 @@
+#!/usr/bin/env perl
+
+# Display names
+
+use strict;
+use warnings;
+
+use FindBin;
+
+use lib "$FindBin::Bin/../lib";
+
+use Trog::SQLite;
+sub _dbh {
+    my $file   = 'schema/auth.schema';
+    my $dbname = "config/auth.db";
+    return Trog::SQLite::dbh($file,$dbname);
+}
+
+my $dbh = _dbh();
+
+$dbh->do("ALTER TABLE user ADD COLUMN display_name TEXT DEFAULT NULL;");

bin/tcms-useradd

@@ -7,12 +7,9 @@ use FindBin::libs;
 
 use Trog::Auth;
 
-my ($user, $pass, $contactemail) = @ARGV;
+my ($user, $display_name, $pass, $contactemail) = @ARGV;
 
 # TODO better arg handling, etc
-die "User must be first arg" unless $user;
-die "Password must be second arg" unless $pass;
-die "contact email must be third arg" unless $contactemail;
 
 Trog::Auth::killsession($user);
-Trog::Auth::useradd($user, $pass, ['admin'], $contactemail);
+Trog::Auth::useradd($user, $display_name, $pass, ['admin'], $contactemail);

lib/TCMS.pm

@@ -62,6 +62,21 @@ my %aliases = $data->aliases();
 # This should eventually be pre-filled from DB.
 my %etags;
 
+# Wrap app to return *our* error handler instead of Plack::Util::run_app's
+my $cur_query = {};
+sub app {
+    return eval { _app(@_) } || do {
+		my $env = shift;
+        $env->{'psgi.errors'}->print($@);
+
+		# Redact the stack trace past line 1, it usually has things which should not be shown
+		$cur_query->{message} = $@;
+		$cur_query->{message} =~ s/\n.*//g if $cur_query->{message};
+
+		return _error($cur_query);
+    };
+}
+
 =head2 app()
 
 Dispatches requests based on %routes built above.
@@ -72,7 +87,7 @@ If a path passed is not a defined route (or regex route), but exists as a file u
 
 =cut
 
-sub app {
+sub _app {
 
     # Start the server timing clock
     my $start = [gettimeofday];
@@ -94,10 +109,14 @@ sub app {
     my $port   = $env->{HTTP_X_FORWARDED_PORT} // $env->{HTTP_PORT};
     my $pport  = defined $port ? ":$port" : "";
     my $scheme = $env->{'psgi.url_scheme'} // 'http';
+    my $method = $env->{REQUEST_METHOD};
 
     # It's important that we log what the user ACTUALLY requested rather than the rewritten path later on.
     my $fullpath = "$scheme://$domain$pport$path";
 
+    # sigdie can now "do the right thing"
+    $cur_query = { route => $path, fullpath => $path, method => $method };
+
     # Check eTags.  If we don't know about it, just assume it's good and lazily fill the cache
     # XXX yes, this allows cache poisoning...but only for logged in users!
     if ( $env->{HTTP_IF_NONE_MATCH} ) {
@@ -191,7 +210,7 @@ sub app {
     @{ $query->{acls} } = grep { $_ ne 'admin' } @{ $query->{acls} } unless $is_admin;
 
     # Ensure any short-circuit routes can log the request
-    $query->{method} = $env->{REQUEST_METHOD};
+    $query->{method} = $method;
     $query->{route}  = $path;
 
     # Disallow any paths that are naughty ( starman auto-removes .. up-traversal)
@@ -323,6 +342,7 @@ sub _generic ( $type, $query ) {
         forbidden  => \&Trog::Routes::HTML::forbidden,
         badrequest => \&Trog::Routes::HTML::badrequest,
         toolong    => \&Trog::Routes::HTML::toolong,
+        error      => \&Trog::Routes::HTML::error,
     );
     return $lookup{$type}->($query);
 }
@@ -347,6 +367,11 @@ sub _toolong($query) {
     return _generic( 'toolong', {} );
 }
 
+sub _error($query) {
+    INFO("$query->{method} 500 $query->{fullpath}");
+    return _generic( 'error', $query );
+}
+
 sub _static ( $fullpath, $path, $start, $streaming, $last_fetch = 0 ) {
 
     DEBUG("Rendering static for $path");

lib/Trog/Auth.pm

@@ -122,6 +122,14 @@ sub totp ( $user, $domain ) {
     }
     $failure = -1 if $secret;
 
+    # Generate a new secret if needed
+    my $secret_is_generated = 0;
+    if (!$secret) {
+        $secret_is_generated = 1;
+        $totp->valid_secret();
+        $secret = $totp->secret();
+    }
+
     my $uri = $totp->generate_otp(
         user   => "$user\@$domain",
         issuer => $domain,
@@ -129,18 +137,14 @@ sub totp ( $user, $domain ) {
         #XXX verifier apps will only do 30s :(
         period => 30,
         digits => 6,
-        $secret ? ( secret => $secret ) : (),
+        secret => $secret,
     );
 
     my $qr = "$user\@$domain.bmp";
-    if ( !$secret ) {
+    if ( $secret_is_generated ) {
         # Liquidate the QR code if it's already there
         unlink "totp/$qr" if -f "totp/$qr";
 
-        # Generate a new secret
-        $totp->valid_secret();
-
-        $secret = $totp->secret();
         $dbh->do( "UPDATE user SET totp_secret=? WHERE name=?", undef, $secret, $user ) or return ( undef, undef, 1, "Failed to store TOTP secret." );
     }
 
@@ -164,10 +168,7 @@ sub totp ( $user, $domain ) {
 sub _totp {
     state $totp;
     if ( !$totp ) {
-        my $cfg           = Trog::Config->get();
-        my $global_secret = $cfg->param('totp.secret');
-        die "Global secret must be set in tCMS configuration totp section!" unless $global_secret;
-        $totp = Authen::TOTP->new( secret => $global_secret );
+        $totp = Authen::TOTP->new();
         die "Cannot instantiate TOTP client!" unless $totp;
         $totp->{DEBUG} = 1 if is_debug();
     }
@@ -277,8 +278,10 @@ Returns True or False (likely false when user already exists).
 
 =cut
 
-sub useradd ( $user, $pass, $acls, $contactemail ) {
+sub useradd ( $user, $displayname, $pass, $acls, $contactemail ) {
 	die "No username set!" unless $user;
+    die "No display name set!" unless $displayname;
+    die "Username and display name cannot be the same" if $user eq $displayname;
 	die "No password set for user!" unless $pass;
 	die "ACLs must be array" unless is_arrayref($acls);
 	die "No contact email set for user!" unless $contactemail;
@@ -286,7 +289,7 @@ sub useradd ( $user, $pass, $acls, $contactemail ) {
     my $dbh  = _dbh();
     my $salt = create_uuid();
     my $hash = sha256( $pass . $salt );
-    my $res  = $dbh->do( "INSERT OR REPLACE INTO user (name,salt,hash,contact_email) VALUES (?,?,?,?)", undef, $user, $salt, $hash, $contactemail );
+    my $res  = $dbh->do( "INSERT OR REPLACE INTO user (name, display_name, salt,hash,contact_email) VALUES (?,?,?,?,?)", undef, $user, $displayname, $salt, $hash, $contactemail );
     return unless $res && ref $acls eq 'ARRAY';
 
     #XXX this is clearly not normalized with an ACL mapping table, will be an issue with large number of users
@@ -304,10 +307,11 @@ sub add_change_request ( %args ) {
 
 sub process_change_request ( $token ) {
     my $dbh  = _dbh();
-    my $rows = $dbh->selectall_arrayref( "SELECT username, type, secret, contact_email FROM change_request_full WHERE processed=0 AND token=?", { Slice => {} }, $token );
+    my $rows = $dbh->selectall_arrayref( "SELECT username, display_name, type, secret, contact_email FROM change_request_full WHERE processed=0 AND token=?", { Slice => {} }, $token );
     return 0 unless ref $rows eq 'ARRAY' && @$rows;
 
     my $user = $rows->[0]{username};
+    my $display = $rows->[0]{display_name};
     my $type = $rows->[0]{type};
     my $secret = $rows->[0]{secret};
     my $contactemail = $rows->[0]{contact_email};
@@ -318,8 +322,8 @@ sub process_change_request ( $token ) {
 			#XXX The fact that this is an INSERT OR REPLACE means all the entries in change_request for this user will get cascade wiped.  Which is good, as the secrets aren't salted.
 			# This is also why we have to snag the user's ACLs or they will be wiped.
 			my @acls = acls4user($user);
-            useradd( $user, $pass, \@acls, $contactemail ) or do {
-               return ''; 
+            useradd( $user, $display, $pass, \@acls, $contactemail ) or do {
+               return '';
             };
             killsession($user);
             return "Password set to $pass for $user";

lib/Trog/Routes/HTML.pm

@@ -398,6 +398,10 @@ sub toolong (@args) {
     return _generic_route( 'toolong', 419, "URI too long", @args );
 }
 
+sub error (@args) {
+    return _generic_route( 'error', 500, "Internal Server Error", @args);
+}
+
 =head2 redirect, redirect_permanent, see_also
 
 Redirects to the provided page.
@@ -500,7 +504,7 @@ sub login ($query) {
     if ( $query->{username} && $query->{password} ) {
         if ( !$hasusers ) {
             # Make the first user
-            Trog::Auth::useradd( $query->{username}, $query->{password}, ['admin'], $query->{contact_email} );
+            Trog::Auth::useradd( $query->{username}, $query->{display_name}, $query->{password}, ['admin'], $query->{contact_email} );
 
             # Add a stub user page and the initial series.
             my $dat = Trog::Data->new($conf);
@@ -873,7 +877,7 @@ sub profile ($query) {
 	#TODO allow username changes
     if ( $query->{password} || $query->{contact_email} ) {
 		my @acls = Trog::Auth::acls4user($query->{username}) || qw{admin};
-        Trog::Auth::useradd( $query->{username}, $query->{password}, \@acls, $query->{contact_email} );
+        Trog::Auth::useradd( $query->{username}, $query->{display_name}, $query->{password}, \@acls, $query->{contact_email} );
     }
 
     #Make sure it is "self-authored", redact pw

schema/auth.schema

@@ -1,5 +1,6 @@
 CREATE TABLE IF NOT EXISTS user (
     name TEXT NOT NULL UNIQUE,
+    display_name TEXT NOT NULL UNIQUE,
     salt TEXT NOT NULL,
     hash TEXT NOT NULL,
     totp_secret TEXT DEFAULT NULL,

www/templates/html/components/config.tx

@@ -23,10 +23,6 @@ If for example, you use mysql it will have to rely on either a local server, val
     </select>
     </div>
     <div>
-    Global TOTP Secret:
-    <input class="cooltext" type="text" name="totp_secret" value="<: $totp_secret :>" />
-    </div>
-    <div>
     Allow Embeds from:
     <input class="cooltext" type="textarea" name="embeds" value="<: $embeds :>" />
     </div>

www/templates/html/components/error.tx

@@ -0,0 +1,3 @@
+<h1>500 Internal Server Error</h1>
+<br /><br />
+<: $message :>

www/templates/html/components/forms/profile.tx

@@ -25,6 +25,7 @@
     <form class="Submissions" action="/profile" method="POST" enctype="multipart/form-data">
         Username *<br /><input required class="cooltext" type="text" name="username" placeholder="AzureDiamond" value="<: $post.user :>" />
         Password *<br /><input <: $post.user ? '' : 'required' :> class="cooltext" type="password" name="password" placeholder="hunter2" />
+        Display Name *<br /><input  <: $post.user ? '' : 'required' :> class="cooltext" type="text" name="display_name" placeholder="Mr. President" />
         Contact Email *<br /><input <: $post.user ? '' : 'required' :> class="cooltext" type="text" name="contact_email" placeholder="test@test.test" />
         Avatar *<br /><input class="cooltext" type="file" name="preview_file" />
         : if ( $post.preview ) {

www/templates/html/login.tx

@@ -33,6 +33,12 @@
       </div>
       <br />
 : } else {
+      Display Name:<br />
+      <div class="input-group">
+        <label for="display_name">📛</label>
+        <input required name="display_name" id="contact_email" placeholder="Mr. President" value="" type="text"></input>
+      </div>
+      <br />
       Contact Email:<br />
       <div class="input-group">
         <label for="contact_email">📧</label>