Use normal user authentication to control access to /admin

 - Adds is_superuser flag to User
 - Logged-in user must be a superuser or have from_body set in order to access
   anything within /admin
 - has_permission_to on a superuser will always return true
 - Only superusers can create/grant superusers
 - New `createsuperuser` command for creating superusers

18 files changed, 207 insertions, 26 deletions

diff --git a/README.md b/README.md
index 5b536a67f..ef8e844c1 100644
--- a/README.md
+++ b/README.md
@@ -56,6 +56,8 @@ web-based cross-browser testing tools for this project.
         - Greatly improve report edit page, including map. #1347
         - Show any waiting reports on admin index page. #1382
         - Allow user's phone number to be edited.
+        - /admin now requires a logged-in user with the `is_superuser` flag set
+        - `createsuperuser` command for creating superusers/granting superuser status.
     - Development improvements:
         - make_css: Add output style option.
         - make_css: Follow symlinks.
diff --git a/Vagrantfile b/Vagrantfile
index 31f645da7..80e79846d 100755
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -49,12 +49,15 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
         # We want to be on port 3000 for development
         sed -i -r -e "s,^( *BASE_URL: .*)',\\1:3000'," fixmystreet/conf/general.yml
     fi
+    # Create a superuser for the admin
+    fixmystreet/bin/createsuperuser superuser@example.org password
     if [ $SUCCESS -eq 0 ]; then
         # All done
         echo "****************"
         echo "You can now ssh into your vagrant box: vagrant ssh"
         echo "The website code is found in: ~/fixmystreet"
         echo "You can run the dev server with: script/fixmystreet_app_server.pl [-d] [-r] [--fork]"
+        echo "Access the admin with username: superuser@example.org and password: password"
     else
         echo "Unfortunately, something appears to have gone wrong with the installation."
         echo "Please see above for any errors, and do ask on our mailing list for help."
