Add 'verified' database columns for email/phone.

These are so we can state whether a user's email address or phone number
have been verified by confirmation email/text.

15 files changed, 95 insertions, 22 deletions

diff --git a/bin/update-schema b/bin/update-schema
index 3f86bdacb..fea316bd6 100755
--- a/bin/update-schema
+++ b/bin/update-schema
@@ -212,6 +212,7 @@ else {
 # (assuming schema change files are never half-applied, which should be the case)
 sub get_db_version {
     return 'EMPTY' if ! table_exists('problem');
+    return '0056' if column_exists('users', 'email_verified');
     return '0055' if column_exists('response_priorities', 'is_default');
     return '0054' if table_exists('state');
     return '0053' if table_exists('report_extra_fields');
diff --git a/db/downgrade_0056---0055.sql b/db/downgrade_0056---0055.sql
new file mode 100644
index 000000000..75b69dd4c
--- /dev/null
+++ b/db/downgrade_0056---0055.sql
@@ -0,0 +1,10 @@
+BEGIN;
+
+ALTER TABLE users DROP email_verified;
+ALTER TABLE users DROP phone_verified;
+
+DELETE FROM users WHERE email IS NULL;
+ALTER TABLE users ALTER email SET NOT NULL;
+ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email);
+
+COMMIT;
diff --git a/db/schema.sql b/db/schema.sql
index f428ff59d..f2197dc52 100644
--- a/db/schema.sql
+++ b/db/schema.sql
@@ -21,9 +21,11 @@ create table sessions (
 -- users table
 create table users (
     id              serial  not null primary key,
-    email           text    not null unique,
+    email           text,
+    email_verified  boolean not null default 'f',
     name            text,
     phone           text,
+    phone_verified  boolean not null default 'f',
     password        text    not null default '',
     from_body       integer,
     flagged         boolean not null default 'f',
@@ -34,6 +36,8 @@ create table users (
     area_id         integer,
     extra           text
 );
+CREATE UNIQUE INDEX users_email_verified_unique ON users (email) WHERE email_verified;
+CREATE UNIQUE INDEX users_phone_verified_unique ON users (phone) WHERE phone_verified;
 
 -- Record details of reporting bodies, including open311 configuration details
 create table body (
diff --git a/db/schema_0056-phone-login.sql b/db/schema_0056-phone-login.sql
new file mode 100644
index 000000000..f5e0b07e4
--- /dev/null
+++ b/db/schema_0056-phone-login.sql
@@ -0,0 +1,12 @@
+BEGIN;
+
+ALTER TABLE users ADD email_verified boolean not null default 'f';
+UPDATE USERS set email_verified = 't';
+ALTER TABLE users ADD phone_verified boolean not null default 'f';
+
+ALTER TABLE users ALTER email DROP NOT NULL;
+ALTER TABLE users DROP CONSTRAINT users_email_key;
+CREATE UNIQUE INDEX users_email_verified_unique ON users (email) WHERE email_verified;
+CREATE UNIQUE INDEX users_phone_verified_unique ON users (phone) WHERE phone_verified;
+
+COMMIT;
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index a47e74f19..71416622a 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -1316,13 +1316,14 @@ sub user_add : Path('user_edit') : Args(0) {
     my $user = $c->model('DB::User')->find_or_create( {
         name => $c->get_param('name'),
         email => lc $c->get_param('email'),
+        email_verified => 1,
         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'
+        key => 'users_email_verified_key'
     } );
     $c->stash->{user} = $user;
     $c->forward('user_cobrand_extra_fields');
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 3e90fb7ca..3eb724ddd 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -76,7 +76,7 @@ sub sign_in : Private {
 
     if (   $email
         && $password
-        && $c->authenticate( { email => $email, password => $password } ) )
+        && $c->authenticate( { email => $email, email_verified => 1, password => $password } ) )
     {
 
         # unless user asked to be remembered limit the session to browser
@@ -233,7 +233,7 @@ sub token : Path('/M') : Args(1) {
     $user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id};
     $user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id};
     $user->update_or_insert;
-    $c->authenticate( { email => $user->email }, 'no_password' );
+    $c->authenticate( { email => $user->email, email_verified => 1 }, 'no_password' );
 
     # send the user to their page
     $c->detach( 'redirect_on_signin', [ $data->{r}, $data->{p} ] );
diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm
index 17ace0205..097cac984 100644
--- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm
@@ -185,7 +185,7 @@ sub oauth_success : Private {
 
     # If we've got here with a full user, log in
     if ($user) {
-        $c->authenticate( { email => $user->email }, 'no_password' );
+        $c->authenticate( { email => $user->email, email_verified => 1 }, 'no_password' );
         $c->stash->{login_success} = 1;
     }
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 562f9445a..3f940d838 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -354,8 +354,12 @@ sub report_import : Path('/import') {
     my $report_user = $c->model('DB::User')->find_or_create(
         {
             email => lc $input{email},
+            email_verified => 1,
             name  => $input{name},
             phone => $input{phone}
+        },
+        {
+            key => 'users_email_verified_key'
         }
     );
 
@@ -447,7 +451,7 @@ sub initialize_report : Private {
 
             if ($report) {
                 # log the problem creation user in to the site
-                $c->authenticate( { email => $report->user->email },
+                $c->authenticate( { email => $report->user->email, email_verified => 1 },
                     'no_password' );
 
                 # save the token to delete at the end
diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm
index a1b0c57ba..1d4438828 100644
--- a/perllib/FixMyStreet/App/Controller/Tokens.pm
+++ b/perllib/FixMyStreet/App/Controller/Tokens.pm
@@ -109,7 +109,7 @@ sub confirm_problem : Path('/P') {
         $problem->user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id};
         $problem->user->update;
     }
-    $c->authenticate( { email => $problem->user->email }, 'no_password' );
+    $c->authenticate( { email => $problem->user->email, email_verified => 1 }, 'no_password' );
     $c->set_session_cookie_expire(0);
 
     $c->stash->{created_report} = 'fromemail';
@@ -170,7 +170,7 @@ sub confirm_alert : Path('/A') {
     }
 
     if (!$alert->confirmed && $c->stash->{confirm_type} ne 'unsubscribe') {
-        $c->authenticate( { email => $alert->user->email }, 'no_password' );
+        $c->authenticate( { email => $alert->user->email, email_verified => 1 }, 'no_password' );
         $c->set_session_cookie_expire(0);
     }
 
@@ -237,7 +237,7 @@ sub confirm_update : Path('/C') {
         $comment->user->update;
     }
 
-    $c->authenticate( { email => $comment->user->email }, 'no_password' );
+    $c->authenticate( { email => $comment->user->email, email_verified => 1 }, 'no_password' );
     $c->set_session_cookie_expire(0);
 
     $c->forward('/report/update/confirm');
@@ -269,7 +269,7 @@ sub questionnaire : Path('/Q') : Args(1) {
 
     my $questionnaire = $c->stash->{questionnaire};
     if (!$questionnaire->whenanswered) {
-        $c->authenticate( { email => $questionnaire->problem->user->email }, 'no_password' );
+        $c->authenticate( { email => $questionnaire->problem->user->email, email_verified => 1 }, 'no_password' );
         $c->set_session_cookie_expire(0);
     }
     $c->forward( '/questionnaire/show' );
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index 19adf5d49..b733b47bd 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -19,7 +19,7 @@ __PACKAGE__->add_columns(
     sequence          => "users_id_seq",
   },
   "email",
-  { data_type => "text", is_nullable => 0 },
+  { data_type => "text", is_nullable => 1 },
   "name",
   { data_type => "text", is_nullable => 1 },
   "phone",
@@ -30,21 +30,24 @@ __PACKAGE__->add_columns(
   { data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
   "flagged",
   { data_type => "boolean", default_value => \"false", is_nullable => 0 },
+  "is_superuser",
+  { data_type => "boolean", default_value => \"false", is_nullable => 0 },
   "title",
   { data_type => "text", is_nullable => 1 },
   "twitter_id",
   { data_type => "bigint", is_nullable => 1 },
   "facebook_id",
   { data_type => "bigint", is_nullable => 1 },
-  "is_superuser",
-  { data_type => "boolean", default_value => \"false", is_nullable => 0 },
   "area_id",
   { data_type => "integer", is_nullable => 1 },
   "extra",
   { data_type => "text", is_nullable => 1 },
+  "email_verified",
+  { data_type => "boolean", default_value => \"false", is_nullable => 0 },
+  "phone_verified",
+  { data_type => "boolean", default_value => \"false", is_nullable => 0 },
 );
 __PACKAGE__->set_primary_key("id");
-__PACKAGE__->add_unique_constraint("users_email_key", ["email"]);
 __PACKAGE__->add_unique_constraint("users_facebook_id_key", ["facebook_id"]);
 __PACKAGE__->add_unique_constraint("users_twitter_id_key", ["twitter_id"]);
 __PACKAGE__->has_many(
@@ -102,8 +105,13 @@ __PACKAGE__->has_many(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07035 @ 2016-09-16 14:22:10
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:7wfF1VnZax2QTXCIPXr+vg
+# Created by DBIx::Class::Schema::Loader v0.07035 @ 2017-09-19 18:02:17
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:OKHKCSahWD3Ov6ulj+2f/w
+
+# These are not fully unique constraints (they only are when the *_verified
+# is true), but this is managed in ResultSet::User's find() wrapper.
+__PACKAGE__->add_unique_constraint("users_email_verified_key", ["email", "email_verified"]);
+__PACKAGE__->add_unique_constraint("users_phone_verified_key", ["phone", "phone_verified"]);
 
 __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn");
 __PACKAGE__->rabx_column('extra');
diff --git a/perllib/FixMyStreet/DB/ResultSet/User.pm b/perllib/FixMyStreet/DB/ResultSet/User.pm
index 7e657a936..9a8a50559 100644
--- a/perllib/FixMyStreet/DB/ResultSet/User.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/User.pm
@@ -4,5 +4,40 @@ use base 'DBIx::Class::ResultSet';
 use strict;
 use warnings;
 
+use Moo;
+
+# The database has a partial unique index on email (when email_verified is
+# true), and phone (when phone_verified is true). In the code, we can only
+# say these are fully unique indices, which they aren't, as there could be
+# multiple identical unverified phone numbers.
+#
+# We assume that any and all calls to find (also called using find_or_new,
+# find_or_create, or update_or_new/create) are to look up verified entries
+# only (it would make no sense to find() a non-unique entry). Therefore we
+# help the code along by specifying the most appropriate key to use, given
+# the data provided, and setting the appropriate verified boolean.
+
+around find => sub {
+    my ($orig, $self) = (shift, shift);
+    # If there's already a key, assume caller knows what they're doing
+    if (ref $_[0] eq 'HASH' && !$_[1]->{key}) {
+        if ($_[0]->{id}) {
+            $_[1]->{key} = 'primary';
+        } elsif (exists $_[0]->{email} && exists $_[0]->{phone}) {
+            # If there's both email and phone, caller must also have specified
+            # a verified boolean so that we know what we're looking for
+            if (!$_[0]->{email_verified} && !$_[0]->{phone_verified}) {
+                die "Cannot perform a User find() with both email and phone and no verified";
+            }
+        } elsif (exists $_[0]->{email}) {
+            $_[0]->{email_verified} = 1;
+            $_[1]->{key} = 'users_email_verified_key';
+        } elsif (exists $_[0]->{phone}) {
+            $_[0]->{phone_verified} = 1;
+            $_[1]->{key} = 'users_phone_verified_key';
+        }
+    }
+    $self->$orig(@_);
+};
 
 1;
diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm
index 4bad1d17b..406382f1d 100644
--- a/perllib/FixMyStreet/TestMech.pm
+++ b/perllib/FixMyStreet/TestMech.pm
@@ -667,8 +667,7 @@ sub create_problems_for_body {
     my $dt = $params->{dt} || DateTime->now();
 
     my $user = $params->{user} ||
-      FixMyStreet::DB->resultset('User')
-      ->find_or_create( { email => 'test@example.com', name => 'Test User' } );
+      FixMyStreet::DB->resultset('User')->find_or_create( { email => 'test@example.com', name => 'Test User' } );
 
     delete $params->{user};
     delete $params->{dt};
diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t
index 1122518ca..883dc2003 100644
--- a/t/app/controller/auth_profile.t
+++ b/t/app/controller/auth_profile.t
@@ -102,7 +102,7 @@ subtest "Test change email page" => sub {
     $mech->content_contains('successfully confirmed');
     ok(FixMyStreet::App->model('DB::User')->find( { email => $test_email2 } ), "got a user");
 
-    ok(FixMyStreet::App->model('DB::User')->create( { email => $test_email } ), "created old user");
+    ok(FixMyStreet::App->model('DB::User')->create( { email => $test_email, email_verified => 1 } ), "created old user");
     $mech->submit_form_ok({ with_fields => { email => $test_email } },
         "change_email back to $test_email"
     );
diff --git a/t/app/model/extra.t b/t/app/model/extra.t
index a5e3e3574..55accb086 100644
--- a/t/app/model/extra.t
+++ b/t/app/model/extra.t
@@ -101,6 +101,7 @@ subtest 'Default hash layout' => sub {
 subtest 'Get named field values' => sub {
     my $user = $db->resultset('User')->create({
         email => 'test-moderation@example.com',
+        email_verified => 1,
         name => 'Test User'
     });
     my $report = $db->resultset('Problem')->create(
diff --git a/t/app/model/photoset.t b/t/app/model/photoset.t
index 4aa5c8992..d171ba88b 100644
--- a/t/app/model/photoset.t
+++ b/t/app/model/photoset.t
@@ -12,9 +12,7 @@ my $UPLOAD_DIR = tempdir( CLEANUP => 1 );
 
 my $db = FixMyStreet::DB->schema;
 
-my $user = $db->resultset('User')->find_or_create({
-        name => 'Bob', email => 'bob@example.com',
-});
+my $user = $db->resultset('User')->find_or_create({ name => 'Bob', email => 'bob@example.com' });
 
 FixMyStreet::override_config {
     UPLOAD_DIR => $UPLOAD_DIR,