Allow users to edit their own settings

Also, don't use the flash anymore for going back to the referer.

src/lib/Hydra/Controller/API.pm

@@ -24,16 +24,6 @@ sub api : Chained('/') PathPart('api') CaptureArgs(0) {
 }
 
 
-sub end : ActionClass('RenderView') {
-    my ($self, $c) = @_;
-    if (scalar @{$c->error}) {
-        $c->stash->{json}->{error} = join "\n", @{$c->error};
-        $c->forward('View::JSON');
-        $c->clear_errors;
-    }
-}
-
-
 sub projectToHash {
     my ($project) = @_;
     return {
@@ -317,8 +307,6 @@ sub push : Chained('api') PathPart('push') Args(0) {
             , where => \ [ 'exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value = ?)', [ 'value', $r ] ]
             });
     }
-
-    $c->forward('View::JSON');
 }
 
 
@@ -326,19 +314,17 @@ sub push_github : Chained('api') PathPart('push-github') Args(0) {
     my ($self, $c) = @_;
 
     $c->{stash}->{json}->{jobsetsTriggered} = [];
-    
+
     my $in = decode_json $c->req->body_params->{payload};
     my $owner = $in->{repository}->{owner}->{name} or die;
     my $repo = $in->{repository}->{name} or die;
     print STDERR "got push from GitHub repository $owner/$repo\n";
-    
+
     triggerJobset($self, $c, $_) foreach $c->model('DB::Jobsets')->search(
         { 'project.enabled' => 1, 'me.enabled' => 1 },
         { join => 'project'
         , where => \ [ 'exists (select 1 from JobsetInputAlts where project = me.project and jobset = me.name and value like ?)', [ 'value', "%github.com%/$owner/$repo.git%" ] ]
         });
-
-    $c->forward('View::JSON');
 }
 
 

src/lib/Hydra/Controller/Admin.pm

@@ -8,11 +8,6 @@ use Hydra::Helper::CatalystUtils;
 use Hydra::Helper::AddBuilds;
 use Data::Dump qw(dump);
 use Digest::SHA1 qw(sha1_hex);
-use Crypt::RandPasswd;
-use Sys::Hostname::Long;
-use Email::Simple;
-use Email::Sender::Simple qw(sendmail);
-use Email::Sender::Transport::SMTP;
 use Config::General;
 
 
@@ -52,146 +47,13 @@ sub admin : Chained('/') PathPart('admin') CaptureArgs(0) {
 }
 
 
-sub updateUser {
-    my ($c, $user) = @_;
-
-    my $username     = trim $c->request->params->{"username"};
-    my $fullname     = trim $c->request->params->{"fullname"};
-    my $emailaddress = trim $c->request->params->{"emailaddress"};
-    my $emailonerror = trim $c->request->params->{"emailonerror"};
-    my $roles         = $c->request->params->{"roles"} ;
-
-    $user->update(
-        { fullname => $fullname
-        , emailaddress => $emailaddress
-        , emailonerror => $emailonerror
-        });
-    $user->userroles->delete_all;
-    if(ref($roles) eq 'ARRAY') {
-        for my $s (@$roles) {
-            $user->userroles->create({ role => $s}) ;
-        }
-    } else {
-        $user->userroles->create({ role => $roles}) if defined $roles ;
-    }
-}
-
-
-sub create_user : Chained('admin') PathPart('create-user') Args(0) {
-    my ($self, $c) = @_;
-
-    requireAdmin($c);
-
-    $c->stash->{template} = 'user.tt';
-    $c->stash->{edit} = 1;
-    $c->stash->{create} = 1;
-}
-
-
-sub create_user_submit : Chained('admin') PathPart('create-user/submit') Args(0) {
-    my ($self, $c) = @_;
-
-    my $username = trim $c->request->params->{username};
-
-    txn_do($c->model('DB')->schema, sub {
-        my $user = $c->model('DB::Users')->create(
-            {username => $username, emailaddress => "", password => ""});
-        updateUser($c, $user);
-    });
-
-    $c->res->redirect("/admin/users");
-}
-
-
-sub user : Chained('admin') PathPart('user') CaptureArgs(1) {
-    my ($self, $c, $username) = @_;
-
-    requireAdmin($c);
-
-    my $user = $c->model('DB::Users')->find($username)
-        or notFound($c, "User $username doesn't exist.");
-
-    $c->stash->{user} = $user;
-}
-
-
 sub users : Chained('admin') PathPart('users') Args(0) {
     my ($self, $c) = @_;
     $c->stash->{users} = [$c->model('DB::Users')->search({}, {order_by => "username"})];
-
     $c->stash->{template} = 'users.tt';
 }
 
 