diff --git a/bin/createsuperuser b/bin/createsuperuser
new file mode 100755
index 000000000..98b36aa36
--- /dev/null
+++ b/bin/createsuperuser
@@ -0,0 +1,33 @@
+#!/usr/bin/env perl
+
+# createsuperuser:
+# Create superusers or grant is_superuser flag to existing users.
+#
+# Usage:
+#
+# Create a new superuser with password 'password123'
+# $ bin/createsuperuser user1@example.org password123
+#
+# Grant superuser status to an existing user:
+# $ bin/createsuperuser user2@example.org
+#
+# Superusers can create superusers and grant/rescind superuser status via /admin
+#
+# Copyright (c) 2016 UK Citizens Online Democracy. All rights reserved.
+# Email: davea@mysociety.org. WWW: http://www.mysociety.org
+
+use strict;
+use warnings;
+
+BEGIN {
+    use File::Basename qw(dirname);
+    use File::Spec;
+    my $d = dirname(File::Spec->rel2abs($0));
+    require "$d/../setenv.pl";
+}
+
+
+use FixMyStreet;
+use FixMyStreet::Script::CreateSuperuser;
+
+FixMyStreet::Script::CreateSuperuser::createsuperuser();
diff --git a/bin/update-schema b/bin/update-schema
index 1393178f8..8f74f34f1 100755
--- a/bin/update-schema
+++ b/bin/update-schema
@@ -194,6 +194,7 @@ else {
 # By querying the database schema, we can see where we're currently at
 # (assuming schema change files are never half-applied, which should be the case)
 sub get_db_version {
+    return '0040' if column_exists('users', 'is_superuser');
     return '0039' if column_exists('users', 'facebook_id');
     return '0038' if column_exists('admin_log', 'time_spent');
     return '0037' if table_exists('response_templates');
diff --git a/conf/apache-vhost.conf.example b/conf/apache-vhost.conf.example
index 583eb0cde..b4a3e78f0 100644
--- a/conf/apache-vhost.conf.example
+++ b/conf/apache-vhost.conf.example
@@ -25,14 +25,6 @@
         AllowOverride None
     </Directory>
 
-    <Location /admin>
-        # 
-        #  WARNING - enable auth here on production machine
-        # 
-    </Location>
-
-    Alias /admin/ /home/yourname/fixmystreet/web-admin/
-
     Alias /jslib/ /home/yourname/fixmystreet/commonlib/jslib/
     <Location /jslib>
         AddOutputFilter DEFLATE js
diff --git a/conf/nginx.conf.example b/conf/nginx.conf.example
index 69416a1c5..e166fc532 100644
--- a/conf/nginx.conf.example
+++ b/conf/nginx.conf.example
@@ -51,12 +51,6 @@ server {
         proxy_set_header X-Real-IP $remote_addr;
     }
 
-    location /admin {
-        auth_basic "FixMyStreet admin interface";
-        auth_basic_user_file /var/www/fixmystreet/admin-htpasswd;
-        try_files $uri @catalyst;
-    }
-
     location / {
         if (-f $document_root/down.html) {
             return 503;
diff --git a/db/downgrade_0040---0039.sql b/db/downgrade_0040---0039.sql
new file mode 100644
index 000000000..8ab45ab24
--- /dev/null
+++ b/db/downgrade_0040---0039.sql
@@ -0,0 +1,3 @@
+begin;
+alter table users drop column is_superuser;
+commit;
diff --git a/db/schema.sql b/db/schema.sql
index 3761553a5..3f73d2325 100644
--- a/db/schema.sql
+++ b/db/schema.sql
@@ -27,6 +27,7 @@ create table users (
     password        text    not null default '',
     from_body       integer,
     flagged         boolean not null default 'f',
+    is_superuser    boolean not null default 'f',
     title           text,
     twitter_id      bigint  unique,
     facebook_id     bigint  unique
diff --git a/db/schema_0040-superuser_flag.sql b/db/schema_0040-superuser_flag.sql
new file mode 100644
index 000000000..e440257ba
--- /dev/null
+++ b/db/schema_0040-superuser_flag.sql
@@ -0,0 +1,3 @@
+begin;
+alter table users add column is_superuser boolean not null default 'f';
+commit;
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index bcf66f36f..43fffd315 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -32,10 +32,12 @@ sub begin : Private {
 
     $c->uri_disposition('relative');
 
-    if ( $c->cobrand->moniker eq 'zurich' || $c->cobrand->moniker eq 'seesomething' ) {
-        $c->detach( '/auth/redirect' ) unless $c->user_exists;
-        $c->detach( '/auth/redirect' ) unless $c->user->from_body;
+    # User must be logged in to see cobrand, and meet whatever checks the
+    # cobrand specifies. Default cobrand just requires superuser flag to be set.
+    unless ( $c->user_exists && $c->cobrand->admin_allow_user($c->user) ) {
+        $c->detach( '/auth/redirect' );
     }
+
     if ( $c->cobrand->moniker eq 'zurich' ) {
         $c->cobrand->admin_type();
     }
@@ -1072,6 +1074,8 @@ sub user_add : Path('user_edit') : Args(0) {
         phone => $c->get_param('phone') || undef,
         from_body => $c->get_param('body') || undef,
         flagged => $c->get_param('flagged') || 0,
+        # Only superusers can create superusers
+        is_superuser => ( $c->user->is_superuser && $c->get_param('is_superuser') ) || 0,
     }, {
         key => 'users_email_key'
     } );
@@ -1114,6 +1118,8 @@ sub user_edit : Path('user_edit') : Args(1) {
         $user->phone( $c->get_param('phone') ) if $c->get_param('phone');
         $user->from_body( $c->get_param('body') || undef );
         $user->flagged( $c->get_param('flagged') || 0 );
+        # Only superusers can grant superuser status
+        $user->is_superuser( ( $c->user->is_superuser && $c->get_param('is_superuser') ) || 0 );
 
         unless ($user->email) {
             $c->stash->{field_errors}->{email} = _('Please enter a valid email');
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index ca4a2fc80..40cd163cf 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -414,8 +414,8 @@ Used after signing in to take the person back to where they were.
 sub redirect_on_signin : Private {
     my ( $self, $c, $redirect ) = @_;
     $redirect = 'my' unless $redirect;
+    $redirect = 'my' if $redirect =~ /^admin/ && !$c->user->is_superuser;
     if ( $c->cobrand->moniker eq 'zurich' ) {
-        $redirect = 'my' if $redirect eq 'admin';
         $redirect = 'admin' if $c->user->from_body;
     }
     $c->res->redirect( $c->uri_for( "/$redirect" ) );
diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm
index 36313cf63..e5ec0c13a 100644
--- a/perllib/FixMyStreet/Cobrand/Default.pm
+++ b/perllib/FixMyStreet/Cobrand/Default.pm
@@ -369,8 +369,8 @@ sub uri {
 
     {
         no warnings 'once';
-        (my $map_class = $FixMyStreet::Map::map_class) =~ s/^FixMyStreet::Map:://;
-        return $uri unless $map_class =~ /OSM|FMS/;
+        my $map_class = $FixMyStreet::Map::map_class;
+        return $uri unless $map_class && $map_class =~ /FixMyStreet::Map::(OSM|FMS)/;
     }
 
     $uri->query_param( zoom => 3 )
@@ -622,6 +622,18 @@ Show the problem creation graph in the admin interface
 
 sub admin_show_creation_graph { 1 }
 
+=head2 admin_allow_user
+
+Perform checks on whether this user can access admin. By default only superusers
+are allowed.
+
+=cut
+
+sub admin_allow_user {
+    my ( $self, $user ) = @_;
+    return 1 if $user->is_superuser;
+}
+
 =head2 area_types
 
 The MaPit types this site handles
diff --git a/perllib/FixMyStreet/Cobrand/SeeSomething.pm b/perllib/FixMyStreet/Cobrand/SeeSomething.pm
index 22750aafa..4d4dd000e 100644
--- a/perllib/FixMyStreet/Cobrand/SeeSomething.pm
+++ b/perllib/FixMyStreet/Cobrand/SeeSomething.pm
@@ -60,6 +60,11 @@ sub allow_anonymous_reports { 1; }
 
 sub anonymous_account { return { name => 'Anonymous Submission', email => FixMyStreet->config('DO_NOT_REPLY_EMAIL') }; }
 
+sub admin_allow_user {
+    my ( $self, $user ) = @_;
+    return 1 if ( $user->from_body || $user->is_superuser );
+}
+
 sub admin_pages {
     my $self = shift;
 
diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm
index d13408321..1bf9cb9a5 100644
--- a/perllib/FixMyStreet/Cobrand/Zurich.pm
+++ b/perllib/FixMyStreet/Cobrand/Zurich.pm
@@ -371,6 +371,12 @@ sub update_admin_log {
     $c->forward( 'log_edit', [ $problem->id, 'problem', $text, $time_spent ] );
 }
 
+# Any user with from_body set can view admin
+sub admin_allow_user {
+    my ( $self, $user ) = @_;
+    return 1 if $user->from_body;
+}
+
 # Specific administrative displays
 
 sub admin_pages {
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index 7356969d1..65dd1dab1 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -26,16 +26,18 @@ __PACKAGE__->add_columns(
   { data_type => "text", is_nullable => 1 },
   "password",
   { data_type => "text", default_value => "", is_nullable => 0 },
-  "from_body",
-  { data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
   "flagged",
   { data_type => "boolean", default_value => \"false", is_nullable => 0 },
+  "from_body",
+  { data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
   "title",
   { data_type => "text", is_nullable => 1 },
-  "twitter_id",
-  { data_type => "bigint", is_nullable => 1 },
   "facebook_id",
   { data_type => "bigint", is_nullable => 1 },
+  "twitter_id",
+  { data_type => "bigint", is_nullable => 1 },
+  "is_superuser",
+  { data_type => "boolean", default_value => \"false", is_nullable => 0 },
 );
 __PACKAGE__->set_primary_key("id");
 __PACKAGE__->add_unique_constraint("users_email_key", ["email"]);
@@ -90,8 +92,8 @@ __PACKAGE__->has_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07035 @ 2015-12-09 16:02:08
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:hCq6ZDZfV/6iiu3HFhPPOg
+# Created by DBIx::Class::Schema::Loader v0.07035 @ 2016-07-11 12:49:31
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:SG86iN6Fr4/JIq7U2zYkug
 
 __PACKAGE__->add_columns(
     "password" => {
@@ -230,6 +232,8 @@ sub split_name {
 sub has_permission_to {
     my ($self, $permission_type, $body_id) = @_;
 
+    return 1 if $self->is_superuser;
+
     return unless $self->belongs_to_body($body_id);
 
     my $permission = $self->user_body_permissions->find({ 
diff --git a/perllib/FixMyStreet/Script/CreateSuperuser.pm b/perllib/FixMyStreet/Script/CreateSuperuser.pm
new file mode 100644
index 000000000..69d165abb
--- /dev/null
+++ b/perllib/FixMyStreet/Script/CreateSuperuser.pm
@@ -0,0 +1,25 @@
+package FixMyStreet::Script::CreateSuperuser;
+
+use strict;
+use warnings;
+
+use FixMyStreet;
+use FixMyStreet::DB;
+
+sub createsuperuser {
+    die "Specify a single email address and optionally password to create a superuser or grant superuser status to." if (@ARGV < 1 || @ARGV > 2);
+
+    my $user = FixMyStreet::DB->resultset('User')->find_or_new({ email => $ARGV[0] });
+    if ( !$user->in_storage ) {
+        die "Specify a password for this new user." if (@ARGV < 2);
+        $user->password($ARGV[1]);
+        $user->is_superuser(1);
+        $user->insert;
+    } else {
+        $user->update({ is_superuser => 1 });
+    }
+    print $user->email . " is now a superuser.\n";
+}
+
+
+1;
\ No newline at end of file
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index d7fcb30e6..9b083ce42 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -17,6 +17,17 @@ my $user2 =
   ->find_or_create( { email => 'test2@example.com', name => 'Test User 2' } );
 ok $user2, "created second test user";
 
+my $superuser =
+  FixMyStreet::App->model('DB::User')
+  ->find_or_create( { email => 'superuser@example.com', name => 'Super User', is_superuser => 1 } );
+ok $superuser, "created superuser";
+
+my $oxfordshire = $mech->create_body_ok(2237, 'Oxfordshire County Council', id => 2237 );
+my $counciluser =
+  FixMyStreet::App->model('DB::User')
+  ->find_or_create( { email => 'counciluser@example.com', name => 'Council User', from_body => $oxfordshire->id } );
+ok $counciluser, "created council user";
+
 
 my $user3 =
   FixMyStreet::App->model('DB::User')
@@ -70,6 +81,8 @@ my $alert = FixMyStreet::App->model('DB::Alert')->find_or_create(
     },
 );
 
+$mech->log_in_ok( $superuser->email );
+
 subtest 'check summary counts' => sub {
     my $problems = FixMyStreet::App->model('DB::Problem')->search( { state => { -in => [qw/confirmed fixed closed investigating planned/, 'in progress', 'fixed - user', 'fixed - council'] } } );
 
@@ -1131,6 +1144,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             name => 'Changed User',
@@ -1146,6 +1160,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             email => 'changed@example.com',
@@ -1161,6 +1176,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             body => $southend->id,
@@ -1176,6 +1192,7 @@ for my $test (
             body => $southend->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             flagged => 'on',
@@ -1191,6 +1208,7 @@ for my $test (
             body => $southend->id,
             phone => '',
             flagged => 'on',
+            is_superuser => undef,
         },
         changes => {
             flagged => undef,
@@ -1198,6 +1216,38 @@ for my $test (
         log_count => 4,
         log_entries => [qw/edit edit edit edit/],
     },
+    {
+        desc => 'edit user add is_superuser',
+        fields => {
+            name => 'Changed User',
+            email => 'changed@example.com',
+            body => $southend->id,
+            phone => '',
+            flagged => undef,
+            is_superuser => undef,
+        },
+        changes => {
+            is_superuser => 'on',
+        },
+        log_count => 5,
+        log_entries => [qw/edit edit edit edit edit/],
+    },
+    {
+        desc => 'edit user remove is_superuser',
+        fields => {
+            name => 'Changed User',
+            email => 'changed@example.com',
+            body => $southend->id,
+            phone => '',
+            flagged => undef,
+            is_superuser => 'on',
+        },
+        changes => {
+            is_superuser => undef,
+        },
+        log_count => 5,
+        log_entries => [qw/edit edit edit edit edit/],
+    },
 ) {
     subtest $test->{desc} => sub {
         $mech->get_ok( '/admin/user_edit/' . $user->id );
@@ -1237,9 +1287,39 @@ subtest "Check admin_base_url" => sub {
         'get_admin_url OK');
 };
 
+# Finished with the superuser tests
+$mech->log_out_ok;
+
+subtest "Users without from_body can't access admin" => sub {
+    $user->from_body( undef );
+    $user->update;
+
+    $mech->log_in_ok( $user->email );
+
+    $mech->get_ok('/admin');
+    is $mech->uri->path, '/my', "redirected to correct page";
+    is $mech->res->code, 200, "got 200 for final destination";
+    is $mech->res->previous->code, 302, "got 302 for redirect";
+
+    $mech->log_out_ok;
+};
+
+subtest "Users with from_body can access admin" => sub {
+    $mech->log_in_ok( $counciluser->email );
+
+    $mech->get_ok('/admin');
+    $mech->content_contains( 'FixMyStreet admin:' );
+
+    $mech->log_out_ok;
+};
+
+
+
 $mech->delete_user( $user );
 $mech->delete_user( $user2 );
 $mech->delete_user( $user3 );
+$mech->delete_user( $superuser );
+$mech->delete_user( $counciluser );
 $mech->delete_user( 'test4@example.com' );
 
 done_testing();
diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html
index d6456f3d9..2942494a7 100644
--- a/templates/web/base/admin/user-form.html
+++ b/templates/web/base/admin/user-form.html
@@ -55,6 +55,17 @@
           
           [% loc('Flagged:') %] <input type="checkbox" id="flagged" name="flagged"[% user.flagged ? ' checked' : '' %]>
         </li>
+
+        [% IF c.user.is_superuser %]
+          <li>
+            <div class="admin-hint">
+              <p>
+                [% loc("Superusers have permission to perform <strong>all actions</strong> within the admin.") %]
+              </p>
+            </div>
+            [% loc('Superuser:') %] <input type="checkbox" id="is_superuser" name="is_superuser"[% user.is_superuser ? ' checked' : '' %]>
+          </li>
+        [% END %]
       [% END %]
     </ul>
     <input type="submit" name="Submit changes" value="[% loc('Submit changes') %]" >