diff options
| author | Matthew Somerville <matthew-github@dracos.co.uk> | 2017-09-30 22:14:48 +0100 |
|---|---|---|
| committer | Matthew Somerville <matthew-github@dracos.co.uk> | 2017-09-30 22:14:48 +0100 |
| commit | 8e9cd7cdb0a0f0fe591c2ecc25ae480fa0a5f799 (patch) | |
| tree | de46361b23538b6ba9644898a9f5a55b4aeed9e4 | |
| parent | a369e1c1a178697d0f92cf82c5bdb23c1883925e (diff) | |
| parent | 83825ebf47cfd40ac649853347c8352944a3d32a (diff) | |
Merge branch 'phone-number-work'
88 files changed, 3471 insertions, 1137 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ea73749e..6eb66566a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ * Unreleased - New features: + - Optional verification of reports and updates, and logging in, + using confirmation by phone text. + - Improved email/phone management in your profile. - Area summary statistics page in admin #1834 - Bugfixes - Shortlist menu item always remains a link #1855 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/conf/general.yml-example b/conf/general.yml-example index 345a6426d..528fc1012 100644 --- a/conf/general.yml-example +++ b/conf/general.yml-example @@ -205,6 +205,13 @@ TESTING_COUNCILS: '' # if you're using Message Manager, include the URL here (see https://github.com/mysociety/message-manager/) MESSAGE_MANAGER_URL: '' +# If you enable login via SMS authentication, you'll need a twilio account +SMS_AUTHENTICATION: 0 +PHONE_COUNTRY: '' +TWILIO_ACCOUNT_SID: '' +TWILIO_AUTH_TOKEN: '' +TWILIO_FROM_PARAMETER: '' + # If you want to hide all pages from non-logged-in users, set this to 1. LOGIN_REQUIRED: 0 @@ -80,6 +80,7 @@ requires 'Net::Domain::TLD', '1.75'; requires 'Net::Facebook::Oauth2', '0.10'; requires 'Net::OAuth'; requires 'Net::Twitter::Lite::WithAPIv1_1', '0.12008'; +requires 'Number::Phone'; requires 'Path::Class'; requires 'POSIX'; requires 'Readonly'; @@ -92,6 +93,7 @@ requires 'Text::CSV'; requires 'URI', '1.71'; requires 'URI::Escape'; requires 'URI::QueryParam'; +requires 'WWW::Twilio::API'; requires 'XML::RSS'; requires 'XML::Simple'; requires 'YAML'; diff --git a/cpanfile.snapshot b/cpanfile.snapshot index 238e54c1c..3e3dec251 100644 --- a/cpanfile.snapshot +++ b/cpanfile.snapshot @@ -116,24 +116,29 @@ DISTRIBUTIONS IO::Scalar 0 Module::Build 0.36 Test::More 0 - CPAN-Meta-2.132140 - pathname: D/DA/DAGOLDEN/CPAN-Meta-2.132140.tar.gz - provides: - CPAN::Meta 2.132140 - CPAN::Meta::Converter 2.132140 - CPAN::Meta::Feature 2.132140 - CPAN::Meta::History 2.132140 - CPAN::Meta::Prereqs 2.132140 - CPAN::Meta::Spec 2.132140 - CPAN::Meta::Validator 2.132140 + CPAN-Meta-2.150010 + pathname: D/DA/DAGOLDEN/CPAN-Meta-2.150010.tar.gz + provides: + CPAN::Meta 2.150010 + CPAN::Meta::Converter 2.150010 + CPAN::Meta::Feature 2.150010 + CPAN::Meta::History 2.150010 + CPAN::Meta::Merge 2.150010 + CPAN::Meta::Prereqs 2.150010 + CPAN::Meta::Spec 2.150010 + CPAN::Meta::Validator 2.150010 + Parse::CPAN::Meta 2.150010 requirements: CPAN::Meta::Requirements 2.121 - CPAN::Meta::YAML 0.008 + CPAN::Meta::YAML 0.011 Carp 0 - ExtUtils::MakeMaker 6.30 - JSON::PP 2.27200 - Parse::CPAN::Meta 1.4403 + Encode 0 + Exporter 0 + ExtUtils::MakeMaker 6.17 + File::Spec 0.80 + JSON::PP 2.27300 Scalar::Util 0 + perl 5.008001 strict 0 version 0.88 warnings 0 @@ -167,22 +172,19 @@ DISTRIBUTIONS strict 0 version 0.77 warnings 0 - CPAN-Meta-YAML-0.008 - pathname: D/DA/DAGOLDEN/CPAN-Meta-YAML-0.008.tar.gz + CPAN-Meta-YAML-0.018 + pathname: D/DA/DAGOLDEN/CPAN-Meta-YAML-0.018.tar.gz provides: - CPAN::Meta::YAML 0.008 + CPAN::Meta::YAML 0.018 requirements: + B 0 Carp 0 Exporter 0 ExtUtils::MakeMaker 6.17 - File::Find 0 - File::Spec 0 - File::Spec::Functions 0 - File::Temp 0 + Fcntl 0 Scalar::Util 0 - Test::More 0 + perl 5.008001 strict 0 - vars 0 warnings 0 CSS-Sass-3.3.3 pathname: O/OC/OCBNET/CSS-Sass-3.3.3.tar.gz @@ -1307,6 +1309,47 @@ DISTRIBUTIONS Try::Tiny 0 namespace::clean 0.23 perl 5.008001 + DBM-Deep-2.0014 + pathname: R/RK/RKINYON/DBM-Deep-2.0014.tar.gz + provides: + DBM::Deep 2.0014 + DBM::Deep::Array undef + DBM::Deep::Engine undef + DBM::Deep::Engine::DBI undef + DBM::Deep::Engine::File undef + DBM::Deep::Hash undef + DBM::Deep::Iterator undef + DBM::Deep::Iterator::DBI undef + DBM::Deep::Iterator::File undef + DBM::Deep::Iterator::File::BucketList undef + DBM::Deep::Iterator::File::Index undef + DBM::Deep::Null undef + DBM::Deep::Sector undef + DBM::Deep::Sector::DBI undef + DBM::Deep::Sector::DBI::Reference undef + DBM::Deep::Sector::DBI::Scalar undef + DBM::Deep::Sector::File undef + DBM::Deep::Sector::File::BucketList undef + DBM::Deep::Sector::File::Data undef + DBM::Deep::Sector::File::Index undef + DBM::Deep::Sector::File::Null undef + DBM::Deep::Sector::File::Reference undef + DBM::Deep::Sector::File::Scalar undef + DBM::Deep::Storage undef + DBM::Deep::Storage::DBI undef + DBM::Deep::Storage::File undef + requirements: + Digest::MD5 1.00 + Fcntl 0.01 + File::Path 0.01 + File::Temp 0.01 + Pod::Usage 1.3 + Scalar::Util 1.14 + Test::Deep 0.095 + Test::Exception 0.21 + Test::More 0.88 + Test::Warn 0.08 + perl 5.008_004 Data-Compare-1.22 pathname: D/DC/DCANTRELL/Data-Compare-1.22.tar.gz provides: @@ -2914,17 +2957,31 @@ DISTRIBUTIONS File::Spec 3.29 Test::More 0.42 perl 5.00503 - File-ShareDir-1.03 - pathname: A/AD/ADAMK/File-ShareDir-1.03.tar.gz + File-ShareDir-1.104 + pathname: R/RE/REHSACK/File-ShareDir-1.104.tar.gz provides: - File::ShareDir 1.03 + File::ShareDir 1.104 requirements: Carp 0 Class::Inspector 1.12 - ExtUtils::MakeMaker 6.42 + ExtUtils::MakeMaker 0 + File::ShareDir::Install 0.03 File::Spec 0.80 - Test::More 0.47 - perl 5.005 + perl 5.008001 + warnings 0 + File-ShareDir-Install-0.11 + pathname: E/ET/ETHER/File-ShareDir-Install-0.11.tar.gz + provides: + File::ShareDir::Install 0.11 + requirements: + Carp 0 + Exporter 0 + File::Spec 0 + IO::Dir 0 + Module::Build::Tiny 0.034 + perl 5.008 + strict 0 + warnings 0 File-Slurp-9999.19 pathname: U/UR/URI/File-Slurp-9999.19.tar.gz provides: @@ -3310,14 +3367,15 @@ DISTRIBUTIONS JSON::PP 2.27202 Scalar::Util 0 perl 5.006 - JSON-PP-2.27202 - pathname: M/MA/MAKAMAKA/JSON-PP-2.27202.tar.gz + JSON-PP-2.94 + pathname: I/IS/ISHIGAKI/JSON-PP-2.94.tar.gz provides: - JSON::PP 2.27202 - JSON::PP::Boolean 2.27202 - JSON::PP::IncrParser 2.27202 + JSON::PP 2.94 + JSON::PP::Boolean 2.94 + JSON::PP::IncrParser 2.94 requirements: ExtUtils::MakeMaker 0 + Scalar::Util 1.08 Test::More 0 LWP-MediaTypes-6.02 pathname: G/GA/GAAS/LWP-MediaTypes-6.02.tar.gz @@ -3620,35 +3678,30 @@ DISTRIBUTIONS requirements: ExtUtils::MakeMaker 0 Memoize 0.52 - Module-Build-0.4007 - pathname: L/LE/LEONT/Module-Build-0.4007.tar.gz - provides: - Module::Build 0.4007 - Module::Build::Base 0.4007 - Module::Build::Compat 0.4007 - Module::Build::Config 0.4007 - Module::Build::Cookbook 0.4007 - Module::Build::Dumper 0.4007 - Module::Build::ModuleInfo 0.4007 - Module::Build::Notes 0.4007 - Module::Build::PPMMaker 0.4007 - Module::Build::Platform::Default 0.4007 - Module::Build::Platform::MacOS 0.4007 - Module::Build::Platform::Unix 0.4007 - Module::Build::Platform::VMS 0.4007 - Module::Build::Platform::VOS 0.4007 - Module::Build::Platform::Windows 0.4007 - Module::Build::Platform::aix 0.4007 - Module::Build::Platform::cygwin 0.4007 - Module::Build::Platform::darwin 0.4007 - Module::Build::Platform::os2 0.4007 - Module::Build::PodParser 0.4007 - Module::Build::Version 0.87 - Module::Build::YAML 1.41 - inc::latest 0.4007 - inc::latest::private 0.4007 - requirements: - CPAN::Meta 2.110420 + Module-Build-0.4224 + pathname: L/LE/LEONT/Module-Build-0.4224.tar.gz + provides: + Module::Build 0.4224 + Module::Build::Base 0.4224 + Module::Build::Compat 0.4224 + Module::Build::Config 0.4224 + Module::Build::Cookbook 0.4224 + Module::Build::Dumper 0.4224 + Module::Build::Notes 0.4224 + Module::Build::PPMMaker 0.4224 + Module::Build::Platform::Default 0.4224 + Module::Build::Platform::MacOS 0.4224 + Module::Build::Platform::Unix 0.4224 + Module::Build::Platform::VMS 0.4224 + Module::Build::Platform::VOS 0.4224 + Module::Build::Platform::Windows 0.4224 + Module::Build::Platform::aix 0.4224 + Module::Build::Platform::cygwin 0.4224 + Module::Build::Platform::darwin 0.4224 + Module::Build::Platform::os2 0.4224 + Module::Build::PodParser 0.4224 + requirements: + CPAN::Meta 2.142060 CPAN::Meta::YAML 0.003 Cwd 0 Data::Dumper 0 @@ -3669,7 +3722,7 @@ DISTRIBUTIONS Parse::CPAN::Meta 1.4401 Perl::OSType 1 Pod::Man 2.17 - Test::Harness 3.16 + TAP::Harness 3.29 Test::More 0.49 Text::Abbrev 0 Text::ParseWords 0 @@ -4442,6 +4495,306 @@ DISTRIBUTIONS Carp 0 ExtUtils::MakeMaker 0 POSIX 0 + Number-Phone-3.4002 + pathname: D/DC/DCANTRELL/Number-Phone-3.4002.tar.gz + provides: + Number::Phone 3.4002 + Number::Phone::Country 1.93 + Number::Phone::Country::Data 1.6 + Number::Phone::Formatter::Raw undef + Number::Phone::Lib 1.0 + Number::Phone::NANP 1.5 + Number::Phone::NANP::AG 1.1 + Number::Phone::NANP::AI 1.1 + Number::Phone::NANP::AS 1.1 + Number::Phone::NANP::BB 1.1 + Number::Phone::NANP::BM 1.1 + Number::Phone::NANP::BS 1.1 + Number::Phone::NANP::CA 1.1 + Number::Phone::NANP::DM 1.1 + Number::Phone::NANP::DO 1.1 + Number::Phone::NANP::Data 1.20170908113144 + Number::Phone::NANP::GD 1.1 + Number::Phone::NANP::GU 1.1 + Number::Phone::NANP::JM 1.1 + Number::Phone::NANP::KN 1.1 + Number::Phone::NANP::KY 1.1 + Number::Phone::NANP::LC 1.1 + Number::Phone::NANP::MP 1.1 + Number::Phone::NANP::MS 1.1 + Number::Phone::NANP::PR 1.1 + Number::Phone::NANP::SX 1 + Number::Phone::NANP::TC 1.1 + Number::Phone::NANP::TT 1.1 + Number::Phone::NANP::US 1.1 + Number::Phone::NANP::VC 1.1 + Number::Phone::NANP::VG 1.1 + Number::Phone::NANP::VI 1.1 + Number::Phone::StubCountry 1.3 + Number::Phone::StubCountry::AC 1.20170908113147 + Number::Phone::StubCountry::AD 1.20170908113147 + Number::Phone::StubCountry::AE 1.20170908113147 + Number::Phone::StubCountry::AF 1.20170908113147 + Number::Phone::StubCountry::AG 1.20170908113147 + Number::Phone::StubCountry::AI 1.20170908113147 + Number::Phone::StubCountry::AL 1.20170908113147 + Number::Phone::StubCountry::AM 1.20170908113147 + Number::Phone::StubCountry::AO 1.20170908113147 + Number::Phone::StubCountry::AR 1.20170908113147 + Number::Phone::StubCountry::AS 1.20170908113147 + Number::Phone::StubCountry::AT 1.20170908113147 + Number::Phone::StubCountry::AU 1.20170908113147 + Number::Phone::StubCountry::AW 1.20170908113147 + Number::Phone::StubCountry::AX 1.20170908113147 + Number::Phone::StubCountry::AZ 1.20170908113147 + Number::Phone::StubCountry::BA 1.20170908113147 + Number::Phone::StubCountry::BB 1.20170908113147 + Number::Phone::StubCountry::BD 1.20170908113147 + Number::Phone::StubCountry::BE 1.20170908113147 + Number::Phone::StubCountry::BF 1.20170908113147 + Number::Phone::StubCountry::BG 1.20170908113147 + Number::Phone::StubCountry::BH 1.20170908113147 + Number::Phone::StubCountry::BI 1.20170908113147 + Number::Phone::StubCountry::BJ 1.20170908113147 + Number::Phone::StubCountry::BL 1.20170908113147 + Number::Phone::StubCountry::BM 1.20170908113147 + Number::Phone::StubCountry::BN 1.20170908113147 + Number::Phone::StubCountry::BO 1.20170908113147 + Number::Phone::StubCountry::BQ 1.20170908113147 + Number::Phone::StubCountry::BR 1.20170908113147 + Number::Phone::StubCountry::BS 1.20170908113147 + Number::Phone::StubCountry::BT 1.20170908113147 + Number::Phone::StubCountry::BW 1.20170908113147 + Number::Phone::StubCountry::BY 1.20170908113147 + Number::Phone::StubCountry::BZ 1.20170908113147 + Number::Phone::StubCountry::CA 1.20170908113147 + Number::Phone::StubCountry::CC 1.20170908113147 + Number::Phone::StubCountry::CD 1.20170908113147 + Number::Phone::StubCountry::CF 1.20170908113147 + Number::Phone::StubCountry::CG 1.20170908113147 + Number::Phone::StubCountry::CH 1.20170908113147 + Number::Phone::StubCountry::CI 1.20170908113147 + Number::Phone::StubCountry::CK 1.20170908113147 + Number::Phone::StubCountry::CL 1.20170908113147 + Number::Phone::StubCountry::CM 1.20170908113147 + Number::Phone::StubCountry::CN 1.20170908113147 + Number::Phone::StubCountry::CO 1.20170908113148 + Number::Phone::StubCountry::CR 1.20170908113148 + Number::Phone::StubCountry::CU 1.20170908113148 + Number::Phone::StubCountry::CV 1.20170908113148 + Number::Phone::StubCountry::CW 1.20170908113148 + Number::Phone::StubCountry::CX 1.20170908113148 + Number::Phone::StubCountry::CY 1.20170908113148 + Number::Phone::StubCountry::CZ 1.20170908113148 + Number::Phone::StubCountry::DE 1.20170908113148 + Number::Phone::StubCountry::DJ 1.20170908113148 + Number::Phone::StubCountry::DK 1.20170908113148 + Number::Phone::StubCountry::DM 1.20170908113148 + Number::Phone::StubCountry::DO 1.20170908113148 + Number::Phone::StubCountry::DZ 1.20170908113148 + Number::Phone::StubCountry::EC 1.20170908113148 + Number::Phone::StubCountry::EE 1.20170908113148 + Number::Phone::StubCountry::EG 1.20170908113148 + Number::Phone::StubCountry::EH 1.20170908113148 + Number::Phone::StubCountry::ER 1.20170908113148 + Number::Phone::StubCountry::ES 1.20170908113148 + Number::Phone::StubCountry::ET 1.20170908113148 + Number::Phone::StubCountry::FI 1.20170908113148 + Number::Phone::StubCountry::FJ 1.20170908113148 + Number::Phone::StubCountry::FK 1.20170908113148 + Number::Phone::StubCountry::FM 1.20170908113148 + Number::Phone::StubCountry::FO 1.20170908113148 + Number::Phone::StubCountry::FR 1.20170908113148 + Number::Phone::StubCountry::GA 1.20170908113148 + Number::Phone::StubCountry::GB 1.20170908113148 + Number::Phone::StubCountry::GD 1.20170908113148 + Number::Phone::StubCountry::GE 1.20170908113148 + Number::Phone::StubCountry::GF 1.20170908113148 + Number::Phone::StubCountry::GG 1.20170908113148 + Number::Phone::StubCountry::GH 1.20170908113148 + Number::Phone::StubCountry::GI 1.20170908113148 + Number::Phone::StubCountry::GL 1.20170908113148 + Number::Phone::StubCountry::GM 1.20170908113148 + Number::Phone::StubCountry::GN 1.20170908113148 + Number::Phone::StubCountry::GP 1.20170908113148 + Number::Phone::StubCountry::GQ 1.20170908113148 + Number::Phone::StubCountry::GR 1.20170908113148 + Number::Phone::StubCountry::GT 1.20170908113148 + Number::Phone::StubCountry::GU 1.20170908113148 + Number::Phone::StubCountry::GW 1.20170908113148 + Number::Phone::StubCountry::GY 1.20170908113148 + Number::Phone::StubCountry::HK 1.20170908113148 + Number::Phone::StubCountry::HN 1.20170908113148 + Number::Phone::StubCountry::HR 1.20170908113148 + Number::Phone::StubCountry::HT 1.20170908113148 + Number::Phone::StubCountry::HU 1.20170908113148 + Number::Phone::StubCountry::ID 1.20170908113148 + Number::Phone::StubCountry::IE 1.20170908113148 + Number::Phone::StubCountry::IL 1.20170908113148 + Number::Phone::StubCountry::IM 1.20170908113148 + Number::Phone::StubCountry::IN 1.20170908113148 + Number::Phone::StubCountry::IO 1.20170908113148 + Number::Phone::StubCountry::IQ 1.20170908113148 + Number::Phone::StubCountry::IR 1.20170908113148 + Number::Phone::StubCountry::IS 1.20170908113148 + Number::Phone::StubCountry::IT 1.20170908113148 + Number::Phone::StubCountry::JE 1.20170908113148 + Number::Phone::StubCountry::JM 1.20170908113148 + Number::Phone::StubCountry::JO 1.20170908113148 + Number::Phone::StubCountry::JP 1.20170908113148 + Number::Phone::StubCountry::KE 1.20170908113148 + Number::Phone::StubCountry::KG 1.20170908113148 + Number::Phone::StubCountry::KH 1.20170908113148 + Number::Phone::StubCountry::KI 1.20170908113148 + Number::Phone::StubCountry::KM 1.20170908113148 + Number::Phone::StubCountry::KN 1.20170908113148 + Number::Phone::StubCountry::KP 1.20170908113148 + Number::Phone::StubCountry::KR 1.20170908113148 + Number::Phone::StubCountry::KW 1.20170908113148 + Number::Phone::StubCountry::KY 1.20170908113148 + Number::Phone::StubCountry::KZ 1.20170908113148 + Number::Phone::StubCountry::LA 1.20170908113148 + Number::Phone::StubCountry::LB 1.20170908113148 + Number::Phone::StubCountry::LC 1.20170908113148 + Number::Phone::StubCountry::LI 1.20170908113148 + Number::Phone::StubCountry::LK 1.20170908113148 + Number::Phone::StubCountry::LR 1.20170908113148 + Number::Phone::StubCountry::LS 1.20170908113148 + Number::Phone::StubCountry::LT 1.20170908113148 + Number::Phone::StubCountry::LU 1.20170908113148 + Number::Phone::StubCountry::LV 1.20170908113148 + Number::Phone::StubCountry::LY 1.20170908113148 + Number::Phone::StubCountry::MA 1.20170908113148 + Number::Phone::StubCountry::MC 1.20170908113148 + Number::Phone::StubCountry::MD 1.20170908113148 + Number::Phone::StubCountry::ME 1.20170908113148 + Number::Phone::StubCountry::MF 1.20170908113148 + Number::Phone::StubCountry::MG 1.20170908113148 + Number::Phone::StubCountry::MH 1.20170908113148 + Number::Phone::StubCountry::MK 1.20170908113148 + Number::Phone::StubCountry::ML 1.20170908113148 + Number::Phone::StubCountry::MM 1.20170908113148 + Number::Phone::StubCountry::MN 1.20170908113148 + Number::Phone::StubCountry::MO 1.20170908113148 + Number::Phone::StubCountry::MP 1.20170908113148 + Number::Phone::StubCountry::MQ 1.20170908113148 + Number::Phone::StubCountry::MR 1.20170908113148 + Number::Phone::StubCountry::MS 1.20170908113148 + Number::Phone::StubCountry::MT 1.20170908113148 + Number::Phone::StubCountry::MU 1.20170908113148 + Number::Phone::StubCountry::MV 1.20170908113148 + Number::Phone::StubCountry::MW 1.20170908113148 + Number::Phone::StubCountry::MX 1.20170908113148 + Number::Phone::StubCountry::MY 1.20170908113148 + Number::Phone::StubCountry::MZ 1.20170908113148 + Number::Phone::StubCountry::NA 1.20170908113148 + Number::Phone::StubCountry::NC 1.20170908113148 + Number::Phone::StubCountry::NE 1.20170908113148 + Number::Phone::StubCountry::NF 1.20170908113148 + Number::Phone::StubCountry::NG 1.20170908113148 + Number::Phone::StubCountry::NI 1.20170908113148 + Number::Phone::StubCountry::NL 1.20170908113148 + Number::Phone::StubCountry::NO 1.20170908113148 + Number::Phone::StubCountry::NP 1.20170908113148 + Number::Phone::StubCountry::NR 1.20170908113148 + Number::Phone::StubCountry::NU 1.20170908113148 + Number::Phone::StubCountry::NZ 1.20170908113148 + Number::Phone::StubCountry::OM 1.20170908113148 + Number::Phone::StubCountry::PA 1.20170908113148 + Number::Phone::StubCountry::PE 1.20170908113148 + Number::Phone::StubCountry::PF 1.20170908113148 + Number::Phone::StubCountry::PG 1.20170908113148 + Number::Phone::StubCountry::PH 1.20170908113148 + Number::Phone::StubCountry::PK 1.20170908113148 + Number::Phone::StubCountry::PL 1.20170908113148 + Number::Phone::StubCountry::PM 1.20170908113148 + Number::Phone::StubCountry::PR 1.20170908113149 + Number::Phone::StubCountry::PS 1.20170908113149 + Number::Phone::StubCountry::PT 1.20170908113149 + Number::Phone::StubCountry::PW 1.20170908113149 + Number::Phone::StubCountry::PY 1.20170908113149 + Number::Phone::StubCountry::QA 1.20170908113149 + Number::Phone::StubCountry::RE 1.20170908113149 + Number::Phone::StubCountry::RO 1.20170908113149 + Number::Phone::StubCountry::RS 1.20170908113149 + Number::Phone::StubCountry::RU 1.20170908113149 + Number::Phone::StubCountry::RW 1.20170908113149 + Number::Phone::StubCountry::SA 1.20170908113149 + Number::Phone::StubCountry::SB 1.20170908113149 + Number::Phone::StubCountry::SC 1.20170908113149 + Number::Phone::StubCountry::SD 1.20170908113149 + Number::Phone::StubCountry::SE 1.20170908113149 + Number::Phone::StubCountry::SG 1.20170908113149 + Number::Phone::StubCountry::SH 1.20170908113149 + Number::Phone::StubCountry::SI 1.20170908113149 + Number::Phone::StubCountry::SJ 1.20170908113149 + Number::Phone::StubCountry::SK 1.20170908113149 + Number::Phone::StubCountry::SL 1.20170908113149 + Number::Phone::StubCountry::SM 1.20170908113149 + Number::Phone::StubCountry::SN 1.20170908113149 + Number::Phone::StubCountry::SO 1.20170908113149 + Number::Phone::StubCountry::SR 1.20170908113149 + Number::Phone::StubCountry::SS 1.20170908113149 + Number::Phone::StubCountry::ST 1.20170908113149 + Number::Phone::StubCountry::SV 1.20170908113149 + Number::Phone::StubCountry::SX 1.20170908113149 + Number::Phone::StubCountry::SY 1.20170908113149 + Number::Phone::StubCountry::SZ 1.20170908113149 + Number::Phone::StubCountry::TA 1.20170908113149 + Number::Phone::StubCountry::TC 1.20170908113149 + Number::Phone::StubCountry::TD 1.20170908113149 + Number::Phone::StubCountry::TG 1.20170908113149 + Number::Phone::StubCountry::TH 1.20170908113149 + Number::Phone::StubCountry::TJ 1.20170908113149 + Number::Phone::StubCountry::TK 1.20170908113149 + Number::Phone::StubCountry::TL 1.20170908113149 + Number::Phone::StubCountry::TM 1.20170908113149 + Number::Phone::StubCountry::TN 1.20170908113149 + Number::Phone::StubCountry::TO 1.20170908113149 + Number::Phone::StubCountry::TR 1.20170908113149 + Number::Phone::StubCountry::TT 1.20170908113149 + Number::Phone::StubCountry::TV 1.20170908113149 + Number::Phone::StubCountry::TW 1.20170908113149 + Number::Phone::StubCountry::TZ 1.20170908113149 + Number::Phone::StubCountry::UA 1.20170908113149 + Number::Phone::StubCountry::UG 1.20170908113149 + Number::Phone::StubCountry::US 1.20170908113149 + Number::Phone::StubCountry::UY 1.20170908113149 + Number::Phone::StubCountry::UZ 1.20170908113149 + Number::Phone::StubCountry::VA 1.20170908113149 + Number::Phone::StubCountry::VC 1.20170908113149 + Number::Phone::StubCountry::VE 1.20170908113149 + Number::Phone::StubCountry::VG 1.20170908113149 + Number::Phone::StubCountry::VI 1.20170908113149 + Number::Phone::StubCountry::VN 1.20170908113149 + Number::Phone::StubCountry::VU 1.20170908113149 + Number::Phone::StubCountry::WF 1.20170908113149 + Number::Phone::StubCountry::WS 1.20170908113149 + Number::Phone::StubCountry::YE 1.20170908113149 + Number::Phone::StubCountry::YT 1.20170908113149 + Number::Phone::StubCountry::ZA 1.20170908113149 + Number::Phone::StubCountry::ZM 1.20170908113149 + Number::Phone::StubCountry::ZW 1.20170908113149 + Number::Phone::UK 1.67 + Number::Phone::UK::Data 2.0 + Number::Phone::UK::Exchanges 1.20060823121334 + Number::Phone::UK::GG 1 + Number::Phone::UK::IM 1 + Number::Phone::UK::JE 1 + requirements: + Cwd 0 + DBM::Deep 2.0008 + Digest::MD5 0 + ExtUtils::Install 0 + ExtUtils::MakeMaker 6.52 + ExtUtils::Manifest 0 + File::Basename 0 + File::ShareDir 1.104 + File::ShareDir::Install 0.11 + File::Spec 0 + Scalar::Util 1.48 + Test::More 0.96 + Test::utf8 0 Object-Signature-1.07 pathname: A/AD/ADAMK/Object-Signature-1.07.tar.gz provides: @@ -5199,13 +5552,13 @@ DISTRIBUTIONS Safe::Isa 1.000002 requirements: ExtUtils::MakeMaker 0 - Scalar-List-Utils-1.47 - pathname: P/PE/PEVANS/Scalar-List-Utils-1.47.tar.gz + Scalar-List-Utils-1.49 + pathname: P/PE/PEVANS/Scalar-List-Utils-1.49.tar.gz provides: - List::Util 1.47 - List::Util::XS 1.47 - Scalar::Util 1.47 - Sub::Util 1.47 + List::Util 1.49 + List::Util::XS 1.49 + Scalar::Util 1.49 + Sub::Util 1.49 requirements: ExtUtils::MakeMaker 0 Test::More 0 @@ -6325,6 +6678,16 @@ DISTRIBUTIONS Fcntl 0 URI 1.10 perl 5.008001 + WWW-Twilio-API-0.21 + pathname: S/SC/SCOTTW/WWW-Twilio-API-0.21.tar.gz + provides: + WWW::Twilio::API 0.21 + requirements: + ExtUtils::MakeMaker 0 + LWP::Protocol::https 0 + LWP::UserAgent 2.03 + List::Util 1.29 + URI::Escape 3.28 Web-Scraper-0.37 pathname: M/MI/MIYAGAWA/Web-Scraper-0.37.tar.gz provides: 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..0caa25710 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -14,6 +14,7 @@ use List::MoreUtils 'uniq'; use mySociety::ArrayUtils; use FixMyStreet::SendReport; +use FixMyStreet::SMS; =head1 NAME @@ -678,6 +679,10 @@ sub reports : Path('reports') { my $like_search = "%$search%"; + my $parsed = FixMyStreet::SMS->parse_username($search); + my $valid_phone = $parsed->{phone}; + my $valid_email = $parsed->{email}; + # when DBIC creates the join it does 'JOIN users user' in the # SQL which makes PostgreSQL unhappy as user is a reserved # word. So look up user ID for email separately. @@ -686,10 +691,19 @@ sub reports : Path('reports') { }, { columns => [ 'id' ] } )->all; @user_ids = map { $_->id } @user_ids; - if (is_valid_email($search)) { + my @user_ids_phone = $c->model('DB::User')->search({ + phone => { ilike => $like_search }, + }, { columns => [ 'id' ] } )->all; + @user_ids_phone = map { $_->id } @user_ids_phone; + + if ($valid_email) { $query->{'-or'} = [ 'me.user_id' => { -in => \@user_ids }, ]; + } elsif ($valid_phone) { + $query->{'-or'} = [ + 'me.user_id' => { -in => \@user_ids_phone }, + ]; } elsif ($search =~ /^id:(\d+)$/) { $query->{'-or'} = [ 'me.id' => int($1), @@ -705,7 +719,7 @@ sub reports : Path('reports') { } else { $query->{'-or'} = [ 'me.id' => $search_n, - 'me.user_id' => { -in => \@user_ids }, + 'me.user_id' => { -in => [ @user_ids, @user_ids_phone ] }, 'me.external_id' => { ilike => $like_search }, 'me.name' => { ilike => $like_search }, 'me.title' => { ilike => $like_search }, @@ -726,10 +740,14 @@ sub reports : Path('reports') { $c->stash->{problems} = [ $problems->all ]; $c->stash->{problems_pager} = $problems->pager; - if (is_valid_email($search)) { + if ($valid_email) { $query = [ 'me.user_id' => { -in => \@user_ids }, ]; + } elsif ($valid_phone) { + $query = [ + 'me.user_id' => { -in => \@user_ids_phone }, + ]; } elsif ($search =~ /^id:(\d+)$/) { $query = [ 'me.id' => int($1), @@ -741,7 +759,7 @@ sub reports : Path('reports') { $query = [ 'me.id' => $search_n, 'problem.id' => $search_n, - 'me.user_id' => { -in => \@user_ids }, + 'me.user_id' => { -in => [ @user_ids, @user_ids_phone ] }, 'me.name' => { ilike => $like_search }, text => { ilike => $like_search }, 'me.cobrand_data' => { ilike => $like_search }, @@ -834,7 +852,7 @@ sub report_edit : Path('report_edit') : Args(1) { return if $done; } - $c->forward('check_email_for_abuse', [ $problem->user->email ] ); + $c->forward('check_username_for_abuse', [ $problem->user ] ); $c->stash->{updates} = [ $c->model('DB::Comment') @@ -884,11 +902,12 @@ sub report_edit : Path('report_edit') : Args(1) { $c->forward( '/admin/report_edit_category', [ $problem ] ); - my $email = lc $c->get_param('email'); - if ( $email ne $problem->user->email ) { - my $user = $c->model('DB::User')->find_or_create({ email => $email }); - $user->insert unless $user->in_storage; - $problem->user( $user ); + my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username')); + if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) { + my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} }); + if ($user->id && $user->id != $problem->user->id) { + $problem->user( $user ); + } } # Deal with photos @@ -1145,28 +1164,15 @@ sub users: Path('users') : Args(0) { { -or => [ email => { ilike => $isearch }, + phone => { ilike => $isearch }, name => { ilike => $isearch }, from_body => $search_n, ] } ); my @users = $users->all; - my %email2user = map { $_->email => $_ } @users; $c->stash->{users} = [ @users ]; - - if ( $c->user->is_superuser ) { - my $emails = $c->model('DB::Abuse')->search( - { email => { ilike => $isearch } } - ); - foreach my $email ($emails->all) { - # Slight abuse of the boolean flagged value - if ($email2user{$email->email}) { - $email2user{$email->email}->flagged( 2 ); - } else { - push @{$c->stash->{users}}, { email => $email->email, flagged => 2 }; - } - } - } + $c->forward('add_flags', [ { email => { ilike => $isearch } } ]); } else { $c->forward('/auth/get_csrf_token'); @@ -1178,9 +1184,7 @@ sub users: Path('users') : Args(0) { { order_by => 'name' } ); my @users = $users->all; - my %email2user = map { $_->email => $_ } @users; $c->stash->{users} = \@users; - } return 1; @@ -1203,7 +1207,7 @@ sub update_edit : Path('update_edit') : Args(1) { return 1; } - $c->forward('check_email_for_abuse', [ $update->user->email ] ); + $c->forward('check_username_for_abuse', [ $update->user ] ); if ( $c->get_param('banuser') ) { $c->forward('ban_user'); @@ -1227,9 +1231,7 @@ sub update_edit : Path('update_edit') : Args(1) { # $update->name can be null which makes ne unhappy my $name = $update->name || ''; - my $email = lc $c->get_param('email'); if ( $c->get_param('name') ne $name - || $email ne $update->user->email || $c->get_param('anonymous') ne $update->anonymous || $c->get_param('text') ne $update->text ) { $edited = 1; @@ -1249,10 +1251,13 @@ sub update_edit : Path('update_edit') : Args(1) { $update->anonymous( $c->get_param('anonymous') ); $update->state( $new_state ); - if ( $email ne $update->user->email ) { - my $user = $c->model('DB::User')->find_or_create({ email => $email }); - $user->insert unless $user->in_storage; - $update->user($user); + my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username')); + if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) { + my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} }); + if ($user->id && $user->id != $update->user->id) { + $edited = 1; + $update->user( $user ); + } } if ( $new_state eq 'confirmed' and $old_state eq 'unconfirmed' ) { @@ -1305,24 +1310,53 @@ sub user_add : Path('user_edit') : Args(0) { $c->forward('/auth/check_csrf_token'); $c->stash->{field_errors} = {}; - unless ($c->get_param('email')) { + my $email = lc $c->get_param('email'); + my $phone = $c->get_param('phone'); + my $email_v = $c->get_param('email_verified'); + my $phone_v = $c->get_param('phone_verified'); + + unless ($email || $phone) { + $c->stash->{field_errors}->{username} = _('Please enter a valid email or phone number'); + } + if (!$email_v && !$phone_v) { + $c->stash->{field_errors}->{username} = _('Please verify at least one of email/phone'); + } + if ($email && !is_valid_email($email)) { $c->stash->{field_errors}->{email} = _('Please enter a valid email'); } unless ($c->get_param('name')) { $c->stash->{field_errors}->{name} = _('Please enter a name'); } + + if ($phone_v) { + my $parsed = FixMyStreet::SMS->parse_username($phone); + if ($parsed->{phone} && $parsed->{phone}->is_mobile) { + $phone = $parsed->{username}; + } elsif ($parsed->{phone}) { + $c->stash->{field_errors}->{phone} = _('Please enter a mobile number'); + } else { + $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct'); + } + } + + my $existing_email = $email_v && $c->model('DB::User')->find( { email => $email } ); + my $existing_phone = $phone_v && $c->model('DB::User')->find( { phone => $phone } ); + if ($existing_email || $existing_phone) { + $c->stash->{field_errors}->{username} = _('User already exists'); + } + return if %{$c->stash->{field_errors}}; - my $user = $c->model('DB::User')->find_or_create( { + my $user = $c->model('DB::User')->create( { name => $c->get_param('name'), - email => lc $c->get_param('email'), - phone => $c->get_param('phone') || undef, + email => $email ? $email : undef, + email_verified => $email && $email_v ? 1 : 0, + phone => $phone || undef, + phone_verified => $phone && $phone_v ? 1 : 0, 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' } ); $c->stash->{user} = $user; $c->forward('user_cobrand_extra_fields'); @@ -1366,19 +1400,72 @@ sub user_edit : Path('user_edit') : Args(1) { my $edited = 0; my $email = lc $c->get_param('email'); - if ( $user->email ne $email || + my $phone = $c->get_param('phone'); + my $email_v = $c->get_param('email_verified') || 0; + my $phone_v = $c->get_param('phone_verified') || 0; + + $c->stash->{field_errors} = {}; + + unless ($email || $phone) { + $c->stash->{field_errors}->{username} = _('Please enter a valid email or phone number'); + } + if (!$email_v && !$phone_v) { + $c->stash->{field_errors}->{username} = _('Please verify at least one of email/phone'); + } + if ($email && !is_valid_email($email)) { + $c->stash->{field_errors}->{email} = _('Please enter a valid email'); + } + + if ($phone_v) { + my $parsed = FixMyStreet::SMS->parse_username($phone); + if ($parsed->{phone} && $parsed->{phone}->is_mobile) { + $phone = $parsed->{username}; + } elsif ($parsed->{phone}) { + $c->stash->{field_errors}->{phone} = _('Please enter a mobile number'); + } else { + $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct'); + } + } + + unless ($user->name) { + $c->stash->{field_errors}->{name} = _('Please enter a name'); + } + + my $email_params = { email => $email, email_verified => 1, id => { '!=', $user->id } }; + my $phone_params = { phone => $phone, phone_verified => 1, id => { '!=', $user->id } }; + my $existing_email = $email_v && $c->model('DB::User')->search($email_params)->first; + my $existing_phone = $phone_v && $c->model('DB::User')->search($phone_params)->first; + my $existing_user = $existing_email || $existing_phone; + my $existing_email_cobrand = $email_v && $c->cobrand->users->search($email_params)->first; + my $existing_phone_cobrand = $phone_v && $c->cobrand->users->search($phone_params)->first; + my $existing_user_cobrand = $existing_email_cobrand || $existing_phone_cobrand; + if ($existing_phone_cobrand && $existing_email_cobrand && $existing_email_cobrand->id != $existing_phone_cobrand->id) { + $c->stash->{field_errors}->{username} = _('User already exists'); + } + + return if %{$c->stash->{field_errors}}; + + if ( ($user->email || "") ne $email || $user->name ne $c->get_param('name') || - ($user->phone || "") ne $c->get_param('phone') || + ($user->phone || "") ne $phone || ($user->from_body && $c->get_param('body') && $user->from_body->id ne $c->get_param('body')) || (!$user->from_body && $c->get_param('body')) ) { $edited = 1; } + if ($existing_user_cobrand) { + $existing_user->adopt($user); + $c->forward( 'log_edit', [ $id, 'user', 'merge' ] ); + return $c->res->redirect( $c->uri_for( 'user_edit', $existing_user->id ) ); + } + + $user->email($email) if !$existing_email; + $user->phone($phone) if !$existing_phone; + $user->email_verified( $email_v ); + $user->phone_verified( $phone_v ); $user->name( $c->get_param('name') ); - my $original_email = $user->email; - $user->email( $email ); - $user->phone( $c->get_param('phone') ) if $c->get_param('phone'); + $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 ); @@ -1457,8 +1544,6 @@ sub user_edit : Path('user_edit') : Args(1) { } } - $c->stash->{field_errors} = {}; - # Update the categories this user operates in if ( $user->from_body ) { $c->stash->{body} = $user->from_body; @@ -1469,33 +1554,12 @@ sub user_edit : Path('user_edit') : Args(1) { $user->set_extra_metadata('categories', \@new_contact_ids); } - unless ($user->email) { - $c->stash->{field_errors}->{email} = _('Please enter a valid email'); - } - unless ($user->name) { - $c->stash->{field_errors}->{name} = _('Please enter a name'); - } - return if %{$c->stash->{field_errors}}; - - my $existing_user = $c->model('DB::User')->search({ email => $user->email, id => { '!=', $user->id } })->first; - my $existing_user_cobrand = $c->cobrand->users->search({ email => $user->email, id => { '!=', $user->id } })->first; - if ($existing_user_cobrand) { - $existing_user->adopt($user); - $c->forward( 'log_edit', [ $id, 'user', 'merge' ] ); - $c->res->redirect( $c->uri_for( 'user_edit', $existing_user->id ) ); - } else { - if ($existing_user) { - # Tried to change email to an existing one lacking permission - # so make sure it's switched back - $user->email($original_email); - } - $user->update; - if ($edited) { - $c->forward( 'log_edit', [ $id, 'user', 'edit' ] ); - } - $c->flash->{status_message} = _("Updated!"); - $c->res->redirect( $c->uri_for( 'user_edit', $user->id ) ); + $user->update; + if ($edited) { + $c->forward( 'log_edit', [ $id, 'user', 'edit' ] ); } + $c->flash->{status_message} = _("Updated!"); + return $c->res->redirect( $c->uri_for( 'user_edit', $user->id ) ); } if ( $user->from_body ) { @@ -1526,6 +1590,27 @@ sub user_cobrand_extra_fields : Private { } } +sub add_flags : Private { + my ( $self, $c, $search ) = @_; + + return unless $c->user->is_superuser; + + my $users = $c->stash->{users}; + my %email2user = map { $_->email => $_ } grep { $_->email } @$users; + my %phone2user = map { $_->phone => $_ } grep { $_->phone } @$users; + my %username2user = (%email2user, %phone2user); + my $usernames = $c->model('DB::Abuse')->search($search); + + foreach my $username (map { $_->email } $usernames->all) { + # Slight abuse of the boolean flagged value + if ($username2user{$username}) { + $username2user{$username}->flagged( 2 ); + } else { + push @{$c->stash->{users}}, { email => $username, flagged => 2 }; + } + } +} + sub flagged : Path('flagged') : Args(0) { my ( $self, $c ) = @_; @@ -1535,23 +1620,10 @@ sub flagged : Path('flagged') : Args(0) { # which has to use an array ref for sql quoting reasons $c->stash->{problems} = [ $problems->all ]; - my $users = $c->cobrand->users->search( { flagged => 1 } ); - my @users = $users->all; - my %email2user = map { $_->email => $_ } @users; + my @users = $c->cobrand->users->search( { flagged => 1 } )->all; $c->stash->{users} = [ @users ]; - my @abuser_emails = $c->model('DB::Abuse')->all() - if $c->user->is_superuser; - - foreach my $email (@abuser_emails) { - # Slight abuse of the boolean flagged value - if ($email2user{$email->email}) { - $email2user{$email->email}->flagged( 2 ); - } else { - push @{$c->stash->{users}}, { email => $email->email, flagged => 2 }; - } - } - + $c->forward('add_flags', [ {} ]); return 1; } @@ -1734,47 +1806,60 @@ sub log_edit : Private { =head2 ban_user -Add the email address in the email param of the request object to -the abuse table if they are not already in there and sets status_message -accordingly +Add the user's email address/phone number to the abuse table if they are not +already in there and sets status_message accordingly. =cut sub ban_user : Private { my ( $self, $c ) = @_; - my $email = lc $c->get_param('email'); - - return unless $email; - - my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $email }); - - if ( $abuse->in_storage ) { - $c->stash->{status_message} = _('Email already in abuse list'); - } else { - $abuse->insert; - $c->stash->{status_message} = _('Email added to abuse list'); + my $user; + if ($c->stash->{problem}) { + $user = $c->stash->{problem}->user; + } elsif ($c->stash->{update}) { + $user = $c->stash->{update}->user; } + return unless $user; - $c->stash->{email_in_abuse} = 1; - + if ($user->email_verified && $user->email) { + my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $user->email }); + if ( $abuse->in_storage ) { + $c->stash->{status_message} = _('User already in abuse list'); + } else { + $abuse->insert; + $c->stash->{status_message} = _('User added to abuse list'); + } + $c->stash->{username_in_abuse} = 1; + } + if ($user->phone_verified && $user->phone) { + my $abuse = $c->model('DB::Abuse')->find_or_new({ email => $user->phone }); + if ( $abuse->in_storage ) { + $c->stash->{status_message} = _('User already in abuse list'); + } else { + $abuse->insert; + $c->stash->{status_message} = _('User added to abuse list'); + } + $c->stash->{username_in_abuse} = 1; + } return 1; } =head2 flag_user -Sets the flag on a user with the given email +Sets the flag on a user =cut sub flag_user : Private { my ( $self, $c ) = @_; - my $email = lc $c->get_param('email'); - - return unless $email; - - my $user = $c->cobrand->users->find({ email => $email }); + my $user; + if ($c->stash->{problem}) { + $user = $c->stash->{problem}->user; + } elsif ($c->stash->{update}) { + $user = $c->stash->{update}->user; + } if ( !$user ) { $c->stash->{status_message} = _('Could not find user'); @@ -1791,18 +1876,19 @@ sub flag_user : Private { =head2 remove_user_flag -Remove the flag on a user with the given email +Remove the flag on a user =cut sub remove_user_flag : Private { my ( $self, $c ) = @_; - my $email = lc $c->get_param('email'); - - return unless $email; - - my $user = $c->cobrand->users->find({ email => $email }); + my $user; + if ($c->stash->{problem}) { + $user = $c->stash->{problem}->user; + } elsif ($c->stash->{update}) { + $user = $c->stash->{update}->user; + } if ( !$user ) { $c->stash->{status_message} = _('Could not find user'); @@ -1816,20 +1902,20 @@ sub remove_user_flag : Private { } -=head2 check_email_for_abuse +=head2 check_username_for_abuse - $c->forward('check_email_for_abuse', [ $email ] ); + $c->forward('check_username_for_abuse', [ $user ] ); -Checks if $email is in the abuse table and sets email_in_abuse accordingly +Checks if $user is in the abuse table and sets username_in_abuse accordingly. =cut -sub check_email_for_abuse : Private { - my ( $self, $c, $email ) =@_; +sub check_username_for_abuse : Private { + my ( $self, $c, $user ) = @_; - my $is_abuse = $c->model('DB::Abuse')->find({ email => $email }); + my $is_abuse = $c->model('DB::Abuse')->find({ email => [ $user->phone, $user->email ] }); - $c->stash->{email_in_abuse} = 1 if $is_abuse; + $c->stash->{username_in_abuse} = 1 if $is_abuse; return 1; } diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 825066026..b453f593b 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -5,12 +5,10 @@ use namespace::autoclean; BEGIN { extends 'Catalyst::Controller'; } use Email::Valid; -use Net::Domain::TLD; use Digest::HMAC_SHA1 qw(hmac_sha1); use JSON::MaybeXS; use MIME::Base64; -use Net::Facebook::Oauth2; -use Net::Twitter::Lite::WithAPIv1_1; +use FixMyStreet::SMS; =head1 NAME @@ -38,19 +36,19 @@ sub general : Path : Args(0) { # all done unless we have a form posted to us return unless $c->req->method eq 'POST'; - my $clicked_email = $c->get_param('email_sign_in'); - my $data_address = $c->get_param('email'); + my $clicked_sign_in_by_code = $c->get_param('sign_in_by_code'); + my $data_username = $c->get_param('username'); my $data_password = $c->get_param('password_sign_in'); my $data_email = $c->get_param('name') || $c->get_param('password_register'); # decide which action to take - $c->detach('email_sign_in') if $clicked_email || ($data_email && !$data_password); - if (!$data_address && !$data_password && !$data_email) { - $c->detach('facebook_sign_in') if $c->get_param('facebook_sign_in'); - $c->detach('twitter_sign_in') if $c->get_param('twitter_sign_in'); + $c->detach('code_sign_in') if $clicked_sign_in_by_code || ($data_email && !$data_password); + if (!$data_username && !$data_password && !$data_email) { + $c->detach('social/facebook_sign_in') if $c->get_param('facebook_sign_in'); + $c->detach('social/twitter_sign_in') if $c->get_param('twitter_sign_in'); } - $c->forward( 'sign_in' ) + $c->forward( 'sign_in', [ $data_username ] ) && $c->detach( 'redirect_on_signin', [ $c->get_param('r') ] ); } @@ -60,6 +58,13 @@ sub general_test : Path('_test_') : Args(0) { $c->stash->{template} = 'auth/token.html'; } +sub authenticate : Private { + my ($self, $c, $type, $username, $password) = @_; + return 1 if $type eq 'email' && $c->authenticate({ email => $username, email_verified => 1, password => $password }); + return 1 if FixMyStreet->config('SMS_AUTHENTICATION') && $type eq 'phone' && $c->authenticate({ phone => $username, phone_verified => 1, password => $password }); + return 0; +} + =head2 sign_in Allow the user to sign in with a username and a password. @@ -67,21 +72,18 @@ Allow the user to sign in with a username and a password. =cut sub sign_in : Private { - my ( $self, $c, $email ) = @_; + my ( $self, $c, $username ) = @_; - $email ||= $c->get_param('email') || ''; - $email = lc $email; + $username ||= ''; my $password = $c->get_param('password_sign_in') || ''; my $remember_me = $c->get_param('remember_me') || 0; # Sign out just in case $c->logout(); - if ( $email - && $password - && $c->authenticate( { email => $email, password => $password } ) ) - { + my $parsed = FixMyStreet::SMS->parse_username($username); + if ($parsed->{username} && $password && $c->forward('authenticate', [ $parsed->{type}, $parsed->{username}, $password ])) { # unless user asked to be remembered limit the session to browser $c->set_session_cookie_expire(0) unless $remember_me; @@ -94,25 +96,40 @@ sub sign_in : Private { $c->stash( sign_in_error => 1, - email => $email, + username => $username, remember_me => $remember_me, ); return; } -=head2 email_sign_in +=head2 code_sign_in -Email the user the details they need to sign in. Don't check for an account - if -there isn't one we can create it when they come back with a token (which -contains the email address). +Either email the user a link to sign in, or send an SMS token to do so. + +Don't check for an account - if there isn't one we can create it when +they come back with a token (which contains the email/phone). =cut -sub email_sign_in : Private { +sub code_sign_in : Private { my ( $self, $c ) = @_; + my $username = $c->stash->{username} = $c->get_param('username') || ''; + + my $parsed = FixMyStreet::SMS->parse_username($username); + + if ($parsed->{type} eq 'phone' && FixMyStreet->config('SMS_AUTHENTICATION')) { + $c->forward('phone/sign_in', [ $parsed->{phone} ]); + } else { + $c->forward('email_sign_in', [ $parsed->{username} ]); + } +} + +sub email_sign_in : Private { + my ( $self, $c, $email ) = @_; + # check that the email is valid - otherwise flag an error - my $raw_email = lc( $c->get_param('email') || '' ); + my $raw_email = lc( $email || '' ); my $email_checker = Email::Valid->new( -mxcheck => 1, @@ -122,9 +139,7 @@ sub email_sign_in : Private { my $good_email = $email_checker->address($raw_email); if ( !$good_email ) { - $c->stash->{email} = $raw_email; - $c->stash->{email_error} = - $raw_email ? $email_checker->details : 'missing'; + $c->stash->{username_error} = $raw_email ? $email_checker->details : 'missing_email'; return; } @@ -133,7 +148,7 @@ sub email_sign_in : Private { # NB this uses the same template as a successful sign in to stop # enumeration of valid email addresses. if ( FixMyStreet->config('SIGNUPS_DISABLED') - && !$c->model('DB::User')->search({ email => $good_email })->count + && !$c->model('DB::User')->find({ email => $good_email }) && !$c->stash->{current_user} # don't break the change email flow ) { $c->stash->{template} = 'auth/token.html'; @@ -156,7 +171,7 @@ sub email_sign_in : Private { $token_data->{twitter_id} = $c->session->{oauth}{twitter_id} if $c->get_param('oauth_need_email') && $c->session->{oauth}{twitter_id}; if ($c->stash->{current_user}) { - $token_data->{old_email} = $c->stash->{current_user}->email; + $token_data->{old_user_id} = $c->stash->{current_user}->id; $token_data->{r} = 'auth/change_email/success'; } @@ -171,6 +186,20 @@ sub email_sign_in : Private { $c->stash->{template} = 'auth/token.html'; } +sub get_token : Private { + my ( $self, $c, $token, $scope ) = @_; + + $c->stash->{token_not_found} = 1, return unless $token; + + my $token_obj = $c->model('DB::Token')->find({ scope => $scope, token => $token }); + + $c->stash->{token_not_found} = 1, return unless $token_obj; + $c->stash->{token_not_found} = 1, return if $token_obj->created < DateTime->now->subtract( days => 1 ); + + my $data = $token_obj->data; + return $data; +} + =head2 token Handle the 'email_sign_in' tokens. Find the account for the email address @@ -181,53 +210,43 @@ Handle the 'email_sign_in' tokens. Find the account for the email address sub token : Path('/M') : Args(1) { my ( $self, $c, $url_token ) = @_; - # retrieve the token or return - my $token_obj = $url_token - ? $c->model('DB::Token')->find( { - scope => 'email_sign_in', token => $url_token - } ) - : undef; + my $data = $c->forward('get_token', [ $url_token, 'email_sign_in' ]) || return; - if ( !$token_obj ) { - $c->stash->{token_not_found} = 1; - return; - } - - if ( $token_obj->created < DateTime->now->subtract( days => 1 ) ) { - $c->stash->{token_not_found} = 1; - return; - } + $c->stash->{token_not_found} = 1, return + if $data->{old_user_id} && (!$c->user_exists || $c->user->id ne $data->{old_user_id}); - # find or create the user related to the token. - my $data = $token_obj->data; + my $type = $data->{login_type} || 'email'; + $c->detach( '/auth/process_login', [ $data, $type ] ); +} - if ($data->{old_email} && (!$c->user_exists || $c->user->email ne $data->{old_email})) { - $c->stash->{token_not_found} = 1; - return; - } +sub process_login : Private { + my ( $self, $c, $data, $type ) = @_; # sign out in case we are another user $c->logout(); - my $user = $c->model('DB::User')->find_or_new({ email => $data->{email} }); + my $user = $c->model('DB::User')->find_or_new({ $type => $data->{$type} }); + my $ver = "${type}_verified"; # Bail out if this is a new user and SIGNUPS_DISABLED is set $c->detach( '/page_error_403_access_denied', [] ) - if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_email}; + if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_user_id}; - if ($data->{old_email}) { - # Were logged in as old_email, want to switch to email ($user) + if ($data->{old_user_id}) { + # Were logged in as old_user_id, want to switch to $user if ($user->in_storage) { - my $old_user = $c->model('DB::User')->find({ email => $data->{old_email} }); + my $old_user = $c->model('DB::User')->find({ id => $data->{old_user_id} }); if ($old_user) { $old_user->adopt($user); $user = $old_user; - $user->email($data->{email}); + $user->$type($data->{$type}); + $user->$ver(1); } } else { - # Updating to a new (to the db) email address, easier! - $user = $c->model('DB::User')->find({ email => $data->{old_email} }); - $user->email($data->{email}); + # Updating to a new (to the db) email address/phone number, easier! + $user = $c->model('DB::User')->find({ id => $data->{old_user_id} }); + $user->$type($data->{$type}); + $user->$ver(1); } } @@ -236,193 +255,12 @@ 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( { $type => $data->{$type}, $ver => 1 }, 'no_password' ); # send the user to their page $c->detach( 'redirect_on_signin', [ $data->{r}, $data->{p} ] ); } -=head2 facebook_sign_in - -Starts the Facebook authentication sequence. - -=cut - -sub fb : Private { - my ($self, $c) = @_; - Net::Facebook::Oauth2->new( - application_id => $c->config->{FACEBOOK_APP_ID}, - application_secret => $c->config->{FACEBOOK_APP_SECRET}, - callback => $c->uri_for('/auth/Facebook'), - ); -} - -sub facebook_sign_in : Private { - my ( $self, $c ) = @_; - - $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); - - my $fb = $c->forward('/auth/fb'); - my $url = $fb->get_authorization_url(scope => ['email']); - - my %oauth; - $oauth{return_url} = $c->get_param('r'); - $oauth{detach_to} = $c->stash->{detach_to}; - $oauth{detach_args} = $c->stash->{detach_args}; - $c->session->{oauth} = \%oauth; - $c->res->redirect($url); -} - -=head2 facebook_callback - -Handles the Facebook callback request and completes the authentication sequence. - -=cut - -sub facebook_callback: Path('/auth/Facebook') : Args(0) { - my ( $self, $c ) = @_; - - $c->detach('oauth_failure') if $c->get_param('error_code'); - - my $fb = $c->forward('/auth/fb'); - my $access_token; - eval { - $access_token = $fb->get_access_token(code => $c->get_param('code')); - }; - if ($@) { - (my $message = $@) =~ s/at [^ ]*Auth.pm.*//; - $c->detach('/page_error_500_internal_error', [ $message ]); - } - - # save this token in session - $c->session->{oauth}{token} = $access_token; - - my $info = $fb->get('https://graph.facebook.com/me?fields=name,email')->as_hash(); - my $email = lc ($info->{email} || ""); - $c->forward('oauth_success', [ 'facebook', $info->{id}, $info->{name}, $email ]); -} - -=head2 twitter_sign_in - -Starts the Twitter authentication sequence. - -=cut - -sub tw : Private { - my ($self, $c) = @_; - Net::Twitter::Lite::WithAPIv1_1->new( - ssl => 1, - consumer_key => $c->config->{TWITTER_KEY}, - consumer_secret => $c->config->{TWITTER_SECRET}, - ); -} - -sub twitter_sign_in : Private { - my ( $self, $c ) = @_; - - $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); - - my $twitter = $c->forward('/auth/tw'); - my $url = $twitter->get_authentication_url(callback => $c->uri_for('/auth/Twitter')); - - my %oauth; - $oauth{return_url} = $c->get_param('r'); - $oauth{detach_to} = $c->stash->{detach_to}; - $oauth{detach_args} = $c->stash->{detach_args}; - $oauth{token} = $twitter->request_token; - $oauth{token_secret} = $twitter->request_token_secret; - $c->session->{oauth} = \%oauth; - $c->res->redirect($url); -} - -=head2 twitter_callback - -Handles the Twitter callback request and completes the authentication sequence. - -=cut - -sub twitter_callback: Path('/auth/Twitter') : Args(0) { - my ( $self, $c ) = @_; - - my $request_token = $c->req->param('oauth_token'); - my $verifier = $c->req->param('oauth_verifier'); - my $oauth = $c->session->{oauth}; - - $c->detach('oauth_failure') if $c->get_param('denied') || $request_token ne $oauth->{token}; - - my $twitter = $c->forward('/auth/tw'); - $twitter->request_token($oauth->{token}); - $twitter->request_token_secret($oauth->{token_secret}); - - eval { - # request_access_token no longer returns UID or name - $twitter->request_access_token(verifier => $verifier); - }; - if ($@) { - (my $message = $@) =~ s/at [^ ]*Auth.pm.*//; - $c->detach('/page_error_500_internal_error', [ $message ]); - } - - my $info = $twitter->verify_credentials(); - $c->forward('oauth_success', [ 'twitter', $info->{id}, $info->{name} ]); -} - -sub oauth_failure : Private { - my ( $self, $c ) = @_; - - $c->stash->{oauth_failure} = 1; - if ($c->session->{oauth}{detach_to}) { - $c->detach($c->session->{oauth}{detach_to}, $c->session->{oauth}{detach_args}); - } else { - $c->stash->{template} = 'auth/general.html'; - $c->detach; - } -} - -sub oauth_success : Private { - my ($self, $c, $type, $uid, $name, $email) = @_; - - my $user; - if ($email) { - # Only Facebook gets here - # We've got an ID and an email address - # Remove any existing mention of this ID - my $existing = $c->model('DB::User')->find( { facebook_id => $uid } ); - $existing->update( { facebook_id => undef } ) if $existing; - # Get or create a user, give it this Facebook ID - $user = $c->model('DB::User')->find_or_new( { email => $email } ); - $user->facebook_id($uid); - $user->name($name); - $user->in_storage() ? $user->update : $user->insert; - } else { - # We've got an ID, but no email - $user = $c->model('DB::User')->find( { $type . '_id' => $uid } ); - if ($user) { - # Matching ID in our database - $user->name($name); - $user->update; - } else { - # No matching ID, store ID for use later - $c->session->{oauth}{$type . '_id'} = $uid; - $c->stash->{oauth_need_email} = 1; - } - } - - # If we've got here with a full user, log in - if ($user) { - $c->authenticate( { email => $user->email }, 'no_password' ); - $c->stash->{login_success} = 1; - } - - if ($c->session->{oauth}{detach_to}) { - $c->detach($c->session->{oauth}{detach_to}, $c->session->{oauth}{detach_args}); - } elsif ($c->stash->{oauth_need_email}) { - $c->stash->{template} = 'auth/general.html'; - } else { - $c->detach( 'redirect_on_signin', [ $c->session->{oauth}{return_url} ] ); - } -} - =head2 redirect_on_signin Used after signing in to take the person back to where they were. @@ -478,69 +316,6 @@ sub redirect : Private { } -=head2 change_password - -Let the user change their password. - -=cut - -sub change_password : Local { - my ( $self, $c ) = @_; - - $c->detach( 'redirect' ) unless $c->user; - - $c->forward('get_csrf_token'); - - # If not a post then no submission - return unless $c->req->method eq 'POST'; - - $c->forward('check_csrf_token'); - - # get the passwords - my $new = $c->get_param('new_password') // ''; - my $confirm = $c->get_param('confirm') // ''; - - # check for errors - my $password_error = - !$new && !$confirm ? 'missing' - : $new ne $confirm ? 'mismatch' - : ''; - - if ($password_error) { - $c->stash->{password_error} = $password_error; - $c->stash->{new_password} = $new; - $c->stash->{confirm} = $confirm; - return; - } - - # we should have a usable password - save it to the user - $c->user->obj->update( { password => $new } ); - $c->stash->{password_changed} = 1; - -} - -=head2 change_email - -Let the user change their email. - -=cut - -sub change_email : Local { - my ( $self, $c ) = @_; - - $c->detach( 'redirect' ) unless $c->user; - - $c->forward('get_csrf_token'); - - # If not a post then no submission - return unless $c->req->method eq 'POST'; - - $c->forward('check_csrf_token'); - $c->stash->{current_user} = $c->user; - $c->stash->{email_template} = 'change_email.txt'; - $c->forward('email_sign_in'); -} - sub get_csrf_token : Private { my ( $self, $c ) = @_; @@ -588,7 +363,7 @@ sub ajax_sign_in : Path('ajax/sign_in') { my ( $self, $c ) = @_; my $return = {}; - if ( $c->forward( 'sign_in' ) ) { + if ( $c->forward( 'sign_in', [ $c->get_param('email') ] ) ) { $return->{name} = $c->user->name; } else { $return->{error} = 1; diff --git a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm new file mode 100644 index 000000000..4e9f92596 --- /dev/null +++ b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm @@ -0,0 +1,100 @@ +package FixMyStreet::App::Controller::Auth::Phone; +use Moose; +use namespace::autoclean; + +BEGIN { extends 'Catalyst::Controller'; } + +use FixMyStreet::SMS; + +=head1 NAME + +FixMyStreet::App::Controller::Auth::Phone - Catalyst Controller + +=head1 DESCRIPTION + +Controller for phone SMS based authentication + +=head1 METHODS + +=head2 code + +Handle the submission of a code sent by text to a mobile number. + +=cut + +sub code : Path('') { + my ( $self, $c ) = @_; + $c->stash->{template} = 'auth/smsform.html'; + + my $token = $c->stash->{token} = $c->get_param('token'); + my $code = $c->get_param('code') || ''; + + my $data = $c->forward('/auth/get_token', [ $token, 'phone_sign_in' ]) || return; + + $c->stash->{incorrect_code} = 1, return if $data->{code} ne $code; + + $c->detach( '/auth/process_login', [ $data, 'phone' ] ); +} + +=head2 sign_in + +When signing in with a mobile phone number, we are sent here. +This sends a text to that number with a confirmation code, +and sets up the token/etc to deal with the response. + +=cut + +sub sign_in : Private { + my ( $self, $c, $phone ) = @_; + + unless ($phone) { + $c->stash->{username_error} = 'other_phone'; + return; + } + + unless ($phone->is_mobile) { + $c->stash->{username_error} = 'nonmobile'; + return; + } + + (my $number = $phone->format) =~ s/\s+//g; + + if ( FixMyStreet->config('SIGNUPS_DISABLED') + && !$c->model('DB::User')->find({ phone => $number }) + && !$c->stash->{current_user} # don't break the change phone flow + ) { + $c->stash->{template} = 'auth/token.html'; + return; + } + + my $user_params = {}; + $user_params->{password} = $c->get_param('password_register') + if $c->get_param('password_register'); + my $user = $c->model('DB::User')->new( $user_params ); + + my $token_data = { + phone => $number, + r => $c->get_param('r'), + name => $c->get_param('name'), + password => $user->password, + }; + if ($c->stash->{current_user}) { + $token_data->{old_user_id} = $c->stash->{current_user}->id; + $token_data->{r} = 'auth/change_phone/success'; + } + + $c->forward('send_token', [ $token_data, 'phone_sign_in', $number ]); +} + +sub send_token : Private { + my ( $self, $c, $token_data, $token_scope, $to ) = @_; + + my $result = FixMyStreet::SMS->send_token($token_data, $token_scope, $to); + $c->stash->{token} = $result->{token}; + $c->log->debug("Sending text containing code *$result->{random}*"); + $c->stash->{template} = 'auth/smsform.html'; +} + +__PACKAGE__->meta->make_immutable; + +1; diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm new file mode 100644 index 000000000..ecf009150 --- /dev/null +++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm @@ -0,0 +1,151 @@ +package FixMyStreet::App::Controller::Auth::Profile; +use Moose; +use namespace::autoclean; + +BEGIN { extends 'Catalyst::Controller'; } + +=head1 NAME + +FixMyStreet::App::Controller::Auth::Profile - Catalyst Controller + +=head1 DESCRIPTION + +Controller for all the authentication profile related pages - adding/ changing/ +verifying email, phone, password. + +=head1 METHODS + +=cut + +sub auto { + my ( $self, $c ) = @_; + + $c->detach( '/auth/redirect' ) unless $c->user; + + return 1; +} + +=head2 change_password + +Let the user change their password. + +=cut + +sub change_password : Path('/auth/change_password') { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'auth/change_password.html'; + + $c->forward('/auth/get_csrf_token'); + + # If not a post then no submission + return unless $c->req->method eq 'POST'; + + $c->forward('/auth/check_csrf_token'); + + # get the passwords + my $new = $c->get_param('new_password') // ''; + my $confirm = $c->get_param('confirm') // ''; + + # check for errors + my $password_error = + !$new && !$confirm ? 'missing' + : $new ne $confirm ? 'mismatch' + : ''; + + if ($password_error) { + $c->stash->{password_error} = $password_error; + $c->stash->{new_password} = $new; + $c->stash->{confirm} = $confirm; + return; + } + + # we should have a usable password - save it to the user + $c->user->obj->update( { password => $new } ); + $c->stash->{password_changed} = 1; + +} + +=head2 change_email + +Let the user change their email. + +=cut + +sub change_email : Path('/auth/change_email') { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'auth/change_email.html'; + + $c->forward('/auth/get_csrf_token'); + + # If not a post then no submission + return unless $c->req->method eq 'POST'; + + $c->forward('/auth/check_csrf_token'); + $c->stash->{current_user} = $c->user; + $c->stash->{email_template} = 'change_email.txt'; + $c->forward('/auth/email_sign_in', [ $c->get_param('email') ]); +} + +sub change_phone : Path('/auth/change_phone') { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'auth/change_phone.html'; + + $c->forward('/auth/get_csrf_token'); + + # If not a post then no submission + return unless $c->req->method eq 'POST'; + + $c->forward('/auth/check_csrf_token'); + $c->stash->{current_user} = $c->user; + + my $phone = $c->stash->{username} = $c->get_param('username') || ''; + my $parsed = FixMyStreet::SMS->parse_username($phone); + + # Allow removal of phone number, if we have verified email + if (!$phone && !$c->stash->{verifying} && $c->user->email_verified) { + $c->user->update({ phone => undef, phone_verified => 0 }); + $c->flash->{flash_message} = _('You have successfully removed your phone number.'); + $c->res->redirect('/my'); + $c->detach; + } + + $c->stash->{username_error} = 'missing_phone', return unless $phone; + $c->stash->{username_error} = 'other_phone', return unless $parsed->{phone}; + + # If we've not used a mobile and we're not specifically verifying, + # and phone isn't our only verified way of logging in, + # then allow change of number (for e.g. landline). + if (!FixMyStreet->config('SMS_AUTHENTICATION') || (!$parsed->{phone}->is_mobile && !$c->stash->{verifying} && $c->user->email_verified)) { + $c->user->update({ phone => $phone, phone_verified => 0 }); + $c->flash->{flash_message} = _('You have successfully added your phone number.'); + $c->res->redirect('/my'); + $c->detach; + } + + $c->forward('/auth/phone/sign_in', [ $parsed->{phone} ]); +} + +sub verify_item : Path('/auth/verify') : Args(1) { + my ( $self, $c, $type ) = @_; + $c->stash->{verifying} = 1; + $c->detach("change_$type"); +} + +sub change_email_success : Path('/auth/change_email/success') { + my ( $self, $c ) = @_; + $c->flash->{flash_message} = _('You have successfully confirmed your email address.'); + $c->res->redirect('/my'); +} + +sub change_phone_success : Path('/auth/change_phone/success') { + my ( $self, $c ) = @_; + $c->flash->{flash_message} = _('You have successfully verified your phone number.'); + $c->res->redirect('/my'); +} + +__PACKAGE__->meta->make_immutable; + +1; diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm new file mode 100644 index 000000000..097cac984 --- /dev/null +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -0,0 +1,203 @@ +package FixMyStreet::App::Controller::Auth::Social; +use Moose; +use namespace::autoclean; + +BEGIN { extends 'Catalyst::Controller'; } + +use Net::Facebook::Oauth2; +use Net::Twitter::Lite::WithAPIv1_1; + +=head1 NAME + +FixMyStreet::App::Controller::Auth::Social - Catalyst Controller + +=head1 DESCRIPTION + +Controller for the Facebook/Twitter authentication. + +=head1 METHODS + +=head2 facebook_sign_in + +Starts the Facebook authentication sequence. + +=cut + +sub fb : Private { + my ($self, $c) = @_; + Net::Facebook::Oauth2->new( + application_id => $c->config->{FACEBOOK_APP_ID}, + application_secret => $c->config->{FACEBOOK_APP_SECRET}, + callback => $c->uri_for('/auth/Facebook'), + ); +} + +sub facebook_sign_in : Private { + my ( $self, $c ) = @_; + + $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); + + my $fb = $c->forward('fb'); + my $url = $fb->get_authorization_url(scope => ['email']); + + my %oauth; + $oauth{return_url} = $c->get_param('r'); + $oauth{detach_to} = $c->stash->{detach_to}; + $oauth{detach_args} = $c->stash->{detach_args}; + $c->session->{oauth} = \%oauth; + $c->res->redirect($url); +} + +=head2 facebook_callback + +Handles the Facebook callback request and completes the authentication sequence. + +=cut + +sub facebook_callback: Path('/auth/Facebook') : Args(0) { + my ( $self, $c ) = @_; + + $c->detach('oauth_failure') if $c->get_param('error_code'); + + my $fb = $c->forward('fb'); + my $access_token; + eval { + $access_token = $fb->get_access_token(code => $c->get_param('code')); + }; + if ($@) { + (my $message = $@) =~ s/at [^ ]*Auth.pm.*//; + $c->detach('/page_error_500_internal_error', [ $message ]); + } + + # save this token in session + $c->session->{oauth}{token} = $access_token; + + my $info = $fb->get('https://graph.facebook.com/me?fields=name,email')->as_hash(); + my $email = lc ($info->{email} || ""); + $c->forward('oauth_success', [ 'facebook', $info->{id}, $info->{name}, $email ]); +} + +=head2 twitter_sign_in + +Starts the Twitter authentication sequence. + +=cut + +sub tw : Private { + my ($self, $c) = @_; + Net::Twitter::Lite::WithAPIv1_1->new( + ssl => 1, + consumer_key => $c->config->{TWITTER_KEY}, + consumer_secret => $c->config->{TWITTER_SECRET}, + ); +} + +sub twitter_sign_in : Private { + my ( $self, $c ) = @_; + + $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); + + my $twitter = $c->forward('tw'); + my $url = $twitter->get_authentication_url(callback => $c->uri_for('/auth/Twitter')); + + my %oauth; + $oauth{return_url} = $c->get_param('r'); + $oauth{detach_to} = $c->stash->{detach_to}; + $oauth{detach_args} = $c->stash->{detach_args}; + $oauth{token} = $twitter->request_token; + $oauth{token_secret} = $twitter->request_token_secret; + $c->session->{oauth} = \%oauth; + $c->res->redirect($url); +} + +=head2 twitter_callback + +Handles the Twitter callback request and completes the authentication sequence. + +=cut + +sub twitter_callback: Path('/auth/Twitter') : Args(0) { + my ( $self, $c ) = @_; + + my $request_token = $c->req->param('oauth_token'); + my $verifier = $c->req->param('oauth_verifier'); + my $oauth = $c->session->{oauth}; + + $c->detach('oauth_failure') if $c->get_param('denied') || $request_token ne $oauth->{token}; + + my $twitter = $c->forward('tw'); + $twitter->request_token($oauth->{token}); + $twitter->request_token_secret($oauth->{token_secret}); + + eval { + # request_access_token no longer returns UID or name + $twitter->request_access_token(verifier => $verifier); + }; + if ($@) { + (my $message = $@) =~ s/at [^ ]*Auth.pm.*//; + $c->detach('/page_error_500_internal_error', [ $message ]); + } + + my $info = $twitter->verify_credentials(); + $c->forward('oauth_success', [ 'twitter', $info->{id}, $info->{name} ]); +} + +sub oauth_failure : Private { + my ( $self, $c ) = @_; + + $c->stash->{oauth_failure} = 1; + if ($c->session->{oauth}{detach_to}) { + $c->detach($c->session->{oauth}{detach_to}, $c->session->{oauth}{detach_args}); + } else { + $c->stash->{template} = 'auth/general.html'; + $c->detach; + } +} + +sub oauth_success : Private { + my ($self, $c, $type, $uid, $name, $email) = @_; + + my $user; + if ($email) { + # Only Facebook gets here + # We've got an ID and an email address + # Remove any existing mention of this ID + my $existing = $c->model('DB::User')->find( { facebook_id => $uid } ); + $existing->update( { facebook_id => undef } ) if $existing; + # Get or create a user, give it this Facebook ID + $user = $c->model('DB::User')->find_or_new( { email => $email } ); + $user->facebook_id($uid); + $user->name($name); + $user->in_storage() ? $user->update : $user->insert; + } else { + # We've got an ID, but no email + $user = $c->model('DB::User')->find( { $type . '_id' => $uid } ); + if ($user) { + # Matching ID in our database + $user->name($name); + $user->update; + } else { + # No matching ID, store ID for use later + $c->session->{oauth}{$type . '_id'} = $uid; + $c->stash->{oauth_need_email} = 1; + } + } + + # If we've got here with a full user, log in + if ($user) { + $c->authenticate( { email => $user->email, email_verified => 1 }, 'no_password' ); + $c->stash->{login_success} = 1; + } + + if ($c->session->{oauth}{detach_to}) { + $c->detach($c->session->{oauth}{detach_to}, $c->session->{oauth}{detach_args}); + } elsif ($c->stash->{oauth_need_email}) { + $c->stash->{template} = 'auth/general.html'; + } else { + $c->detach( '/auth/redirect_on_signin', [ $c->session->{oauth}{return_url} ] ); + } +} + +__PACKAGE__->meta->make_immutable; + +1; diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm index e2ab16b6b..1313b5071 100644 --- a/perllib/FixMyStreet/App/Controller/Moderate.pm +++ b/perllib/FixMyStreet/App/Controller/Moderate.pm @@ -106,19 +106,21 @@ sub report_moderate_audit : Private { reason => (sprintf '%s (%s)', $reason, $types_csv), }); - my $token = $c->model("DB::Token")->create({ - scope => 'moderation', - data => { id => $problem->id } - }); - - $c->send_email( 'problem-moderated.txt', { - to => [ [ $problem->user->email, $problem->name ] ], - types => $types_csv, - user => $problem->user, - problem => $problem, - report_uri => $c->stash->{report_uri}, - report_complain_uri => $c->stash->{cobrand_base} . '/contact?m=' . $token->token, - }); + if ($problem->user->email_verified) { + my $token = $c->model("DB::Token")->create({ + scope => 'moderation', + data => { id => $problem->id } + }); + + $c->send_email( 'problem-moderated.txt', { + to => [ [ $problem->user->email, $problem->name ] ], + types => $types_csv, + user => $problem->user, + problem => $problem, + report_uri => $c->stash->{report_uri}, + report_complain_uri => $c->stash->{cobrand_base} . '/contact?m=' . $token->token, + }); + } } sub report_moderate_hide : Private { diff --git a/perllib/FixMyStreet/App/Controller/My.pm b/perllib/FixMyStreet/App/Controller/My.pm index 5b80a4a08..9647fae9a 100644 --- a/perllib/FixMyStreet/App/Controller/My.pm +++ b/perllib/FixMyStreet/App/Controller/My.pm @@ -176,6 +176,10 @@ sub setup_page_data : Private { any_zoom => 1, ) if @$pins; + + foreach (qw(flash_message)) { + $c->stash->{$_} = $c->flash->{$_} if $c->flash->{$_}; + } } sub planned_change : Path('planned/change') { diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index f92a5cb22..fa3967bf3 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -13,6 +13,7 @@ use Path::Class; use Utils; use mySociety::EmailUtil; use JSON::MaybeXS; +use FixMyStreet::SMS; =head1 NAME @@ -116,19 +117,25 @@ sub report_new : Path : Args(0) { $c->forward('redirect_or_confirm_creation'); } -# This is for the new phonegap versions of the app. It looks a lot like -# report_new but there's a few workflow differences as we only ever want -# to sent JSON back here - sub report_new_test : Path('_test_') : Args(0) { my ( $self, $c ) = @_; $c->stash->{template} = 'email_sent.html'; $c->stash->{email_type} = $c->get_param('email_type'); } +# This is for the new phonegap versions of the app. It looks a lot like +# report_new but there's a few workflow differences as we only ever want +# to sent JSON back here + sub report_new_ajax : Path('mobile') : Args(0) { my ( $self, $c ) = @_; + # Apps are sending email as username + # Prepare for when they upgrade + if (!$c->get_param('username')) { + $c->set_param('username', $c->get_param('email')); + } + # create the report - loading a partial if available $c->forward('initialize_report'); @@ -354,8 +361,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 +458,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 @@ -733,14 +744,12 @@ sub process_user : Private { # Extract all the params to a hash to make them easier to work with my %params = map { $_ => $c->get_param($_) } - ( 'email', 'name', 'phone', 'password_register', 'fms_extra_title' ); - - my $user_title = Utils::trim_text( $params{fms_extra_title} ); + ( 'username', 'email', 'name', 'phone', 'password_register', 'fms_extra_title' ); if ( $c->cobrand->allow_anonymous_reports ) { my $anon_details = $c->cobrand->anonymous_account; - for my $key ( qw( email name ) ) { + for my $key ( qw( username email name ) ) { $params{ $key } ||= $anon_details->{ $key }; } } @@ -755,34 +764,29 @@ sub process_user : Private { last; } - $user->name( Utils::trim_text( $params{name} ) ) if $params{name}; - $user->phone( Utils::trim_text( $params{phone} ) ); - $user->title( $user_title ) if $user_title; $report->user( $user ); + $c->forward('update_user', [ \%params ]); if ($c->stash->{contributing_as_body} = $user->contributing_as('body', $c, $c->stash->{bodies}) or $c->stash->{contributing_as_anonymous_user} = $user->contributing_as('anonymous_user', $c, $c->stash->{bodies})) { $report->name($user->from_body->name); $user->name($user->from_body->name) unless $user->name; $c->stash->{no_reporter_alert} = 1; - } else { - $report->name($user->name); } return 1; } } - # cleanup the email address - my $email = $params{email} ? lc $params{email} : ''; - $email =~ s{\s+}{}g; - - $report->user( $c->model('DB::User')->find_or_new( { email => $email } ) ) + my $parsed = FixMyStreet::SMS->parse_username($params{username}); + my $type = $parsed->{type} || 'email'; + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $report->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $report->user; - # The user is trying to sign in. We only care about email from the params. + # The user is trying to sign in. We only care about username from the params. if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) { - unless ( $c->forward( '/auth/sign_in' ) ) { - $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.'); + unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) { + $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.'); return 1; } my $user = $c->user->obj; @@ -794,17 +798,28 @@ sub process_user : Private { return 1; } - # set the user's name, phone, and password - $report->user->name( Utils::trim_text( $params{name} ) ) if $params{name}; - $report->user->phone( Utils::trim_text( $params{phone} ) ); + $c->forward('update_user', [ \%params ]); $report->user->password( Utils::trim_text( $params{password_register} ) ) if $params{password_register}; - $report->user->title( $user_title ) if $user_title; - $report->name( Utils::trim_text( $params{name} ) ); return 1; } +sub update_user : Private { + my ($self, $c, $params) = @_; + my $report = $c->stash->{report}; + my $user = $report->user; + $user->name( Utils::trim_text( $params->{name} ) ); + $report->name($user->name); + if (!$user->phone_verified) { + $user->phone( Utils::trim_text( $params->{phone} ) ); + } elsif (!$user->email_verified) { + $user->email( Utils::trim_text( $params->{email} ) ); + } + my $user_title = Utils::trim_text( $params->{fms_extra_title} ); + $user->title( $user_title ) if $user_title; +} + =head2 process_report Looking at the parameters passed in create a new item and return it. Does not @@ -1027,11 +1042,11 @@ sub check_for_errors : Private { delete $field_errors{name}; } - # if using social login then we don't care about name and email errors + # if using social login then we don't care about other errors $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; - delete $field_errors{email}; + delete $field_errors{username}; } # add the photo error if there is one. @@ -1052,7 +1067,8 @@ sub tokenize_user : Private { my ($self, $c, $report) = @_; $c->stash->{token_data} = { name => $report->user->name, - phone => $report->user->phone, + (!$report->user->phone_verified ? (phone => $report->user->phone) : ()), + (!$report->user->email_verified ? (email => $report->user->email) : ()), password => $report->user->password, title => $report->user->title, }; @@ -1085,6 +1101,114 @@ sub send_problem_confirm_email : Private { } ); } +sub send_problem_confirm_text : Private { + my ( $self, $c ) = @_; + my $data = $c->stash->{token_data} || {}; + my $report = $c->stash->{report}; + + $data->{id} = $report->id; + $c->forward('/auth/phone/send_token', [ $data, 'problem', $report->user->phone ]); + $c->stash->{submit_url} = '/report/new/text'; +} + +sub confirm_by_text : Path('text') { + my ( $self, $c ) = @_; + + my $token = $c->stash->{token} = $c->get_param('token'); + my $code = $c->get_param('code') || ''; + + my $data = $c->stash->{token_data} = $c->forward('/auth/get_token', [ $token, 'problem' ]) || return; + if ($data->{code} ne $code) { + $c->stash->{template} = 'auth/smsform.html'; + $c->stash->{submit_url} = '/report/new/text'; + $c->stash->{incorrect_code} = 1; + return; + } + + $c->detach('process_confirmation'); +} + +sub process_confirmation : Private { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'tokens/confirm_problem.html'; + my $data = $c->stash->{token_data}; + + unless ($c->stash->{report}) { + # Look at all problems, not just cobrand, in case am approving something we don't actually show + $c->stash->{report} = $c->model('DB::Problem')->find({ id => $data->{id} }) || return; + } + my $problem = $c->stash->{report}; + + # check that this email or domain are not the cause of abuse. If so hide it. + if ( $problem->is_from_abuser ) { + $problem->update( + { state => 'hidden', lastupdate => \'current_timestamp' } ); + $c->stash->{template} = 'tokens/abuse.html'; + return; + } + + # For Zurich, email confirmation simply sets a flag, it does not change the + # problem state, log in, or anything else + if ($c->cobrand->moniker eq 'zurich') { + $problem->set_extra_metadata( email_confirmed => 1 ); + $problem->update( { + confirmed => \'current_timestamp', + } ); + + if ( $data->{name} || $data->{password} ) { + $problem->user->name( $data->{name} ) if $data->{name}; + $problem->user->phone( $data->{phone} ) if $data->{phone}; + $problem->user->update; + } + + return 1; + } + + if ($problem->state ne 'unconfirmed') { + my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; + $c->res->redirect($report_uri); + return; + } + + # We have an unconfirmed problem + $problem->update( + { + state => 'confirmed', + confirmed => \'current_timestamp', + lastupdate => \'current_timestamp', + } + ); + + # Subscribe problem reporter to email updates + $c->forward( '/report/new/create_reporter_alert' ); + + # log the problem creation user in to the site + if ( $data->{name} || $data->{password} ) { + if (!$problem->user->email_verified) { + $problem->user->email( $data->{email} ) if $data->{email}; + } elsif (!$problem->user->phone_verified) { + $problem->user->phone( $data->{phone} ) if $data->{phone}; + } + $problem->user->password( $data->{password}, 1 ) if $data->{password}; + for (qw(name title facebook_id twitter_id)) { + $problem->user->$_( $data->{$_} ) if $data->{$_}; + } + $problem->user->update; + } + if ($problem->user->email_verified) { + $c->authenticate( { email => $problem->user->email, email_verified => 1 }, 'no_password' ); + } elsif ($problem->user->phone_verified) { + $c->authenticate( { phone => $problem->user->phone, phone_verified => 1 }, 'no_password' ); + } else { + warn "Reached user authentication with no username verification"; + } + $c->set_session_cookie_expire(0); + + $c->stash->{created_report} = 'fromemail'; + return 1; +} + =head2 save_user_and_report Save the user and the report. @@ -1131,19 +1255,15 @@ sub save_user_and_report : Private { $c->stash->{detach_args} = [$token->token]; if ( $c->get_param('facebook_sign_in') ) { - $c->detach('/auth/facebook_sign_in'); + $c->detach('/auth/social/facebook_sign_in'); } elsif ( $c->get_param('twitter_sign_in') ) { - $c->detach('/auth/twitter_sign_in'); + $c->detach('/auth/social/twitter_sign_in'); } } # Save or update the user if appropriate if ( $c->cobrand->never_confirm_reports ) { - if ( $report->user->in_storage() ) { - $report->user->update(); - } else { - $report->user->insert(); - } + $report->user->update_or_insert; $report->confirm(); } elsif ( $c->forward('created_as_someone_else', [ $c->stash->{bodies} ]) ) { # If created on behalf of someone else, we automatically confirm it, @@ -1153,7 +1273,11 @@ sub save_user_and_report : Private { # User does not exist. $c->forward('tokenize_user', [ $report ]); $report->user->name( undef ); - $report->user->phone( undef ); + if (!$report->user->email_verified) { + $report->user->email( undef ); + } elsif (!$report->user->phone_verified) { + $report->user->phone( undef ); + } $report->user->password( '', 1 ); $report->user->title( undef ); $report->user->insert(); @@ -1173,8 +1297,7 @@ sub save_user_and_report : Private { $c->log->info($report->user->id . ' exists, but is not logged in for this report'); } - # save the report; - $report->in_storage ? $report->update : $report->insert(); + $report->update_or_insert; # tidy up if ( my $token = $c->stash->{partial_token} ) { @@ -1260,13 +1383,20 @@ sub redirect_or_confirm_creation : Private { return 1; } - # otherwise email a confirm token to them. - $c->forward( 'send_problem_confirm_email' ); - - # tell user that they've been sent an email - $c->stash->{template} = 'email_sent.html'; - $c->stash->{email_type} = 'problem'; - $c->log->info($report->user->id . ' created ' . $report->id . ', email sent, ' . ($c->stash->{token_data}->{password} ? 'password set' : 'password not set')); + # otherwise email or text a confirm token to them. + my $thing = 'email'; + if ($report->user->email_verified) { + $c->forward( 'send_problem_confirm_email' ); + # tell user that they've been sent an email + $c->stash->{template} = 'email_sent.html'; + $c->stash->{email_type} = 'problem'; + } elsif ($report->user->phone_verified) { + $c->forward( 'send_problem_confirm_text' ); + $thing = 'text'; + } else { + warn "Reached problem confirmation with no username verification"; + } + $c->log->info($report->user->id . ' created ' . $report->id . ", $thing sent, " . ($c->stash->{token_data}->{password} ? 'password set' : 'password not set')); } sub create_reporter_alert : Private { diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index 033f5c017..66724f2d1 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -36,18 +36,6 @@ sub report_update : Path : Args(0) { $c->forward('redirect_or_confirm_creation'); } -sub confirm : Private { - my ( $self, $c ) = @_; - - $c->stash->{update}->confirm; - $c->stash->{update}->update; - - $c->forward('update_problem'); - $c->forward('signup_for_alerts'); - - return 1; -} - sub update_problem : Private { my ( $self, $c ) = @_; @@ -109,6 +97,10 @@ sub process_user : Private { my $update = $c->stash->{update}; + # Extract all the params to a hash to make them easier to work with + my %params = map { $_ => $c->get_param($_) } + ( 'username', 'name', 'password_register', 'fms_extra_title' ); + # Extra block to use 'last' if ( $c->user_exists ) { { my $user = $c->user->obj; @@ -118,13 +110,9 @@ sub process_user : Private { last; } - my $name = $c->get_param('name'); - $user->name( Utils::trim_text( $name ) ) if $name; - my $title = $c->get_param('fms_extra_title'); - if ( $title ) { - $c->log->debug( 'user exists and title is ' . $title ); - $user->title( Utils::trim_text( $title ) ); - } + $user->name( Utils::trim_text( $params{name} ) ) if $params{name}; + my $title = Utils::trim_text( $params{fms_extra_title} ); + $user->title( $title ) if $title; $update->user( $user ); # Just in case, make sure the user will have a name @@ -135,21 +123,16 @@ sub process_user : Private { return 1; } } - # Extract all the params to a hash to make them easier to work with - my %params = map { $_ => $c->get_param($_) } - ( 'rznvy', 'name', 'password_register', 'fms_extra_title' ); - - # cleanup the email address - my $email = $params{rznvy} ? lc $params{rznvy} : ''; - $email =~ s{\s+}{}g; - - $update->user( $c->model('DB::User')->find_or_new( { email => $email } ) ) + my $parsed = FixMyStreet::SMS->parse_username($params{username}); + my $type = $parsed->{type} || 'email'; + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $update->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $update->user; - # The user is trying to sign in. We only care about email from the params. + # The user is trying to sign in. We only care about username from the params. if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) { - unless ( $c->forward( '/auth/sign_in', [ $email ] ) ) { - $c->stash->{field_errors}->{password} = _('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.'); + unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) { + $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.'); return 1; } my $user = $c->user->obj; @@ -328,8 +311,6 @@ sub process_update : Private { $update->extra( $extra ); } - $c->log->debug( 'name is ' . $c->get_param('name') ); - $c->stash->{add_alert} = $c->get_param('add_alert'); return 1; @@ -372,7 +353,7 @@ sub check_for_errors : Private { $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; - delete $field_errors{email}; + delete $field_errors{username}; } if ( my $photo_error = delete $c->stash->{photo_error} ) { @@ -438,18 +419,14 @@ sub save_update : Private { $c->stash->{detach_args} = [$token->token]; if ( $c->get_param('facebook_sign_in') ) { - $c->detach('/auth/facebook_sign_in'); + $c->detach('/auth/social/facebook_sign_in'); } elsif ( $c->get_param('twitter_sign_in') ) { - $c->detach('/auth/twitter_sign_in'); + $c->detach('/auth/social/twitter_sign_in'); } } if ( $c->cobrand->never_confirm_updates ) { - if ( $update->user->in_storage() ) { - $update->user->update(); - } else { - $update->user->insert(); - } + $update->user->update_or_insert; $update->confirm(); } elsif ( $c->forward('/report/new/created_as_someone_else', [ $update->problem->bodies_str ]) ) { # If created on behalf of someone else, we automatically confirm it, @@ -464,7 +441,6 @@ sub save_update : Private { } elsif ( $c->user && $c->user->id == $update->user->id ) { # Logged in and same user, so can confirm update straight away - $c->log->debug( 'user exists' ); $update->user->update; $update->confirm; } else { @@ -473,12 +449,7 @@ sub save_update : Private { $update->user->discard_changes(); } - if ( $update->in_storage ) { - $update->update; - } - else { - $update->insert; - } + $update->update_or_insert; return 1; } @@ -507,28 +478,108 @@ sub redirect_or_confirm_creation : Private { return 1; } - # otherwise create a confirm token and email it to them. - my $data = $c->stash->{token_data} || {}; - my $token = $c->model("DB::Token")->create( - { - scope => 'comment', - data => { - %$data, - id => $update->id, - add_alert => ( $c->get_param('add_alert') ? 1 : 0 ), - } - } - ); + my $data = $c->stash->{token_data}; + $data->{id} = $update->id; + $data->{add_alert} = $c->get_param('add_alert') ? 1 : 0; + + if ($update->user->email_verified) { + $c->forward('send_confirmation_email'); + # tell user that they've been sent an email + $c->stash->{template} = 'email_sent.html'; + $c->stash->{email_type} = 'update'; + } elsif ($update->user->phone_verified) { + $c->forward('send_confirmation_text'); + } else { + warn "Reached update confirmation with no username verification"; + } + + return 1; +} + +sub send_confirmation_email : Private { + my ( $self, $c ) = @_; + + my $update = $c->stash->{update}; + my $token = $c->model("DB::Token")->create( { + scope => 'comment', + data => $c->stash->{token_data}, + } ); + my $template = 'update-confirm.txt'; $c->stash->{token_url} = $c->uri_for_email( '/C', $token->token ); - $c->send_email( 'update-confirm.txt', { - to => $update->name - ? [ [ $update->user->email, $update->name ] ] - : $update->user->email, + $c->send_email( $template, { + to => [ $update->name ? [ $update->user->email, $update->name ] : $update->user->email ], } ); +} + +sub send_confirmation_text : Private { + my ( $self, $c ) = @_; + my $update = $c->stash->{update}; + $c->forward('/auth/phone/send_token', [ $c->stash->{token_data}, 'comment', $update->user->phone ]); + $c->stash->{submit_url} = '/report/update/text'; +} + +sub confirm_by_text : Path('text') { + my ( $self, $c ) = @_; + + my $token = $c->stash->{token} = $c->get_param('token'); + my $code = $c->get_param('code') || ''; + + my $data = $c->stash->{token_data} = $c->forward('/auth/get_token', [ $token, 'comment' ]) || return; + if ($data->{code} ne $code) { + $c->stash->{template} = 'auth/smsform.html'; + $c->stash->{submit_url} = '/report/update/text'; + $c->stash->{incorrect_code} = 1; + return; + } + + $c->detach('process_confirmation'); +} + +sub process_confirmation : Private { + my ( $self, $c ) = @_; + + $c->stash->{template} = 'tokens/confirm_update.html'; + my $data = $c->stash->{token_data}; - # tell user that they've been sent an email - $c->stash->{template} = 'email_sent.html'; - $c->stash->{email_type} = 'update'; + unless ($c->stash->{update}) { + $c->stash->{update} = $c->model('DB::Comment')->find({ id => $data->{id} }) || return; + } + my $comment = $c->stash->{update}; + + # check that this email or domain are not the cause of abuse. If so hide it. + if ( $comment->is_from_abuser ) { + $c->stash->{template} = 'tokens/abuse.html'; + return; + } + + if ( $comment->state ne 'unconfirmed' ) { + my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; + $c->res->redirect($report_uri); + return; + } + + if ( $data->{name} || $data->{password} ) { + for (qw(name facebook_id twitter_id)) { + $comment->user->$_( $data->{$_} ) if $data->{$_}; + } + $comment->user->password( $data->{password}, 1 ) if $data->{password}; + $comment->user->update; + } + + if ($comment->user->email_verified) { + $c->authenticate( { email => $comment->user->email, email_verified => 1 }, 'no_password' ); + } elsif ($comment->user->phone_verified) { + $c->authenticate( { phone => $comment->user->phone, phone_verified => 1 }, 'no_password' ); + } else { + warn "Reached user authentication with no username verification"; + } + $c->set_session_cookie_expire(0); + + $c->stash->{update}->confirm; + $c->stash->{update}->update; + $c->forward('update_problem'); + $c->stash->{add_alert} = $data->{add_alert}; + $c->forward('signup_for_alerts'); return 1; } diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm index a1b0c57ba..bb6140e0a 100644 --- a/perllib/FixMyStreet/App/Controller/Tokens.pm +++ b/perllib/FixMyStreet/App/Controller/Tokens.pm @@ -45,10 +45,10 @@ sub confirm_problem : Path('/P') { # Load the problem my $data = $auth_token->data; $data = { id => $data } unless ref $data; + $c->stash->{token_data} = $data; - my $problem_id = $data->{id}; # Look at all problems, not just cobrand, in case am approving something we don't actually show - my $problem = $c->model('DB::Problem')->find( { id => $problem_id } ) + my $problem = $c->model('DB::Problem')->find( { id => $data->{id} } ) || $c->detach('token_error'); $c->stash->{report} = $problem; @@ -56,64 +56,7 @@ sub confirm_problem : Path('/P') { if $problem->state eq 'unconfirmed' && $auth_token->created < DateTime->now->subtract( months => 1 ); - # check that this email or domain are not the cause of abuse. If so hide it. - if ( $problem->is_from_abuser ) { - $problem->update( - { state => 'hidden', lastupdate => \'current_timestamp' } ); - $c->stash->{template} = 'tokens/abuse.html'; - return; - } - - # For Zurich, email confirmation simply sets a flag, it does not change the - # problem state, log in, or anything else - if ($c->cobrand->moniker eq 'zurich') { - $problem->set_extra_metadata( email_confirmed => 1 ); - $problem->update( { - confirmed => \'current_timestamp', - } ); - - if ( $data->{name} || $data->{password} ) { - $problem->user->name( $data->{name} ) if $data->{name}; - $problem->user->phone( $data->{phone} ) if $data->{phone}; - $problem->user->update; - } - - return 1; - } - - if ($problem->state ne 'unconfirmed') { - my $report_uri = $c->cobrand->base_url_for_report( $problem ) . $problem->url; - $c->res->redirect($report_uri); - return; - } - - # We have an unconfirmed problem - $problem->update( - { - state => 'confirmed', - confirmed => \'current_timestamp', - lastupdate => \'current_timestamp', - } - ); - - # Subscribe problem reporter to email updates - $c->forward( '/report/new/create_reporter_alert' ); - - # log the problem creation user in to the site - if ( $data->{name} || $data->{password} ) { - $problem->user->name( $data->{name} ) if $data->{name}; - $problem->user->phone( $data->{phone} ) if $data->{phone}; - $problem->user->password( $data->{password}, 1 ) if $data->{password}; - $problem->user->title( $data->{title} ) if $data->{title}; - $problem->user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; - $problem->user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; - $problem->user->update; - } - $c->authenticate( { email => $problem->user->email }, 'no_password' ); - $c->set_session_cookie_expire(0); - - $c->stash->{created_report} = 'fromemail'; - return 1; + $c->forward('/report/new/process_confirmation'); } =head2 redirect_to_partial_problem @@ -170,7 +113,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); } @@ -205,11 +148,9 @@ sub confirm_update : Path('/C') { $c->forward( 'load_auth_token', [ $token_code, 'comment' ] ); # Load the update - my $data = $auth_token->data; - my $comment_id = $data->{id}; - $c->stash->{add_alert} = $data->{add_alert}; + my $data = $c->stash->{token_data} = $auth_token->data; - my $comment = $c->model('DB::Comment')->find( { id => $comment_id } ) + my $comment = $c->model('DB::Comment')->find( { id => $data->{id} } ) || $c->detach('token_error'); $c->stash->{update} = $comment; @@ -217,32 +158,7 @@ sub confirm_update : Path('/C') { if $comment->state ne 'confirmed' && $auth_token->created < DateTime->now->subtract( months => 1 ); - # check that this email or domain are not the cause of abuse. If so hide it. - if ( $comment->is_from_abuser ) { - $c->stash->{template} = 'tokens/abuse.html'; - return; - } - - if ( $comment->state ne 'unconfirmed' ) { - my $report_uri = $c->cobrand->base_url_for_report( $comment->problem ) . $comment->problem->url; - $c->res->redirect($report_uri); - return; - } - - if ( $data->{name} || $data->{password} ) { - $comment->user->name( $data->{name} ) if $data->{name}; - $comment->user->password( $data->{password}, 1 ) if $data->{password}; - $comment->user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; - $comment->user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; - $comment->user->update; - } - - $c->authenticate( { email => $comment->user->email }, 'no_password' ); - $c->set_session_cookie_expire(0); - - $c->forward('/report/update/confirm'); - - return 1; + $c->forward('/report/update/process_confirmation'); } sub load_questionnaire : Private { @@ -269,7 +185,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/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index b954a68ab..56601480d 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -485,12 +485,21 @@ Return a url for this problem report that logs a user in sub tokenised_url { my ($self, $user, $params) = @_; + my %params; + if ($user->email_verified) { + $params{email} = $user->email; + } elsif ($user->phone_verified) { + $params{phone} = $user->phone; + # This is so the email token can look up/ log in a phone user + $params{login_type} = 'phone'; + } + my $token = FixMyStreet::App->model('DB::Token')->create( { scope => 'email_sign_in', data => { + %params, id => $self->id, - email => $user->email, r => $self->url, p => $params, } diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 19adf5d49..296cf997d 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,13 +105,19 @@ __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'); use Moo; +use FixMyStreet::SMS; use mySociety::EmailUtil; use namespace::clean -except => [ 'meta' ]; @@ -125,6 +134,26 @@ __PACKAGE__->add_columns( }, ); +=head2 username + +Returns a verified email or phone for this user, preferring email, +or undef if neither verified (shouldn't happen). + +=cut + +sub username { + my $self = shift; + return $self->email if $self->email_verified; + return $self->phone_display if $self->phone_verified; +} + +sub phone_display { + my $self = shift; + return $self->phone unless $self->phone; + my $parsed = FixMyStreet::SMS->parse_username($self->phone); + return $parsed->{phone} ? $parsed->{phone}->format : $self->phone; +} + sub latest_anonymity { my $self = shift; my $p = $self->problems->search(undef, { order_by => { -desc => 'id' } } )->first; @@ -157,11 +186,19 @@ sub check_for_errors { $errors{name} = _('Please enter your name'); } - if ( $self->email !~ /\S/ ) { - $errors{email} = _('Please enter your email'); - } - elsif ( !mySociety::EmailUtil::is_valid_email( $self->email ) ) { - $errors{email} = _('Please enter a valid email'); + if ($self->email_verified) { + if ($self->email !~ /\S/) { + $errors{username} = _('Please enter your email'); + } elsif (!mySociety::EmailUtil::is_valid_email($self->email)) { + $errors{username} = _('Please enter a valid email'); + } + } elsif ($self->phone_verified) { + my $parsed = FixMyStreet::SMS->parse_username($self->phone); + if (!$parsed->{phone}) { + $errors{username} = _('Please check your phone number is correct'); + } elsif (!$parsed->{phone}->is_mobile) { + $errors{username} = _('Please enter a mobile number'); + } } return \%errors; 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/Roles/Abuser.pm b/perllib/FixMyStreet/Roles/Abuser.pm index fc76565ca..e2e9eb19e 100644 --- a/perllib/FixMyStreet/Roles/Abuser.pm +++ b/perllib/FixMyStreet/Roles/Abuser.pm @@ -13,9 +13,9 @@ Returns true if the user's email or its domain is listed in the 'abuse' table. sub is_from_abuser { my $self = shift; - # get the domain my $email = $self->user->email; - my ($domain) = $email =~ m{ @ (.*) \z }x; + my ($domain) = $email =~ m{ @ (.*) \z }x if $email; + my $phone = $self->user->phone; # search for an entry in the abuse table my $abuse_rs = $self->result_source->schema->resultset('Abuse'); @@ -23,6 +23,7 @@ sub is_from_abuser { return $abuse_rs->find( { email => $email } ) || $abuse_rs->find( { email => $domain } ) + || $abuse_rs->find( { email => $phone } ) || undef; } diff --git a/perllib/FixMyStreet/SMS.pm b/perllib/FixMyStreet/SMS.pm new file mode 100644 index 000000000..ec9251a1a --- /dev/null +++ b/perllib/FixMyStreet/SMS.pm @@ -0,0 +1,110 @@ +package FixMyStreet::SMS; + +use strict; +use warnings; + +# use JSON::MaybeXS; +use Moo; +use Number::Phone::Lib; +use WWW::Twilio::API; + +use FixMyStreet; +use mySociety::EmailUtil qw(is_valid_email); +use FixMyStreet::DB; + +has twilio => ( + is => 'lazy', + default => sub { + WWW::Twilio::API->new( + AccountSid => FixMyStreet->config('TWILIO_ACCOUNT_SID'), + AuthToken => FixMyStreet->config('TWILIO_AUTH_TOKEN'), + utf8 => 1, + ); + }, +); + +has from => ( + is => 'lazy', + default => sub { FixMyStreet->config('TWILIO_FROM_PARAMETER') }, +); + +sub send_token { + my ($class, $token_data, $token_scope, $to) = @_; + + # Random number between 10,000 and 75,535 + my $random = 10000 + unpack('n', mySociety::Random::random_bytes(2, 1)); + $token_data->{code} = $random; + my $token_obj = FixMyStreet::DB->resultset("Token")->create({ + scope => $token_scope, + data => $token_data, + }); + my $body = sprintf(_("Your verification code is %s"), $random); + + my $result = $class->new->send(to => $to, body => $body); + return { + random => $random, + token => $token_obj->token, + result => $result, + }; +} + +sub send { + my ($self, %params) = @_; + my $output = $self->twilio->POST('Messages.json', + From => $self->from, + To => $params{to}, + Body => $params{body}, + ); + # At present, we do nothing and assume sent okay. + # TODO add error checking + # my $data = decode_json($output->{content}); + # if ($output->{code} != 200) { + # return { error => "$data->{message} ($data->{code})" }; + # } + # return { success => $data->{sid} }; +} + +=head2 parse_username + +Given a string that might be an email address or a phone number, +return what we think it is, and if it's valid one of those. Or +undef if it's empty. + +=cut + +sub parse_username { + my ($class, $username) = @_; + + return { type => 'email', username => $username } unless $username; + + $username = lc $username; + $username =~ s/\s+//g; + + return { type => 'email', email => $username, username => $username } if is_valid_email($username); + + my $type = $username =~ /^[^a-z]+$/i ? 'phone' : 'email'; + my $phone = do { + if ($username =~ /^\+/) { + # If already in international format, use that + Number::Phone::Lib->new($username) + } else { + # Otherwise, assume it is country configured + my $country = FixMyStreet->config('PHONE_COUNTRY'); + Number::Phone::Lib->new($country, $username); + } + }; + + if ($phone) { + $type = 'phone'; + # Store phone without spaces + ($username = $phone->format) =~ s/\s+//g; + } + + return { + type => $type, + phone => $phone, + username => $username, + }; +} + +1; diff --git a/perllib/FixMyStreet/Script/Alerts.pm b/perllib/FixMyStreet/Script/Alerts.pm index c001cc311..86f11c7b5 100644 --- a/perllib/FixMyStreet/Script/Alerts.pm +++ b/perllib/FixMyStreet/Script/Alerts.pm @@ -109,7 +109,7 @@ sub send() { my $user = $schema->resultset('User')->find( { id => $row->{alert_user_id} } ); - $data{alert_email} = $user->email; + $data{alert_user} = $user; my $token_obj = $schema->resultset('Token')->create( { scope => 'alert_to_reporter', data => { @@ -209,7 +209,7 @@ sub send() { template => $template, data => [], alert_id => $alert->id, - alert_email => $alert->user->email, + alert_user => $alert->user, lang => $alert->lang, cobrand => $cobrand, cobrand_data => $alert->cobrand_data, @@ -258,16 +258,20 @@ sub _send_aggregated_alert_email(%) { $cobrand->set_lang_and_domain( $data{lang}, 1, FixMyStreet->path_to('locale')->stringify ); FixMyStreet::Map::set_map_class($cobrand->map_type); - if (!$data{alert_email}) { + if (!$data{alert_user}) { my $user = $data{schema}->resultset('User')->find( { id => $data{alert_user_id} } ); - $data{alert_email} = $user->email; + $data{alert_user} = $user; } - my ($domain) = $data{alert_email} =~ m{ @ (.*) \z }x; + # Ignore phone-only users + return unless $data{alert_user}->email_verified; + + my $email = $data{alert_user}->email; + my ($domain) = $email =~ m{ @ (.*) \z }x; return if $data{schema}->resultset('Abuse')->search( { - email => [ $data{alert_email}, $domain ] + email => [ $email, $domain ] } )->first; my $token = $data{schema}->resultset("Token")->new_result( { @@ -275,7 +279,7 @@ sub _send_aggregated_alert_email(%) { data => { id => $data{alert_id}, type => 'unsubscribe', - email => $data{alert_email}, + email => $email, } } ); $data{unsubscribe_url} = $cobrand->base_url( $data{cobrand_data} ) . '/A/' . $token->token; @@ -286,7 +290,7 @@ sub _send_aggregated_alert_email(%) { "$data{template}.txt", \%data, { - To => $data{alert_email}, + To => $email, }, $sender, 0, diff --git a/perllib/FixMyStreet/Script/Questionnaires.pm b/perllib/FixMyStreet/Script/Questionnaires.pm index ec6139d2d..5fc01512d 100644 --- a/perllib/FixMyStreet/Script/Questionnaires.pm +++ b/perllib/FixMyStreet/Script/Questionnaires.pm @@ -49,7 +49,11 @@ sub send_questionnaires_period { # Not all cobrands send questionnaires next unless $cobrand->send_questionnaires; - next if $row->is_from_abuser; + + if ($row->is_from_abuser || !$row->user->email_verified) { + $row->update( { send_questionnaire => 0 } ); + next; + } # Cobranded and non-cobranded messages can share a database. In this case, the conf file # should specify a vhost to send the reports for each cobrand, so that they don't get sent diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm index 1e5fd55bb..1ea98342c 100644 --- a/perllib/FixMyStreet/Script/Reports.pm +++ b/perllib/FixMyStreet/Script/Reports.pm @@ -84,7 +84,6 @@ sub send(;$) { $h{query} = $row->postcode; $h{url} = $email_base_url . $row->url; $h{admin_url} = $row->admin_url($cobrand); - $h{phone_line} = $h{phone} ? _('Phone:') . " $h{phone}\n\n" : ''; if ($row->photo) { $h{has_photo} = _("This web page also contains a photo of the problem, provided by the user.") . "\n\n"; $h{image_url} = $email_base_url . $row->photos->[0]->{url_full}; @@ -299,6 +298,9 @@ sub _send_report_sent_email { my $nomail = shift; my $cobrand = shift; + # Don't send 'report sent' text + return unless $row->user->email_verified; + FixMyStreet::Email::send_cron( $row->result_source->schema, 'confirm_report_sent.txt', diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm index eefb14553..0aacc14a1 100644 --- a/perllib/FixMyStreet/SendReport/Email.pm +++ b/perllib/FixMyStreet/SendReport/Email.pm @@ -74,14 +74,21 @@ sub send { my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->cobrand)->new(); my $params = { To => $self->to, - From => $self->send_from( $row ), }; $cobrand->call_hook(munge_sendreport_params => $row, $h, $params); $params->{Bcc} = $self->bcc if @{$self->bcc}; - my $sender = FixMyStreet::Email::unique_verp_id('report', $row->id); + my $sender; + if ($row->user->email && $row->user->email_verified) { + $sender = FixMyStreet::Email::unique_verp_id('report', $row->id); + $params->{From} = $self->send_from( $row ); + } else { + $sender = FixMyStreet->config('DO_NOT_REPLY_EMAIL'); + my $name = sprintf(_("On behalf of %s"), $params->{From}[1]); + $params->{From} = [ $sender, $name ]; + } if (FixMyStreet::Email::test_dmarc($params->{From}[0]) || Utils::Email::same_domain($params->{From}, $params->{To})) { diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index 4bad1d17b..dbfc94286 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -65,11 +65,12 @@ Create a test user (or find it and return if it already exists). sub create_user_ok { my $self = shift; - my ( $email, %extra ) = @_; + my ( $username, %extra ) = @_; - my $params = { email => $email, %extra }; + my $params = { %extra }; + $username =~ /@/ ? $params->{email} = $username : $params->{phone} = $username; my $user = FixMyStreet::DB->resultset('User')->find_or_create($params); - ok $user, "found/created user for $email"; + ok $user, "found/created user for $username"; return $user; } @@ -78,15 +79,15 @@ sub create_user_ok { $user = $mech->log_in_ok( $email_address ); -Log in with the email given. If email does not match an account then create one. +Log in with the email/phone given. If email/phone does not match an account then create one. =cut sub log_in_ok { my $mech = shift; - my $email = shift; + my $username = shift; - my $user = $mech->create_user_ok($email); + my $user = $mech->create_user_ok($username); # remember the old password and then change it to a known one my $old_password = $user->password || ''; @@ -95,7 +96,7 @@ sub log_in_ok { # log in $mech->get_ok('/auth'); $mech->submit_form_ok( - { with_fields => { email => $email, password_sign_in => 'secret' } }, + { with_fields => { username => $username, password_sign_in => 'secret' } }, "sign in using form" ); $mech->logged_in_ok; @@ -135,6 +136,7 @@ sub log_out_ok { $mech->delete_user( $user ); $mech->delete_user( $email ); + $mech->delete_user( $phone ); Delete the current user, including linked objects like problems etc. Can be either a user object or an email address. @@ -142,14 +144,14 @@ either a user object or an email address. =cut sub delete_user { - my $mech = shift; - my $email_or_user = shift; + my $mech = shift; + my $user_or_username = shift; - my $user = - ref $email_or_user - ? $email_or_user - : FixMyStreet::DB->resultset('User') - ->find( { email => $email_or_user } ); + my $user = ref $user_or_username ? $user_or_username : undef; + $user = FixMyStreet::DB->resultset('User')->find( { email => $user_or_username } ) + unless $user; + $user = FixMyStreet::DB->resultset('User')->find( { phone => $user_or_username } ) + unless $user; # If no user found we can't delete them return 1 unless $user; @@ -667,8 +669,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/perllib/Open311.pm b/perllib/Open311.pm index da5a0a377..90b593256 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -127,13 +127,15 @@ sub _populate_service_request_params { my ( $firstname, $lastname ) = ( $problem->name =~ /(\w+)\.?\s+(.+)/ ); my $params = { - email => $problem->user->email, description => $description, service_code => $service_code, first_name => $firstname, last_name => $lastname || '', }; + $params->{phone} = $problem->user->phone if $problem->user->phone; + $params->{email} = $problem->user->email if $problem->user->email; + # if you click nearby reports > skip map then it's possible # to end up with used_map = f and nothing in postcode if ( $problem->used_map || $self->always_send_latlong @@ -153,10 +155,6 @@ sub _populate_service_request_params { $params->{address_string} = $problem->postcode; } - if ( $problem->user->phone ) { - $params->{ phone } = $problem->user->phone; - } - if ( $extra->{image_url} ) { $params->{media_url} = $extra->{image_url}; } @@ -327,12 +325,14 @@ sub _populate_service_request_update_params { updated_datetime => DateTime::Format::W3CDTF->format_datetime($comment->confirmed->set_nanosecond(0)), service_request_id => $comment->problem->external_id, status => $status, - email => $comment->user->email, description => $comment->text, last_name => $lastname, first_name => $firstname, }; + $params->{phone} = $comment->user->phone if $comment->user->phone; + $params->{email} = $comment->user->email if $comment->user->email; + if ( $self->use_extended_updates ) { $params->{public_anonymity_required} = $comment->anonymous ? 'TRUE' : 'FALSE', $params->{update_id_ext} = $comment->id; diff --git a/t/Mock/Twilio.pm b/t/Mock/Twilio.pm new file mode 100644 index 000000000..eaad30b76 --- /dev/null +++ b/t/Mock/Twilio.pm @@ -0,0 +1,28 @@ +package t::Mock::Twilio; + +use Web::Simple; + +has texts => ( + is => 'ro', + default => sub { [] }, +); + +sub get_text_code { + my $self = shift; + my $text = shift @{$self->texts}; + return unless $text; + my ($code) = $text->{Body} =~ /(\d+)/; + return $code; +} + +sub dispatch_request { + my $self = shift; + + sub (POST + /2010-04-01/Accounts/*/Messages.json + %*) { + my ($self, $sid, $data) = @_; + push @{$self->texts}, $data; + return [ 200, [ 'Content-Type' => 'application/json' ], [ '{}' ] ]; + }, +} + +__PACKAGE__->run_if_script; diff --git a/t/app/controller/admin.t b/t/app/controller/admin.t index bd0f9e408..9f1b0cfb9 100644 --- a/t/app/controller/admin.t +++ b/t/app/controller/admin.t @@ -316,7 +316,7 @@ foreach my $test ( detail => 'Detail for Report to Edit', state => 'confirmed', name => 'Test User', - email => $user->email, + username => $user->email, anonymous => 0, flagged => undef, non_public => undef, @@ -332,7 +332,7 @@ foreach my $test ( detail => 'Detail for Report to Edit', state => 'confirmed', name => 'Test User', - email => $user->email, + username => $user->email, anonymous => 0, flagged => undef, non_public => undef, @@ -348,7 +348,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Test User', - email => $user->email, + username => $user->email, anonymous => 0, flagged => undef, non_public => undef, @@ -365,7 +365,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user->email, + username => $user->email, anonymous => 0, flagged => undef, non_public => undef, @@ -384,12 +384,12 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user->email, + username => $user->email, anonymous => 0, flagged => 'on', non_public => undef, }, - changes => { email => $user2->email, }, + changes => { username => $user2->email, }, log_entries => [qw/edit edit edit edit edit/], resend => 0, user => $user2, @@ -401,7 +401,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 0, flagged => 'on', non_public => undef, @@ -417,7 +417,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'unconfirmed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 0, flagged => 'on', non_public => undef, @@ -433,7 +433,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 0, flagged => 'on', non_public => undef, @@ -450,7 +450,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'fixed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 0, flagged => 'on', non_public => undef, @@ -468,7 +468,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'hidden', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 0, flagged => 'on', non_public => undef, @@ -489,7 +489,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 1, flagged => 'on', non_public => undef, @@ -507,7 +507,7 @@ foreach my $test ( detail => 'Edited Detail', state => 'confirmed', name => 'Edited User', - email => $user2->email, + username => $user2->email, anonymous => 1, flagged => 'on', non_public => undef, @@ -555,7 +555,7 @@ foreach my $test ( $test->{changes}->{flagged} = 1 if $test->{changes}->{flagged}; $test->{changes}->{non_public} = 1 if $test->{changes}->{non_public}; - is $report->$_, $test->{changes}->{$_}, "$_ updated" for grep { $_ ne 'email' } keys %{ $test->{changes} }; + is $report->$_, $test->{changes}->{$_}, "$_ updated" for grep { $_ ne 'username' } keys %{ $test->{changes} }; if ( $test->{user} ) { is $report->user->id, $test->{user}->id, 'user changed'; @@ -603,7 +603,7 @@ subtest 'change email to new user' => sub { detail => $report->detail, state => $report->state, name => $report->name, - email => $report->user->email, + username => $report->user->email, category => 'Other', anonymous => 1, flagged => 'on', @@ -616,12 +616,10 @@ subtest 'change email to new user' => sub { is_deeply( $mech->visible_form_values(), $fields, 'initial form values' ); my $changes = { - email => 'test3@example.com' + username => 'test3@example.com' }; - $user3 = - FixMyStreet::App->model('DB::User') - ->find( { email => 'test3@example.com', name => 'Test User 2' } ); + $user3 = FixMyStreet::App->model('DB::User')->find( { email => 'test3@example.com' } ); ok !$user3, 'user not in database'; @@ -640,9 +638,7 @@ subtest 'change email to new user' => sub { is $log_entries->first->action, 'edit', 'log action'; is_deeply( $mech->visible_form_values(), $new_fields, 'changed form values' ); - $user3 = - FixMyStreet::App->model('DB::User') - ->find( { email => 'test3@example.com', name => 'Test User 2' } ); + $user3 = FixMyStreet::App->model('DB::User')->find( { email => 'test3@example.com' } ); $report->discard_changes; @@ -657,18 +653,18 @@ subtest 'adding email to abuse list from report page' => sub { $abuse->delete if $abuse; $mech->get_ok( '/admin/report_edit/' . $report->id ); - $mech->content_contains('Ban email address'); + $mech->content_contains('Ban user'); $mech->click_ok('banuser'); - $mech->content_contains('Email added to abuse list'); - $mech->content_contains('<small>(Email in abuse table)</small>'); + $mech->content_contains('User added to abuse list'); + $mech->content_contains('<small>(User in abuse table)</small>'); $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $email } ); ok $abuse, 'entry created in abuse table'; $mech->get_ok( '/admin/report_edit/' . $report->id ); - $mech->content_contains('<small>(Email in abuse table)</small>'); + $mech->content_contains('<small>(User in abuse table)</small>'); }; subtest 'flagging user from report page' => sub { @@ -742,7 +738,7 @@ for my $test ( state => 'confirmed', name => '', anonymous => 1, - email => 'test@example.com', + username => 'test@example.com', }, changes => { text => 'this is a changed update', @@ -757,7 +753,7 @@ for my $test ( state => 'confirmed', name => '', anonymous => 1, - email => 'test@example.com', + username => 'test@example.com', }, changes => { name => 'A User', @@ -772,7 +768,7 @@ for my $test ( state => 'confirmed', name => 'A User', anonymous => 1, - email => 'test@example.com', + username => 'test@example.com', }, changes => { anonymous => 0, @@ -787,11 +783,10 @@ for my $test ( state => 'confirmed', name => 'A User', anonymous => 0, - email => $update->user->email, - email => 'test@example.com', + username => 'test@example.com', }, changes => { - email => 'test2@example.com', + username => 'test2@example.com', }, log_count => 4, log_entries => [qw/edit edit edit edit/], @@ -804,7 +799,7 @@ for my $test ( state => 'confirmed', name => 'A User', anonymous => 0, - email => 'test2@example.com', + username => 'test2@example.com', }, changes => { state => 'unconfirmed', @@ -819,7 +814,7 @@ for my $test ( state => 'unconfirmed', name => 'A User', anonymous => 0, - email => 'test2@example.com', + username => 'test2@example.com', }, changes => { text => 'this is a twice changed update', @@ -849,7 +844,7 @@ for my $test ( $update->discard_changes; - is $update->$_, $test->{changes}->{$_} for grep { $_ ne 'email' } keys %{ $test->{changes} }; + is $update->$_, $test->{changes}->{$_} for grep { $_ ne 'username' } keys %{ $test->{changes} }; if ( $test->{changes}{state} && $test->{changes}{state} eq 'confirmed' ) { isnt $update->confirmed, undef; } @@ -935,9 +930,7 @@ for my $test ( } subtest 'editing update email creates new user if required' => sub { - my $user = FixMyStreet::App->model('DB::User')->find( - { email => 'test4@example.com' } - ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'test4@example.com' } ); $user->delete if $user; @@ -946,14 +939,12 @@ subtest 'editing update email creates new user if required' => sub { state => 'hidden', name => 'A User', anonymous => 0, - email => 'test4@example.com', + username => 'test4@example.com', }; $mech->submit_form_ok( { with_fields => $fields } ); - $user = FixMyStreet::App->model('DB::User')->find( - { email => 'test4@example.com' } - ); + $user = FixMyStreet::App->model('DB::User')->find( { email => 'test4@example.com' } ); is_deeply $mech->visible_form_values, $fields, 'submitted form values'; @@ -970,18 +961,18 @@ subtest 'adding email to abuse list from update page' => sub { $abuse->delete if $abuse; $mech->get_ok( '/admin/update_edit/' . $update->id ); - $mech->content_contains('Ban email address'); + $mech->content_contains('Ban user'); $mech->click_ok('banuser'); - $mech->content_contains('Email added to abuse list'); - $mech->content_contains('<small>(Email in abuse table)</small>'); + $mech->content_contains('User added to abuse list'); + $mech->content_contains('<small>(User in abuse table)</small>'); $abuse = FixMyStreet::App->model('DB::Abuse')->find( { email => $email } ); ok $abuse, 'entry created in abuse table'; $mech->get_ok( '/admin/update_edit/' . $update->id ); - $mech->content_contains('<small>(Email in abuse table)</small>'); + $mech->content_contains('<small>(User in abuse table)</small>'); }; subtest 'flagging user from update page' => sub { @@ -1029,13 +1020,12 @@ subtest 'hiding comment marked as fixed reopens report' => sub { $report->state('fixed - user'); $report->update; - my $fields = { text => 'this is a changed update', state => 'hidden', name => 'A User', anonymous => 0, - email => 'test2@example.com', + username => 'test2@example.com', }; $mech->submit_form_ok( { with_fields => $fields } ); @@ -1092,7 +1082,7 @@ subtest 'report search' => sub { subtest 'search abuse' => sub { $mech->get_ok( '/admin/users?search=example' ); - $mech->content_like(qr{test4\@example.com.*</td>\s*<td>.*?</td>\s*<td>\(Email in abuse table}s); + $mech->content_like(qr{test4\@example.com.*</td>\s*<td>.*?</td>\s*<td>\(User in abuse table}s); }; subtest 'show flagged entries' => sub { @@ -1168,6 +1158,69 @@ $user->update; my $southend = $mech->create_body_ok(2607, 'Southend-on-Sea Borough Council'); +for my $test ( + { + desc => 'add user - blank form', + fields => { + email => '', email_verified => 0, + phone => '', phone_verified => 0, + }, + error => ['Please verify at least one of email/phone', 'Please enter a name'], + }, + { + desc => 'add user - blank, verify phone', + fields => { + email => '', email_verified => 0, + phone => '', phone_verified => 1, + }, + error => ['Please enter a valid email or phone number', 'Please enter a name'], + }, + { + desc => 'add user - bad email', + fields => { + name => 'Norman', + email => 'bademail', email_verified => 0, + phone => '', phone_verified => 0, + }, + error => ['Please enter a valid email'], + }, + { + desc => 'add user - bad phone', + fields => { + name => 'Norman', + phone => '01214960000000', phone_verified => 1, + }, + error => ['Please check your phone number is correct'], + }, + { + desc => 'add user - landline', + fields => { + name => 'Norman Name', + phone => '+441214960000', + phone_verified => 1, + }, + error => ['Please enter a mobile number'], + }, + { + desc => 'add user - good details', + fields => { + name => 'Norman Name', + phone => '+61491570156', + phone_verified => 1, + }, + }, +) { + subtest $test->{desc} => sub { + $mech->get_ok('/admin/users'); + $mech->submit_form_ok( { with_fields => $test->{fields} } ); + if ($test->{error}) { + $mech->content_contains($_) for @{$test->{error}}; + } else { + $mech->content_contains('Updated'); + } + }; +} + my %default_perms = ( "permissions[moderate]" => undef, "permissions[planned_reports]" => undef, @@ -1358,6 +1411,32 @@ FixMyStreet::override_config { } }; +FixMyStreet::override_config { + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, +}, sub { + subtest "Test edit user add verified phone" => sub { + $mech->get_ok( '/admin/user_edit/' . $user->id ); + $mech->submit_form_ok( { with_fields => { + phone => '+61491570157', + phone_verified => 1, + } } ); + $mech->content_contains( 'Updated!' ); + }; + + subtest "Test changing user to an existing one" => sub { + my $existing_user = $mech->create_user_ok('existing@example.com', name => 'Existing User'); + $mech->create_problems_for_body(2, 2514, 'Title', { user => $existing_user }); + my $count = FixMyStreet::DB->resultset('Problem')->search({ user_id => $user->id })->count; + $mech->get_ok( '/admin/user_edit/' . $user->id ); + $mech->submit_form_ok( { with_fields => { email => 'existing@example.com' } }, 'submit email change' ); + is $mech->uri->path, '/admin/user_edit/' . $existing_user->id, 'redirected'; + my $p = FixMyStreet::DB->resultset('Problem')->search({ user_id => $existing_user->id })->count; + is $p, $count + 2, 'reports merged'; + }; + +}; + subtest "Test setting a report from unconfirmed to something else doesn't cause a front end error" => sub { $report->update( { confirmed => undef, state => 'unconfirmed', non_public => 0 } ); $mech->get_ok("/admin/report_edit/$report_id"); @@ -1380,6 +1459,7 @@ subtest "Check admin_base_url" => sub { $mech->log_out_ok; subtest "Users without from_body can't access admin" => sub { + $user = FixMyStreet::App->model('DB::User')->find( { email => 'existing@example.com' } ); $user->from_body( undef ); $user->update; diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index cb7d16969..661f99412 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -4,7 +4,6 @@ use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; my $test_email = 'test@example.com'; -my $test_email2 = 'test@example.net'; my $test_email3 = 'newuser@example.org'; my $test_password = 'foobar'; @@ -41,8 +40,8 @@ for my $test ( $mech->submit_form_ok( { form_name => 'general_auth', - fields => { email => $email, }, - button => 'email_sign_in', + fields => { username => $email, }, + button => 'sign_in_by_code', }, "try to create an account with email '$email'" ); @@ -60,8 +59,8 @@ $mech->get_ok('/auth'); $mech->submit_form_ok( { form_name => 'general_auth', - fields => { email => $test_email, }, - button => 'email_sign_in', + fields => { username => $test_email, password_register => $test_password }, + button => 'sign_in_by_code', }, "create an account for '$test_email'" ); @@ -101,125 +100,6 @@ $mech->not_logged_in_ok; $mech->log_out_ok; } -# get a sign in email and change password -{ - $mech->clear_emails_ok; - $mech->get_ok('/auth'); - $mech->submit_form_ok( - { - form_name => 'general_auth', - fields => { - email => "$test_email", - r => 'faq', # Just as a test - }, - button => 'email_sign_in', - }, - "email_sign_in with '$test_email'" - ); - - # rest is as before so no need to test - - # follow link and change password - check not prompted for old password - $mech->not_logged_in_ok; - - my $link = $mech->get_link_from_email; - $mech->get_ok($link); - is $mech->uri->path, '/faq', "redirected to the Help page"; - - $mech->get_ok('/auth/change_password'); - - ok my $form = $mech->form_name('change_password'), - "found change password form"; - is_deeply [ sort grep { $_ } map { $_->name } $form->inputs ], # - [ 'confirm', 'new_password', 'token' ], - "check we got expected fields (ie not old_password)"; - - # check the various ways the form can be wrong - for my $test ( - { new => '', conf => '', err => 'enter a password', }, - { new => 'secret', conf => '', err => 'do not match', }, - { new => '', conf => 'secret', err => 'do not match', }, - { new => 'secret', conf => 'not_secret', err => 'do not match', }, - ) - { - $mech->get_ok('/auth/change_password'); - $mech->content_lacks( $test->{err}, "did not find expected error" ); - $mech->submit_form_ok( - { - form_name => 'change_password', - fields => - { new_password => $test->{new}, confirm => $test->{conf}, }, - }, - "change_password with '$test->{new}' and '$test->{conf}'" - ); - $mech->content_contains( $test->{err}, "found expected error" ); - } - - my $user = - FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); - ok $user, "got a user"; - ok !$user->password, "user has no password"; - - $mech->get_ok('/auth/change_password'); - $mech->submit_form_ok( - { - form_name => 'change_password', - fields => - { new_password => $test_password, confirm => $test_password, }, - }, - "change_password with '$test_password' and '$test_password'" - ); - is $mech->uri->path, '/auth/change_password', - "still on change password page"; - $mech->content_contains( 'password has been changed', - "found password changed" ); - - $user->discard_changes(); - ok $user->password, "user now has a password"; -} - -subtest "Test change email page" => sub { - # Still signed in from the above test - $mech->get_ok('/my'); - $mech->follow_link_ok({url => '/auth/change_email'}); - $mech->submit_form_ok( - { with_fields => { email => "" } }, - "submit blank change email form" - ); - $mech->content_contains( 'Please enter your email', "found expected error" ); - $mech->submit_form_ok({ with_fields => { email => $test_email2 } }, "change_email to $test_email2"); - is $mech->uri->path, '/auth/change_email', "still on change email page"; - $mech->content_contains( 'Now check your email', "found check your email" ); - my $link = $mech->get_link_from_email; - $mech->get_ok($link); - is $mech->uri->path, '/auth/change_email/success', "redirected to the change_email page"; - $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"); - $mech->submit_form_ok({ with_fields => { email => $test_email } }, - "change_email back to $test_email" - ); - is $mech->uri->path, '/auth/change_email', "still on change email page"; - $mech->content_contains( 'Now check your email', "found check your email" ); - $link = $mech->get_link_from_email; - $mech->get_ok($link); - is $mech->uri->path, '/auth/change_email/success', "redirected to the change_email page"; - $mech->content_contains('successfully confirmed'); - - # Test you can't click the link if logged out - $mech->submit_form_ok({ with_fields => { email => $test_email } }, - "change_email back to $test_email" - ); - is $mech->uri->path, '/auth/change_email', "still on change email page"; - $mech->content_contains( 'Now check your email', "found check your email" ); - $link = $mech->get_link_from_email; - $mech->log_out_ok; - $mech->get_ok($link); - isnt $mech->uri->path, '/auth/change_email/success', "not redirected to the change_email page"; - $mech->content_contains('Sorry'); -}; - foreach my $remember_me ( '1', '0' ) { subtest "sign in using valid details (remember_me => '$remember_me')" => sub { $mech->get_ok('/auth'); @@ -227,11 +107,11 @@ foreach my $remember_me ( '1', '0' ) { { form_name => 'general_auth', fields => { - email => $test_email, + username => $test_email, password_sign_in => $test_password, remember_me => ( $remember_me ? 1 : undef ), }, - button => 'sign_in', + button => 'sign_in_by_password', }, "sign in with '$test_email' & '$test_password'" ); @@ -253,15 +133,15 @@ $mech->submit_form_ok( { form_name => 'general_auth', fields => { - email => $test_email, + username => $test_email, password_sign_in => 'not the password', }, - button => 'sign_in', + button => 'sign_in_by_password', }, "sign in with '$test_email' & 'not the password'" ); is $mech->uri->path, '/auth', "redirected to correct page"; -$mech->content_contains( 'problem with your email/password combination', 'found error message' ); +$mech->content_contains( 'problem with your login information', 'found error message' ); subtest "sign in but have email form autofilled" => sub { $mech->get_ok('/auth'); @@ -269,11 +149,11 @@ subtest "sign in but have email form autofilled" => sub { { form_name => 'general_auth', fields => { - email => $test_email, + username => $test_email, password_sign_in => $test_password, name => 'Auto-completed from elsewhere', }, - button => 'sign_in', + button => 'sign_in_by_password', }, "sign in with '$test_email' and auto-completed name" ); @@ -289,10 +169,10 @@ subtest "sign in with uppercase email" => sub { { form_name => 'general_auth', fields => { - email => $uc_test_email, + username => $uc_test_email, password_sign_in => $test_password, }, - button => 'sign_in', + button => 'sign_in_by_password', }, "sign in with '$uc_test_email' and auto-completed name" ); @@ -317,8 +197,8 @@ FixMyStreet::override_config { $mech->submit_form_ok( { form_name => 'general_auth', - fields => { email => $test_email3, }, - button => 'email_sign_in', + fields => { username => $test_email3, }, + button => 'sign_in_by_code', }, "create a new account" ); @@ -338,13 +218,13 @@ FixMyStreet::override_config { { form_name => 'general_auth', fields => { - email => "$test_email", + username => "$test_email", password_register => $new_password, r => 'faq', # Just as a test }, - button => 'email_sign_in', + button => 'sign_in_by_code', }, - "email_sign_in with '$test_email'" + "sign_in_by_code with '$test_email'" ); $mech->not_logged_in_ok; @@ -361,10 +241,10 @@ FixMyStreet::override_config { { form_name => 'general_auth', fields => { - email => $test_email, + username => $test_email, password_sign_in => $new_password, }, - button => 'sign_in', + button => 'sign_in_by_password', }, "sign in with '$test_email' and new password" ); diff --git a/t/app/controller/auth_phone.t b/t/app/controller/auth_phone.t new file mode 100644 index 000000000..8673f5c62 --- /dev/null +++ b/t/app/controller/auth_phone.t @@ -0,0 +1,94 @@ +use FixMyStreet::TestMech; + +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +my $mech = FixMyStreet::TestMech->new; + +subtest 'Log in with invalid number, fail' => sub { + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + form_name => 'general_auth', + fields => { username => '01214960000000' }, + button => 'sign_in_by_code', + }, "sign in using bad number"); + $mech->content_contains('Please check your phone number is correct'); + }; +}; + +subtest 'Log in using landline, fail' => sub { + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + form_name => 'general_auth', + fields => { username => '01214960000' }, + button => 'sign_in_by_code', + }, "sign in using landline"); + $mech->content_contains('Please enter a mobile number'); + }; +}; + +subtest 'Log in using mobile, by text' => sub { + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok({ + form_name => 'general_auth', + fields => { username => '+61491570156', password_register => 'secret' }, + button => 'sign_in_by_code', + }, "sign in using mobile"); + + $mech->submit_form_ok({ + with_fields => { code => '00000' } + }, 'submit incorrect code'); + $mech->content_contains('Try again'); + + my $code = $twilio->get_text_code; + $mech->submit_form_ok({ + with_fields => { code => $code } + }, 'submit correct code'); + + my $user = FixMyStreet::App->model('DB::User')->find( { phone => '+61491570156' } ); + ok $user, "user created"; + is $mech->uri->path, '/my', "redirected to the 'my' section of site"; + $mech->logged_in_ok; + $mech->log_out_ok; + }; +}; + +subtest 'Log in using mobile, by password' => sub { + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + }, sub { + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + form_name => 'general_auth', + fields => { username => '+61491570156', password_sign_in => 'incorrect' }, + button => 'sign_in_by_password', + }, "sign in using wrong password"); + $mech->content_contains('There was a problem'); + $mech->submit_form_ok({ + form_name => 'general_auth', + fields => { username => '+61491570156', password_sign_in => 'secret' }, + button => 'sign_in_by_password', + }, "sign in using password"); + + is $mech->uri->path, '/my', "redirected to the 'my' section of site"; + $mech->logged_in_ok; + }; +}; + +done_testing(); diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t new file mode 100644 index 000000000..519086ff5 --- /dev/null +++ b/t/app/controller/auth_profile.t @@ -0,0 +1,262 @@ +use FixMyStreet::TestMech; +my $mech = FixMyStreet::TestMech->new; + +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +my $test_email = 'test@example.com'; +my $test_email2 = 'test@example.net'; +my $test_password = 'foobar'; + +END { + done_testing(); +} + +# get a sign in email and change password +subtest "Test change password page" => sub { + $mech->clear_emails_ok; + $mech->get_ok('/auth'); + $mech->submit_form_ok( + { + form_name => 'general_auth', + fields => { + username => $test_email, + r => 'faq', # Just as a test + }, + button => 'sign_in_by_code', + }, + "sign_in_by_code with '$test_email'" + ); + + # follow link and change password - check not prompted for old password + $mech->not_logged_in_ok; + + my $link = $mech->get_link_from_email; + $mech->get_ok($link); + is $mech->uri->path, '/faq', "redirected to the Help page"; + + $mech->get_ok('/auth/change_password'); + + ok my $form = $mech->form_name('change_password'), + "found change password form"; + is_deeply [ sort grep { $_ } map { $_->name } $form->inputs ], # + [ 'confirm', 'new_password', 'token' ], + "check we got expected fields (ie not old_password)"; + + # check the various ways the form can be wrong + for my $test ( + { new => '', conf => '', err => 'enter a password', }, + { new => 'secret', conf => '', err => 'do not match', }, + { new => '', conf => 'secret', err => 'do not match', }, + { new => 'secret', conf => 'not_secret', err => 'do not match', }, + ) + { + $mech->get_ok('/auth/change_password'); + $mech->content_lacks( $test->{err}, "did not find expected error" ); + $mech->submit_form_ok( + { + form_name => 'change_password', + fields => + { new_password => $test->{new}, confirm => $test->{conf}, }, + }, + "change_password with '$test->{new}' and '$test->{conf}'" + ); + $mech->content_contains( $test->{err}, "found expected error" ); + } + + my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); + ok $user, "got a user"; + ok !$user->password, "user has no password"; + + $mech->get_ok('/auth/change_password'); + $mech->submit_form_ok( + { + form_name => 'change_password', + fields => + { new_password => $test_password, confirm => $test_password, }, + }, + "change_password with '$test_password' and '$test_password'" + ); + is $mech->uri->path, '/auth/change_password', + "still on change password page"; + $mech->content_contains( 'password has been changed', + "found password changed" ); + + $user->discard_changes(); + ok $user->password, "user now has a password"; +}; + +subtest "Test change email page" => sub { + $mech->create_problems_for_body(1, 2514, 'Title1', { user => FixMyStreet::DB->resultset('User')->find( { email => $test_email } ) } ); + + # Still signed in from the above test + $mech->get_ok('/my'); + $mech->follow_link_ok({url => '/auth/change_email'}); + $mech->submit_form_ok( + { with_fields => { email => "" } }, + "submit blank change email form" + ); + $mech->content_contains( 'Please enter your email', "found expected error" ); + $mech->submit_form_ok({ with_fields => { email => $test_email2 } }, "change_email to $test_email2"); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + my $link = $mech->get_link_from_email; + $mech->get_ok($link); + is $mech->uri->path, '/my', "redirected to /my page"; + $mech->content_contains('successfully confirmed'); + ok(FixMyStreet::App->model('DB::User')->find( { email => $test_email2 } ), "got a user"); + + my $p = FixMyStreet::DB->resultset("Problem")->first; + is $p->user->email, $test_email2, 'problem user updated'; + + my $user1 = FixMyStreet::App->model('DB::User')->create( { email => $test_email, email_verified => 1 } ); + ok($user1, "created old user"); + $mech->create_problems_for_body(1, 2514, 'Title1', { user => $user1 } ); + + $mech->follow_link_ok({url => '/auth/change_email'}); + $mech->submit_form_ok({ with_fields => { email => $test_email } }, + "change_email back to $test_email" + ); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + $link = $mech->get_link_from_email; + $mech->get_ok($link); + is $mech->uri->path, '/my', "redirected to /my page"; + $mech->content_contains('successfully confirmed'); + + for (FixMyStreet::DB->resultset("Problem")->all) { + is $_->user->email, $test_email; + } + + # Test you can't click the link if logged out + $mech->follow_link_ok({url => '/auth/change_email'}); + $mech->submit_form_ok({ with_fields => { email => $test_email } }, + "change_email back to $test_email" + ); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + $link = $mech->get_link_from_email; + $mech->log_out_ok; + $mech->get_ok($link); + isnt $mech->uri->path, '/auth/change_email/success', "not redirected to the change_email page"; + $mech->content_contains('Sorry'); +}; + +my $test_phone_bad = '01214960000000'; +my $test_landline = '01214960000'; +my $test_mobile = '+61491570156'; +my $test_mobile2 = '+61491570157'; + +my $user_mob2 = FixMyStreet::App->model('DB::User')->create( { + phone => $test_mobile, + phone_verified => 1, + name => 'Aus Mobile user', +} ); +$mech->create_problems_for_body(1, 2514, 'Title1', { user => $user_mob2 } ); + +subtest "Test add/verify/change phone page" => sub { + $mech->get_ok('/auth'); + $mech->submit_form_ok({ + with_fields => { + username => $test_email, + password_sign_in => $test_password, + }, + }); + + $mech->follow_link_ok({url => '/auth/change_phone'}); + $mech->submit_form_ok( { with_fields => { username => "" } }, "submit blank change phone form" ); + is $mech->uri->path, '/my', 'redirected'; + $mech->content_contains('successfully removed'); + + $mech->follow_link_ok({url => '/auth/change_phone'}); + $mech->submit_form_ok({ with_fields => { username => $test_phone_bad } }); + $mech->content_contains( 'Please check your phone number is correct', "found expected error" ); + + FixMyStreet::override_config({ + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->submit_form_ok({ with_fields => { username => $test_landline } }); + }); + is $mech->uri->path, '/my', 'redirected'; + $mech->content_contains('successfully added'); + + FixMyStreet::override_config({ + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->follow_link_ok({url => '/auth/verify/phone'}); + $mech->submit_form_ok({ with_fields => { username => $test_landline } }); + }); + $mech->content_contains( 'Please enter a mobile number', "found expected error" ); + + FixMyStreet::override_config({ + SMS_AUTHENTICATION => 1, + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok({ with_fields => { username => $test_mobile } }); + }); + is $mech->uri->path, '/auth/verify/phone', "still on change phone page"; + $mech->content_contains( 'Now check your phone', "found check your phone" ); + + $mech->submit_form_ok({ + with_fields => { code => '00000' } + }, 'submit incorrect code'); + $mech->content_contains('Try again'); + + my $code = $twilio->get_text_code; + $mech->submit_form_ok({ + with_fields => { code => $code } + }, 'submit correct code'); + + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_mobile } ); + ok $user, "user exists"; + is $user->email_verified, 1; + is $user->email, $test_email, 'email still same'; + is $mech->uri->path, '/my', "redirected to /my page"; + $mech->content_contains('successfully verified'); + $mech->logged_in_ok; +}; + +subtest "Test change phone to existing account" => sub { + $mech->get_ok('/auth'); + FixMyStreet::override_config({ + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok({ + with_fields => { + username => $test_mobile, + password_sign_in => $test_password, + }, + }); + }); + + $mech->follow_link_ok({url => '/auth/change_phone'}); + + FixMyStreet::override_config({ + SMS_AUTHENTICATION => 1, + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok({ with_fields => { username => $test_mobile2 } }); + }); + is $mech->uri->path, '/auth/change_phone', "still on change phone page"; + $mech->content_contains( 'Now check your phone', "found check your phone" ); + + my $code = $twilio->get_text_code; + $mech->submit_form_ok({ with_fields => { code => $code } }, 'submit correct code'); + + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_mobile } ); + ok !$user, 'old user does not exist'; + $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_mobile2 } ); + ok $user, "new mobile user exists"; + is $user->email_verified, 1; + is $user->email, $test_email, 'email still same'; + is $mech->uri->path, '/my', "redirected to /my page"; + $mech->content_contains('successfully verified'); + + for (FixMyStreet::DB->resultset("Problem")->all) { + is $_->user->email, $test_email; + } +}; diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t index 726d264bd..031fb8d9e 100644 --- a/t/app/controller/auth_social.t +++ b/t/app/controller/auth_social.t @@ -102,11 +102,7 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { $mech->content_contains('We need your email address, please give it below.'); # We don't have an email, so check that we can still submit it, # and the ID carries through the confirmation - if ($page eq 'update') { - $fields->{rznvy} = $fb_email; - } else { - $fields->{email} = $fb_email; - } + $fields->{username} = $fb_email; $fields->{name} = 'Ffion Tester'; $mech->submit_form(with_fields => $fields); $mech->content_contains('Nearly done! Now check your email'); @@ -214,11 +210,7 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) { $mech->content_contains('We need your email address, please give it below.'); # We don't have an email, so check that we can still submit it, # and the ID carries through the confirmation - if ($page eq 'update') { - $fields->{rznvy} = $tw_email; - } else { - $fields->{email} = $tw_email; - } + $fields->{username} = $tw_email; $fields->{name} = 'Ffion Tester'; $mech->submit_form(with_fields => $fields); $mech->content_contains('Nearly done! Now check your email'); diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t index 457eceade..14bd76c41 100644 --- a/t/app/controller/dashboard.t +++ b/t/app/controller/dashboard.t @@ -31,7 +31,7 @@ FixMyStreet::override_config { $mech->content_contains( 'sign in' ); $mech->submit_form( - with_fields => { email => $test_user, password_sign_in => $test_pass } + with_fields => { username => $test_user, password_sign_in => $test_pass } ); is $mech->status, '404', 'If not council user get 404'; @@ -42,7 +42,7 @@ FixMyStreet::override_config { $mech->log_out_ok; $mech->get_ok('/dashboard'); $mech->submit_form_ok( { - with_fields => { email => $test_user, password_sign_in => $test_pass } + with_fields => { username => $test_user, password_sign_in => $test_pass } } ); $mech->content_contains( 'Area 2651' ); diff --git a/t/app/controller/report_as_other.t b/t/app/controller/report_as_other.t index daa213e8c..91644e8ce 100644 --- a/t/app/controller/report_as_other.t +++ b/t/app/controller/report_as_other.t @@ -47,7 +47,7 @@ subtest "Body user, has permission to add report as another user" => sub { detail => 'Test report details.', category => 'Potholes', name => 'Another User', - email => 'another@example.net', + username => 'another@example.net', ); is $report->name, 'Another User', 'report name is given name'; is $report->user->name, 'Another User', 'user name matches'; @@ -66,7 +66,7 @@ subtest "Body user, has permission to add report as another (existing) user" => detail => 'Test report details.', category => 'Potholes', name => 'Existing Yooser', - email => 'existing@example.net', + username => 'existing@example.net', ); is $report->name, 'Existing Yooser', 'report name is given name'; is $report->user->name, 'Existing User', 'user name remains same'; @@ -108,7 +108,7 @@ subtest "Body user, has permission to add update as another user" => sub { form_as => 'another_user', update => 'Test Update', name => 'Another User', - rznvy => 'another2@example.net', + username => 'another2@example.net', ); is $update->name, 'Another User', 'update name is given name'; is $update->user->name, 'Another User', 'user name matches'; @@ -124,7 +124,7 @@ subtest "Body user, has permission to add update as another (existing) user" => form_as => 'another_user', update => 'Test Update', name => 'Existing Yooser', - rznvy => 'existing@example.net', + username => 'existing@example.net', ); is $update->name, 'Existing Yooser', 'update name is given name'; is $update->user->name, 'Existing User', 'user name remains same'; diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index 4d73a5204..f0913fbd2 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -128,7 +128,7 @@ subtest "test a good report" => sub { my %fields = ( name => '', - rznvy => '', + username => '', update => '', add_alert => 1, # defaults to true fixed => undef diff --git a/t/app/controller/report_import.t b/t/app/controller/report_import.t index 47113198e..e4a202db7 100644 --- a/t/app/controller/report_import.t +++ b/t/app/controller/report_import.t @@ -362,14 +362,12 @@ subtest "Submit a correct entry (with location) to cobrand" => sub { photo2 => '', photo3 => '', phone => '', - email => 'test-ll@example.com', + username => 'test-ll@example.com', }, "check imported fields are shown" or diag Dumper( $mech->visible_form_values ); use Data::Dumper; - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'test-ll@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'test-ll@example.com' } ); ok $user, "Found a user"; my $report = $user->problems->first; diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index ab6b5d78e..efe392eab 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -101,6 +101,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', password_sign_in => '', @@ -127,6 +128,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Something bad', @@ -156,6 +158,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -182,6 +185,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => undef, + username => '', email => '', phone => '', category => 'Street lighting', @@ -208,6 +212,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => undef, + username => '', email => '', phone => '', category => 'Street lighting', @@ -233,6 +238,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -258,6 +264,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -283,6 +290,7 @@ foreach my $test ( photo3 => '', name => 'DUDE', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -307,6 +315,7 @@ foreach my $test ( photo3 => '', name => 'anonymous', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -331,14 +340,15 @@ foreach my $test ( photo3 => '', name => 'Joe Smith', may_show_name => '1', - email => 'not an email', + username => 'not an email', + email => '', phone => '', category => 'Street lighting', password_sign_in => '', password_register => '', remember_me => undef, }, - changes => { email => 'notanemail', }, + changes => { username => 'notanemail', email => 'notanemail' }, errors => [ 'Please enter a valid email', ], }, { @@ -352,6 +362,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -379,7 +390,8 @@ foreach my $test ( photo3 => '', name => ' Bob Jones ', may_show_name => '1', - email => ' BOB @ExAmplE.COM ', + username => ' BOB @ExAmplE.COM ', + email => '', phone => '', category => 'Street lighting', password_sign_in => '', @@ -388,6 +400,7 @@ foreach my $test ( }, changes => { name => 'Bob Jones', + username => 'bob@example.com', email => 'bob@example.com', }, errors => [ 'Please enter a subject', 'Please enter some details', ], @@ -403,6 +416,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -426,6 +440,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -449,6 +464,7 @@ foreach my $test ( photo3 => '', name => 'Bob Jones', may_show_name => '1', + username => 'bob@example.com', email => 'bob@example.com', phone => '', category => 'Street lighting', @@ -560,7 +576,7 @@ foreach my $test ( photo1 => '', name => 'Joe Bloggs', may_show_name => '1', - email => 'test-1@example.com', + username => 'test-1@example.com', phone => '07903 123 456', category => 'Street lighting', password_register => $test->{password} ? 'secret' : '', @@ -674,7 +690,7 @@ subtest "test password errors for a user who is signing in as they report" => su title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', password_sign_in => 'secret1', category => 'Street lighting', } @@ -685,7 +701,7 @@ subtest "test password errors for a user who is signing in as they report" => su # check that we got the errors expected is_deeply $mech->page_errors, [ - "There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the \x{2018}sign in by email\x{2019} section of the form.", + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", ], "check there were errors"; }; @@ -726,7 +742,7 @@ subtest "test report creation for a user who is signing in as they report" => su title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', password_sign_in => 'secret2', category => 'Street lighting', } @@ -947,7 +963,7 @@ subtest "test report creation for a category that is non public" => sub { title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', name => 'Joe Bloggs', category => 'Street lighting', } @@ -1135,7 +1151,7 @@ for my $test ( title => "Test Report", detail => 'Test report details.', photo1 => '', - email => 'firstlast@example.com', + username => 'firstlast@example.com', may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1167,9 +1183,7 @@ for my $test ( # confirm token in order to update the user details $mech->get_ok($url); - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'firstlast@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'firstlast@example.com' } ); my $report = $user->problems->first; ok $report, "Found the report"; @@ -1284,7 +1298,7 @@ subtest "test Hart" => sub { $mech->submit_form_ok( { with_fields => { pc => 'GU51 4AE' } }, "submit location" ); $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); my %optional_fields = $test->{confirm} ? () : - ( email => $test_email, phone => '07903 123 456' ); + ( username => $test_email, phone => '07903 123 456' ); # we do this as otherwise test::www::mechanize::catalyst # goes to the value set in ->host above irregardless and @@ -1424,7 +1438,7 @@ subtest "unresponsive body handling works" => sub { detail => 'Test report details.', photo1 => '', name => 'Joe Bloggs', - email => $test_email, + username => $test_email, may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1497,7 +1511,7 @@ subtest "unresponsive body handling works" => sub { detail => 'Test report details.', photo1 => '', name => 'Joe Bloggs', - email => $test_email, + username => $test_email, may_show_name => '1', phone => '07903 123 456', category => 'Trees', @@ -1618,7 +1632,7 @@ subtest "extra google analytics code displayed on email confirmation problem cre title => "Test Report", detail => 'Test report details.', photo1 => '', - email => 'firstlast@example.com', + username => 'firstlast@example.com', name => 'Test User', may_show_name => '1', phone => '07903 123 456', @@ -1639,9 +1653,7 @@ subtest "extra google analytics code displayed on email confirmation problem cre $mech->get_ok($url); # find the report - my $user = - FixMyStreet::App->model('DB::User') - ->find( { email => 'firstlast@example.com' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { email => 'firstlast@example.com' } ); my $report = $user->problems->first; ok $report, "Found the report"; diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t index 9a4a81182..0224e7e47 100644 --- a/t/app/controller/report_new_open311.t +++ b/t/app/controller/report_new_open311.t @@ -54,6 +54,7 @@ foreach my $test ( photo3 => '', name => '', may_show_name => '1', + username => '', email => '', phone => '', category => 'Street lighting', @@ -76,7 +77,7 @@ foreach my $test ( title => 'test', detail => 'test detail', name => 'Test User', - email => 'testopen311@example.com', + username => 'testopen311@example.com', category => 'Street lighting', number => 27, }, @@ -100,7 +101,7 @@ foreach my $test ( $mech->clear_emails_ok; # check that the user does not exist - my $test_email = $test->{submit_with}->{email}; + my $test_email = $test->{submit_with}->{username}; my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } ); if ( $user ) { $user->problems->delete; diff --git a/t/app/controller/report_new_text.t b/t/app/controller/report_new_text.t new file mode 100644 index 000000000..94f350eba --- /dev/null +++ b/t/app/controller/report_new_text.t @@ -0,0 +1,371 @@ +use FixMyStreet::TestMech; +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +# disable info logs for this test run +FixMyStreet::App->log->disable('info'); +END { FixMyStreet::App->log->enable('info'); } + +my $mech = FixMyStreet::TestMech->new; + +my $body = $mech->create_body_ok(2651, 'City of Edinburgh Council'); +$mech->create_contact_ok( body_id => $body->id, category => 'Street lighting', email => 'highways@example.com' ); +$mech->create_contact_ok( body_id => $body->id, category => 'Trees', email => 'trees@example.com' ); + +# test that phone number validation works okay +foreach my $test ( + { + msg => 'invalid number', + pc => 'EH1 1BB', + fields => { + username => '0121 4960000000', email => '', phone => '', + title => 'Title', detail => 'Detail', name => 'Bob Jones', + category => 'Street lighting', + may_show_name => '1', remember_me => undef, + photo1 => '', photo2 => '', photo3 => '', + password_register => '', password_sign_in => '', + }, + changes => { + username => '01214960000000', + phone => '01214960000000', + }, + errors => [ 'Please check your phone number is correct' ], + }, + { + msg => 'landline number', + pc => 'EH1 1BB', + fields => { + username => '0121 4960000', email => '', phone => '', + title => 'Title', detail => 'Detail', name => 'Bob Jones', + category => 'Street lighting', + may_show_name => '1', remember_me => undef, + photo1 => '', photo2 => '', photo3 => '', + password_register => '', password_sign_in => '', + }, + changes => { + username => '+44 121 496 0000', + phone => '+44 121 496 0000', + }, + errors => [ 'Please enter a mobile number', ], + }, + ) +{ + subtest "check form errors where $test->{msg}" => sub { + $mech->get_ok('/around'); + + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => $test->{pc} } }, + "submit location" ); + is_deeply $mech->page_errors, [], "no errors for pc '$test->{pc}'"; + + # click through to the report page + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, + "follow 'skip this step' link" ); + + # submit the main form + $mech->submit_form_ok( { with_fields => $test->{fields} }, "submit form" ); + }; + + # check that we got the errors expected + is_deeply [ sort @{$mech->page_errors} ], [ sort @{$test->{errors}} ], "check errors"; + + # check that fields have changed as expected + my $new_values = { + %{ $test->{fields} }, # values added to form + %{ $test->{changes} }, # changes we expect + }; + is_deeply $mech->visible_form_values, $new_values, + "values correctly changed"; + }; +} + +my $test_phone = '+61491570156'; +my $first_user; +foreach my $test ( + { + desc => 'does not have an account, does not set a password', + user => 0, password => 0, + }, + { + desc => 'does not have an account, sets a password', + user => 0, password => 1, + }, + { + desc => 'does have an account and is not signed in; does not sign in, does not set a password', + user => 1, password => 0, + }, + { + desc => 'does have an account and is not signed in; does not sign in, sets a password', + user => 1, password => 1, + }, +) { + subtest "test report creation for a user who " . $test->{desc} => sub { + $mech->log_out_ok; + + if ($test->{user}) { + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ); + ok $user, "test user does exist"; + $user->problems->delete; + $user->name( 'Old Name' ); + $user->password( 'old_password' ); + $user->update; + } elsif (!$first_user) { + ok !FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ), + "test user does not exist"; + $first_user = 1; + } else { + # Not first pass, so will exist, but want no user to start, so delete it. + $mech->delete_user($test_phone); + } + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_register', + with_fields => { + title => 'Test Report', detail => 'Test report details.', + photo1 => '', + name => 'Joe Bloggs', may_show_name => '1', + username => $test_phone, + category => 'Street lighting', + password_register => $test->{password} ? 'secret' : '', + } + }, + "submit good details" + ); + }; + + is_deeply $mech->page_errors, [], "check there were no errors"; + + my $user = FixMyStreet::App->model('DB::User')->find( { phone => $test_phone } ); + ok $user, "user found"; + if ($test->{user}) { + is $user->name, 'Old Name', 'name unchanged'; + ok $user->check_password('old_password'), 'password unchanged'; + } else { + is $user->name, undef, 'name not yet set'; + is $user->password, '', 'password not yet set for new user'; + } + + my $report = $user->problems->first; + ok $report, "Found the report"; + is $report->state, 'unconfirmed', "report not confirmed"; + is $report->bodies_str, $body->id; + + $mech->submit_form_ok({ with_fields => { code => '00000' } }); + $mech->content_contains('Try again'); + + my $code = $twilio->get_text_code; + $mech->submit_form_ok({ with_fields => { code => $code } }); + + $report->discard_changes; + is $report->state, 'confirmed', "Report is now confirmed"; + + $mech->get_ok( '/report/' . $report->id ); + + is $report->name, 'Joe Bloggs', 'name updated correctly'; + if ($test->{password}) { + ok $report->user->check_password('secret'), 'password updated correctly'; + } elsif ($test->{user}) { + ok $report->user->check_password('old_password'), 'password unchanged, as no new one given'; + } else { + is $report->user->password, '', 'password still not set, as none given'; + } + + # check that the reporter has an alert + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + # user is created and logged in + $mech->logged_in_ok; + + # cleanup + $mech->delete_user($user) + if $test->{user} && $test->{password}; + }; +} + +# this test to make sure that we don't see spurious error messages about +# the name being blank when there is a sign in error +subtest "test password errors for a user who is signing in as they report" => sub { + $mech->log_out_ok; + + my $user = $mech->create_user_ok($test_phone); + ok $user->update( { + name => 'Joe Bloggs', + email => 'joe@example.net', + password => 'secret2', + } ), "set user details"; + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => { + title => 'Test Report', + detail => 'Test report details.', + photo1 => '', + username => $test_phone, + password_sign_in => 'secret1', + category => 'Street lighting', + } + }, + "submit with wrong password" + ); + }; + + # check that we got the errors expected + is_deeply $mech->page_errors, [ + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", + ], "check there were errors"; +}; + +subtest "test report creation for a user who is signing in as they report" => sub { + $mech->log_out_ok; + $mech->cookie_jar({}); + + my $user = $mech->create_user_ok($test_phone); + + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => { + title => 'Test Report', + detail => 'Test report details.', + photo1 => '', + username => $test_phone, + password_sign_in => 'secret2', + category => 'Street lighting', + } + }, + "submit good details" + ); + + # check that we got the message expected + $mech->content_contains( 'You have successfully signed in; please check and confirm your details are accurate:' ); + + # Now submit with a name + $mech->submit_form_ok( + { + with_fields => { + name => 'Joe Bloggs', + } + }, + "submit good details" + ); + }; + + my $report = $user->problems->first; + ok $report, "Found the report"; + $mech->content_contains('Thank you for reporting this issue'); + is $report->bodies_str, $body->id; + is $report->state, 'confirmed', "report is now confirmed"; + $mech->get_ok( '/report/' . $report->id ); + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + $mech->logged_in_ok; +}; + +subtest "test report creation for a user who is logged in" => sub { + my $user = $mech->create_user_ok($test_phone); + $mech->get_ok('/around'); + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, sub { + $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } }, "submit location" ); + $mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" ); + is_deeply( + $mech->visible_form_values, + { + title => '', + detail => '', + may_show_name => '1', + name => 'Joe Bloggs', + email => 'joe@example.net', + photo1 => '', + photo2 => '', + photo3 => '', + category => '-- Pick a category --', + }, + "user's details prefilled" + ); + + $mech->submit_form_ok( + { + with_fields => { + title => "Test Report at café", + detail => 'Test report details.', + photo1 => '', + name => 'Joe Bloggs', + may_show_name => '1', + category => 'Street lighting', + } + }, + "submit good details" + ); + }; + + my $report = $user->problems->first; + ok $report, "Found the report"; + is $report->bodies_str, $body->id; + $mech->content_contains('Thank you for reporting this issue'); + is $report->state, 'confirmed', "report is now confirmed"; + $mech->get_ok( '/report/' . $report->id ); + my $alert = FixMyStreet::App->model('DB::Alert')->find( { + user => $report->user, + alert_type => 'new_updates', + parameter => $report->id, + } ); + ok $alert, "created new alert"; + + $mech->logged_in_ok; + + $mech->get_ok( + '/ajax?bbox=' . ($report->longitude - 0.01) . ',' . ($report->latitude - 0.01) + . ',' . ($report->longitude + 0.01) . ',' . ($report->latitude + 0.01) + ); + $mech->content_contains( "Test Report at caf\xc3\xa9" ); +}; + +done_testing(); diff --git a/t/app/controller/report_update_text.t b/t/app/controller/report_update_text.t new file mode 100644 index 000000000..45b4e78c2 --- /dev/null +++ b/t/app/controller/report_update_text.t @@ -0,0 +1,307 @@ +use FixMyStreet::TestMech; +use t::Mock::Twilio; + +my $twilio = t::Mock::Twilio->new; +LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com'); + +my $mech = FixMyStreet::TestMech->new; +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'); + +my $dt = DateTime->new( + year => 2011, + month => 04, + day => 16, + hour => 15, + minute => 47, + second => 23 +); + +my $report = FixMyStreet::App->model('DB::Problem')->find_or_create( + { + postcode => 'SW1A 1AA', + bodies_str => $body->id, + areas => ',105255,11806,11828,2247,2504,', + category => 'Other', + title => 'Test 2', + detail => 'Test 2 Detail', + used_map => 't', + name => 'Test User', + anonymous => 'f', + state => 'confirmed', + confirmed => $dt->ymd . ' ' . $dt->hms, + lang => 'en-gb', + service => '', + cobrand => 'default', + cobrand_data => '', + send_questionnaire => 't', + latitude => '51.5016605453401', + longitude => '-0.142497580865087', + user_id => $user->id, + } +); +my $report_id = $report->id; +ok $report, "created test report - $report_id"; + +my $comment = FixMyStreet::App->model('DB::Comment')->find_or_create( { + problem_id => $report_id, + user_id => $user2->id, + name => 'Other User', + mark_fixed => 'false', + text => 'This is some update text', + state => 'confirmed', + confirmed => $dt->ymd . ' ' . $dt->hms, + anonymous => 'f', +}); + +my $comment_id = $comment->id; +ok $comment, "created test update - $comment_id"; + +for my $test ( + { + desc => 'Invalid phone', + fields => { + username => '01214960000000', + update => 'Update', + name => 'Name', + photo1 => '', + photo2 => '', + photo3 => '', + fixed => undef, + add_alert => 1, + may_show_name => undef, + remember_me => undef, + password_sign_in => '', + password_register => '', + }, + changes => {}, + field_errors => [ 'Please check your phone number is correct' ] + }, + { + desc => 'landline number', + fields => { + username => '01214960000', + update => 'Update', + name => 'Name', + photo1 => '', + photo2 => '', + photo3 => '', + fixed => undef, + add_alert => 1, + may_show_name => undef, + remember_me => undef, + password_register => '', + password_sign_in => '', + }, + changes => { + username => '+44 121 496 0000', + }, + field_errors => [ 'Please enter a mobile number' ] + }, + ) +{ + subtest "submit an update - $test->{desc}" => sub { + $mech->get_ok("/report/$report_id"); + + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + PHONE_COUNTRY => 'GB', + }, sub { + $mech->submit_form_ok( { with_fields => $test->{fields} }, 'submit update' ); + }; + + is_deeply $mech->page_errors, $test->{field_errors}, 'field errors'; + + my $values = { + %{ $test->{fields} }, + %{ $test->{changes} }, + }; + + is_deeply $mech->visible_form_values('updateForm'), $values, 'form changes'; + }; +} + +my $test_phone = '+61491570156'; +for my $test ( + { + desc => 'submit an update, unregistered, logged out', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from an unregistered user', + add_alert => undef, + name => 'Unreg User', + may_show_name => undef, + }, + }, + { + desc => 'submit an update, unregistered, logged out, sign up for alerts', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from an unregistered user', + add_alert => 1, + name => 'Unreg User', + may_show_name => undef, + }, + }, + { + desc => 'submit an update, registered, logged out, confirming by text', + registered => 1, + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a registered user', + add_alert => undef, + name => 'Reg User', + password_register => 'new_secret', + }, + }, +) { + subtest $test->{desc} => sub { + $mech->log_out_ok(); + my $user; + if ($test->{registered}) { + $user = $mech->create_user_ok( $test_phone ); + $user->update( { name => 'Mr Reg', password => 'secret2' } ); + } + + $mech->get_ok("/report/$report_id"); + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + TWILIO_ACCOUNT_SID => 'AC123', + }, sub { + $mech->submit_form_ok( { with_fields => $test->{form_values} }, 'submit update'); + }; + $mech->content_contains('Nearly done! Now check your phone'); + + if ($user) { + $user->discard_changes; + ok $user->check_password( 'secret2' ), 'password unchanged'; + is $user->name, 'Mr Reg', 'name unchanged'; + } + + my ($token) = $mech->content =~ /name="token" value="([^"]*)"/; + $token = FixMyStreet::App->model('DB::Token')->find({ + token => $token, + scope => 'comment' + }); + ok $token, 'Token found in database'; + + my $update_id = $token->data->{id}; + my $add_alerts = $token->data->{add_alert}; + my $update = FixMyStreet::App->model('DB::Comment')->find( { id => $update_id } ); + + ok $update, 'found update in database'; + is $update->state, 'unconfirmed', 'update unconfirmed'; + my $details = $test->{form_values}; + is $update->user->phone, $details->{username}, 'update phone'; + is $update->user->phone_verified, 1; + is $update->text, $details->{update}, 'update text'; + is $add_alerts, $details->{add_alert} ? 1 : 0, 'do not sign up for alerts'; + + my $code = $twilio->get_text_code; + $mech->submit_form_ok( { with_fields => { code => '00000' } }); + $mech->content_contains('Try again'); + $mech->submit_form_ok( { with_fields => { code => $code } }); + + $mech->content_contains("/report/$report_id#update_$update_id"); + + if ($user) { + $user->discard_changes; + ok $user->check_password( 'new_secret' ), 'password changed'; + is $user->name, 'Reg User', 'name changed'; + } else { + $user = FixMyStreet::App->model( 'DB::User' )->find( { phone => $details->{username} } ); + ok $user, 'found user'; + } + + my $alert = FixMyStreet::App->model( 'DB::Alert' )->find( + { user => $user, alert_type => 'new_updates', confirmed => 1, } + ); + + ok $details->{add_alert} ? defined( $alert ) : !defined( $alert ), 'sign up for alerts'; + + $update->discard_changes; + is $update->state, 'confirmed', 'update confirmed'; + $mech->delete_user( $user ); + }; +} + +for my $test ( + { + desc => 'submit an update for a registered user, signing in with wrong password', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a user', + add_alert => undef, + password_sign_in => 'secret', + }, + field_errors => [ + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", + 'Please enter your name', # FIXME Not really necessary error + ], + }, + { + desc => 'submit an update for a registered user and sign in', + form_values => { + submit_update => 1, + username => $test_phone, + update => 'Update from a user', + add_alert => undef, + password_sign_in => 'secret2', + }, + message => 'You have successfully signed in; please check and confirm your details are accurate:', + } +) { + subtest $test->{desc} => sub { + # Set things up + my $user = $mech->create_user_ok( $test->{form_values}->{username} ); + my $pw = 'secret2'; + $user->update( { name => 'Mr Reg', password => $pw } ); + $report->comments->delete; + + $mech->log_out_ok(); + $mech->clear_emails_ok(); + $mech->get_ok("/report/$report_id"); + FixMyStreet::override_config { + SMS_AUTHENTICATION => 1, + }, sub { + $mech->submit_form_ok( + { + button => 'submit_sign_in', + with_fields => $test->{form_values} + }, + 'submit update' + ); + }; + + $mech->content_contains($test->{message}) if $test->{message}; + + is_deeply $mech->page_errors, $test->{field_errors}, 'check there were errors' + if $test->{field_errors}; + + SKIP: { + skip( "Incorrect password", 4 ) unless $test->{form_values}{password_sign_in} eq $pw; + + # Now submit with a name + $mech->submit_form_ok( + { with_fields => { name => 'Joe Bloggs', } }, + "submit good details" + ); + + $mech->content_contains('Thank you for updating this issue'); + + my $update = $report->comments->first; + ok $update, 'found update'; + is $update->text, $test->{form_values}->{update}, 'update text'; + is $update->user->phone, $test->{form_values}->{username}, 'update user'; + is $update->state, 'confirmed', 'update confirmed'; + $mech->delete_user( $update->user ); + } + }; +} + +done_testing(); diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t index 0526b2fd7..7cb547081 100644 --- a/t/app/controller/report_updates.t +++ b/t/app/controller/report_updates.t @@ -196,7 +196,7 @@ for my $test ( { desc => 'No email, no message', fields => { - rznvy => '', + username => '', update => '', name => '', photo1 => '', @@ -215,7 +215,7 @@ for my $test ( { desc => 'Invalid email, no message', fields => { - rznvy => 'test', + username => 'test', update => '', name => '', photo1 => '', @@ -234,7 +234,7 @@ for my $test ( { desc => 'email with spaces, no message', fields => { - rznvy => 'test @ example. com', + username => 'test @ example. com', update => '', name => '', photo1 => '', @@ -248,14 +248,14 @@ for my $test ( password_sign_in => '', }, changes => { - rznvy => 'test@example.com', + username => 'test@example.com', }, field_errors => [ 'Please enter a message', 'Please enter your name' ] }, { desc => 'email with uppercase, no message', fields => { - rznvy => 'test@EXAMPLE.COM', + username => 'test@EXAMPLE.COM', update => '', name => '', photo1 => '', @@ -269,7 +269,7 @@ for my $test ( password_sign_in => '', }, changes => { - rznvy => 'test@example.com', + username => 'test@example.com', }, field_errors => [ 'Please enter a message', 'Please enter your name' ] }, @@ -297,7 +297,7 @@ for my $test ( desc => 'submit an update for a non registered user', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -311,7 +311,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, name => 'Unreg User', @@ -323,7 +323,7 @@ for my $test ( desc => 'submit an update for a non registered user and sign up', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -337,7 +337,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => "update from an\r\n\r\nunregistered user", add_alert => 1, name => 'Unreg User', @@ -395,14 +395,14 @@ for my $test ( ok $update, 'found update in database'; is $update->state, 'unconfirmed', 'update unconfirmed'; - is $update->user->email, $details->{rznvy}, 'update email'; + is $update->user->email, $details->{username}, 'update email'; is $update->text, $details->{update}, 'update text'; is $add_alerts, $details->{add_alert} ? 1 : 0, 'do not sign up for alerts'; $mech->get_ok( $url ); $mech->content_contains("/report/$report_id#update_$update_id"); - my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{rznvy} } ); + my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{username} } ); ok $unreg_user, 'found user'; @@ -427,7 +427,7 @@ for my $test ( desc => 'overriding email confirmation allows report confirmation with no email sent', initial_values => { name => '', - rznvy => '', + username => '', may_show_name => 1, add_alert => 1, photo1 => '', @@ -441,7 +441,7 @@ for my $test ( }, form_values => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => "update no email confirm", add_alert => 1, name => 'Unreg User', @@ -493,10 +493,10 @@ for my $test ( ok $update, 'found update in database'; is $update->state, 'confirmed', 'update confirmed'; - is $update->user->email, $details->{rznvy}, 'update email'; + is $update->user->email, $details->{username}, 'update email'; is $update->text, $details->{update}, 'update text'; - my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{rznvy} } ); + my $unreg_user = FixMyStreet::App->model( 'DB::User' )->find( { email => $details->{username} } ); ok $unreg_user, 'found user'; @@ -972,13 +972,13 @@ for my $test ( desc => 'submit an update for a registered user, signing in with wrong password', form_values => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, password_sign_in => 'secret', }, field_errors => [ - "There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the \x{2018}sign in by email\x{2019} section of the form.", + "There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the \x{2018}No\x{2019} section of the form.", 'Please enter your name', # FIXME Not really necessary error ], }, @@ -986,7 +986,7 @@ for my $test ( desc => 'submit an update for a registered user and sign in', form_values => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, password_sign_in => 'secret2', @@ -996,7 +996,7 @@ for my $test ( ) { subtest $test->{desc} => sub { # Set things up - my $user = $mech->create_user_ok( $test->{form_values}->{rznvy} ); + my $user = $mech->create_user_ok( $test->{form_values}->{username} ); my $pw = 'secret2'; $user->update( { name => 'Mr Reg', password => $pw } ); $report->comments->delete; @@ -1036,7 +1036,7 @@ for my $test ( my $update = $report->comments->first; ok $update, 'found update'; is $update->text, $test->{form_values}->{update}, 'update text'; - is $update->user->email, $test->{form_values}->{rznvy}, 'update user'; + is $update->user->email, $test->{form_values}->{username}, 'update user'; is $update->state, 'confirmed', 'update confirmed'; $mech->delete_user( $update->user ); } @@ -1053,7 +1053,7 @@ subtest 'submit an update for a registered user, creating update by email' => su $mech->submit_form_ok( { with_fields => { submit_update => 1, - rznvy => 'registered@example.com', + username => 'registered@example.com', update => 'Update from a user', add_alert => undef, name => 'New Name', @@ -1502,7 +1502,7 @@ for my $test ( fields => { submit_update => 1, name => 'Test User', - rznvy => 'test@example.com', + username => 'test@example.com', may_show_name => 1, update => 'update from owner', add_alert => undef, @@ -1524,7 +1524,7 @@ for my $test ( submit_update => 1, name => 'Test User', may_show_name => 1, - rznvy => 'test@example.com', + username => 'test@example.com', update => 'update from owner', add_alert => undef, fixed => 1, @@ -1589,7 +1589,7 @@ for my $test ( my $update = $report->comments->first; ok $update, 'found update'; is $update->text, $results->{update}, 'update text'; - is $update->user->email, $test->{fields}->{rznvy}, 'update user'; + is $update->user->email, $test->{fields}->{username}, 'update user'; is $update->state, 'unconfirmed', 'update confirmed'; is $update->anonymous, $test->{anonymous}, 'user anonymous'; 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, diff --git a/t/cobrand/bromley.t b/t/cobrand/bromley.t index f3053c29a..a64337085 100644 --- a/t/cobrand/bromley.t +++ b/t/cobrand/bromley.t @@ -74,7 +74,7 @@ for my $test ( cobrand => 'bromley', fields => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, first_name => 'Unreg', @@ -87,7 +87,7 @@ for my $test ( cobrand => 'fixmystreet', fields => { submit_update => 1, - rznvy => 'unregistered@example.com', + username => 'unregistered@example.com', update => 'Update from an unregistered user', add_alert => undef, name => 'Unreg User', diff --git a/t/cobrand/form_extras.t b/t/cobrand/form_extras.t index f450d908e..84ded5bc1 100644 --- a/t/cobrand/form_extras.t +++ b/t/cobrand/form_extras.t @@ -37,7 +37,7 @@ FixMyStreet::override_config { detail => 'Test report details.', name => 'Joe Bloggs', may_show_name => '1', - email => 'test-1@example.com', + username => 'test-1@example.com', passport => '123456', password_register => '', } diff --git a/t/cobrand/oxfordshire.t b/t/cobrand/oxfordshire.t index a79a8f2a4..abafa1fe8 100644 --- a/t/cobrand/oxfordshire.t +++ b/t/cobrand/oxfordshire.t @@ -217,7 +217,7 @@ subtest 'response times messages displayed' => sub { title => 'Test Report', detail => 'Test report details.', photo1 => '', - email => 'test-2@example.com', + username => 'test-2@example.com', name => 'Test User', category => 'Pothole', } diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index 03b20b087..e0671db2a 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -808,7 +808,7 @@ subtest "photo must be supplied for categories that require it" => sub { $mech->get_ok('/report/new?lat=47.381817&lon=8.529156'); $mech->submit_form_ok({ with_fields => { detail => 'Problem-Bericht', - email => 'user@example.org', + username => 'user@example.org', category => 'Graffiti - photo required', }}); is $mech->res->code, 200, "missing photo shouldn't return anything but 200"; diff --git a/templates/email/default/submit.html b/templates/email/default/submit.html index 85511b2e4..582670f98 100644 --- a/templates/email/default/submit.html +++ b/templates/email/default/submit.html @@ -27,7 +27,13 @@ of a local problem that they believe might require your attention.</p> </tr> <tr> <th style="[% contact_th_style %]">Email</th> - <td style="[% contact_td_style %]"><a href="mailto:[% email | html %]">[% email | html %]</a></td> + <td style="[% contact_td_style %]"> + [%~ IF email ~%] + <a href="mailto:[% email | html %]">[% email | html %]</a> + [%~ ELSE ~%] + <strong>No email address provided, only phone number</strong> + [%~ END ~%] + </td> </tr> [%~ IF phone %] <tr> diff --git a/templates/email/default/submit.txt b/templates/email/default/submit.txt index 5d79f3b41..8c88a17f8 100644 --- a/templates/email/default/submit.txt +++ b/templates/email/default/submit.txt @@ -15,9 +15,11 @@ please visit the following link: Name: [% name %] -Email: [% email %] +Email: [% email OR "None provided" %] -[% phone_line %][% category_line %]Subject: [% title %] +Phone: [% phone OR "None provided" %] + +[% category_line %]Subject: [% title %] Details: [% detail %] diff --git a/templates/email/fiksgatami/nn/submit.txt b/templates/email/fiksgatami/nn/submit.txt index 32a895632..dfbdd9457 100644 --- a/templates/email/fiksgatami/nn/submit.txt +++ b/templates/email/fiksgatami/nn/submit.txt @@ -15,9 +15,11 @@ problemet, ver venleg og besøk følgjande lenkje: Namn: [% name %] -E-post: [% email %] +E-post: [% email OR '-' %] -[% phone_line %][% category_line %]Tema: [% title %] +Telefon: [% phone OR '-' %] + +[% category_line %]Tema: [% title %] Detaljer: [% detail %] diff --git a/templates/email/fiksgatami/submit.txt b/templates/email/fiksgatami/submit.txt index a0e0687eb..165b804f1 100644 --- a/templates/email/fiksgatami/submit.txt +++ b/templates/email/fiksgatami/submit.txt @@ -15,9 +15,11 @@ vennligst besøk følgende lenke: Navn: [% name %] -E-post: [% email %] +E-post: [% email OR '-' %] -[% phone_line %][% category_line %]Tema: [% title %] +Telefon: [% phone OR '-' %] + +[% category_line %]Tema: [% title %] Detajer: [% detail %] diff --git a/templates/email/fixamingata/submit.html b/templates/email/fixamingata/submit.html index da4e7f48f..dfc56589a 100644 --- a/templates/email/fixamingata/submit.html +++ b/templates/email/fixamingata/submit.html @@ -27,8 +27,14 @@ tror medborgaren behöver er uppmärksamhet.</p> </tr> <tr> <th style="[% contact_th_style %]">Epost</th> - <td style="[% contact_td_style %]"><a href="mailto:[% email | html %]">[% email | html %]</a></td> + <td style="[% contact_td_style %]"> + [%~ IF email ~%] + <a href="mailto:[% email | html %]">[% email | html %]</a> + [%~ ELSE ~%] + [%~ END ~%] + </td> </tr> + [% END %] [%~ IF phone %] <tr> <th style="[% contact_th_style %]">Telefon</th> diff --git a/templates/email/fixamingata/submit.txt b/templates/email/fixamingata/submit.txt index 95365b87e..fc8b65886 100644 --- a/templates/email/fixamingata/submit.txt +++ b/templates/email/fixamingata/submit.txt @@ -16,9 +16,11 @@ tror medborgaren behöver er uppmärksamhet. Namn: [% name %] -Epost: [% email %] +Epost: [% email OR '-' %] -[% phone_line %]** Information om ärendet +Telefonnummer: [% phone OR '-' %] + +** Information om ärendet ID: [% id %] diff --git a/templates/email/fixmystreet.com/submit.html b/templates/email/fixmystreet.com/submit.html index fc2b6c095..22e025085 100644 --- a/templates/email/fixmystreet.com/submit.html +++ b/templates/email/fixmystreet.com/submit.html @@ -27,7 +27,13 @@ of a local problem that they believe might require your attention.</p> </tr> <tr> <th style="[% contact_th_style %]">Email</th> - <td style="[% contact_td_style %]"><a href="mailto:[% email | html %]">[% email | html %]</a></td> + <td style="[% contact_td_style %]"> + [%~ IF email ~%] + <a href="mailto:[% email | html %]">[% email | html %]</a> + [%~ ELSE ~%] + <strong>No email address provided, only phone number</strong> + [%~ END ~%] + </td> </tr> [%~ IF phone %] <tr> diff --git a/templates/email/fixmystreet.com/submit.txt b/templates/email/fixmystreet.com/submit.txt index aa05316bf..5a0845b14 100644 --- a/templates/email/fixmystreet.com/submit.txt +++ b/templates/email/fixmystreet.com/submit.txt @@ -15,9 +15,11 @@ please visit the following link: Name: [% name %] -Email: [% email %] +Email: [% email OR 'None provided' %] -[% phone_line %][% category_line %]Subject: [% title %] +Phone: [% phone OR 'None provided' %] + +[% category_line %]Subject: [% title %] Details: [% detail %] diff --git a/templates/web/base/admin/report_blocks.html b/templates/web/base/admin/report_blocks.html index f5896b88f..8e8b56393 100644 --- a/templates/web/base/admin/report_blocks.html +++ b/templates/web/base/admin/report_blocks.html @@ -15,7 +15,7 @@ SET state_groups = c.cobrand.state_groups_admin; [% BLOCK abuse_button -%] [% IF allowed_pages.abuse_edit -%] -[% IF email_in_abuse %]<small>[% loc('(Email in abuse table)') %]</small>[% ELSE %]<input type="submit" class="btn" name="banuser" value="[% loc('Ban email address') %]" />[% END %] +[% IF username_in_abuse %]<small>[% loc('(User in abuse table)') %]</small>[% ELSE %]<input type="submit" class="btn" name="banuser" value="[% loc('Ban user') %]" />[% END %] [%- END %] [%- END %] diff --git a/templates/web/base/admin/report_edit.html b/templates/web/base/admin/report_edit.html index 3c8134b80..c58b2d605 100644 --- a/templates/web/base/admin/report_edit.html +++ b/templates/web/base/admin/report_edit.html @@ -126,12 +126,17 @@ class="admin-offsite-link">[% problem.latitude %], [% problem.longitude %]</a> </select></li> <li><label for="name">[% loc('Name:') %]</label> <input type='text' class="form-control" name='name' id='name' value='[% problem.name | html %]'></li> -<li><label for="email">[% loc('Email:') %]</label> - <input type='text' class="form-control" id='email' name='email' value='[% problem.user.email | html %]'> +<li><label for="username">[% loc('User:') %]</label> + <input type='text' class="form-control" id='username' name='username' value='[% problem.user.username | html %]'> [% PROCESS abuse_button %] [% PROCESS flag_button user=problem.user %] </li> -<li>[% loc('Phone:') %] [% problem.user.phone | html %]</li> +[% IF problem.user.phone_display != problem.user.username %] +<li>[% loc('Phone:') %] [% problem.user.phone_display | html %]</li> +[% END %] +[% IF problem.user.email != problem.user.username %] +<li>[% loc('Email:') %] [% problem.user.email | html %]</li> +[% END %] <li><label class="inline-text" for="flagged">[% loc('Flagged:') %]</label> <input type="checkbox" id="flagged" name="flagged"[% ' checked' IF problem.flagged %]></li> <li><label class="inline-text" for="non_public">[% loc('Private') %]:</label> diff --git a/templates/web/base/admin/update_edit.html b/templates/web/base/admin/update_edit.html index 2b20c50b3..34e64310f 100644 --- a/templates/web/base/admin/update_edit.html +++ b/templates/web/base/admin/update_edit.html @@ -31,8 +31,10 @@ <option [% 'selected ' IF state.0 == update.state %] value="[% state.0 %]">[% state.1 %]</option> [% END %] </select></li> -<li>[% loc('Name:') %] <input type='text' class="form-control" name='name' id='name' value='[% update.name | html %]'></li> -<li>[% loc('Email:') %] <input type='text' class="form-control" id='email' name='email' value='[% update.user.email | html %]'> +<li><label for="name">[% loc('Name:') %]</label> + <input type='text' class="form-control" name='name' id='name' value='[% update.name | html %]'></li> +<li><label for="username">[% loc('User:') %]</label> + <input type='text' class="form-control" id='username' name='username' value='[% update.user.username | html %]'> [%- IF update.user.from_body && update.user.from_body.id == update.problem.bodies_str %] [% ' (' _ tprintf(loc('user is from same council as problem - %d'), update.user.from_body.id ) _')' %] [% END -%] diff --git a/templates/web/base/admin/user-form.html b/templates/web/base/admin/user-form.html index dbd554b1e..5637252e2 100644 --- a/templates/web/base/admin/user-form.html +++ b/templates/web/base/admin/user-form.html @@ -18,8 +18,20 @@ </li> <li><label for="email">[% loc('Email:') %]</label> <input type='text' class="form-control" id='email' name='email' value='[% user.email | html %]'></li> + [% IF c.config.SMS_AUTHENTICATION %] + <li><label class="inline" for="email_verified">[% loc('Email verified:') %]</label> + <input type="checkbox" id="email_verified" name="email_verified" value="1" [% user.email_verified ? ' checked' : '' %]> + [% ELSE %] + <input type="hidden" name="email_verified" value="1"> + [% END %] <li><label for="phone">[% loc('Phone:') %]</label> <input type='text' class="form-control" id='phone' name='phone' value='[% user.phone | html %]'></li> + [% IF c.config.SMS_AUTHENTICATION %] + <li><label class="inline" for="phone_verified">[% loc('Phone verified:') %]</label> + <input type="checkbox" id="phone_verified" name="phone_verified" value="1" [% user.phone_verified ? ' checked' : '' %]> + [% ELSE %] + <input type="hidden" name="phone_verified" value="0"> + [% END %] [% IF c.user.is_superuser || c.cobrand.moniker == 'zurich' %] <li> diff --git a/templates/web/base/admin/users.html b/templates/web/base/admin/users.html index 8e35e1c31..d367c18d8 100644 --- a/templates/web/base/admin/users.html +++ b/templates/web/base/admin/users.html @@ -29,7 +29,7 @@ [% IF user.is_superuser %] * [% END %] </td> [% IF c.cobrand.moniker != 'zurich' %] - <td>[% user.flagged == 2 ? loc('(Email in abuse table)') : user.flagged ? loc('Yes') : ' ' %]</td> + <td>[% user.flagged == 2 ? loc('(User in abuse table)') : user.flagged ? loc('Yes') : ' ' %]</td> [% END %] <td>[% IF user.id %]<a href="[% c.uri_for( 'user_edit', user.id ) %]">[% loc('Edit') %]</a>[% END %]</td> </tr> diff --git a/templates/web/base/auth/change_email.html b/templates/web/base/auth/change_email.html index a444b8c31..b3bec6b3e 100644 --- a/templates/web/base/auth/change_email.html +++ b/templates/web/base/auth/change_email.html @@ -1,33 +1,48 @@ -[% INCLUDE 'header.html', title = loc('Change email address'), bodyclass = 'authpage' %] - -<h1>[% loc('Change email address') %]</h1> - -[% IF c.req.args.0 == 'success' %] - <p class="form-success">[% loc('You have successfully confirmed your email address.') %]</p> -[% END %] - +[% +IF c.user.email_verified OR (c.user.email AND NOT verifying); + SET title = loc('Change email address'); +ELSIF c.user.email; + SET title = loc('Verify email address'); +ELSE; + SET title = loc('Add email address'); +END +-%] +[% INCLUDE 'header.html' bodyclass = 'authpage' %] + +<h1>[% title %]</h1> + +[% IF c.user.email_verified OR (c.user.email AND NOT verifying) %] [% loc('Your email address') %]: [% c.user.email %] +[% ELSIF c.user.email %] +[% DEFAULT username = c.user.email %] +[% END %] -<form action="[% c.uri_for('change_email') %]" method="post" name="change_email"> +<form method="post" name="change_email"> <input type="hidden" name="token" value="[% csrf_token %]"> <fieldset> - [% IF email_error; + [% IF username_error; errors = { - missing = loc('Please enter your email'), - other = loc('Please check your email address is correct') + missing_email = loc('Please enter your email'), + other_email = loc('Please check your email address is correct') }; - loc_email_error = errors.$email_error || errors.other; + loc_username_error = errors.$username_error || errors.other_email; %] - <div class="form-error">[% loc_email_error %]</div> + <div class="form-error">[% loc_username_error %]</div> [% END %] <div class="form-field"> - <label for="email">[% loc('New email address:') %]</label> - <input class="form-control" type="email" name="email" id="email" value="[% email | html %]"> + <label for="email"> + [% IF NOT c.user.email_verified AND c.user.email AND verifying %] + [% loc('Email address') %]: + [% ELSE %] + [% loc('New email address:') %] + [% END %] + </label> + <input class="form-control" type="email" name="email" id="email" value="[% username | html %]"> </div> <div class="final-submit"> - <input type="submit" class="btn" value="[% loc('Change email address') %]"> + <input type="submit" class="btn" value="[% title %]"> </div> </fieldset> diff --git a/templates/web/base/auth/change_password.html b/templates/web/base/auth/change_password.html index 094d131eb..a32dbaf9c 100644 --- a/templates/web/base/auth/change_password.html +++ b/templates/web/base/auth/change_password.html @@ -15,7 +15,7 @@ INCLUDE 'header.html', title = loc('Change password'), bodyclass = bclass <h1>[% loc('Change password') %]</h1> -<form action="[% c.uri_for('change_password') %]" method="post" name="change_password" class="fieldset"> +<form action="[% c.uri_for_action('/auth/profile/change_password') %]" method="post" name="change_password" class="fieldset"> <input type="hidden" name="token" value="[% csrf_token %]"> <fieldset> diff --git a/templates/web/base/auth/change_phone.html b/templates/web/base/auth/change_phone.html new file mode 100644 index 000000000..4b181f6c5 --- /dev/null +++ b/templates/web/base/auth/change_phone.html @@ -0,0 +1,57 @@ +[% +IF c.user.phone_verified OR (c.user.phone AND NOT verifying); + SET title = loc('Change phone number'); +ELSIF c.user.phone; + SET title = loc('Verify phone number'); +ELSE; + SET title = loc('Add phone number'); +END +-%] +[% INCLUDE 'header.html' bodyclass = 'authpage' %] + +<h1>[% title %]</h1> + +[% IF c.req.args.0 == 'success' %] + <p class="form-success">[% loc('You have successfully confirmed your phone number.') %]</p> +[% END %] + +[% IF c.user.phone_verified OR (c.user.phone AND NOT verifying) %] +[% loc('Your phone number') %]: [% c.user.phone_display %] +[% ELSIF c.user.phone %] +[% DEFAULT username = c.user.phone %] +[% END %] + +<form method="post" name="change_phone"> + <input type="hidden" name="token" value="[% csrf_token %]"> + + <fieldset> + [% IF username_error; + errors = { + nonmobile = loc('Please enter a mobile number'), + missing_phone = loc('Please enter your phone number'), + other_phone = loc('Please check your phone number is correct') + }; + loc_username_error = errors.$username_error || errors.other_phone; + %] + <div class="form-error">[% loc_username_error %]</div> + [% END %] + + <div class="form-field"> + <label for="phone"> + [% IF NOT c.user.phone_verified AND c.user.phone AND verifying %] + [% loc('Phone number') %]: + [% ELSE %] + [% loc('New phone number:') %] + [% END %] + </label> + <input class="form-control" type="tel" name="username" id="phone" value="[% username | html %]"> + </div> + <div class="final-submit"> + <input type="submit" class="btn" value="[% title %]"> + </div> + + </fieldset> +</form> + + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/base/auth/general.html b/templates/web/base/auth/general.html index 2a8bea402..e64230b41 100644 --- a/templates/web/base/auth/general.html +++ b/templates/web/base/auth/general.html @@ -11,7 +11,7 @@ <p class="form-error">[% loc('Sorry, we could not log you in. Please fill in the form below.') %]</p> [% END %] -<form action="[% c.uri_for() %]" method="post" name="general_auth" class="validate"> +<form action="/auth" method="post" name="general_auth" class="validate"> <fieldset> <input type="hidden" name="r" value="[% c.req.params.r | html %]"> @@ -36,25 +36,34 @@ <div id="js-social-email-hide"> [% END %] - [% IF email_error; + [% IF username_error; # other keys include fqdn, mxcheck if you'd like to write a custom error message errors = { - missing => loc('Please enter your email'), - other => loc('Please check your email address is correct') + nonmobile = loc('Please enter a mobile number'), + missing_email = loc('Please enter your email'), + other_email = loc('Please check your email address is correct') + missing_phone = loc('Please enter your phone number'), + other_phone = loc('Please check your phone number is correct') }; - loc_email_error = errors.$email_error || errors.other; + loc_username_error = errors.$username_error || errors.other_email; END %] - <label class="n" for="email">[% loc('Email') %]</label> - [% IF loc_email_error %] - <div class="form-error">[% loc_email_error %]</div> +[% IF c.config.SMS_AUTHENTICATION %] + [% SET username_label = loc('Your email or mobile') %] +[% ELSE %] + [% SET username_label = loc('Your email') %] +[% END %] + + <label class="n" for="username">[% username_label %]</label> + [% IF loc_username_error %] + <div class="form-error">[% loc_username_error %]</div> [% ELSIF sign_in_error %] - <div class="form-error">[% loc('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.') %]</div> + <div class="form-error">[% loc('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.') %]</div> [% END %] - <input type="email" class="form-control required email" id="email" name="email" value="[% email | html %]" placeholder="[% loc('Your email address') %]" autofocus> + <input type="text" class="form-control required" id="username" name="username" value="[% username | html %]" autofocus> <div id="form_sign_in"> <h3>[% tprintf(loc("Do you have a %s password?", "%s is the site name"), site_name) %]</h3> @@ -85,7 +94,7 @@ <div class="form-txt-submit-box"> <input type="password" name="password_sign_in" class="form-control" id="password_sign_in" value="" placeholder="[% loc('Your password') %]"> - <input class="green-btn" type="submit" name="sign_in" value="[% loc('Sign in') %]"> + <input class="green-btn" type="submit" name="sign_in_by_password" value="[% loc('Sign in') %]"> </div> <div class="checkbox-group"> @@ -95,7 +104,11 @@ <div class="general-notes"> <p><strong>[% loc('Forgotten your password?') %]</strong> + [% IF c.config.SMS_AUTHENTICATION %] + [% loc('Sign in by email or text, providing a new password. When you click the link in your email or enter the SMS authentication code, your password will be updated.') %]</p> + [% ELSE %] [% loc('Sign in by email instead, providing a new password. When you click the link in your email, your password will be updated.') %]</p> + [% END %] </div> </div> @@ -103,7 +116,11 @@ [% BLOCK form_sign_in_no %] <div id="form_sign_in_no" class="form-box"> + [% IF c.config.SMS_AUTHENTICATION %] + <h5>[% loc('<strong>No</strong> let me sign in by email or text') %]</h5> + [% ELSE %] <h5>[% loc('<strong>No</strong> let me sign in by email') %]</h5> + [% END %] <label for="name">[% loc('Name') %]</label> <input class="form-control" type="text" name="name" value="" placeholder="[% loc('Your name') %]"> @@ -116,7 +133,7 @@ <div class="form-txt-submit-box"> <input class="form-control" type="password" name="password_register" id="password_register" value="" placeholder="[% loc('Enter a password') %]"> - <input class="green-btn" type="submit" name="email_sign_in" value="[% loc('Sign in') %]"> + <input class="green-btn" type="submit" name="sign_in_by_code" value="[% loc('Sign in') %]"> </div> </div> [% END %] diff --git a/templates/web/base/auth/smsform.html b/templates/web/base/auth/smsform.html new file mode 100644 index 000000000..a475dd2f6 --- /dev/null +++ b/templates/web/base/auth/smsform.html @@ -0,0 +1,34 @@ +[% INCLUDE 'header.html', bodyclass = 'fullwidthpage', title = loc('Confirm account') %] + +[% IF token_not_found %] + + <div class="confirmation-header confirmation-header--failure"> + <h1>[% loc('Sorry, that wasn’t a valid link') %]</h1> + <p>[% loc('The link might have expired, or maybe you didn’t quite copy and paste it correctly.') %]</p> + </div> + +[% ELSE %] + +[% DEFAULT submit_url = '/auth/phone' %] + + <div class="confirmation-header confirmation-header--phone"> + [% IF incorrect_code %] + <h1>[% loc('Sorry, that wasn’t the correct code') %]</h1> + <p>[% loc('Try again') %]:</p> + [% ELSE %] + <h1>[% loc("Nearly done! Now check your phone…") %]</h1> + <p>[% loc("We have sent a confirmation code to your phone. Please enter it below:") %]</p> + [% END %] + <form action="[% submit_url %]" method="post"> + <input type="hidden" name="token" value="[% token | html %]"> + <label for="code">[% loc('Code') %]</label> + <div class="form-txt-submit-box"> + <input class="form-control" type="number" id="code" name="code" value="" required> + <input type="submit" value="[% loc('Submit') %]" class="btn-primary"> + </div> + </form> + </div> + +[% END %] + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/base/js/translation_strings.html b/templates/web/base/js/translation_strings.html index bc2f013ff..9bdf3b498 100644 --- a/templates/web/base/js/translation_strings.html +++ b/templates/web/base/js/translation_strings.html @@ -59,7 +59,11 @@ upload_cancel_confirmation: '[% loc ('Are you sure you want to cancel this upload?') | replace("'", "\\'") %]', upload_invalid_file_type: '[% loc ('Please upload an image only') | replace("'", "\\'") %]', + [% IF c.config.SMS_AUTHENTICATION ~%] + login_with_email: '[% loc('Log in with email/text') | replace("'", "\\'") %]', + [% ELSE ~%] login_with_email: '[% loc('Log in with email') | replace("'", "\\'") %]', + [% END ~%] offline: { your_reports: '[% loc('Your offline reports') | replace("'", "\\'") %]', diff --git a/templates/web/base/my/my.html b/templates/web/base/my/my.html index 1aaad6dc9..68fbc2ee8 100644 --- a/templates/web/base/my/my.html +++ b/templates/web/base/my/my.html @@ -17,11 +17,50 @@ <h1>[% loc('Your account') %]</h1> -<p>[% c.user.name %] [% c.user.email %]</p> +[% IF flash_message %] +<p class="form-success">[% flash_message %]</p> +[% END %] + +<style> +/* TODO XXX */ +li .my-account-buttons { + float: right; + margin: 0; +} +li .my-account-buttons a { + padding: 0 0.5em; +} +</style> +<ul> +<li>[% loc('Name:') %] [% c.user.name %] +<li>[% loc('Email:') %] [% c.user.email OR '-' %] + <p class="my-account-buttons"> + [% IF NOT c.user.email %] + <a href="/auth/change_email">[% loc('Add') %]</a> + [% ELSIF c.user.email_verified %] + <a href="/auth/change_email">[% loc('Change') %]</a> + [% ELSE %] + <a href="/auth/verify/email">[% loc('Verify') %]</a> + <a href="/auth/change_email">[% loc('Change') %]</a> + [% END %] + </p> +<li>[% loc('Phone:') %] [% c.user.phone_display OR '-' %] + <p class="my-account-buttons"> + [% IF NOT c.user.phone %] + <a href="/auth/change_phone">[% loc('Add') %]</a> + [% ELSIF c.user.phone_verified %] + <a href="/auth/change_phone">[% loc('Change') %]</a> + [% ELSE %] + [% IF c.config.SMS_AUTHENTICATION %] + <a href="/auth/verify/phone">[% loc('Verify') %]</a> + [% END %] + <a href="/auth/change_phone">[% loc('Change') %]</a> + [% END %] + </p> +</ul> <p class="my-account-buttons"> <a href="/auth/change_password">[% loc('Change password') %]</a> - <a href="/auth/change_email">[% loc('Change email') %]</a> <a href="/auth/sign_out">[% loc('Sign out') %]</a> </p> diff --git a/templates/web/base/report/new/form_user_loggedin.html b/templates/web/base/report/new/form_user_loggedin.html index e841845bf..bd4ce1cf7 100644 --- a/templates/web/base/report/new/form_user_loggedin.html +++ b/templates/web/base/report/new/form_user_loggedin.html @@ -29,12 +29,19 @@ </select> [% END %] - <label for="form_email">[% loc('Email address') %]</label> - <input class="form-control" id="form_email" name="email" +[% IF c.user.phone_verified %] + <label for="form_phone">[% loc('Phone number') %]</label> + <input class="form-control" id="form_phone" name="phone" disabled type="text" value="[% c.user.phone | html %]"> +[% END %] + +[% IF c.user.email_verified %] + <label for="form_username">[% loc('Email address') %]</label> + <input class="form-control" id="form_username" name="username" [%- IF NOT can_contribute_as_another_user -%] disabled [%- END -%] type="text" value="[% c.user.email | html %]"> +[% END %] [% INCLUDE 'report/new/extra_name.html' %] [% PROCESS 'user/_anonymity.html' anonymous = report.anonymous %] @@ -56,8 +63,14 @@ <label class="inline" for="form_may_show_name">[% loc('Show my name publicly') %] </label> </div> +[% IF NOT c.user.phone_verified %] <label for="form_phone">[% loc('Phone number (optional)') %]</label> <input class="form-control" type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone"> +[% END %] +[% IF NOT c.user.email_verified %] + <label for="form_username">[% loc('Email address (optional)') %]</label> + <input class="form-control" type="text" value="[% report.user.email | html %]" name="email" id="form_email"> +[% END %] <div class="form-txt-submit-box"> <input class="green-btn js-submit_register" type="submit" name="submit_register" value="[% loc('Submit') %]"> diff --git a/templates/web/base/report/new/form_user_loggedout_by_email.html b/templates/web/base/report/new/form_user_loggedout_by_email.html index 409fd4bbf..e9519f573 100644 --- a/templates/web/base/report/new/form_user_loggedout_by_email.html +++ b/templates/web/base/report/new/form_user_loggedout_by_email.html @@ -1,5 +1,9 @@ <div id="form_sign_in_no" class="form-box"> + [% IF c.config.SMS_AUTHENTICATION %] + <h5>[% loc('<strong>No</strong> Let me confirm my report by email/text') %]</h5> + [% ELSE %] <h5>[% loc('<strong>No</strong> Let me confirm my report by email') %]</h5> + [% END %] [% INCLUDE 'report/new/extra_name.html' %] [% PROCESS 'user/_anonymity.html' anonymous = report.anonymous %] @@ -22,8 +26,14 @@ <label class="inline" for="form_may_show_name">[% loc('Show my name publicly') %]</label> </div> - <label class="form-focus-hidden" for="form_phone">[% loc('Phone number (optional)') %]</label> - <input class="form-control form-focus-hidden" type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone" placeholder="[% loc('Your phone number') %]"> + <div id="js-hide-if-username-phone"> + <label class="form-focus-hidden" for="form_phone">[% loc('Phone number (optional)') %]</label> + <input class="form-control form-focus-hidden" type="text" value="[% report.user.phone_display | html %]" name="phone" id="form_phone"> + </div> + <div id="js-hide-if-username-email"> + <label class="form-focus-hidden" for="form_email">[% loc('Email address (optional)') %]</label> + <input class="form-control form-focus-hidden" type="text" value="[% report.user.email | html %]" name="email" id="form_email"> + </div> <label class="form-focus-hidden" for="password_register">[% loc('Password (optional)') %]</label> diff --git a/templates/web/base/report/new/form_user_loggedout_email.html b/templates/web/base/report/new/form_user_loggedout_email.html index 39e9fd779..734eb6f35 100644 --- a/templates/web/base/report/new/form_user_loggedout_email.html +++ b/templates/web/base/report/new/form_user_loggedout_email.html @@ -1,7 +1,17 @@ -<label for="form_email">[% loc('Your email') %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +[% IF c.config.SMS_AUTHENTICATION %] + [% SET username_label = loc('Your email or mobile') %] + [% SET username_type = 'text' %] + [% SET username_value = report.user.username %] +[% ELSE %] + [% SET username_label = loc('Your email') %] + [% SET username_type = 'email' %] + [% SET username_value = report.user.email %] [% END %] -<input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" + +<label for="form_username">[% username_label %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> +[% END %] +<input type="[% username_type %]" value="[% username_value | html %]" name="username" id="form_username" [% IF required %]required[% END %] - class="required"> + class="form-control required"> diff --git a/templates/web/base/report/update/form_name.html b/templates/web/base/report/update/form_name.html index e4f7ac60c..f366895a5 100644 --- a/templates/web/base/report/update/form_name.html +++ b/templates/web/base/report/update/form_name.html @@ -20,8 +20,8 @@ <option value="body">[% c.user.from_body.name %]</option> [% END %] </select> - <label for="form_email">[% loc('Email address') %]</label> - <input class="form-control" name="rznvy" id="form_email" type="text" value="[% c.user.email | html %]"> + <label for="form_username">[% loc('Email address') %]</label> + <input class="form-control" name="username" id="form_username" type="text" value="[% c.user.email | html %]"> [% END %] <label for="form_name">[% loc('Name') %]</label> diff --git a/templates/web/base/report/update/form_user_loggedout_by_email.html b/templates/web/base/report/update/form_user_loggedout_by_email.html index 04a842bef..7d10fe391 100644 --- a/templates/web/base/report/update/form_user_loggedout_by_email.html +++ b/templates/web/base/report/update/form_user_loggedout_by_email.html @@ -1,5 +1,9 @@ <div id="form_sign_in_no" class="form-box"> + [% IF c.config.SMS_AUTHENTICATION %] + <h5>[% loc('<strong>No</strong> Let me confirm my update by email/text') %]</h5> + [% ELSE %] <h5>[% loc('<strong>No</strong> Let me confirm my update by email') %]</h5> + [% END %] [% INCLUDE 'report/update/form_name.html' %] diff --git a/templates/web/base/report/update/form_user_loggedout_email.html b/templates/web/base/report/update/form_user_loggedout_email.html index ccea2de02..f4228969b 100644 --- a/templates/web/base/report/update/form_user_loggedout_email.html +++ b/templates/web/base/report/update/form_user_loggedout_email.html @@ -1,7 +1,17 @@ -<label for="form_rznvy">[% loc('Your email' ) %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +[% IF c.config.SMS_AUTHENTICATION %] + [% SET username_label = loc('Your email or mobile') %] + [% SET username_type = 'text' %] + [% SET username_value = update.user.username %] +[% ELSE %] + [% SET username_label = loc('Your email') %] + [% SET username_type = 'email' %] + [% SET username_value = update.user.email %] [% END %] -<input type="email" name="rznvy" id="form_rznvy" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" + +<label for="form_username">[% username_label %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> +[% END %] +<input type="[% username_type %]" name="username" id="form_username" value="[% username_value | html %]" [% IF required %]required[% END %] class="form-control required"> diff --git a/templates/web/bromley/report/new/form_user.html b/templates/web/bromley/report/new/form_user.html index 634e18c10..e6749f5ab 100644 --- a/templates/web/bromley/report/new/form_user.html +++ b/templates/web/bromley/report/new/form_user.html @@ -50,11 +50,11 @@ </div> [% ELSE %] - <label for="form_email">[% loc('Your email') %]</label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + <label for="form_username">[% loc('Your email') %]</label> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> + <input class="form-control" type="email" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <div id="form_sign_in"> diff --git a/templates/web/bromley/report/update-form.html b/templates/web/bromley/report/update-form.html index e71e27528..22dfb1c08 100644 --- a/templates/web/bromley/report/update-form.html +++ b/templates/web/bromley/report/update-form.html @@ -78,14 +78,14 @@ [% ELSE %] - <label for="form_rznvy">[% loc('Email' ) %] + <label for="form_username">[% loc('Email' ) %] <span class="muted">([% loc('We never show your email') %])</span> </label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" name="rznvy" id="form_rznvy" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" required> + <input class="form-control" type="email" name="username" id="form_username" value="[% update.user.email | html %]" placeholder="[% loc('Your email address' ) %]" required> <div id="form_sign_in"> <p>To submit your update you now need to confirm it either by email or by using a FixMyStreet password.</p> diff --git a/templates/web/fixamingata/report/new/form_user_loggedout.html b/templates/web/fixamingata/report/new/form_user_loggedout.html index 24834454c..1f7cf2aeb 100644 --- a/templates/web/fixamingata/report/new/form_user_loggedout.html +++ b/templates/web/fixamingata/report/new/form_user_loggedout.html @@ -1,8 +1,8 @@ -<label for="form_email">[% loc('Your email') %]</label> -[% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> +<label for="form_username">[% loc('Your email') %]</label> +[% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] -<input type="email" class="form-control" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> +<input type="email" class="form-control" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <div id="form_sign_in"> <h3>[% loc("Now to submit your report…") %]</h3> diff --git a/templates/web/zurich/admin/report_edit-sdm.html b/templates/web/zurich/admin/report_edit-sdm.html index 07f0332d5..9f5433d8b 100644 --- a/templates/web/zurich/admin/report_edit-sdm.html +++ b/templates/web/zurich/admin/report_edit-sdm.html @@ -64,7 +64,7 @@ <br> [% problem.user.email | html %] [% IF NOT problem.extra.email_confirmed %]<span class="error">[% loc('Unconfirmed') %]</span>[% END %] - <input type='hidden' id='email' name='email' value='[% problem.user.email | html %]'> + <input type='hidden' id='username' name='username' value='[% problem.user.username | html %]'> <br> [% IF problem.user.phone %][% problem.user.phone | html %][% ELSE %]<em>[% loc('(No phone number)') %]</em>[% END %] </dd> diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html index 35075a9f0..aa5029eb7 100644 --- a/templates/web/zurich/admin/report_edit.html +++ b/templates/web/zurich/admin/report_edit.html @@ -92,7 +92,7 @@ <br> [% problem.user.email | html %] [% IF NOT problem.extra.email_confirmed %]<span class="error">[% loc('Unconfirmed') %]</span>[% END %] - <input type='hidden' id='email' name='email' value='[% problem.user.email | html %]'> + <input type='hidden' id='username' name='username' value='[% problem.user.username | html %]'> <br> [% IF problem.user.phone %][% problem.user.phone | html %][% ELSE %]<em>[% loc('(No phone number)') %]</em>[% END %] </dd> diff --git a/templates/web/zurich/admin/update_edit.html b/templates/web/zurich/admin/update_edit.html index b2cde0b92..bcf849732 100644 --- a/templates/web/zurich/admin/update_edit.html +++ b/templates/web/zurich/admin/update_edit.html @@ -20,7 +20,7 @@ [% END %] </select></li> <input type='hidden' name='name' id='name' value='[% update.name | html %]'> -<input type='hidden' id='email' name='email' value='[% update.user.email | html %]'> +<input type='hidden' id='username' name='username' value='[% update.user.username | html %]'> [% IF update.problem_state %] <li>[% tprintf(loc('Update changed problem state to %s'), update.problem_state) %]</li> [% END %] diff --git a/templates/web/zurich/auth/general.html b/templates/web/zurich/auth/general.html index fd34b79f8..899f0ca71 100644 --- a/templates/web/zurich/auth/general.html +++ b/templates/web/zurich/auth/general.html @@ -1,18 +1,18 @@ [% INCLUDE 'header.html', title = loc('Sign in or create an account') %] -[% IF email_error; +[% IF username_error; # other keys include fqdn, mxcheck if you'd like to write a custom error message errors = { - missing => loc('Please enter your email'), - other => loc('Please check your email address is correct') + missing_email = loc('Please enter your email'), + other_email = loc('Please check your email address is correct') }; - loc_email_error = errors.$email_error || errors.other; + loc_username_error = errors.$username_error || errors.other_email; END %] -<form action="[% c.uri_for() %]" method="post" name="general_auth_login" class="validate"> +<form action="/auth" method="post" name="general_auth_login" class="validate"> <fieldset> <h1>[% loc('Sign in') %]</h1> @@ -21,18 +21,18 @@ END %] <div id="form_sign_in_yes" class="form-box"> - <label class="n" for="email">[% loc('Email') %]</label> - [% IF loc_email_error %] - <div class="form-error">[% loc_email_error %]</div> + <label class="n" for="username">[% loc('Email') %]</label> + [% IF loc_username_error %] + <div class="form-error">[% loc_username_error %]</div> [% ELSIF sign_in_error %] - <div class="form-error">[% loc('There was a problem with your email/password combination. If you cannot remember your password, or do not have one, please fill in the ‘sign in by email’ section of the form.') %]</div> + <div class="form-error">[% loc('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.') %]</div> [% END %] - <input type="email" class="required email" id="email" name="email" value="[% email | html %]" placeholder="[% loc('Your email address') %]" autofocus> + <input type="email" class="required email" id="username" name="username" value="[% username | html %]" placeholder="[% loc('Your email address') %]" autofocus> <label for="password_sign_in">[% loc('Password (optional)') %]</label> <div class="form-txt-submit-box"> <input type="password" class="required" name="password_sign_in" id="password_sign_in" value="" placeholder="[% loc('Your password') %]"> - <input class="green-btn" type="submit" name="sign_in" value="[% loc('Sign in') %]"> + <input class="green-btn" type="submit" name="sign_in_by_password" value="[% loc('Sign in') %]"> </div> <div class="form-txt-submit-box"> @@ -44,18 +44,18 @@ END %] </fieldset> </form> -<form action="[% c.uri_for() %]" method="post" name="general_auth_register" class="validate"> +<form action="/auth" method="post" name="general_auth_register" class="validate"> <fieldset> <input type="hidden" name="r" value="[% c.req.params.r | html %]"> <h1>[% loc('<strong>No</strong> let me sign in by email') %]</h1> <div id="form_sign_in_no" class="form-box"> - <label class="n" for="email2">[% loc('Email') %]</label> - [% IF loc_email_error %] - <div class="form-error">[% loc_email_error %]</div> + <label class="n" for="username2">[% loc('Email') %]</label> + [% IF loc_username_error %] + <div class="form-error">[% loc_username_error %]</div> [% END %] - <input type="email" class="required email" id="email2" name="email" value="[% email | html %]" placeholder="[% loc('Your email address') %]"> + <input type="email" class="required email" id="username2" name="username" value="[% username | html %]" placeholder="[% loc('Your email address') %]"> <label for="name">[% loc('Name') %]</label> <input type="text" class="required" name="name" value="" placeholder="[% loc('Your name') %]"> @@ -63,7 +63,7 @@ END %] <label for="password_register">[% loc('Password (optional)') %]</label> <div class="form-txt-submit-box"> <input type="password" class="required" name="password_register" id="password_register" value="" placeholder="[% loc('Enter a password') %]"> - <input class="green-btn" type="submit" name="email_sign_in" value="Registrieren"> + <input class="green-btn" type="submit" name="sign_in_by_code" value="Registrieren"> </div> </div> diff --git a/templates/web/zurich/report/new/fill_in_details_form.html b/templates/web/zurich/report/new/fill_in_details_form.html index 77d764950..fd21e0fff 100644 --- a/templates/web/zurich/report/new/fill_in_details_form.html +++ b/templates/web/zurich/report/new/fill_in_details_form.html @@ -50,11 +50,11 @@ [% PROCESS "report/new/category_wrapper.html" %] - <label for="form_email">[% loc('Your email') %]</label> - [% IF field_errors.email %] - <p class='form-error'>[% field_errors.email %]</p> + <label for="form_username">[% loc('Your email') %]</label> + [% IF field_errors.username %] + <p class='form-error'>[% field_errors.username %]</p> [% END %] - <input class="form-control" type="email" value="[% report.user.email | html %]" name="email" id="form_email" placeholder="[% loc('Please enter your email address') %]" required> + <input class="form-control" type="email" value="[% report.user.email | html %]" name="username" id="form_username" placeholder="[% loc('Please enter your email address') %]" required> <label for="form_name">[% loc('Name') %] [% loc('(optional)') %]</label> [% IF field_errors.name %] diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 8673b6b76..3bf78ee31 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -343,9 +343,8 @@ $.extend(fixmystreet.set_up, { } ); $('#facebook_sign_in, #twitter_sign_in').click(function(e){ - $('#form_email').removeClass(); - $('#form_rznvy').removeClass(); - $('#email').removeClass(); + $('#form_username').removeClass(); + $('#username').removeClass(); }); $('#planned_form').submit(function(e) { @@ -737,6 +736,19 @@ $.extend(fixmystreet.set_up, { } }, + reporting_hide_phone_email: function() { + $('#form_username').on('keyup change', function() { + var username = $(this).val(); + if (/^[^a-z]+$/i.test(username)) { + $('#js-hide-if-username-phone').hide(); + $('#js-hide-if-username-email').show(); + } else { + $('#js-hide-if-username-phone').show(); + $('#js-hide-if-username-email').hide(); + } + }); + }, + fancybox_images: function() { // Fancybox fullscreen images if (typeof $.fancybox == 'function') { diff --git a/web/cobrands/fixmystreet/images/phone-in-circle-100px.png b/web/cobrands/fixmystreet/images/phone-in-circle-100px.png Binary files differnew file mode 100644 index 000000000..0fab32f8a --- /dev/null +++ b/web/cobrands/fixmystreet/images/phone-in-circle-100px.png diff --git a/web/cobrands/fixmystreet/staff.js b/web/cobrands/fixmystreet/staff.js index c3d1650a6..48bd36909 100644 --- a/web/cobrands/fixmystreet/staff.js +++ b/web/cobrands/fixmystreet/staff.js @@ -171,7 +171,7 @@ $.extend(fixmystreet.set_up, { var opt = this.options[this.selectedIndex], val = opt.value, txt = opt.text; - var $emailInput = $('input[name=email]').add('input[name=rznvy]'); + var $emailInput = $('input[name=username]'); var $nameInput = $('input[name=name]'); var $phoneInput = $('input[name=phone]'); var $showNameCheckbox = $('input[name=may_show_name]'); diff --git a/web/cobrands/sass/_base.scss b/web/cobrands/sass/_base.scss index ce28badab..9d6052c45 100644 --- a/web/cobrands/sass/_base.scss +++ b/web/cobrands/sass/_base.scss @@ -228,6 +228,8 @@ input[type=file] { width: 100%; } +input[type=tel], +input[type=number], input[type=text], input[type=password], input[type=email], @@ -402,6 +404,8 @@ select.form-control { .form-txt-submit-box { @include clearfix(); input[type=password], + input[type=tel], + input[type=number], input[type=text], input[type=email] { width: 65%; @@ -2200,6 +2204,10 @@ table.nicetable { background-image: url(/cobrands/fixmystreet/images/inbox-in-circle-100px.png); } + &.confirmation-header--phone { + background-image: url(/cobrands/fixmystreet/images/phone-in-circle-100px.png); + } + h1, h2 { margin: 0; line-height: 1.2em; |