-sub user_edit : Chained('user') PathPart('edit') Args(0) {
-    my ($self, $c) = @_;
-
-    $c->stash->{template} = 'user.tt';
-    $c->stash->{edit} = 1;
-}
-
-
-sub user_edit_submit : Chained('user') PathPart('submit') Args(0) {
-    my ($self, $c) = @_;
-    requirePost($c);
-
-    txn_do($c->model('DB')->schema, sub {
-        if (($c->request->params->{submit} || "") eq "delete") {
-            $c->stash->{user}->delete;
-        } else {
-            updateUser($c, $c->stash->{user});
-        }
-    });
-
-    $c->res->redirect("/admin/users");
-}
-
-
-sub sendemail {
-   my ($to, $subject, $body) = @_;
-
-   my $url = hostname_long;
-   my $sender = ($ENV{'USER'} || "hydra") .  "@" . $url;
-
-   my $email = Email::Simple->create(
-        header => [
-            To      => $to,
-            From    => "Hydra <$sender>",
-            Subject => $subject
-        ],
-        body => $body
-    );
-
-    sendmail($email);
-}
-
-
-sub reset_password : Chained('user') PathPart('reset-password') Args(0) {
-    my ($self, $c) = @_;
-
-    # generate password
-    my $password = Crypt::RandPasswd->word(8,10);
-
-    # calculate hash
-    my $hashed = sha1_hex($password);
-
-    $c->stash->{user}-> update({ password => $hashed}) ;
-
-    # send email
-
-    sendemail(
-        $c->stash->{user}->emailaddress,
-        "New password for Hydra",
-        "Hi,\n\n".
-        "Your password has been reset. Your new password is '$password'.\n".
-        "You can change your password at " . $c->config()->{'base_uri'} . "/change-password .\n".
-        "With regards, Hydra\n"
-    );
-
-    $c->res->redirect("/admin/users");
-}
-
-
 sub machines : Chained('admin') PathPart('machines') Args(0) {
     my ($self, $c) = @_;
     $c->stash->{machines} = [$c->model('DB::BuildMachines')->search({}, {order_by => "hostname"})];
@@ -312,6 +174,7 @@ sub machine_enable : Chained('machine') PathPart('enable') Args(0) {
     $c->res->redirect("/admin/machines");
 }
 
+
 sub machine_disable : Chained('machine') PathPart('disable') Args(0) {
     my ($self, $c) = @_;
     $c->stash->{machine}->update({ enabled => 0});

src/lib/Hydra/Controller/Root.pm

@@ -156,7 +156,15 @@ sub default :Path {
 sub end : ActionClass('RenderView') {
     my ($self, $c) = @_;
 
-    if (scalar @{$c->error}) {
+    if (defined $c->stash->{json}) {
+        if (scalar @{$c->error}) {
+            $c->stash->{json}->{error} = join "\n", @{$c->error};
+            $c->clear_errors;
+        }
+        $c->forward('View::JSON');
+    }
+
+    elsif (scalar @{$c->error}) {
         $c->stash->{template} = 'error.tt';
         $c->stash->{errors} = $c->error;
         $c->response->status(500) if $c->response->status == 200;
@@ -216,33 +224,6 @@ sub narinfo :LocalRegex('^([a-z0-9]+).narinfo$') :Args(0) {
 }
 
 
-sub change_password : Path('change-password') :Args(0) {
-    my ($self, $c) = @_;
-
-    requireLogin($c) if !$c->user_exists;
-
-    $c->stash->{template} = 'change-password.tt';
-}
-
-
-sub change_password_submit : Path('change-password/submit') : Args(0) {
-    my ($self, $c) = @_;
-
-    requireLogin($c) if !$c->user_exists;
-
-    my $password = $c->request->params->{"password"};
-    my $password_check = $c->request->params->{"password_check"};
-    print STDERR "$password \n";
-    print STDERR "$password_check \n";
-    error($c, "Passwords did not match, go back and try again!") if $password ne $password_check;
-
-    my $hashed = sha1_hex($password);
-    $c->user->update({ password => $hashed}) ;
-
-    $c->res->redirect("/");
-}
-
-
 sub logo :Local {
     my ($self, $c) = @_;
     my $path = $ENV{"HYDRA_LOGO"} or die("Logo not set!");

src/lib/Hydra/Controller/User.pm

@@ -1,8 +1,10 @@
 package Hydra::Controller::User;
 
+use utf8;
 use strict;
 use warnings;
 use base 'Catalyst::Controller';
+use Crypt::RandPasswd;
 use Digest::SHA1 qw(sha1_hex);
 use Hydra::Helper::Nix;
 use Hydra::Helper::CatalystUtils;
@@ -17,23 +19,17 @@ sub login :Local {
     my $username = $c->request->params->{username} || "";
     my $password = $c->request->params->{password} || "";
 
-    if ($username eq "" && $password eq "" && !defined $c->flash->{referer}) {
+    if ($username eq "" && $password eq "" && !defined $c->session->{referer}) {
         my $baseurl = $c->uri_for('/');
-        my $refurl = $c->request->referer;
+        my $referer = $c->request->referer;
-        $c->flash->{referer} = $refurl if $refurl =~ m/^($baseurl)/;
+        $c->session->{referer} = $referer if defined $referer && $referer =~ m/^($baseurl)/;
     }
 
     if ($username && $password) {
-        if ($c->authenticate({username => $username, password => $password})) {
+        backToReferer($c) if $c->authenticate({username => $username, password => $password});
-            $c->response->redirect($c->flash->{referer} || $c->uri_for('/'));
-            $c->flash->{referer} = undef;
-            return;
-        }
         $c->stash->{errorMsg} = "Bad username or password.";
     }
 
-    $c->keep_flash("referer");
-
     $c->stash->{template} = 'login.tt';
 }
 
@@ -51,6 +47,18 @@ sub captcha :Local Args(0) {
 }
 
 
+sub isValidPassword {
+    my ($password) = @_;
+    return length($password) >= 6;
+}
+
+
+sub setPassword {
+    my ($user, $password) = @_;
+    $user->update({ password => sha1_hex($password) });
+}
+
+
 sub register :Local Args(0) {
     my ($self, $c) = @_;
 
@@ -81,7 +89,7 @@ sub register :Local Args(0) {
     return fail($c, "Your must specify your full name.") if $fullName eq "";
 
     return fail($c, "You must specify a password of at least 6 characters.")
-        if length($password) < 6;
+        unless isValidPassword($password);
 
     return fail($c, "The passwords you specified did not match.")
         if $password ne trim $c->req->params->{password2};
@@ -90,22 +98,111 @@ sub register :Local Args(0) {
         my $user = $c->model('DB::Users')->create(
             { username => $userName
             , fullname => $fullName
-            , password => sha1_hex($password)
+            , password => "!"
             , emailaddress => "",
             });
+        setPassword($user, $password);
     });
 
-    $c->authenticate({username => $userName, password => $password})
+    unless ($c->user_exists) {
-        or error($c, "Unable to authenticate the new user!");
+        $c->authenticate({username => $userName, password => $password})
+            or error($c, "Unable to authenticate the new user!");
+    }
 
     $c->flash->{successMsg} = "User <tt>$userName</tt> has been created.";
-    $c->response->redirect($c->flash->{referer} || $c->uri_for('/'));
+    backToReferer($c);
 }
 
 
-sub preferences :Local Args(0) {
+sub user :Chained('/') PathPart('user') CaptureArgs(1) {
+    my ($self, $c, $userName) = @_;
+
+    requireLogin($c) if !$c->user_exists;
+
+    error($c, "You do not have permission to edit other users.")
+        if $userName ne $c->user->username && !isAdmin($c);
+
+    $c->stash->{user} = $c->model('DB::Users')->find($userName)
+        or notFound($c, "User $userName doesn't exist.");
+}
+
+
+sub deleteUser {
+    my ($self, $c, $user) = @_;
+    my ($project) = $c->model('DB::Projects')->search({ owner => $user->username });
+    error($c, "User " . $user->username . " is still owner of project " . $project->name . ".")
+        if defined $project;
+    $c->logout() if $user->username eq $c->user->username;
+    $user->delete;
+}
+
+
+sub edit :Chained('user') Args(0) {
     my ($self, $c) = @_;
-    error($c, "Not implemented.");
+
+    my $user = $c->stash->{user};
+
+    $c->stash->{template} = 'user.tt';
+
+    $c->session->{referer} = $c->request->referer if !defined $c->session->{referer};
+
+    if ($c->request->method ne "POST") {
+        $c->stash->{fullname} = $user->fullname;
+        $c->stash->{emailonerror} = $user->emailonerror;
+        return;
+    }
+
+    if (($c->request->params->{submit} // "") eq "delete") {
+        deleteUser($self, $c, $user);
+        backToReferer($c);
+    }
+
+    if (($c->request->params->{submit} // "") eq "reset-password") {
+        $c->stash->{json} = {};
+        error($c, "No email address is set for this user.")
+            unless $user->emailaddress;
+        my $password = Crypt::RandPasswd->word(8,10);
+        setPassword($user, $password);
+        sendEmail($c,
+            $user->emailaddress,
+            "Hydra password reset",
+            "Hi,\n\n".
+            "Your password has been reset. Your new password is '$password'.\n\n".
+            "You can change your password at " . $c->uri_for($self->action_for('edit'), [$user->username]) . ".\n\n".
+            "With regards,\n\nHydra.\n"
+        );
+        return;
+    }
+
+    my $fullName = trim $c->req->params->{fullname};
+
+    txn_do($c->model('DB')->schema, sub {
+
+        error($c, "Your must specify your full name.") if $fullName eq "";
+
+        $user->update(
+            { fullname => $fullName
+            , emailonerror => $c->request->params->{"emailonerror"} ? 1 : 0
+            });
+
+        my $password = $c->req->params->{password} // "";
+        if ($password ne "") {
+            error($c, "You must specify a password of at least 6 characters.")
+                unless isValidPassword($password);
+            error($c, "The passwords you specified did not match.")
+                if $password ne trim $c->req->params->{password2};
+            setPassword($user, $password);
+        }
+
+        if (isAdmin($c)) {
+            $user->userroles->delete_all;
+            $user->userroles->create({ role => $_})
+                foreach paramToList($c, "roles");
+        }
+
+    });
+
+    backToReferer($c);
 }
 
 

src/lib/Hydra/Helper/CatalystUtils.pm

@@ -4,6 +4,10 @@ use utf8;
 use strict;
 use Exporter;
 use Readonly;
+use Email::Simple;
+use Email::Sender::Simple qw(sendmail);
+use Email::Sender::Transport::SMTP;
+use Sys::Hostname::Long;
 use Nix::Store;
 use Hydra::Helper::Nix;
 
@@ -15,6 +19,9 @@ our @EXPORT = qw(
     trim
     getLatestFinishedEval
     parseJobsetName
+    sendEmail
+    paramToList
+    backToReferer
     $pathCompRE $relPathRE $relNameRE $projectNameRE $jobsetNameRE $jobNameRE $systemRE $userNameRE
     @buildListColumns
 );
@@ -97,8 +104,17 @@ sub notFound {
 }
 
 
+sub backToReferer {
+    my ($c) = @_;
+    $c->response->redirect($c->session->{referer} || $c->uri_for('/'));
+    $c->session->{referer} = undef;
+    $c->detach;
+}
+
+
 sub requireLogin {
     my ($c) = @_;
+    $c->session->{referer} = $c->request->uri;
     $c->response->redirect($c->uri_for('/login'));
     $c->detach; # doesn't return
 }
@@ -162,6 +178,39 @@ sub getLatestFinishedEval {
 }
 
 
+sub sendEmail {
+    my ($c, $to, $subject, $body) = @_;
+
+    my $sender = $c->config->{'notification_sender'} ||
+        (($ENV{'USER'} || "hydra") .  "@" . hostname_long);
+
+    my $email = Email::Simple->create(
+        header => [
+            To      => $to,
+            From    => "Hydra <$sender>",
+            Subject => $subject
+        ],
+        body => $body
+    );
+
+    print STDERR "Sending email:\n", $email->as_string if $ENV{'HYDRA_MAIL_TEST'};
+
+    sendmail($email);
+}
+
+
+# Catalyst request parameters can be an array or a scalar or
+# undefined, making them annoying to handle.  So this utility function
+# always returns a request parameter as a list.
+sub paramToList {
+    my ($c, $name) = @_;
+    my $x = $c->request->params->{$name};
+    return () unless defined $x;
+    return @$x if ref($x) eq 'ARRAY';
+    return ($x);
+}
+
+
 # Security checking of filenames.
 Readonly our $pathCompRE    => "(?:[A-Za-z0-9-\+\._\$][A-Za-z0-9-\+\._\$]*)";
 Readonly our $relPathRE     => "(?:$pathCompRE(?:/$pathCompRE)*)";

src/root/change-password.tt

@@ -1,23 +0,0 @@
-[% WRAPPER layout.tt title="Change password" %]
-[% PROCESS common.tt %]
-
-<form action="[% c.uri_for('/change-password/submit') %]" method="post">
-
-<h2>Change password</h2>
-
-<table class="info-table">
-  <tr>
-    <th>Enter password:</th>
-    <td><input type="password" class="string" id="password" name="password" /></td>
-  </tr>
-  <tr>
-    <th>Enter password again:</th>
-    <td><input type="password" class="string" id="password_check" name="password_check" /></td>
-  </tr>
-</table>
-
-<p><button type="submit"><img src="/static/images/success.gif" />Change</button></p>
-
-</form>
-
-[% END %]

src/root/topbar.tt

@@ -202,7 +202,7 @@
   <ul class="nav" id="top-menu">
 
     [% IF c.user_exists %]
-      [% INCLUDE makeLink uri = c.uri_for(c.controller('Root').action_for('preferences')) title = "Preferences" %]
+      [% INCLUDE makeLink uri = c.uri_for(c.controller('User').action_for('edit'), [c.user.username]) title = "Preferences" %]
       [% INCLUDE makeLink uri = c.uri_for(c.controller('Root').action_for('logout')) title = "Sign out" %]
     [% ELSE %]
       [% INCLUDE makeLink uri = c.uri_for(c.controller('Root').action_for('login')) title = "Sign in" %]

src/root/user.tt

@@ -1,4 +1,4 @@
-[% WRAPPER layout.tt title=(create ? "Register new user" : "User $user.username") %]
+[% WRAPPER layout.tt title=(create ? "Register new user" : "Editing user $user.username") %]
 [% PROCESS common.tt %]
 
 [% BLOCK roleoption %]
@@ -61,7 +61,7 @@
       <div class="control-group">
         <div class="controls">
           <label class="checkbox">
-            <input type="checkbox" name="enabled" [% IF 1; 'checked="checked"'; END %]></input>Receive evaluation error notifications
+            <input type="checkbox" name="emailonerror" [% IF emailonerror; 'checked="checked"'; END %]></input>Receive evaluation error notifications
           </label>
         </div>
       </div>
@@ -99,6 +99,30 @@
         <i class="icon-ok icon-white"></i>
         [%IF create %]Create[% ELSE %]Apply changes[% END %]
       </button>
+      [% IF !create && c.check_user_roles('admin') %]
+        <button id="reset-password" type="submit" class="btn btn-warning" name="submit" value="reset-password">
+          <i class="icon-trash icon-white"></i>
+          Reset password
+        </button>
+        <script type="text/javascript">
+          $("#reset-password").click(function() {
+            bootbox.confirm(
+              'Are you sure you want to reset the password for this user?',
+              function(c) {
+                if (!c) return;
+                $.post("[% c.uri_for(c.controller('User').action_for('edit'), [user.username]) %]", { submit: 'reset-password' })
+                  .done(function(data) {
+                    if (data.error)
+                      bootbox.alert("Unable to reset password: " + data.error);
+                    else
+                      bootbox.alert("An email containing the new password has been sent to the user.");
+                  })
+                  .fail(function() { bootbox.alert("Server request failed!"); });
+              });
+            return false;
+          });
+        </script>
+      [% END %]
       [% IF !create %]
         <button id="delete-user" type="submit" class="btn btn-danger" name="submit" value="delete">
           <i class="icon-trash icon-white"></i>

src/root/users.tt

@@ -9,24 +9,22 @@
       <th>Email</th>
       <th>Roles</th>
       <th>Eval. notifications</th>
-      <th>Options</th>
     </tr>
   </thead>
   <tbody>
     [% FOREACH u IN users %]
       <tr>
-        <td><a class="row-link" href="[% c.uri_for(c.controller('Admin').action_for('user_edit'), [u.username]) %]">[% u.username %]</a></td>
+        <td><a class="row-link" href="[% c.uri_for(c.controller('User').action_for('edit'), [u.username]) %]">[% u.username %]</a></td>
         <td>[% u.fullname %]</td>
         <td>[% u.emailaddress %]</td>
         <td>[% FOREACH r IN u.userroles %]<i>[% r.role %]</i> [% END %]</td>
         <td>[% IF u.emailonerror %]Yes[% ELSE %]No[% END %]</td>
-        <td>[% INCLUDE maybeLink uri = c.uri_for(c.controller('Admin').action_for('reset_password'), [u.username]) content = "Reset password" confirmmsg = "Are you sure you want to reset the password for this user?" class = "btn btn-mini" %]</td>
       </tr>
     [% END %]
   </tbody>
 </table>
 
-<p><a class="btn" href="[% c.uri_for(c.controller('Admin').action_for('create_user')) %]">
+<p><a class="btn" href="[% c.uri_for(c.controller('Root').action_for('register')) %]">
   <i class="icon-plus"></i> Add a new user
 </a></p>
 

src/script/hydra-build

@@ -114,7 +114,7 @@ sub sendEmailNotification {
 
     my $status =  statusDescription($build->buildstatus);
 
-    my $baseurl = hostname_long ;
+    my $baseurl = hostname_long;
     my $sender = $config->{'notification_sender'} ||
         (($ENV{'USER'} || "hydra") .  "@" . $baseurl);
 

tests/Setup.pm

@@ -12,7 +12,7 @@ our @EXPORT = qw(hydra_setup nrBuildsForJobset queuedBuildsForJobset nrQueuedBui
 
 sub hydra_setup {
     my ($db) = @_;
-    $db->resultset('Users')->create({ username => "root", emailaddress => 'root@email.com', password => '' });
+    $db->resultset('Users')->create({ username => "root", emailaddress => 'root@invalid.org', password => '' });
 }
 
 sub nrBuildsForJobset {

tests/jobs/bzr-update.sh

@@ -14,7 +14,7 @@ export BZR_HOME; # Set by the Makefile
 case $state in
     (0) echo "::Create repo. -- continue -- updated::"
     bzr init bzr-repo
-    bzr whoami "build <build@donotemail.com>" -d bzr-repo
+    bzr whoami "build <build@invalid.org>" -d bzr-repo
     touch bzr-repo/bzr-file
     bzr add bzr-repo/bzr-file
     bzr commit -m "add bzr-file" bzr-repo/bzr-file