diff options
Diffstat (limited to 'perllib/FixMyStreet')
36 files changed, 711 insertions, 321 deletions
diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm index e47336b7c..008aea595 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -18,13 +18,14 @@ use URI; use URI::QueryParam; use Catalyst ( - 'Static::Simple', # + 'Static::Simple', 'Unicode::Encoding', 'Session', 'Session::Store::DBIC', 'Session::State::Cookie', # FIXME - we're using our own override atm 'Authentication', 'SmartURI', + 'FixMyStreet::Session::StoreSessions', ); extends 'Catalyst'; @@ -61,10 +62,19 @@ __PACKAGE__->config( 'Plugin::Authentication' => { default_realm => 'default', default => { - credential => { # Catalyst::Authentication::Credential::Password - class => 'Password', - password_field => 'password', - password_type => 'self_check', + credential => { + class => 'MultiFactor', + factors => [ + # Catalyst::Authentication::Credential::Password + { + class => 'Password', + password_field => 'password', + password_type => 'self_check', + }, + { + class => '2FA', + }, + ], }, store => { # Catalyst::Authentication::Store::DBIx::Class class => 'DBIx::Class', diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index a5c29fce3..85b6204fc 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -892,7 +892,7 @@ sub report_edit : Path('report_edit') : Args(1) { $self->remove_photo($c, $problem, $remove_photo_param); } - if ( $remove_photo_param || $problem->state eq 'hidden' ) { + if ($problem->state eq 'hidden') { $problem->get_photoset->delete_cached; } @@ -1274,8 +1274,14 @@ sub update_edit : Path('update_edit') : Args(1) { $self->remove_photo($c, $update, $remove_photo_param); } - if ( $remove_photo_param || $new_state eq 'hidden' ) { - $update->get_photoset->delete_cached; + $c->stash->{status_message} = '<p><em>' . _('Updated!') . '</em></p>'; + + # Must call update->hide while it's not hidden (so is_latest works) + if ($new_state eq 'hidden') { + my $outcome = $update->hide; + $c->stash->{status_message} .= + '<p><em>' . _('Problem marked as open.') . '</em></p>' + if $outcome->{reopened}; } $update->name( $c->get_param('name') || '' ); @@ -1296,19 +1302,6 @@ sub update_edit : Path('update_edit') : Args(1) { $update->update; - $c->stash->{status_message} = '<p><em>' . _('Updated!') . '</em></p>'; - - # If we're hiding an update, see if it marked as fixed and unfix if so - if ( $new_state eq 'hidden' && $update->mark_fixed ) { - if ( $update->problem->state =~ /^fixed/ ) { - $update->problem->state('confirmed'); - $update->problem->update; - } - - $c->stash->{status_message} .= - '<p><em>' . _('Problem marked as open.') . '</em></p>'; - } - if ( $new_state ne $old_state ) { $c->forward( 'log_edit', [ $update->id, 'update', 'state_change' ] ); @@ -1426,11 +1419,19 @@ sub user_edit : Path('user_edit') : Args(1) { '<p><em>' . $c->flash->{status_message} . '</em></p>'; } + $c->forward('/auth/check_csrf_token') if $c->get_param('submit'); + if ( $c->get_param('submit') and $c->get_param('unban') ) { - $c->forward('/auth/check_csrf_token'); $c->forward('unban_user', [ $user ]); + } elsif ( $c->get_param('submit') and $c->get_param('logout_everywhere') ) { + $c->forward('user_logout_everywhere', [ $user ]); + } elsif ( $c->get_param('submit') and $c->get_param('anon_everywhere') ) { + $c->forward('user_anon_everywhere', [ $user ]); + } elsif ( $c->get_param('submit') and $c->get_param('hide_everywhere') ) { + $c->forward('user_hide_everywhere', [ $user ]); + } elsif ( $c->get_param('submit') and $c->get_param('remove_account') ) { + $c->forward('user_remove_account', [ $user ]); } elsif ( $c->get_param('submit') ) { - $c->forward('/auth/check_csrf_token'); my $edited = 0; @@ -1759,6 +1760,58 @@ sub ban_user : Private { return 1; } +sub user_logout_everywhere : Private { + my ( $self, $c, $user ) = @_; + my $sessions = $user->get_extra_metadata('sessions'); + foreach (grep { $_ ne $c->sessionid } @$sessions) { + $c->delete_session_data("session:$_"); + } + $c->stash->{status_message} = _('That user has been logged out.'); +} + +sub user_anon_everywhere : Private { + my ( $self, $c, $user ) = @_; + $user->problems->update({anonymous => 1}); + $user->comments->update({anonymous => 1}); + $c->stash->{status_message} = _('That user has been made anonymous on all reports and updates.'); +} + +sub user_hide_everywhere : Private { + my ( $self, $c, $user ) = @_; + my $problems = $user->problems->search({ state => { '!=' => 'hidden' } }); + while (my $problem = $problems->next) { + $problem->get_photoset->delete_cached; + $problem->update({ state => 'hidden' }); + } + my $updates = $user->comments->search({ state => { '!=' => 'hidden' } }); + while (my $update = $updates->next) { + $update->hide; + } + $c->stash->{status_message} = _('That user’s reports and updates have been hidden.'); +} + +# Anonymize and remove name from all problems/updates, disable all alerts. +# Remove their account's email address, phone number, password, etc. +sub user_remove_account : Private { + my ( $self, $c, $user ) = @_; + $c->forward('user_logout_everywhere', [ $user ]); + $user->problems->update({ anonymous => 1, name => '', send_questionnaire => 0 }); + $user->comments->update({ anonymous => 1, name => '' }); + $user->alerts->update({ whendisabled => \'current_timestamp' }); + $user->password('', 1); + $user->update({ + email => 'removed-' . $user->id . '@' . FixMyStreet->config('EMAIL_DOMAIN'), + email_verified => 0, + name => '', + phone => '', + phone_verified => 0, + title => undef, + twitter_id => undef, + facebook_id => undef, + }); + $c->stash->{status_message} = _('That user’s personal details have been removed.'); +} + sub unban_user : Private { my ( $self, $c, $user ) = @_; @@ -1904,6 +1957,7 @@ sub remove_photo : Private { my ($self, $c, $object, $keys) = @_; if ($keys eq 'ALL') { $object->photo(undef); + $object->get_photoset->delete_cached; } else { my $fileids = $object->get_photoset->remove_images($keys); $object->photo($fileids); @@ -1938,11 +1992,9 @@ sub check_page_allowed : Private { sub fetch_all_bodies : Private { my ($self, $c ) = @_; - my @bodies = $c->model('DB::Body')->all_translated; + my @bodies = $c->model('DB::Body')->translated->all_sorted; if ( $c->cobrand->moniker eq 'zurich' ) { @bodies = $c->cobrand->admin_fetch_all_bodies( @bodies ); - } else { - @bodies = sort { strcoll($a->name, $b->name) } @bodies; } $c->stash->{bodies} = \@bodies; @@ -1952,20 +2004,15 @@ sub fetch_all_bodies : Private { sub fetch_body_areas : Private { my ($self, $c, $body ) = @_; - my $body_area = $body->body_areas->first; - - unless ( $body_area ) { + my $children = $body->first_area_children; + unless ($children) { # Body doesn't have any areas defined. delete $c->stash->{areas}; delete $c->stash->{fetched_areas_body_id}; return; } - my $areas = mySociety::MaPit::call('area/children', [ $body_area->area_id ], - type => $c->cobrand->area_types_children, - ); - - $c->stash->{areas} = [ sort { strcoll($a->{name}, $b->{name}) } values %$areas ]; + $c->stash->{areas} = [ sort { strcoll($a->{name}, $b->{name}) } values %$children ]; # Keep track of the areas we've fetched to prevent a duplicate fetch later on $c->stash->{fetched_areas_body_id} = $body->id; } diff --git a/perllib/FixMyStreet/App/Controller/Alert.pm b/perllib/FixMyStreet/App/Controller/Alert.pm index 5c9fbad1b..9d522dbc9 100644 --- a/perllib/FixMyStreet/App/Controller/Alert.pm +++ b/perllib/FixMyStreet/App/Controller/Alert.pm @@ -281,20 +281,25 @@ then display confirmation page. sub send_confirmation_email : Private { my ( $self, $c ) = @_; + my $user = $c->stash->{alert}->user; + + # Superusers using 2FA can not log in by code + $c->detach( '/page_error_403_access_denied', [] ) if $user->has_2fa; + my $token = $c->model("DB::Token")->create( { scope => 'alert', data => { id => $c->stash->{alert}->id, type => 'subscribe', - email => $c->stash->{alert}->user->email + email => $user->email } } ); $c->stash->{token_url} = $c->uri_for_email( '/A', $token->token ); - $c->send_email( 'alert-confirm.txt', { to => $c->stash->{alert}->user->email } ); + $c->send_email( 'alert-confirm.txt', { to => $user->email } ); $c->stash->{email_type} = 'alert'; $c->stash->{template} = 'email_sent.html'; diff --git a/perllib/FixMyStreet/App/Controller/Around.pm b/perllib/FixMyStreet/App/Controller/Around.pm index da17cbd56..ae7d83f55 100644 --- a/perllib/FixMyStreet/App/Controller/Around.pm +++ b/perllib/FixMyStreet/App/Controller/Around.pm @@ -225,10 +225,7 @@ sub check_and_stash_category : Private { my ( $self, $c ) = @_; my $all_areas = $c->stash->{all_areas}; - my @bodies = $c->model('DB::Body')->search( - { 'body_areas.area_id' => [ keys %$all_areas ], deleted => 0 }, - { join => 'body_areas' } - )->all; + my @bodies = $c->model('DB::Body')->active->for_areas(keys %$all_areas)->all; my %bodies = map { $_->id => $_ } @bodies; my @contacts = $c->model('DB::Contact')->not_deleted->search( @@ -243,7 +240,7 @@ sub check_and_stash_category : Private { )->all; my @categories = map { { name => $_->category, value => $_->category_display } } @contacts; $c->stash->{filter_categories} = \@categories; - my %categories_mapped = map { $_ => 1 } @categories; + my %categories_mapped = map { $_->{name} => 1 } @categories; my $categories = [ $c->get_param_list('filter_category', 1) ]; my %valid_categories = map { $_ => 1 } grep { $_ && $categories_mapped{$_} } @$categories; @@ -287,11 +284,11 @@ sub map_features : Private { Handle the ajax calls that the map makes when it is dragged. The info returned is used to update the pins on the map and the text descriptions on the side of -the map. +the map. Used via /around?ajax=1 but also available at /ajax for mobile app. =cut -sub ajax : Private { +sub ajax : Path('/ajax') { my ( $self, $c ) = @_; my $ret = $c->forward('/location/determine_location_from_bbox'); diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 455022e03..533e6a9be 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -5,6 +5,7 @@ use namespace::autoclean; BEGIN { extends 'Catalyst::Controller'; } use Email::Valid; +use Data::Password::Common 'found'; use Digest::HMAC_SHA1 qw(hmac_sha1); use JSON::MaybeXS; use MIME::Base64; @@ -84,6 +85,12 @@ sub sign_in : Private { my $parsed = FixMyStreet::SMS->parse_username($username); if ($parsed->{username} && $password && $c->forward('authenticate', [ $parsed->{type}, $parsed->{username}, $password ])) { + # Upgrade hash count if necessary + my $cost = sprintf("%02d", FixMyStreet::DB::Result::User->cost); + if ($c->user->password !~ /^\$2a\$$cost\$/) { + $c->user->update({ password => $password }); + } + # unless user asked to be remembered limit the session to browser $c->set_session_cookie_expire(0) unless $remember_me; @@ -143,6 +150,11 @@ sub email_sign_in : Private { return; } + my $password = $c->get_param('password_register'); + if ($password) { + return unless $c->forward('/auth/test_password', [ $password ]); + } + # If user registration is disabled then bail out at this point # if there's not already a user with this email address. # NB this uses the same template as a successful sign in to stop @@ -156,8 +168,7 @@ sub email_sign_in : Private { } my $user_params = {}; - $user_params->{password} = $c->get_param('password_register') - if $c->get_param('password_register'); + $user_params->{password} = $password if $password; my $user = $c->model('DB::User')->new( $user_params ); my $token_data = { @@ -232,6 +243,9 @@ sub process_login : Private { $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_user_id}; + # Superusers using 2FA can not log in by code + $c->detach( '/page_error_403_access_denied', [] ) if $user->has_2fa; + if ($data->{old_user_id}) { # Were logged in as old_user_id, want to switch to $user if ($user->in_storage) { @@ -270,6 +284,11 @@ Used after signing in to take the person back to where they were. sub redirect_on_signin : Private { my ( $self, $c, $redirect, $params ) = @_; + + if ($c->stash->{detach_to}) { + $c->detach($c->stash->{detach_to}, $c->stash->{detach_args}); + } + unless ( $redirect ) { $c->detach('redirect_to_categories') if $c->user->from_body && scalar @{ $c->user->categories }; $redirect = 'my'; @@ -348,6 +367,55 @@ sub no_csrf_token : Private { $c->detach('/page_error_400_bad_request', []); } +=item common_password + +Returns 1/0 depending on if password is common or not. + +=cut + +sub common_password : Local : Args(0) { + my ($self, $c) = @_; + + my $password = $c->get_param('password_register'); + + my $return = JSON->true; + if (!$c->cobrand->call_hook('bypass_password_checks') && found($password)) { + $return = _('Please choose a less commonly-used password'); + } + + my $body = JSON->new->utf8->allow_nonref->encode($return); + $c->res->content_type('application/json; charset=utf-8'); + $c->res->body($body); +} + +=item test_password + +Checks a password is not too weak; returns true if okay, +false if weak (and sets stash error). + +=cut + +sub test_password : Private { + my ($self, $c, $password) = @_; + + return 1 if $c->cobrand->call_hook('bypass_password_checks'); + + my @errors; + + my $min_length = $c->cobrand->password_minimum_length; + push @errors, sprintf(_('Please make sure your password is at least %d characters long'), $min_length) + if length($password) < $min_length; + + push @errors, _('Please choose a less commonly-used password') + if found($password); + + if (@errors) { + $c->stash->{field_errors}->{password_register} = join('<br>', @errors); + return 0; + } + return 1; +} + =head2 sign_out Log the user out. Tell them we've done so. diff --git a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm index 8387b9d64..8e3150df9 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm @@ -59,6 +59,11 @@ sub sign_in : Private { return; } + my $password = $c->get_param('password_register'); + if ($password) { + return unless $c->forward('/auth/test_password', [ $password ]); + } + (my $number = $parsed->{phone}->format) =~ s/\s+//g; if ( FixMyStreet->config('SIGNUPS_DISABLED') @@ -70,8 +75,7 @@ sub sign_in : Private { } my $user_params = {}; - $user_params->{password} = $c->get_param('password_register') - if $c->get_param('password_register'); + $user_params->{password} = $password if $password; my $user = $c->model('DB::User')->new( $user_params ); my $token_data = { diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm index 5e6fe6266..87aff2261 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm @@ -19,7 +19,7 @@ verifying email, phone, password. =cut -sub auto { +sub auto : Private { my ( $self, $c ) = @_; $c->detach( '/auth/redirect' ) unless $c->user; @@ -49,10 +49,20 @@ sub change_password : Path('/auth/change_password') { my $new = $c->get_param('new_password') // ''; my $confirm = $c->get_param('confirm') // ''; + my $password_error; + + # Check existing password, if available + if ($c->user->password) { + my $current = $c->get_param('current_password') // ''; + $c->stash->{current_password} = $current; + $password_error = 'incorrect' unless $c->user->check_password($current); + } + # check for errors - my $password_error = + $password_error ||= !$new && !$confirm ? 'missing' : $new ne $confirm ? 'mismatch' + : !$c->forward('/auth/test_password', [ $new ]) ? 'failed' : ''; if ($password_error) { @@ -62,10 +72,17 @@ sub change_password : Path('/auth/change_password') { return; } - # we should have a usable password - save it to the user - $c->user->obj->update( { password => $new } ); - $c->stash->{password_changed} = 1; - + if ($c->user->password) { + # we should have a usable password - save it to the user + $c->user->obj->update( { password => $new } ); + $c->stash->{password_changed} = 1; + } else { + # Set up arguments for code sign in + $c->set_param('username', $c->user->username); + $c->set_param('password_register', $new); + $c->set_param('r', 'auth/change_password/success'); + $c->detach('/auth/code_sign_in'); + } } =head2 change_email @@ -148,6 +165,12 @@ sub change_phone_success : Path('/auth/change_phone/success') { $c->res->redirect('/my'); } +sub change_password_success : Path('/auth/change_password/success') { + my ( $self, $c ) = @_; + $c->flash->{flash_message} = _('Your password has been changed'); + $c->res->redirect('/my'); +} + sub generate_token : Path('/auth/generate_token') { my ($self, $c) = @_; @@ -157,14 +180,34 @@ sub generate_token : Path('/auth/generate_token') { $c->stash->{template} = 'auth/generate_token.html'; $c->forward('/auth/get_csrf_token'); + my $has_2fa = $c->user->get_extra_metadata('2fa_secret'); + if ($c->req->method eq 'POST') { $c->forward('/auth/check_csrf_token'); - my $token = mySociety::AuthToken::random_token(); - $c->user->set_extra_metadata('access_token', $token); + + if ($c->get_param('generate_token')) { + my $token = mySociety::AuthToken::random_token(); + $c->user->set_extra_metadata('access_token', $token); + $c->stash->{token_generated} = 1; + } + + if ($c->get_param('toggle_2fa') && $c->user->is_superuser) { + if ($has_2fa) { + $c->user->unset_extra_metadata('2fa_secret'); + $c->stash->{toggle_2fa_off} = 1; + } else { + my $auth = Auth::GoogleAuth->new; + $c->stash->{qr_code} = $auth->qr_code(undef, $c->user->email, 'FixMyStreet'); + $c->stash->{secret32} = $auth->secret32; + $c->user->set_extra_metadata('2fa_secret', $auth->secret32); + $c->stash->{toggle_2fa_on} = 1; + } + } + $c->user->update(); - $c->stash->{token_generated} = 1; } + $c->stash->{has_2fa} = $has_2fa ? 1 : 0; $c->stash->{existing_token} = $c->user->get_extra_metadata('access_token'); } diff --git a/perllib/FixMyStreet/App/Controller/Dashboard.pm b/perllib/FixMyStreet/App/Controller/Dashboard.pm index 926e941f6..032c36e05 100644 --- a/perllib/FixMyStreet/App/Controller/Dashboard.pm +++ b/perllib/FixMyStreet/App/Controller/Dashboard.pm @@ -86,11 +86,7 @@ sub index : Path : Args(0) { if ($body) { $c->stash->{body_name} = $body->name; - my $area_id = $body->body_areas->first->area_id; - my $children = mySociety::MaPit::call('area/children', $area_id, - type => $c->cobrand->area_types_children, - ); - $c->stash->{children} = $children; + my $children = $c->stash->{children} = $body->first_area_children; $c->forward('/admin/fetch_contacts'); $c->stash->{contacts} = [ $c->stash->{contacts}->all ]; @@ -103,7 +99,8 @@ sub index : Path : Args(0) { $c->stash->{body_name} = join "", map { $children->{$_}->{name} } grep { $children->{$_} } $c->user->area_id; } } else { - $c->forward('/admin/fetch_all_bodies'); + my @bodies = $c->model('DB::Body')->active->translated->with_area_count->all_sorted; + $c->stash->{bodies} = \@bodies; } $c->stash->{start_date} = $c->get_param('start_date'); diff --git a/perllib/FixMyStreet/App/Controller/Moderate.pm b/perllib/FixMyStreet/App/Controller/Moderate.pm index a8e0b7a3c..86143b5ea 100644 --- a/perllib/FixMyStreet/App/Controller/Moderate.pm +++ b/perllib/FixMyStreet/App/Controller/Moderate.pm @@ -128,6 +128,7 @@ sub report_moderate_hide : Private { if ($c->get_param('problem_hide')) { $problem->update({ state => 'hidden' }); + $problem->get_photoset->delete_cached; $c->res->redirect( '/' ); # Go directly to front-page $c->detach( 'report_moderate_audit', ['hide'] ); # break chain here. @@ -267,7 +268,7 @@ sub update_moderate_hide : Private { my $comment = $c->stash->{comment} or die; if ($c->get_param('update_hide')) { - $comment->update({ state => 'hidden' }); + $comment->hide; $c->detach( 'update_moderate_audit', ['hide'] ); # break chain here. } return; diff --git a/perllib/FixMyStreet/App/Controller/Open311.pm b/perllib/FixMyStreet/App/Controller/Open311.pm index 95b29d116..c7e4e5bee 100644 --- a/perllib/FixMyStreet/App/Controller/Open311.pm +++ b/perllib/FixMyStreet/App/Controller/Open311.pm @@ -284,7 +284,7 @@ sub output_requests : Private { my $display_photos = $c->cobrand->allow_photo_display($problem); if ($display_photos && $problem->photo) { my $url = $c->cobrand->base_url(); - my $imgurl = $url . $problem->photos->[0]->{url_full}; + my $imgurl = $url . $problem->photos->[$display_photos-1]->{url_full}; $request->{'media_url'} = $imgurl; } push(@problemlist, $request); diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm index 2302322bf..f41702dcf 100644 --- a/perllib/FixMyStreet/App/Controller/Photo.pm +++ b/perllib/FixMyStreet/App/Controller/Photo.pm @@ -63,7 +63,7 @@ sub index :LocalRegex('^(c/)?([1-9]\d*)(?:\.(\d+))?(?:\.(full|tn|fp))?\.(?:jpeg| $c->detach( 'no_photo' ) unless $item; - $c->detach( 'no_photo' ) unless $c->cobrand->allow_photo_display($item); # Should only be for reports, not updates + $c->detach( 'no_photo' ) unless $c->cobrand->allow_photo_display($item, $photo_number); # Should only be for reports, not updates my $photo; $photo = $item->get_photoset diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index ca4fa2fd2..f9e07dd41 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -109,8 +109,8 @@ sub report_new : Path : Args(0) { return unless $c->forward('check_form_submitted'); $c->forward('/auth/check_csrf_token'); - $c->forward('process_user'); $c->forward('process_report'); + $c->forward('process_user'); $c->forward('/photo/process_photo'); return unless $c->forward('check_for_errors'); $c->forward('save_user_and_report'); @@ -147,8 +147,8 @@ sub report_new_ajax : Path('mobile') : Args(0) { $c->forward('setup_categories_and_bodies'); $c->forward('setup_report_extra_fields'); - $c->forward('process_user'); $c->forward('process_report'); + $c->forward('process_user'); $c->forward('/photo/process_photo'); unless ($c->forward('check_for_errors')) { @@ -418,7 +418,9 @@ sub report_import : Path('/import') { sub oauth_callback : Private { my ( $self, $c, $token_code ) = @_; - $c->stash->{oauth_report} = $token_code; + my $auth_token = $c->forward( + '/tokens/load_auth_token', [ $token_code, 'problem/social' ]); + $c->stash->{oauth_report} = $auth_token->data; $c->detach('report_new'); } @@ -475,9 +477,7 @@ sub initialize_report : Private { } if (!$report && $c->stash->{oauth_report}) { - my $auth_token = $c->forward( '/tokens/load_auth_token', - [ $c->stash->{oauth_report}, 'problem/social' ] ); - $report = $c->model("DB::Problem")->new($auth_token->data); + $report = $c->model("DB::Problem")->new($c->stash->{oauth_report}); } if ($report) { @@ -617,10 +617,7 @@ sub setup_categories_and_bodies : Private { my $all_areas = $c->stash->{all_areas}; my $first_area = ( values %$all_areas )[0]; - my @bodies = $c->model('DB::Body')->search( - { 'body_areas.area_id' => [ keys %$all_areas ], deleted => 0 }, - { join => 'body_areas' } - )->all; + my @bodies = $c->model('DB::Body')->active->for_areas(keys %$all_areas)->all; my %bodies = map { $_->id => $_ } @bodies; my $first_body = ( values %bodies )[0]; @@ -775,7 +772,7 @@ sub process_user : Private { if ( $c->user_exists ) { { my $user = $c->user->obj; - if ($c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $c->stash->{bodies})) { + if ($c->stash->{contributing_as_another_user}) { # Act as if not logged in (and it will be auto-confirmed later on) $report->user(undef); last; @@ -784,8 +781,7 @@ sub process_user : Private { $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})) { + if ($c->stash->{contributing_as_body} or $c->stash->{contributing_as_anonymous_user}) { $report->name($user->from_body->name); $user->name($user->from_body->name) unless $user->name; $c->stash->{no_reporter_alert} = 1; @@ -794,14 +790,27 @@ sub process_user : Private { return 1; } } + if ( $c->stash->{contributing_as_another_user} && !$params{username} ) { + # If the 'username' (i.e. email) field is blank, then use the phone + # field for the username. + $params{username} = $params{phone}; + } + my $parsed = FixMyStreet::SMS->parse_username($params{username}); my $type = $parsed->{type} || 'email'; - $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION') || $c->stash->{contributing_as_another_user}; $report->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $report->user; + $c->stash->{phone_may_be_mobile} = $type eq 'phone' && $parsed->{may_be_mobile}; + # 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') ) { + $c->stash->{tfa_data} = { + detach_to => '/report/new/report_new', + login_success => 1, + oauth_report => { $report->get_inflated_columns } + }; 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; @@ -816,8 +825,10 @@ sub process_user : Private { } $c->forward('update_user', [ \%params ]); - $report->user->password( Utils::trim_text( $params{password_register} ) ) - if $params{password_register}; + if ($params{password_register}) { + $c->forward('/auth/test_password', [ $params{password_register} ]); + $report->user->password(Utils::trim_text($params{password_register})); + } return 1; } @@ -870,6 +881,13 @@ sub process_report : Private { $report->longitude( $c->stash->{longitude} ); $report->send_questionnaire( $c->cobrand->send_questionnaires() ); + if ( $c->user_exists ) { + my $user = $c->user->obj; + $c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $c->stash->{bodies}); + $c->stash->{contributing_as_body} = $user->contributing_as('body', $c, $c->stash->{bodies}); + $c->stash->{contributing_as_anonymous_user} = $user->contributing_as('anonymous_user', $c, $c->stash->{bodies}); + } + # set some simple bool values (note they get inverted) if ($c->stash->{contributing_as_body}) { $report->anonymous(0); @@ -960,7 +978,7 @@ sub contacts_to_bodies : Private { my @contacts = grep { $_->category eq $category } @{$c->stash->{contacts}}; - if ($c->stash->{unresponsive}{$category} || $c->stash->{unresponsive}{ALL}) { + if ($c->stash->{unresponsive}{$category} || $c->stash->{unresponsive}{ALL} || !@contacts) { []; } else { if ( $c->cobrand->call_hook('singleton_bodies_str') ) { @@ -1066,6 +1084,12 @@ sub check_for_errors : Private { delete $field_errors{username}; } + # if we're contributing as someone else then allow landline numbers + if ( $field_errors{phone} && $c->stash->{contributing_as_another_user} && !$c->stash->{phone_may_be_mobile}) { + delete $field_errors{username}; + delete $field_errors{phone}; + } + # add the photo error if there is one. if ( my $photo_error = delete $c->stash->{photo_error} ) { $field_errors{photo} = $photo_error; @@ -1374,10 +1398,11 @@ sub redirect_or_confirm_creation : Private { if ( $report->confirmed ) { # Subscribe problem reporter to email updates $c->forward( 'create_reporter_alert' ); - if ($c->stash->{contributing_as_another_user}) { - $c->send_email( 'other-reported.txt', { - to => [ [ $report->user->email, $report->name ] ], - } ); + if ($c->stash->{contributing_as_another_user} && $report->user->email + && !$c->cobrand->report_sent_confirmation_email) { + $c->send_email( 'other-reported.txt', { + to => [ [ $report->user->email, $report->name ] ], + } ); } # If the user has shortlist permission, and either we're not on a # council cobrand or the just-created problem is owned by the cobrand @@ -1394,6 +1419,9 @@ sub redirect_or_confirm_creation : Private { return 1; } + # Superusers using 2FA can not log in by code + $c->detach( '/page_error_403_access_denied', [] ) if $report->user->has_2fa; + # otherwise email or text a confirm token to them. my $thing = 'email'; if ($report->user->email_verified) { diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index c28039808..99eae8659 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -125,12 +125,19 @@ sub process_user : Private { my $parsed = FixMyStreet::SMS->parse_username($params{username}); my $type = $parsed->{type} || 'email'; - $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION'); + $type = 'email' unless FixMyStreet->config('SMS_AUTHENTICATION') || $c->stash->{contributing_as_another_user}; $update->user( $c->model('DB::User')->find_or_new( { $type => $parsed->{username} } ) ) unless $update->user; + $c->stash->{phone_may_be_mobile} = $type eq 'phone' && $parsed->{may_be_mobile}; + # 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') ) { + $c->stash->{tfa_data} = { + detach_to => '/report/update/report_update', + login_success => 1, + oauth_update => { $update->get_inflated_columns } + }; 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; @@ -144,11 +151,14 @@ sub process_user : Private { $update->user->name( Utils::trim_text( $params{name} ) ) if $params{name}; - $update->user->password( Utils::trim_text( $params{password_register} ) ) - if $params{password_register}; $update->user->title( Utils::trim_text( $params{fms_extra_title} ) ) if $params{fms_extra_title}; + if ($params{password_register}) { + $c->forward('/auth/test_password', [ $params{password_register} ]); + $update->user->password(Utils::trim_text($params{password_register})); + } + return 1; } @@ -161,7 +171,9 @@ what we have so far. sub oauth_callback : Private { my ( $self, $c, $token_code ) = @_; - $c->stash->{oauth_update} = $token_code; + my $auth_token = $c->forward('/tokens/load_auth_token', + [ $token_code, 'update/social' ]); + $c->stash->{oauth_update} = $auth_token->data; $c->detach('report_update'); } @@ -176,9 +188,7 @@ sub initialize_update : Private { my $update; if ($c->stash->{oauth_update}) { - my $auth_token = $c->forward( '/tokens/load_auth_token', - [ $c->stash->{oauth_update}, 'update/social' ] ); - $update = $c->model("DB::Comment")->new($auth_token->data); + $update = $c->model("DB::Comment")->new($c->stash->{oauth_update}); } if ($update) { @@ -356,6 +366,12 @@ sub check_for_errors : Private { delete $field_errors{username}; } + # if we're contributing as someone else then allow landline numbers + if ( $field_errors{phone} && $c->stash->{contributing_as_another_user} && !$c->stash->{phone_may_be_mobile}) { + delete $field_errors{username}; + delete $field_errors{phone}; + } + if ( my $photo_error = delete $c->stash->{photo_error} ) { $field_errors{photo} = $photo_error; } @@ -469,7 +485,7 @@ sub redirect_or_confirm_creation : Private { if ( $update->confirmed ) { $c->forward( 'update_problem' ); $c->forward( 'signup_for_alerts' ); - if ($c->stash->{contributing_as_another_user}) { + if ($c->stash->{contributing_as_another_user} && $update->user->email) { $c->send_email( 'other-updated.txt', { to => [ [ $update->user->email, $update->name ] ], } ); @@ -478,6 +494,9 @@ sub redirect_or_confirm_creation : Private { return 1; } + # Superusers using 2FA can not log in by code + $c->detach( '/page_error_403_access_denied', [] ) if $update->user->has_2fa; + my $data = $c->stash->{token_data}; $data->{id} = $update->id; $data->{add_alert} = $c->get_param('add_alert') ? 1 : 0; diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index ec7a192b3..7c3796c42 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -5,7 +5,6 @@ use namespace::autoclean; use JSON::MaybeXS; use List::MoreUtils qw(any); use Path::Tiny; -use POSIX qw(strcoll); use RABX; use mySociety::MaPit; @@ -93,18 +92,8 @@ sub index : Path : Args(0) { $c->stash->{children} = $children; } } else { - # Fetch all bodies - my @bodies = $c->model('DB::Body')->search({ - deleted => 0, - }, { - '+select' => [ { count => 'area_id' } ], - '+as' => [ 'area_count' ], - join => 'body_areas', - distinct => 1, - })->all; - @bodies = sort { strcoll($a->name, $b->name) } @bodies; + my @bodies = $c->model('DB::Body')->active->translated->with_area_count->all_sorted; $c->stash->{bodies} = \@bodies; - $c->stash->{any_empty_bodies} = any { $_->get_column('area_count') == 0 } @bodies; } # Down here so that error pages aren't cached. @@ -451,16 +440,13 @@ sub summary : Private { # required to stop errors in generate_grouped_data $c->stash->{q_state} = ''; - $c->stash->{ward} = $c->get_param('ward'); + $c->stash->{ward} = $c->get_param('area'); $c->stash->{start_date} = $dtf->format_date($start_date); $c->stash->{end_date} = $c->get_param('end_date'); $c->stash->{group_by_default} = 'category'; - my $area_id = $c->stash->{body}->body_areas->first->area_id; - my $children = mySociety::MaPit::call('area/children', $area_id, - type => $c->cobrand->area_types_children, - ); + my $children = $c->stash->{body}->first_area_children; $c->stash->{children} = $children; $c->forward('/admin/fetch_contacts'); @@ -508,7 +494,7 @@ sub export_summary_csv : Private { 'postcode', 'url', ], - filename => 'fixmystreet-data.csv', + filename => 'fixmystreet-data', }; $c->forward('/dashboard/generate_csv'); } diff --git a/perllib/FixMyStreet/App/Controller/Rss.pm b/perllib/FixMyStreet/App/Controller/Rss.pm index 7cf4783c0..e1da4445d 100755 --- a/perllib/FixMyStreet/App/Controller/Rss.pm +++ b/perllib/FixMyStreet/App/Controller/Rss.pm @@ -282,14 +282,15 @@ sub add_row : Private { $item{pubDate} = $pubDate if $pubDate; $item{category} = encode_entities($row->{category}) if $row->{category}; - if ($c->cobrand->allow_photo_display($row) && $row->{photo}) { + if ((my $photo_to_show = $c->cobrand->allow_photo_display($row)) && $row->{photo}) { # Bit yucky as we don't have full objects here my $photoset = FixMyStreet::App::Model::PhotoSet->new({ db_data => $row->{photo} }); - my $first_fn = $photoset->get_id(0); + my $idx = $photo_to_show - 1; + my $first_fn = $photoset->get_id($idx); my ($hash, $format) = split /\./, $first_fn; my $cachebust = substr($hash, 0, 8); my $key = $alert_type->item_table eq 'comment' ? 'c/' : ''; - $item{description} .= encode_entities("\n<br><img src=\"". $base_url . "/photo/$key$row->{id}.0.$format?$cachebust\">"); + $item{description} .= encode_entities("\n<br><img src=\"". $base_url . "/photo/$key$row->{id}.$idx.$format?$cachebust\">"); } if ( $row->{used_map} ) { diff --git a/perllib/FixMyStreet/App/Response.pm b/perllib/FixMyStreet/App/Response.pm index 16ebf995f..6b32e6ebb 100644 --- a/perllib/FixMyStreet/App/Response.pm +++ b/perllib/FixMyStreet/App/Response.pm @@ -13,7 +13,7 @@ around 'redirect' => sub { return $self->$orig() unless @_; # getter my $agent = $self->_context->request->user_agent; - return $self->$orig(@_) unless $agent =~ /Edge\/14/; # Only care about Edge + return $self->$orig(@_) unless $agent && $agent =~ /Edge\/14/; # Only care about Edge # Instead of a redirect, output HTML that redirects $self->body(<<END diff --git a/perllib/FixMyStreet/Cobrand/Angus.pm b/perllib/FixMyStreet/Cobrand/Angus.pm index 056101574..87dcc1d96 100644 --- a/perllib/FixMyStreet/Cobrand/Angus.pm +++ b/perllib/FixMyStreet/Cobrand/Angus.pm @@ -72,9 +72,7 @@ sub temp_update_contacts { my $contact_rs = $self->{c}->model('DB::Contact'); - my $body = FixMyStreet::DB->resultset('Body')->search({ - 'body_areas.area_id' => $self->council_area_id, - }, { join => 'body_areas' })->first; + my $body = FixMyStreet::DB->resultset('Body')->for_areas($self->council_area_id)->first; my $_update = sub { my ($category, $field, $category_details) = @_; diff --git a/perllib/FixMyStreet/Cobrand/Borsetshire.pm b/perllib/FixMyStreet/Cobrand/Borsetshire.pm index 7ddcff469..d9b018d69 100644 --- a/perllib/FixMyStreet/Cobrand/Borsetshire.pm +++ b/perllib/FixMyStreet/Cobrand/Borsetshire.pm @@ -29,4 +29,6 @@ sub send_questionnaires { return 0; } +sub bypass_password_checks { 1 } + 1; diff --git a/perllib/FixMyStreet/Cobrand/Bristol.pm b/perllib/FixMyStreet/Cobrand/Bristol.pm index b11a52643..4648802bd 100644 --- a/perllib/FixMyStreet/Cobrand/Bristol.pm +++ b/perllib/FixMyStreet/Cobrand/Bristol.pm @@ -20,7 +20,7 @@ sub example_places { } sub map_type { - 'Bristol'; + 'OSM'; } sub default_link_zoom { 6 } diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index c33bda7f3..c6ca5c56b 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -15,7 +15,16 @@ use Carp; use mySociety::MaPit; use mySociety::PostcodeUtil; -=head1 path_to_web_templates +=head1 The default cobrand + +This module provides the default cobrand functions used by the codebase, +if not overridden by the cobrand in use. + +=head1 Functions + +=over + +=item path_to_web_templates $path = $cobrand->path_to_web_templates( ); @@ -32,7 +41,7 @@ sub path_to_web_templates { return $paths; } -=head1 path_to_email_templates +=item path_to_email_templates $path = $cobrand->path_to_email_templates( ); @@ -50,7 +59,15 @@ sub path_to_email_templates { return $paths; } -=head1 country +=item password_minimum_length + +Returns the minimum length a password can be set to. + +=cut + +sub password_minimum_length { 6 } + +=item country Returns the country that this cobrand operates in, as an ISO3166-alpha2 code. Default is none. This is not really used for anything important (minor GB only @@ -62,7 +79,7 @@ sub country { return ''; } -=head1 problems +=item problems Returns a ResultSet of Problems, potentially restricted to a subset if we're on a cobrand that only wants some of the data. @@ -74,7 +91,7 @@ sub problems { return $self->problems_restriction(FixMyStreet::DB->resultset('Problem')); } -=head1 problems_on_map +=item problems_on_map Returns a ResultSet of Problems to be shown on the /around map, potentially restricted to a subset if we're on a cobrand that only wants some of the data. @@ -86,7 +103,7 @@ sub problems_on_map { return $self->problems_on_map_restriction(FixMyStreet::DB->resultset('Problem')); } -=head1 updates +=item updates Returns a ResultSet of Comments, potentially restricted to a subset if we're on a cobrand that only wants some of the data. @@ -98,7 +115,7 @@ sub updates { return $self->updates_restriction(FixMyStreet::DB->resultset('Comment')); } -=head1 problems_restriction/updates_restriction +=item problems_restriction/updates_restriction Used to restricts reports and updates in a cobrand in a particular way. Do nothing by default. @@ -115,7 +132,7 @@ sub updates_restriction { return $rs; } -=head1 categories_restriction +=item categories_restriction Used to restrict categories available when making new report in a cobrand in a particular way. Do nothing by default. @@ -128,7 +145,7 @@ sub categories_restriction { } -=head1 problems_on_map_restriction +=item problems_on_map_restriction Used to restricts reports shown on the /around map in a cobrand in a particular way. Do nothing by default. @@ -140,7 +157,7 @@ sub problems_on_map_restriction { return $rs; } -=head1 users +=item users Returns a ResultSet of Users, potentially restricted to a subset if we're on a cobrand that only wants some of the data. @@ -152,7 +169,7 @@ sub users { return $self->users_restriction(FixMyStreet::DB->resultset('User')); } -=head1 users_restriction +=item users_restriction Used to restricts users in the admin in a cobrand in a particular way. Do nothing by default. @@ -166,7 +183,7 @@ sub users_restriction { sub site_key { return 0; } -=head2 restriction +=item restriction Return a restriction to data saved while using this specific cobrand site. @@ -178,7 +195,7 @@ sub restriction { return $self->moniker ? { cobrand => $self->moniker } : {}; } -=head2 base_url_with_lang +=item base_url_with_lang =cut @@ -187,7 +204,7 @@ sub base_url_with_lang { return $self->base_url; } -=head2 admin_base_url +=item admin_base_url Base URL for the admin interface. @@ -198,7 +215,7 @@ sub admin_base_url { return FixMyStreet->config('ADMIN_BASE_URL') || $self->base_url . "/admin"; } -=head2 base_url +=item base_url Return the base url for the cobranded version of the site @@ -206,7 +223,7 @@ Return the base url for the cobranded version of the site sub base_url { FixMyStreet->config('BASE_URL') } -=head2 base_url_for_report +=item base_url_for_report Return the base url for a report (might be different in a two-tier county, but most of the time will be same as base_url_with_lang). Report may be an object, @@ -219,7 +236,7 @@ sub base_url_for_report { return $self->base_url_with_lang; } -=head2 base_host +=item base_host Return the base host for the cobranded version of the site @@ -231,7 +248,7 @@ sub base_host { return $uri->host; } -=head2 enter_postcode_text +=item enter_postcode_text Return override text that prompts the user to enter their postcode/place name. Can be specified in template. @@ -240,7 +257,7 @@ Can be specified in template. sub enter_postcode_text { } -=head2 set_lang_and_domain +=item set_lang_and_domain my $set_lang = $cobrand->set_lang_and_domain( $lang, $unicode, $dir ) @@ -276,7 +293,7 @@ sub languages { FixMyStreet->config('LANGUAGES') || [] } sub language_domain { } sub language_override { } -=head2 alert_list_options +=item alert_list_options Return HTML for a list of alert options for the cobrand, given QUERY and OPTIONS. @@ -285,7 +302,7 @@ OPTIONS. sub alert_list_options { 0 } -=head2 recent_photos +=item recent_photos Return N recent photos. If EASTING, NORTHING and DISTANCE are supplied, the photos must be attached to problems within DISTANCE of the point defined by @@ -299,7 +316,7 @@ sub recent_photos { return $self->problems->recent_photos(@_); } -=head2 recent +=item recent Return recent problems on the site. @@ -321,7 +338,7 @@ sub shorten_recency_if_new_greater_than_fixed { return 1; } -=head2 front_stats_data +=item front_stats_data Return a data structure containing the front stats information that a template can then format. @@ -353,7 +370,7 @@ sub front_stats_data { return $stats; } -=head2 disambiguate_location +=item disambiguate_location Returns any disambiguating information available. Defaults to none. @@ -361,7 +378,7 @@ Returns any disambiguating information available. Defaults to none. sub disambiguate_location { FixMyStreet->config('GEOCODING_DISAMBIGUATION') or {}; } -=head2 cobrand_data_for_generic_update +=item cobrand_data_for_generic_update Parameter is UPDATE_DATA, a reference to a hash of non-cobranded update data. Return cobrand extra data for the update @@ -370,7 +387,7 @@ Return cobrand extra data for the update sub cobrand_data_for_generic_update { '' } -=head2 cobrand_data_for_generic_update +=item cobrand_data_for_generic_update Parameter is PROBLEM_DATA, a reference to a hash of non-cobranded problem data. Return cobrand extra data for the problem @@ -379,7 +396,7 @@ Return cobrand extra data for the problem sub cobrand_data_for_generic_problem { '' } -=head2 uri +=item uri Given a URL ($_[1]), QUERY, EXTRA_DATA, return a URL with any extra params needed appended to it. @@ -398,7 +415,7 @@ sub uri { } -=head2 header_params +=item header_params Return any params to be added to responses @@ -406,7 +423,7 @@ Return any params to be added to responses sub header_params { return {} } -=head2 map_type +=item map_type Return an override type of map if necessary. @@ -417,7 +434,7 @@ sub map_type { return; } -=head2 reports_per_page +=item reports_per_page The number of reports to show per page on all reports page. @@ -427,7 +444,7 @@ sub reports_per_page { return FixMyStreet->config('ALL_REPORTS_PER_PAGE') || 100; } -=head2 reports_ordering +=item reports_ordering The order_by clause to use for reports on all reports page @@ -437,7 +454,7 @@ sub reports_ordering { return 'updated-desc'; } -=head2 on_map_default_status +=item on_map_default_status Return the default ?status= query parameter to use for filter on map page. @@ -445,7 +462,7 @@ Return the default ?status= query parameter to use for filter on map page. sub on_map_default_status { return 'all'; } -=head2 allow_photo_upload +=item allow_photo_upload Return a boolean indicating whether the cobrand allows photo uploads @@ -453,18 +470,19 @@ Return a boolean indicating whether the cobrand allows photo uploads sub allow_photo_upload { return 1; } -=head2 allow_photo_display +=item allow_photo_display Return a boolean indicating whether the cobrand allows photo display +for the particular report and photo. =cut sub allow_photo_display { - my ( $self, $r ) = @_; + my ( $self, $r, $num ) = @_; return 1; } -=head2 allow_update_reporting +=item allow_update_reporting Return a boolean indication whether users should see links next to updates allowing them to report them as offensive. @@ -473,7 +491,7 @@ allowing them to report them as offensive. sub allow_update_reporting { return 0; } -=head2 geocode_postcode +=item geocode_postcode Given a QUERY, return LAT/LON and/or ERROR. @@ -484,7 +502,7 @@ sub geocode_postcode { return {}; } -=head2 geocoded_string_check +=item geocoded_string_check Parameters are LOCATION, QUERY. Return a boolean indicating whether the string LOCATION passes the cobrands checks. @@ -493,7 +511,7 @@ string LOCATION passes the cobrands checks. sub geocoded_string_check { return 1; } -=head2 find_closest +=item find_closest Used by send-reports and similar to attach nearest things to the bottom of the report. @@ -526,7 +544,7 @@ sub find_closest { return $data; } -=head2 find_closest_address_for_rss +=item find_closest_address_for_rss Used by rss feeds to provide a bit more context @@ -553,7 +571,7 @@ sub find_closest_address_for_rss { return $str; } -=head2 format_postcode +=item format_postcode Takes a postcode string and if it looks like a valid postcode then transforms it into the canonical postcode. @@ -570,7 +588,7 @@ sub format_postcode { return $postcode; } -=head2 area_check +=item area_check Paramters are AREAS, QUERY, CONTEXT. Return a boolean indicating whether AREAS pass any extra checks. CONTEXT is where we are on the site. @@ -579,7 +597,7 @@ AREAS pass any extra checks. CONTEXT is where we are on the site. sub area_check { return ( 1, '' ); } -=head2 all_reports_single_body +=item all_reports_single_body Return a boolean indicating whether the cobrand displays a report of all councils @@ -588,7 +606,7 @@ councils sub all_reports_single_body { 0 } -=head2 ask_ever_reported +=item ask_ever_reported Return a boolean indicating whether people should be asked whether this is the first time they' ve reported a problem @@ -597,7 +615,7 @@ first time they' ve reported a problem sub ask_ever_reported { 1 } -=head2 send_questionnaires +=item send_questionnaires Return a boolean indicating whether people should be sent questionnaire emails. @@ -605,7 +623,7 @@ Return a boolean indicating whether people should be sent questionnaire emails. sub send_questionnaires { 1 } -=head2 admin_pages +=item admin_pages List of names of pages to display on the admin interface @@ -660,14 +678,14 @@ sub admin_pages { return $pages; } -=head2 admin_show_creation_graph +=item admin_show_creation_graph Show the problem creation graph in the admin interface =cut sub admin_show_creation_graph { 1 } -=head2 admin_allow_user +=item admin_allow_user Perform checks on whether this user can access admin. By default only superusers are allowed. @@ -679,7 +697,7 @@ sub admin_allow_user { return 1 if $user->is_superuser; } -=head2 available_permissions +=item available_permissions Grouped lists of permission types available for use in the admin @@ -723,7 +741,7 @@ sub available_permissions { } -=head2 area_types +=item area_types The MaPit types this site handles @@ -732,7 +750,7 @@ The MaPit types this site handles sub area_types { FixMyStreet->config('MAPIT_TYPES') || [ 'ZZZ' ] } sub area_types_children { FixMyStreet->config('MAPIT_TYPES_CHILDREN') || [] } -=head2 contact_name, contact_email +=item contact_name, contact_email Return the contact name or email for the cobranded version of the site (to be used in emails). @@ -827,7 +845,7 @@ sub council_rss_alert_options { return ( \@options, @reported_to_options ? \@reported_to_options : undef ); } -=head2 reports_body_check +=item reports_body_check This function is called by the All Reports page, and lets you do some cobrand specific checking on the URL passed to try and match to a relevant body. @@ -839,7 +857,7 @@ sub reports_body_check { return 0; } -=head2 default_photo_resize +=item default_photo_resize Size that photos are to be resized to for display. If photos aren't to be resized then return 0; @@ -848,7 +866,7 @@ to be resized then return 0; sub default_photo_resize { return 0; } -=head2 get_report_stats +=item get_report_stats Get stats to display on the council reports page @@ -883,7 +901,7 @@ sub example_places { FixMyStreet->config('EXAMPLE_PLACES') || [ 'High Street', 'Main Street' ]; } -=head2 title_list +=item title_list Returns an arrayref of possible titles for a person to send to the mobile app. @@ -891,7 +909,7 @@ Returns an arrayref of possible titles for a person to send to the mobile app. sub title_list { return undef; } -=head2 only_authed_can_create +=item only_authed_can_create If true, only users with the from_body flag set are able to create reports. @@ -901,7 +919,7 @@ sub only_authed_can_create { return 0; } -=head2 areas_on_around +=item areas_on_around If set to an arrayref, will plot those area ID(s) from mapit on all the /around pages. @@ -909,7 +927,7 @@ If set to an arrayref, will plot those area ID(s) from mapit on all the /around sub areas_on_around { []; } -=head2 +=item report_form_extras A list of extra fields we wish to save to the database in the 'extra' column of problems based on variables passed in by the form. Return a list of hashrefs @@ -922,7 +940,7 @@ sub report_form_extras {} sub process_open311_extras {} -=head 2 pin_colour +=item pin_colour Returns the colour of pin to be used for a particular report (so perhaps different depending upon the age of the report). @@ -935,7 +953,7 @@ sub pin_colour { return $p->is_fixed ? 'green' : 'red'; } -=head2 pin_new_report_colour +=item pin_new_report_colour Returns the colour of pin to be used for a new report. @@ -944,7 +962,7 @@ sub pin_new_report_colour { return 'green'; } -=head2 path_to_pin_icons +=item path_to_pin_icons Used to override the path for the pin icons if you want to add custom pin icons for your cobrand. @@ -956,7 +974,7 @@ sub path_to_pin_icons { } -=head2 tweak_all_reports_map +=item tweak_all_reports_map Used to tweak the display settings of the map on the all reports pages. @@ -968,7 +986,7 @@ sub tweak_all_reports_map {} sub can_support_problems { return 0; } -=head2 default_map_zoom / default_link_zoom +=item default_map_zoom / default_link_zoom default_map_zoom is used when displaying a map overriding the default of max-4 or max-3 depending on population density. @@ -983,7 +1001,7 @@ sub default_link_zoom { 3 } sub users_can_hide { return 0; } -=head2 default_show_name +=item default_show_name Returns true if the show name checkbox should be ticked by default. @@ -993,7 +1011,7 @@ sub default_show_name { 1; } -=head2 report_check_for_errors +=item report_check_for_errors Perform validation for new reports. Takes Catalyst context object as an argument @@ -1012,7 +1030,7 @@ sub report_check_for_errors { sub report_sent_confirmation_email { 0; } -=head2 never_confirm_reports +=item never_confirm_reports If true then we never send an email to confirm a report @@ -1020,7 +1038,7 @@ If true then we never send an email to confirm a report sub never_confirm_reports { 0; } -=head2 allow_anonymous_reports +=item allow_anonymous_reports If true then can have reports that are truely anonymous - i.e with no email or name. You need to also put details in the anonymous_account function too. @@ -1029,7 +1047,7 @@ need to also put details in the anonymous_account function too. sub allow_anonymous_reports { 0; } -=head2 anonymous_account +=item anonymous_account Details to use for anonymous reports. This should return a hashref with an email and a name key @@ -1038,7 +1056,7 @@ a name key sub anonymous_account { undef; } -=head2 show_unconfirmed_reports +=item show_unconfirmed_reports Whether reports in state 'unconfirmed' should still be shown on the public site. (They're always included in the admin interface.) @@ -1074,7 +1092,7 @@ sub max_detailed_info_length { 0 } sub prefill_report_fields_for_inspector { 0 } -=head2 never_confirm_updates +=item never_confirm_updates If true then we never send an email to confirm an update @@ -1084,7 +1102,7 @@ sub never_confirm_updates { 0; } sub include_time_in_update_alerts { 0; } -=head2 prettify_dt +=item prettify_dt my $date = $c->prettify_dt( $datetime ); @@ -1099,7 +1117,7 @@ sub prettify_dt { return Utils::prettify_dt( $dt, 1 ); } -=head2 extra_contact_validation +=item extra_contact_validation Perform any extra validation on the contact form. @@ -1108,7 +1126,7 @@ Perform any extra validation on the contact form. sub extra_contact_validation { (); } -=head2 get_geocoder +=item get_geocoder Return the default geocoder from config. @@ -1141,7 +1159,7 @@ sub jurisdiction_id_example { return $self->moniker; } -=head2 body_details_data +=item body_details_data Returns a list of bodies to create with ensure_body. These are mostly just passed to ->find_or_create, but there is some @@ -1174,7 +1192,7 @@ sub body_details_data { return (); } -=head2 contact_details_data +=item contact_details_data Returns a list of contact_data to create with setup_contacts. See Zurich for an example. @@ -1185,7 +1203,7 @@ sub contact_details_data { return () } -=head2 lookup_by_ref_regex +=item lookup_by_ref_regex Returns a regex to match postcode form input against to determine if a lookup by id should be done. @@ -1196,7 +1214,7 @@ sub lookup_by_ref_regex { return qr/^\s*ref:\s*(\d+)\s*$/; } -=head2 category_extra_hidden +=item category_extra_hidden Return true if an Open311 service attribute should be a hidden field. =cut @@ -1206,7 +1224,7 @@ sub category_extra_hidden { return 0; } -=head2 reputation_increment_states/reputation_decrement_states +=item reputation_increment_states/reputation_decrement_states Get a hashref of states that cause the reporting user's reputation to be incremented/decremented, if a report is changed to this state upon inspection. @@ -1224,7 +1242,7 @@ sub traffic_management_options { } -=head2 display_days_ago_threshold +=item display_days_ago_threshold Used to control whether a relative 'n days ago' or absolute date is shown for problems/updates. If a problem/update's `days_ago` value is <= this figure, @@ -1233,7 +1251,7 @@ the 'n days ago' format is used. By default the absolute date is always used. =cut sub display_days_ago_threshold { 0 } -=head2 allow_report_extra_fields +=item allow_report_extra_fields Used to control whether site-wide extra fields are available. If true, users with the category_edit permission can add site-wide fields via the @@ -1250,7 +1268,7 @@ sub social_auth_enabled { } -=head2 send_moderation_notifications +=item send_moderation_notifications Used to control whether an email is sent to the problem reporter when a report is moderated. @@ -1260,6 +1278,8 @@ moderation, so e.g. if a UK council cobrand disables the moderation notifications and a report is moderated on fixmystreet.com, the email will still be sent (because it wasn't disabled on the FixMyStreet cobrand). +=back + =cut sub send_moderation_notifications { 1 } diff --git a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm index 23324e763..00f099278 100644 --- a/perllib/FixMyStreet/Cobrand/Oxfordshire.pm +++ b/perllib/FixMyStreet/Cobrand/Oxfordshire.pm @@ -15,8 +15,6 @@ sub is_council_with_case_management { return FixMyStreet->config('STAGING_SITE'); } -sub map_type { 'OSM' } - sub base_url { my $self = shift; return $self->next::method() if FixMyStreet->config('STAGING_SITE'); diff --git a/perllib/FixMyStreet/Cobrand/UKCouncils.pm b/perllib/FixMyStreet/Cobrand/UKCouncils.pm index f958b525a..753aa2404 100644 --- a/perllib/FixMyStreet/Cobrand/UKCouncils.pm +++ b/perllib/FixMyStreet/Cobrand/UKCouncils.pm @@ -41,9 +41,7 @@ sub restriction { # UK cobrands assume that each MapIt area ID maps both ways with one body sub body { my $self = shift; - my $body = FixMyStreet::DB->resultset('Body')->search({ - 'body_areas.area_id' => $self->council_area_id - }, { join => 'body_areas' })->first; + my $body = FixMyStreet::DB->resultset('Body')->for_areas($self->council_area_id)->first; return $body; } diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index 4dc95b178..f0308d6d7 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -4,6 +4,7 @@ use base 'FixMyStreet::Cobrand::Default'; use DateTime; use POSIX qw(strcoll); use RABX; +use List::Util qw(min); use Scalar::Util 'blessed'; use DateTime::Format::Pg; @@ -223,19 +224,30 @@ sub updates_as_hashref { return $hashref; } +# If $num is undefined, we want to return the minimum photo number that can be +# shown (1-indexed), or false for no display. If $num is defined, return +# boolean whether that indexed photo can be shown. sub allow_photo_display { - my ( $self, $r ) = @_; + my ( $self, $r, $num ) = @_; + my $publish_photo; if (blessed $r) { - return $r->get_extra_metadata( 'publish_photo' ); + $publish_photo = $r->get_extra_metadata('publish_photo'); + } else { + # additional munging in case $r isn't an object, TODO see if we can remove this + my $extra = $r->{extra}; + utf8::encode($extra) if utf8::is_utf8($extra); + my $h = new IO::String($extra); + $extra = RABX::wire_rd($h); + return unless ref $extra eq 'HASH'; + $publish_photo = $extra->{publish_photo}; } + # Old style stored 1/0 integer, which can still be used if present. + return $publish_photo unless ref $publish_photo; + return $publish_photo->{$num} if defined $num; - # additional munging in case $r isn't an object, TODO see if we can remove this - my $extra = $r->{extra}; - utf8::encode($extra) if utf8::is_utf8($extra); - my $h = new IO::String($extra); - $extra = RABX::wire_rd($h); - return unless ref $extra eq 'HASH'; - return $extra->{publish_photo}; + # We return a 1-indexed number so that '0' can be taken as 'not allowed' + my $i = min grep { $publish_photo->{$_} } keys %$publish_photo; + return $i + 1; } sub show_unconfirmed_reports { @@ -250,12 +262,27 @@ sub get_body_sender { # Report overdue functions my %public_holidays = map { $_ => 1 } ( - '2013-01-01', '2013-01-02', '2013-03-29', '2013-04-01', - '2013-04-15', '2013-05-01', '2013-05-09', '2013-05-20', - '2013-08-01', '2013-09-09', '2013-12-25', '2013-12-26', - '2014-01-01', '2014-01-02', '2014-04-18', '2014-04-21', - '2014-04-28', '2014-05-01', '2014-05-29', '2014-06-09', - '2014-08-01', '2014-09-15', '2014-12-25', '2014-12-26', + # New Year's Day, Saint Berchtold, Good Friday, Easter Monday, + # Sechseläuten, Labour Day, Ascension Day, Whit Monday, + # Swiss National Holiday, Knabenschiessen, Christmas, St Stephen's Day + # Extra holidays + + '2018-01-01', '2018-01-02', '2018-03-30', '2018-04-02', + '2018-04-16', '2018-05-01', '2018-05-10', '2018-05-21', + '2018-08-01', '2018-09-10', '2018-12-25', '2018-12-26', + '2018-03-29', '2018-05-11', '2018-12-27', '2018-12-28', '2018-12-31', + + '2019-01-01', '2019-01-02', '2019-04-19', '2019-04-22', + '2019-04-08', '2019-05-01', '2019-05-30', '2019-06-10', + '2019-08-01', '2019-09-09', '2019-12-25', '2019-12-26', + + '2020-01-01', '2020-01-02', '2020-04-10', '2020-04-13', + '2020-04-20', '2020-05-01', '2020-05-21', '2020-06-01', + '2020-09-14', '2020-12-25', + + '2021-01-01', '2021-04-02', '2021-04-05', + '2021-04-19', '2021-05-13', '2021-05-24', + '2021-09-13', ); sub is_public_holiday { @@ -566,7 +593,19 @@ sub admin_report_edit { # Problem updates upon submission if ( ($type eq 'super' || $type eq 'dm') && $c->get_param('submit') ) { - $problem->set_extra_metadata('publish_photo' => $c->get_param('publish_photo') || 0 ); + + my @keys = grep { /^publish_photo/ } keys %{ $c->req->params }; + my %publish_photo; + foreach my $key (@keys) { + my ($index) = $key =~ /(\d+)$/; + $publish_photo{$index} = 1 if $c->get_param($key); + } + + if (%publish_photo) { + $problem->set_extra_metadata('publish_photo' => \%publish_photo); + } else { + $problem->unset_extra_metadata('publish_photo'); + } $problem->set_extra_metadata('third_personal' => $c->get_param('third_personal') || 0 ); # Make sure we have a copy of the original detail field @@ -1005,23 +1044,28 @@ sub _admin_send_email { sub munge_sendreport_params { my ($self, $row, $h, $params) = @_; - if ($row->state =~ /^(closed|investigating)$/ && $row->get_extra_metadata('publish_photo')) { + + if ($row->state =~ /^(closed|investigating)$/) { # we attach images to reports sent to external bodies my $photoset = $row->get_photoset(); my $num = $photoset->num_images or return; my $id = $row->id; my @attachments = map { - my $image = $photoset->get_raw_image($_); - { - body => $image->{data}, - attributes => { - filename => "$id.$_." . $image->{extension}, - content_type => $image->{content_type}, - encoding => 'base64', - # quoted-printable ends up with newlines corrupting binary data - name => "$id.$_." . $image->{extension}, - }, + if ($self->allow_photo_display($row, $_)) { + my $image = $photoset->get_raw_image($_); + { + body => $image->{data}, + attributes => { + filename => "$id.$_." . $image->{extension}, + content_type => $image->{content_type}, + encoding => 'base64', + # quoted-printable ends up with newlines corrupting binary data + name => "$id.$_." . $image->{extension}, + }, + }; + } else { + (); } } (0..$num-1); $params->{_attachments_} = \@attachments; @@ -1165,7 +1209,10 @@ sub admin_stats { $public_response =~ s{\r?\n}{ <br/> }g if $public_response; # Assemble photo URL, if report has a photo - my $media_url = ( @{$report->photos} && $c->cobrand->allow_photo_display($report) ) ? ($c->cobrand->base_url . $report->photos->[0]->{url}) : ''; + my $photo_to_display = $c->cobrand->allow_photo_display($report); + my $media_url = (@{$report->photos} && $photo_to_display) + ? $c->cobrand->base_url . $report->photos->[$photo_to_display-1]->{url} + : ''; my @columns = ( $report->id, @@ -1231,7 +1278,7 @@ sub admin_stats { # pictures taken my $pictures_taken = $c->model('DB::Problem')->search( { photo => { '!=', undef }, %params } )->count; # pictures published - my $pictures_published = $c->model('DB::Problem')->search( { extra => { like => '%publish_photo,I1:1%' }, %params } )->count; + my $pictures_published = $c->model('DB::Problem')->search( { extra => { like => '%publish_photo%' }, %params } )->count; # how many times was a telephone number provided # XXX => How many users have a telephone number stored # my $phone = $c->model('DB::User')->search( { phone => { '!=', undef } } )->count; diff --git a/perllib/FixMyStreet/DB/Factories.pm b/perllib/FixMyStreet/DB/Factories.pm index 7b8234aec..0e99608e1 100644 --- a/perllib/FixMyStreet/DB/Factories.pm +++ b/perllib/FixMyStreet/DB/Factories.pm @@ -22,7 +22,7 @@ use parent "DBIx::Class::Factory"; __PACKAGE__->resultset(FixMyStreet::DB->resultset("Problem")); -__PACKAGE__->exclude(['body']); +__PACKAGE__->exclude(['body', 'photo_id']); __PACKAGE__->fields({ postcode => '', @@ -30,6 +30,7 @@ __PACKAGE__->fields({ detail => __PACKAGE__->seq(sub { 'Detail #' . (shift()+1) }), name => __PACKAGE__->callback(sub { shift->get('user')->name }), bodies_str => __PACKAGE__->callback(sub { shift->get('body')->id }), + photo => __PACKAGE__->callback(sub { shift->get('photo_id') }), confirmed => \'current_timestamp', whensent => \'current_timestamp', state => 'confirmed', diff --git a/perllib/FixMyStreet/DB/Result/Body.pm b/perllib/FixMyStreet/DB/Result/Body.pm index e5cd2b907..07bea276c 100644 --- a/perllib/FixMyStreet/DB/Result/Body.pm +++ b/perllib/FixMyStreet/DB/Result/Body.pm @@ -158,10 +158,11 @@ sub areas { sub first_area_children { my ( $self ) = @_; - my $area_id = $self->body_areas->first->area_id; + my $body_area = $self->body_areas->first; + return unless $body_area; my $cobrand = $self->result_source->schema->cobrand; - my $children = mySociety::MaPit::call('area/children', $area_id, + my $children = mySociety::MaPit::call('area/children', $body_area->area_id, type => $cobrand->area_types_children, ); diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm index 60fd31510..8a4dbe475 100644 --- a/perllib/FixMyStreet/DB/Result/Comment.pm +++ b/perllib/FixMyStreet/DB/Result/Comment.pm @@ -156,26 +156,6 @@ sub url { return "/report/" . $self->problem_id . '#update_' . $self->id; } -sub photos { - my $self = shift; - my $photoset = $self->get_photoset; - my $i = 0; - my $id = $self->id; - my @photos = map { - my $cachebust = substr($_, 0, 8); - my ($hash, $format) = split /\./, $_; - { - id => $hash, - url_temp => "/photo/temp.$hash.$format", - url_temp_full => "/photo/fulltemp.$hash.$format", - url => "/photo/c/$id.$i.$format?$cachebust", - url_full => "/photo/c/$id.$i.full.$format?$cachebust", - idx => $i++, - } - } $photoset->all_ids; - return \@photos; -} - =head2 latest_moderation_log_entry Return most recent ModerationLog object @@ -293,4 +273,29 @@ sub problem_state_display { return $update_state; } +sub is_latest { + my $self = shift; + my $latest_update = $self->result_source->resultset->search( + { problem_id => $self->problem_id, state => 'confirmed' }, + { order_by => [ { -desc => 'confirmed' }, { -desc => 'id' } ] } + )->first; + return $latest_update->id == $self->id; +} + +sub hide { + my $self = shift; + + my $ret = {}; + + # If we're hiding an update, see if it marked as fixed and unfix if so + if ($self->mark_fixed && $self->is_latest && $self->problem->state =~ /^fixed/) { + $self->problem->state('confirmed'); + $self->problem->update; + $ret->{reopened} = 1; + } + $self->get_photoset->delete_cached; + $self->update({ state => 'hidden' }); + return $ret; +} + 1; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index 8625bf17a..c73f7efca 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -867,6 +867,33 @@ sub update_send_failed { } ); } +=head2 updates_sent_to_body + +Returns 1 if updates left on this report will be sent to any of the receiving +bodies by some mechanism. Right now that mechanism is Open311. + +=cut + +sub updates_sent_to_body { + my $self = shift; + return unless $self->send_method_used && $self->send_method_used eq 'Open311'; + + # Some bodies only send updates *to* FMS, they don't receive updates. + # NB See also the list in bin/send-comments + my $excluded = qr{Lewisham|Oxfordshire}; + + my @bodies = values %{ $self->bodies }; + my @updates_sent = grep { + $_->send_comments && + ( + $_->send_method eq 'Open311' || + $_->send_method eq 'Noop' # Sending might be temporarily disabled + ) && + !($_->name =~ /$excluded/) + } @bodies; + return scalar @updates_sent; +} + sub add_send_method { my $self = shift; my $sender = shift; @@ -919,38 +946,6 @@ sub latest_moderation_log_entry { return $self->admin_log_entries->search({ action => 'moderation' }, { order_by => { -desc => 'id' } })->first; } -sub photos { - my $self = shift; - my $photoset = $self->get_photoset; - my $i = 0; - my $id = $self->id; - my @photos = map { - my $cachebust = substr($_, 0, 8); - # Some Varnish configurations (e.g. on mySociety infra) strip cookies from - # images, which means image requests will be redirected to the login page - # if LOGIN_REQUIRED is set. To stop this happening, Varnish should be - # configured to not strip cookies if the cookie_passthrough param is - # present, which this line ensures will be if LOGIN_REQUIRED is set. - my $extra = ''; - if (FixMyStreet->config('LOGIN_REQUIRED')) { - $cachebust .= '&cookie_passthrough=1'; - $extra = '?cookie_passthrough=1'; - } - my ($hash, $format) = split /\./, $_; - { - id => $hash, - url_temp => "/photo/temp.$hash.$format$extra", - url_temp_full => "/photo/fulltemp.$hash.$format$extra", - url => "/photo/$id.$i.$format?$cachebust", - url_full => "/photo/$id.$i.full.$format?$cachebust", - url_tn => "/photo/$id.$i.tn.$format?$cachebust", - url_fp => "/photo/$id.$i.fp.$format?$cachebust", - idx => $i++, - } - } $photoset->all_ids; - return \@photos; -} - __PACKAGE__->has_many( "admin_log_entries", "FixMyStreet::DB::Result::AdminLog", diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index d02039ac3..db68236bf 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -125,11 +125,15 @@ with 'FixMyStreet::Roles::Extra'; __PACKAGE__->many_to_many( planned_reports => 'user_planned_reports', 'report' ); +sub cost { + FixMyStreet->test_mode ? 1 : 12; +} + __PACKAGE__->add_columns( "password" => { encode_column => 1, encode_class => 'Crypt::Eksblowfish::Bcrypt', - encode_args => { cost => 8 }, + encode_args => { cost => cost() }, encode_check_method => 'check_password', }, ); @@ -150,8 +154,9 @@ sub username { sub phone_display { my $self = shift; return $self->phone unless $self->phone; + my $country = FixMyStreet->config('PHONE_COUNTRY'); my $parsed = FixMyStreet::SMS->parse_username($self->phone); - return $parsed->{phone} ? $parsed->{phone}->format : $self->phone; + return $parsed->{phone} ? $parsed->{phone}->format_for_country($country) : $self->phone; } sub latest_anonymity { @@ -195,9 +200,13 @@ sub check_for_errors { } elsif ($self->phone_verified) { my $parsed = FixMyStreet::SMS->parse_username($self->phone); if (!$parsed->{phone}) { + # Errors with the phone number may apply to both the username or + # phone field depending on the form. $errors{username} = _('Please check your phone number is correct'); + $errors{phone} = _('Please check your phone number is correct'); } elsif (!$parsed->{may_be_mobile}) { $errors{username} = _('Please enter a mobile number'); + $errors{phone} = _('Please enter a mobile number'); } } @@ -395,6 +404,11 @@ sub admin_user_body_permissions { }); } +sub has_2fa { + my $self = shift; + return $self->is_superuser && $self->get_extra_metadata('2fa_secret'); +} + sub contributing_as { my ($self, $other, $c, $bodies) = @_; $bodies = [ keys %$bodies ] if ref $bodies eq 'HASH'; diff --git a/perllib/FixMyStreet/DB/ResultSet/Body.pm b/perllib/FixMyStreet/DB/ResultSet/Body.pm index e79d038b1..0aa3e8240 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Body.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Body.pm @@ -3,6 +3,25 @@ use base 'DBIx::Class::ResultSet'; use strict; use warnings; +use POSIX qw(strcoll); + +=head1 Name + +FixMyStreet::DB::ResultSet::Body - a ResultSet class for the Body model. + +=head1 Synopsis + + my @bodies = $rs->for_areas(1, 2)->active->with_area_count->translated->all_sorted; + +=head1 Functions + +=over + +=item for_areas + +This restricts the ResultSet to bodies covering the area IDs provided. + +=cut sub for_areas { my ( $rs, @areas ) = @_; @@ -14,14 +33,64 @@ sub for_areas { return $result; } -sub all_translated { +=item active + +This restricts the ResultSet to bodies that are not marked as deleted. + +=cut + +sub active { + my $rs = shift; + $rs->search({ deleted => 0 }); +} + +=item translated + +This joins the ResultSet to the translation table, adding the `msgstr` +column containing possible translations of the body name. + +=cut + +sub translated { my $rs = shift; my $schema = $rs->result_source->schema; - my @bodies = $rs->search(undef, { + $rs->search(undef, { '+columns' => { 'msgstr' => 'translations.msgstr' }, join => 'translations', bind => [ 'name', $schema->lang, 'body' ], - })->all; + }); +} + +=item with_area_count + +This adds the number of areas associated with each body to the ResultSet, +in the area_count column. + +=cut + +sub with_area_count { + my $rs = shift; + $rs->search(undef, { + '+select' => [ { count => 'area_id' } ], + '+as' => [ 'area_count' ], + join => 'body_areas', + distinct => 1, + }); +} + +=item all_sorted + +This returns all results, as C<all()>, but sorted by their name column +(which will be the translated names if present). + +=back + +=cut + +sub all_sorted { + my $rs = shift; + my @bodies = $rs->all; + @bodies = sort { strcoll($a->name, $b->name) } @bodies; return @bodies; } diff --git a/perllib/FixMyStreet/DB/ResultSet/Problem.pm b/perllib/FixMyStreet/DB/ResultSet/Problem.pm index ae45351c4..3f083c073 100644 --- a/perllib/FixMyStreet/DB/ResultSet/Problem.pm +++ b/perllib/FixMyStreet/DB/ResultSet/Problem.pm @@ -232,12 +232,6 @@ sub categories_summary { return \%categories; } -sub send_reports { - my ( $rs, $site_override ) = @_; - require FixMyStreet::Script::Reports; - return FixMyStreet::Script::Reports::send($site_override); -} - sub include_comment_counts { my $rs = shift; my $order_by = $rs->{attrs}{order_by}; diff --git a/perllib/FixMyStreet/Geocode/Google.pm b/perllib/FixMyStreet/Geocode/Google.pm index e64d02c4c..162101953 100644 --- a/perllib/FixMyStreet/Geocode/Google.pm +++ b/perllib/FixMyStreet/Geocode/Google.pm @@ -8,6 +8,7 @@ package FixMyStreet::Geocode::Google; use strict; use Utils; +use URI::Escape; # string STRING CONTEXT # Looks up on Google Maps API, and caches, a user-inputted location. @@ -19,11 +20,13 @@ sub string { my $params = $c->cobrand->disambiguate_location($s); + my $components = ""; + # For some reason adding gl=uk is no longer sufficient to make google - # think we are in the UK for some locations so we explictly add UK to - # the address. - if ($c->cobrand->country eq 'GB' && $s !~ /, *UK/ && $s !~ /united *kingdom$/) { - $s .= ', UK'; + # think we are in the UK for some locations so we explicitly tell Google + # the country. + if ($c->cobrand->country eq 'GB') { + $components = "country:GB"; } $s = FixMyStreet::Geocode::escape($s); @@ -37,8 +40,13 @@ sub string { } elsif ($params->{country}) { $url .= '®ion=' . $params->{country}; } + if ($params->{components}) { + $components .= ($components ? '|' : '') . URI::Escape::uri_escape_utf8($params->{components}); + } $url .= '&language=' . $params->{lang} if $params->{lang}; + $url .= '&components=' . $components if $components; + my $args = 'key=' . FixMyStreet->config('GOOGLE_MAPS_API_KEY'); my $js = FixMyStreet::Geocode::cache('google', $url, $args, qr/"status"\s*:\s*"(OVER_QUERY_LIMIT|REQUEST_DENIED|INVALID_REQUEST|UNKNOWN_ERROR)"/); if (!$js) { diff --git a/perllib/FixMyStreet/Roles/PhotoSet.pm b/perllib/FixMyStreet/Roles/PhotoSet.pm index 9607b5049..2a6863cff 100644 --- a/perllib/FixMyStreet/Roles/PhotoSet.pm +++ b/perllib/FixMyStreet/Roles/PhotoSet.pm @@ -32,4 +32,38 @@ sub get_first_image_fp { return $self->get_photoset->get_image_data( num => 0, size => 'fp' ); } +sub photos { + my $self = shift; + my $photoset = $self->get_photoset; + my $i = 0; + my $id = $self->id; + my $typ = $self->result_source->name eq 'comment' ? 'c/' : ''; + + my @photos = map { + my $cachebust = substr($_, 0, 8); + # Some Varnish configurations (e.g. on mySociety infra) strip cookies from + # images, which means image requests will be redirected to the login page + # if LOGIN_REQUIRED is set. To stop this happening, Varnish should be + # configured to not strip cookies if the cookie_passthrough param is + # present, which this line ensures will be if LOGIN_REQUIRED is set. + my $extra = ''; + if (FixMyStreet->config('LOGIN_REQUIRED')) { + $cachebust .= '&cookie_passthrough=1'; + $extra = '?cookie_passthrough=1'; + } + my ($hash, $format) = split /\./, $_; + { + id => $hash, + url_temp => "/photo/temp.$hash.$format$extra", + url_temp_full => "/photo/fulltemp.$hash.$format$extra", + url => "/photo/$typ$id.$i.$format?$cachebust", + url_full => "/photo/$typ$id.$i.full.$format?$cachebust", + url_tn => "/photo/$typ$id.$i.tn.$format?$cachebust", + url_fp => "/photo/$typ$id.$i.fp.$format?$cachebust", + idx => $i++, + } + } $photoset->all_ids; + return \@photos; +} + 1; diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm index 04ad1c893..8e4a4aec1 100644 --- a/perllib/FixMyStreet/Script/Reports.pm +++ b/perllib/FixMyStreet/Script/Reports.pm @@ -45,6 +45,7 @@ sub send(;$) { while (my $row = $unsent->next) { my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($row->cobrand)->new(); + FixMyStreet::DB->schema->cobrand($cobrand); if ($debug_mode) { $debug_unsent_count++; @@ -105,10 +106,6 @@ sub send(;$) { $row->user->email eq $cobrand->anonymous_account->{'email'} ) { $h{anonymous_report} = 1; - $h{user_details} = _('This report was submitted anonymously'); - } else { - $h{user_details} = sprintf(_('Name: %s'), $row->name) . "\n\n"; - $h{user_details} .= sprintf(_('Email: %s'), $row->user->email) . "\n\n"; } $cobrand->call_hook(process_additional_metadata_for_email => $row, \%h); @@ -158,7 +155,7 @@ sub send(;$) { } } - if ( $reporters{ $sender }->should_skip( $row ) ) { + if ( $reporters{ $sender }->should_skip( $row, $debug_mode ) ) { $skip = 1; debug_print("skipped by sender " . $sender_info->{method} . " (might be due to previous failed attempts?)", $row->id) if $debug_mode; } else { @@ -309,9 +306,9 @@ sub _send_report_sent_email { $h, { To => $row->user->email, - From => [ FixMyStreet->config('CONTACT_EMAIL'), $cobrand->contact_name ], + From => [ $cobrand->contact_email, $cobrand->contact_name ], }, - FixMyStreet->config('CONTACT_EMAIL'), + $cobrand->contact_email, $nomail, $cobrand, $row->lang, diff --git a/perllib/FixMyStreet/SendReport.pm b/perllib/FixMyStreet/SendReport.pm index 40ec4caf2..2739e3043 100644 --- a/perllib/FixMyStreet/SendReport.pm +++ b/perllib/FixMyStreet/SendReport.pm @@ -19,10 +19,12 @@ has 'unconfirmed_notes' => ( 'is' => 'rw', isa => HashRef, default => sub { {} } sub should_skip { - my $self = shift; - my $row = shift; + my $self = shift; + my $row = shift; + my $debug = shift; return 0 unless $row->send_fail_count; + return 0 if $debug; my $now = DateTime->now( time_zone => FixMyStreet->local_time_zone ); my $diff = $now - $row->send_fail_timestamp; diff --git a/perllib/FixMyStreet/SendReport/Email.pm b/perllib/FixMyStreet/SendReport/Email.pm index 0aacc14a1..583aaaa08 100644 --- a/perllib/FixMyStreet/SendReport/Email.pm +++ b/perllib/FixMyStreet/SendReport/Email.pm @@ -86,7 +86,7 @@ sub send { $params->{From} = $self->send_from( $row ); } else { $sender = FixMyStreet->config('DO_NOT_REPLY_EMAIL'); - my $name = sprintf(_("On behalf of %s"), $params->{From}[1]); + my $name = sprintf(_("On behalf of %s"), @{ $self->send_from($row) }[1]); $params->{From} = [ $sender, $name ]; } diff --git a/perllib/FixMyStreet/Test.pm b/perllib/FixMyStreet/Test.pm index 572ae0a44..aa1a63c21 100644 --- a/perllib/FixMyStreet/Test.pm +++ b/perllib/FixMyStreet/Test.pm @@ -7,6 +7,13 @@ use warnings FATAL => 'all'; use utf8; use Test::More; use mySociety::Locale; + +BEGIN { + use FixMyStreet; + FixMyStreet->test_mode(1); + mySociety::Locale::gettext_domain('FixMyStreet', 1); +} + use FixMyStreet::DB; my $db = FixMyStreet::DB->schema->storage; @@ -19,12 +26,6 @@ sub import { $db->txn_begin; } -BEGIN { - use FixMyStreet; - FixMyStreet->test_mode(1); - mySociety::Locale::gettext_domain('FixMyStreet', 1); -} - END { $db->txn_rollback if $db; } |