Merge branch 'admin-using-normal-login'

36 files changed, 352 insertions, 179 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..35af9a1a6 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();
     }
@@ -73,7 +75,7 @@ sub index : Path : Args(0) {
 
     $c->forward('stats_by_state');
 
-    my @unsent = $c->model('DB::Problem')->search( {
+    my @unsent = $c->cobrand->problems->search( {
         state => [ 'confirmed' ],
         whensent => undef,
         bodies_str => { '!=', undef },
@@ -242,13 +244,15 @@ sub bodies : Path('bodies') : Args(0) {
         $c->forward('/auth/check_csrf_token');
 
         my $params = $c->forward('body_params');
-        my $body = $c->model('DB::Body')->create( $params );
-        my @area_ids = $c->get_param_list('area_ids');
-        foreach (@area_ids) {
-            $c->model('DB::BodyArea')->create( { body => $body, area_id => $_ } );
-        }
+        unless ( keys $c->stash->{body_errors} ) {
+            my $body = $c->model('DB::Body')->create( $params );
+            my @area_ids = $c->get_param_list('area_ids');
+            foreach (@area_ids) {
+                $c->model('DB::BodyArea')->create( { body => $body, area_id => $_ } );
+            }
 
-        $c->stash->{updated} = _('New body added');
+            $c->stash->{updated} = _('New body added');
+        }
     }
 
     $c->forward( 'fetch_all_bodies' );
@@ -313,8 +317,13 @@ sub body : Path('body') : Args(1) {
 
 sub check_for_super_user : Private {
     my ( $self, $c ) = @_;
-    if ( $c->cobrand->moniker eq 'zurich' && $c->stash->{admin_type} ne 'super' ) {
-        $c->detach('/page_error_404_not_found', []);
+
+    my $superuser = $c->user->is_superuser;
+    # Zurich currently has its own way of defining superusers
+    $superuser ||= $c->cobrand->moniker eq 'zurich' && $c->stash->{admin_type} eq 'super';
+
+    unless ( $superuser ) {
+        $c->detach('/page_error_403_access_denied', []);
     }
 }
 
@@ -403,18 +412,20 @@ sub update_contacts : Private {
         $c->forward('/auth/check_csrf_token');
 
         my $params = $c->forward( 'body_params' );
-        $c->stash->{body}->update( $params );
-        my @current = $c->stash->{body}->body_areas->all;
-        my %current = map { $_->area_id => 1 } @current;
-        my @area_ids = $c->get_param_list('area_ids');
-        foreach (@area_ids) {
-            $c->model('DB::BodyArea')->find_or_create( { body => $c->stash->{body}, area_id => $_ } );
-            delete $current{$_};
-        }
-        # Remove any others
-        $c->stash->{body}->body_areas->search( { area_id => [ keys %current ] } )->delete;
+        unless ( keys $c->stash->{body_errors} ) {
+            $c->stash->{body}->update( $params );
+            my @current = $c->stash->{body}->body_areas->all;
+            my %current = map { $_->area_id => 1 } @current;
+            my @area_ids = $c->get_param_list('area_ids');
+            foreach (@area_ids) {
+                $c->model('DB::BodyArea')->find_or_create( { body => $c->stash->{body}, area_id => $_ } );
+                delete $current{$_};
+            }
+            # Remove any others
+            $c->stash->{body}->body_areas->search( { area_id => [ keys %current ] } )->delete;
 
-        $c->stash->{updated} = _('Values updated');
+            $c->stash->{updated} = _('Values updated');
+        }
     }
 }
 
@@ -433,9 +444,20 @@ sub body_params : Private {
         deleted => 0,
     );
     my %params = map { $_ => $c->get_param($_) || $defaults{$_} } keys %defaults;
+    $c->forward('check_body_params', [ \%params ]);
     return \%params;
 }
 
+sub check_body_params : Private {
+    my ( $self, $c, $params ) = @_;
+
+    $c->stash->{body_errors} ||= {};
+
+    unless ($params->{name}) {
+        $c->stash->{body_errors}->{name} = _('Please enter a name for this body');
+    }
+}
+
 sub display_contacts : Private {
     my ( $self, $c ) = @_;
 
@@ -1072,6 +1094,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 +1138,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/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm
index 6e98f4ae0..43f10130a 100644
--- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm
+++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm
@@ -150,4 +150,11 @@ sub base_url_for_report {
     }
 }
 
+sub admin_allow_user {
+    my ( $self, $user ) = @_;
+    return 1 if $user->is_superuser;
+    return undef unless defined $user->from_body;
+    return $user->from_body->id == $self->council_id;
+}
+
 1;
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/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm
index 2ad820d1f..937780a31 100644
--- a/perllib/FixMyStreet/TestMech.pm
+++ b/perllib/FixMyStreet/TestMech.pm
@@ -63,11 +63,10 @@ Create a test user (or find it and return if it already exists).
 
 sub create_user_ok {
     my $self = shift;
-    my ($email) = @_;
+    my ( $email, %extra ) = @_;
 
-    my $user =
-      FixMyStreet::DB->resultset('User')
-      ->find_or_create( { email => $email } );
+    my $params = { email => $email, %extra };
+    my $user = FixMyStreet::DB->resultset('User')->find_or_create($params);
     ok $user, "found/created user for $email";
 
     return $user;
diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t
index d7fcb30e6..51307f756 100644
--- a/t/app/controller/admin.t
+++ b/t/app/controller/admin.t
@@ -6,21 +6,16 @@ use FixMyStreet::TestMech;
 
 my $mech = FixMyStreet::TestMech->new;
 
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test@example.com' } );
-ok $user, "created test user";
-$user->update({ name => 'Test User' });
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
 
-my $user2 =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test2@example.com', name => 'Test User 2' } );
-ok $user2, "created second test user";
+my $user2 = $mech->create_user_ok('test2@example.com', name => 'Test User 2');
 
+my $superuser = $mech->create_user_ok('superuser@example.com', name => 'Super User', is_superuser => 1);
 
-my $user3 =
-  FixMyStreet::App->model('DB::User')
-  ->find( { email => 'test3@example.com', name => 'Test User 2' } );
+my $oxfordshire = $mech->create_body_ok(2237, 'Oxfordshire County Council', id => 2237);
+my $oxfordshireuser = $mech->create_user_ok('counciluser@example.com', name => 'Council User', from_body => $oxfordshire);
+
+my $user3 = $mech->create_user_ok('test3@example.com', name => 'Test User 2');
 
 if ( $user3 ) {
   $mech->delete_user( $user3 );
@@ -70,6 +65,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 +1128,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             name => 'Changed User',
@@ -1146,6 +1144,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             email => 'changed@example.com',
@@ -1161,6 +1160,7 @@ for my $test (
             body => $haringey->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             body => $southend->id,
@@ -1176,6 +1176,7 @@ for my $test (
             body => $southend->id,
             phone => '',
             flagged => undef,
+            is_superuser => undef,
         },
         changes => {
             flagged => 'on',
@@ -1191,6 +1192,7 @@ for my $test (
             body => $southend->id,
             phone => '',
             flagged => 'on',
+            is_superuser => undef,
         },
         changes => {
             flagged => undef,
@@ -1198,6 +1200,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 +1271,73 @@ 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 their own council's admin" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => [ 'oxfordshire' ],
+    }, sub {
+        $mech->log_in_ok( $oxfordshireuser->email );
+
+        $mech->get_ok('/admin');
+        $mech->content_contains( 'FixMyStreet admin:' );
+
+        $mech->log_out_ok;
+    };
+};
+
+subtest "Users with from_body can't access another council's admin" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => [ 'bristol' ],
+    }, sub {
+        $mech->log_in_ok( $oxfordshireuser->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't access fixmystreet.com admin" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => [ 'fixmystreet' ],
+    }, sub {
+        $mech->log_in_ok( $oxfordshireuser->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;
+    };
+};
+
+
+
 $mech->delete_user( $user );
 $mech->delete_user( $user2 );
 $mech->delete_user( $user3 );
+$mech->delete_user( $superuser );
+$mech->delete_user( $oxfordshireuser );
 $mech->delete_user( 'test4@example.com' );
 
 done_testing();
diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t
index 06932f70a..2c20daf9d 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -156,9 +156,7 @@ foreach my $test (
 
         my $type = 'area_problems';
 
-        my $user =
-          FixMyStreet::App->model('DB::User')
-          ->find_or_create( { email => 'test-new@example.com' } );
+        my $user = $mech->create_user_ok('test-new@example.com');
 
         my $alert = FixMyStreet::App->model('DB::Alert')->find(
             {
@@ -204,9 +202,7 @@ foreach my $test (
     subtest $test->{desc} => sub {
         my $type = $test->{type} . '_problems';
 
-        my $user =
-          FixMyStreet::App->model('DB::User')
-          ->find_or_create( { email => $test->{email} } );
+        my $user = $mech->create_user_ok($test->{email});
 
         $mech->log_in_ok( $test->{email} );
         $mech->clear_emails_ok;
@@ -335,13 +331,9 @@ subtest "Test normal alert signups and that alerts are sent" => sub {
     $mech->delete_user( 'reporter@example.com' );
     $mech->delete_user( 'alerts@example.com' );
 
-    my $user1 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'reporter@example.com', name => 'Reporter User' } );
-    ok $user1, "created test user";
+    my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User' );
 
-    my $user2 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'alerts@example.com', name => 'Alert User' } );
-    ok $user2, "created test user";
+    my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User' );
 
     for my $alert (
         {
@@ -489,13 +481,9 @@ subtest "Test signature template is used from cobrand" => sub {
     $mech->delete_user( 'reporter@example.com' );
     $mech->delete_user( 'alerts@example.com' );
 
-    my $user1 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'reporter@example.com', name => 'Reporter User' } );
-    ok $user1, "created test user";
+    my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User' );
 
-    my $user2 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'alerts@example.com', name => 'Alert User' } );
-    ok $user2, "created test user";
+    my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User' );
 
     my $dt = DateTime->now()->add( days => 2);
 
@@ -638,13 +626,9 @@ for my $test (
         $mech->delete_user( 'reporter@example.com' );
         $mech->delete_user( 'alerts@example.com' );
 
-        my $user1 = FixMyStreet::App->model('DB::User')
-          ->find_or_create( { email => 'reporter@example.com', name => 'Reporter User' } );
-        ok $user1, "created test user";
+        my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User');
 
-        my $user2 = FixMyStreet::App->model('DB::User')
-          ->find_or_create( { email => 'alerts@example.com', name => 'Alert User' } );
-        ok $user2, "created test user";
+        my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User');
 
         my $dt = DateTime->now->add( minutes => -30 );
         my $r_dt = $dt->clone->add( minutes => 20 );
@@ -711,17 +695,11 @@ subtest 'check new updates alerts for non public reports only go to report owner
     $mech->delete_user( 'reporter@example.com' );
     $mech->delete_user( 'alerts@example.com' );
 
-    my $user1 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'reporter@example.com', name => 'Reporter User' } );
-    ok $user1, "created test user";
+    my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User');
 
-    my $user2 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'alerts@example.com', name => 'Alert User' } );
-    ok $user2, "created test user";
+    my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User');
 
-    my $user3 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'updates@example.com', name => 'Update User' } );
-    ok $user3, "created test user";
+    my $user3 = $mech->create_user_ok('updates@example.com', name => 'Update User');
 
     my $dt = DateTime->now->add( minutes => -30 );
     my $r_dt = $dt->clone->add( minutes => 20 );
@@ -812,17 +790,11 @@ subtest 'check setting inlude dates in new updates cobrand option' => sub {
     $mech->delete_user( 'reporter@example.com' );
     $mech->delete_user( 'alerts@example.com' );
 
-    my $user1 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'reporter@example.com', name => 'Reporter User' } );
-    ok $user1, "created test user";
+    my $user1 = $mech->create_user_ok('reporter@example.com', name => 'Reporter User');
 
-    my $user2 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'alerts@example.com', name => 'Alert User' } );
-    ok $user2, "created test user";
+    my $user2 = $mech->create_user_ok('alerts@example.com', name => 'Alert User');
 
-    my $user3 = FixMyStreet::App->model('DB::User')
-      ->find_or_create( { email => 'updates@example.com', name => 'Update User' } );
-    ok $user3, "created test user";
+    my $user3 = $mech->create_user_ok('updates@example.com', name => 'Update User');
 
     my $dt = DateTime->now->add( minutes => -30 );
     my $r_dt = $dt->clone->add( minutes => 20 );
diff --git a/t/app/controller/contact.t b/t/app/controller/contact.t
index 4ac69a9f8..1b0f09a85 100644
--- a/t/app/controller/contact.t
+++ b/t/app/controller/contact.t
@@ -52,12 +52,7 @@ for my $test (
   )
 {
     subtest 'check reporting a problem displays correctly' => sub {
-        my $user = FixMyStreet::App->model('DB::User')->find_or_create(
-            {
-                name  => $test->{name},
-                email => $test->{email}
-            }
-        );
+        my $user = $mech->create_user_ok($test->{email}, name => $test->{name});
 
         my $problem = FixMyStreet::App->model('DB::Problem')->create(
             {
@@ -80,12 +75,8 @@ for my $test (
 
         if ( $test->{update} ) {
             my $update_info = $test->{update};
-            my $update_user = FixMyStreet::App->model('DB::User')->find_or_create(
-                {
-                    name  => $update_info->{name},
-                    email => $update_info->{email}
-                }
-            );
+            my $update_user = $mech->create_user_ok($update_info->{email},
+                name => $update_info->{name});
 
             $update = FixMyStreet::App->model('DB::Comment')->create(
                 {
diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t
index 3829873e5..5ea5cb9f5 100644
--- a/t/app/controller/dashboard.t
+++ b/t/app/controller/dashboard.t
@@ -16,14 +16,9 @@ my $test_ward = 20723;
 my $body = $mech->create_body_ok($test_council, 'City of Edinburgh Council');
 
 $mech->delete_user( $test_user );
-my $user = FixMyStreet::App->model('DB::User')->create( {
-    email => $test_user,
-    password => $test_pass,
-} );
-
-my $p_user = FixMyStreet::App->model('DB::User')->find_or_create( {
-    email => 'p_user@example.com'
-} );
+my $user = $mech->create_user_ok($test_user, password => $test_pass);
+
+my $p_user = $mech->create_user_ok('p_user@example.com');
 
 # Dashboard tests assume we are not too early in year, to allow reporting
 # within same year, as a convenience.
diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t
index 38216c708..14c751115 100644
--- a/t/app/controller/moderate.t
+++ b/t/app/controller/moderate.t
@@ -15,9 +15,7 @@ my $body = $mech->create_body_ok( $BROMLEY_ID, 'Bromley Council' );
 
 my $dt = DateTime->now;
 
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test-moderation@example.com', name => 'Test User' } );
+my $user = $mech->create_user_ok('test-moderation@example.com', name => 'Test User');
 $user->user_body_permissions->delete_all;
 $user->discard_changes;
 
diff --git a/t/app/controller/questionnaire.t b/t/app/controller/questionnaire.t
index 7bc6545db..7a46e48bd 100644
--- a/t/app/controller/questionnaire.t
+++ b/t/app/controller/questionnaire.t
@@ -17,10 +17,7 @@ $mech->clear_emails_ok;
 # create a test user and report
 $mech->delete_user('test@example.com');
 
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test@example.com', name => 'Test User' } );
-ok $user, "created test user";
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
 
 my $dt = DateTime->now()->subtract( weeks => 5 );
 my $report_time = $dt->ymd . ' ' . $dt->hms;
diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t
index 265760d86..fb532ddc4 100644
--- a/t/app/controller/report_display.t
+++ b/t/app/controller/report_display.t
@@ -12,15 +12,9 @@ my $mech = FixMyStreet::TestMech->new;
 
 # create a test user and report
 $mech->delete_user('test@example.com');
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test@example.com', name => 'Test User' } );
-ok $user, "created test user";
-
-my $user2 =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test2@example.com', name => 'Other User' } );
-ok $user2, "created test user";
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
+
+my $user2 = $mech->create_user_ok('test2@example.com', name => 'Other User');
 
 my $dt = DateTime->new(
     year   => 2011,
diff --git a/t/app/controller/report_interest_count.t b/t/app/controller/report_interest_count.t
index 4e86789ba..3cb80ea5f 100644
--- a/t/app/controller/report_interest_count.t
+++ b/t/app/controller/report_interest_count.t
@@ -22,10 +22,7 @@ my $mech = FixMyStreet::TestMech->new;
 
 # create a test user and report
 $mech->delete_user('test@example.com');
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test@example.com', name => 'Test User' } );
-ok $user, "created test user";
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
 
 my $dt = DateTime->new(
     year   => 2011,
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 6ea4c9523..2aebfa00b 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -654,8 +654,7 @@ subtest "test password errors for a user who is signing in as they report" => su
     # check that the user does not exist
     my $test_email = 'test-2@example.com';
 
-    my $user = FixMyStreet::App->model('DB::User')->find_or_create( { email => $test_email } );
-    ok $user, "test user does exist";
+    my $user = $mech->create_user_ok($test_email);
 
     # setup the user.
     ok $user->update( {
@@ -707,8 +706,7 @@ subtest "test report creation for a user who is signing in as they report" => su
     # check that the user does not exist
     my $test_email = 'test-2@example.com';
 
-    my $user = FixMyStreet::App->model('DB::User')->find_or_create( { email => $test_email } );
-    ok $user, "test user does exist";
+    my $user = $mech->create_user_ok($test_email);
 
     # setup the user.
     ok $user->update( {
@@ -909,8 +907,7 @@ subtest "test report creation for a category that is non public" => sub {
     # check that the user does not exist
     my $test_email = 'test-2@example.com';
 
-    my $user = FixMyStreet::App->model('DB::User')->find_or_create( { email => $test_email } );
-    ok $user, "test user does exist";
+    my $user = $mech->create_user_ok($test_email);
 
     $contact1->update( { non_public => 1 } );
 
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index 2a3c7c0b3..e077a07c9 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -13,15 +13,9 @@ my $mech = FixMyStreet::TestMech->new;
 $mech->delete_user('commenter@example.com');
 $mech->delete_user('test@example.com');
 
-my $user =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'test@example.com', name => 'Test User' } );
-ok $user, "created test user";
-
-my $user2 =
-  FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'commenter@example.com', name => 'Commenter' } );
-ok $user2, "created comment user";
+my $user = $mech->create_user_ok('test@example.com', name => 'Test User');
+
+my $user2 = $mech->create_user_ok('commenter@example.com', name => 'Commenter');
 
 my $body = $mech->create_body_ok(2504, 'Westminster City Council');
 
diff --git a/t/app/controller/rss.t b/t/app/controller/rss.t
index 3e820cff3..8b39219fa 100644
--- a/t/app/controller/rss.t
+++ b/t/app/controller/rss.t
@@ -13,8 +13,7 @@ my $dt = DateTime->new(
     day     => 10
 );
 
-my $user1 = FixMyStreet::App->model('DB::User')
-  ->find_or_create( { email => 'reporter-rss@example.com', name => 'Reporter User' } );
+my $user1 = $mech->create_user_ok('reporter-rss@example.com', name => 'Reporter User');
 
 my $dt_parser = FixMyStreet::App->model('DB')->schema->storage->datetime_parser;
 
diff --git a/t/app/controller/token.t b/t/app/controller/token.t
index 9ca8b905d..ac88f4f7a 100644
--- a/t/app/controller/token.t
+++ b/t/app/controller/token.t
@@ -6,10 +6,8 @@ use utf8;
 use FixMyStreet::TestMech;
 use FixMyStreet::App;
 
-my $user = FixMyStreet::App->model('DB::User')->find_or_create({
-        name => 'Bob', email => 'bob@example.com',
-    });
 my $mech = FixMyStreet::TestMech->new;
+my $user = $mech->create_user_ok('bob@example.com', name => 'Bob');
 
 subtest 'Zurich special case for C::Tokens->problem_confirm' => sub {
     FixMyStreet::override_config {
diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t
index a595f48c9..e16738b30 100644
--- a/t/cobrand/zurich.t
+++ b/t/cobrand/zurich.t
@@ -698,7 +698,7 @@ subtest "only superuser can edit bodies" => sub {
     }, sub {
         $mech->get( '/admin/body/' . $zurich->id );
     };
-    is $mech->res->code, 404, "only superuser should be able to edit bodies";
+    is $mech->res->code, 403, "only superuser should be able to edit bodies";
     $mech->log_out_ok;
 };
 
diff --git a/templates/web/base/admin/bodies.html b/templates/web/base/admin/bodies.html
index 4c95423c0..e98e2d350 100644
--- a/templates/web/base/admin/bodies.html
+++ b/templates/web/base/admin/bodies.html
@@ -1,5 +1,9 @@
 [% INCLUDE 'admin/header.html' title=loc('Bodies') -%]
 
+[% IF body_errors.size %]
+    <p class="error">[% loc('Please correct the errors below') %]</p>
+[% END %]
+
 [% IF bodies.size == 0 %]
   <p class="fms-admin-info">
     [% loc('Currently no bodies have been created.') %]
@@ -69,7 +73,7 @@
   </table>
 [% END %]
 
-[% IF c.cobrand.moniker != 'zurich' OR admin_type == 'super' %]
+[% IF (c.cobrand.moniker == 'zurich' AND admin_type == 'super') OR c.user.is_superuser %]
     <div class="admin-box">
       <h2>[% loc('Add body') %]</h2>
       [% INCLUDE 'admin/body-form.html', body='' %]
diff --git a/templates/web/base/admin/body-form.html b/templates/web/base/admin/body-form.html
index 8c4956f7f..13e688097 100644
--- a/templates/web/base/admin/body-form.html
+++ b/templates/web/base/admin/body-form.html
@@ -6,6 +6,12 @@
           categories of problem) to each body."
         ) %]
     </div>
+
+    [% IF body_errors.name %]
+      <div class="fms-admin-warning">
+        [% body_errors.name %]
+      </div>
+    [% END %]
     <div class="admin-hint">
       <p>
         [% loc(
diff --git a/templates/web/base/admin/body.html b/templates/web/base/admin/body.html
index 15802fc44..a00c5ca4c 100644
--- a/templates/web/base/admin/body.html
+++ b/templates/web/base/admin/body.html
@@ -7,6 +7,10 @@
   </p>
 [% END %]
 
+[% IF body_errors.size %]
+    <p class="error">[% loc('Please correct the errors below') %]</p>
+[% END %]
+
 [% IF NOT errors %]
 
 <p>
@@ -213,7 +217,7 @@
   </form>
 </div>
 
-[% IF NOT errors %]
+[% IF NOT errors and c.user.is_superuser %]
 <div class="admin-box">
   <h2>[% loc('Edit body details') %]</h2>
   [% INCLUDE 'admin/body-form.html' %]
diff --git a/templates/web/base/admin/index.html b/templates/web/base/admin/index.html
index 3c510471e..beb4dad7f 100644
--- a/templates/web/base/admin/index.html
+++ b/templates/web/base/admin/index.html
@@ -30,19 +30,21 @@ and to receive notices of updates.
         <input type="text" name="search"  size="30" id="search_users" value="[% searched | html %]">
 </form>
 
-<form method="get" action="[% c.uri_for('bodies') %]">
-<label for="search_body">[% loc('Edit body details') %]</label>
-<select id="search_body" name="body">
-[% FOREACH body IN bodies %]
-  [%- SET id = body.id %]
-    <option[% IF body.deleted %] class="adminhidden"[% END %] value="[% body.id %]">
-        [% body.name | html %]
-        [%- IF body.parent %], [% body.parent.name | html %][% END -%]
-    </option>
+[% IF c.user.is_superuser %]
+  <form method="get" action="[% c.uri_for('bodies') %]">
+  <label for="search_body">[% loc('Edit body details') %]</label>
+  <select id="search_body" name="body">
+  [% FOREACH body IN bodies %]
+    [%- SET id = body.id %]
+      <option[% IF body.deleted %] class="adminhidden"[% END %] value="[% body.id %]">
+          [% body.name | html %]
+          [%- IF body.parent %], [% body.parent.name | html %][% END -%]
+      </option>
+  [% END %]
+  </select>
+  <input type="submit" value="[% loc('Go') %]">
+  </form>
 [% END %]
-</select>
-<input type="submit" value="[% loc('Go') %]">
-</form>
 
 [% IF unsent_reports.size %]
 <h2>[% loc('Reports waiting to be sent') %]</h2>
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') %]" >