Show privacy notice before redirecting if not signed (#1212)

* Show privacy notice before redirecting if not signed

* check if user is signed in

* update spec to use facebook (twitter removed)

* agree to terms on password spec

* terms agree on require recent signin

* change how before_action checks if signed in

* alter paths for profile in specs

* use newflow_layout for terms display

* update feature specs for pose

app/assets/stylesheets/newflow.scss

@@ -290,6 +290,18 @@ input.has-error {
     color: var(--input-error-message-color);
 }
 
+.center {
+    text-align: center;
+    margin: 0 auto;
+}
+
+.policy {
+    max-width: 800px;
+    margin: 20px auto;
+    padding: 40px;
+    background-color: #f5f5f5;
+}
+
 // per-page styles
 .cs-form-complete-profile {
     header.page-header {

app/controllers/newflow/login_controller.rb

@@ -12,6 +12,7 @@ class LoginController < BaseController
     before_action :known_signup_role_redirect, only: :login_form
     before_action :cache_alternate_signup_url, only: :login_form
     before_action :redirect_to_signup_if_go_param_present, only: :login_form
+    before_action :did_sign_privacy_notice, if: -> { signed_in? }, only: :login_form
     before_action :redirect_back, if: -> { signed_in? }, only: :login_form
 
     def login
@@ -86,5 +87,12 @@ def should_redirect_to_signup_welcome?
     def cache_alternate_signup_url
       set_alternate_signup_url(params[:signup_at])
     end
+
+    def did_sign_privacy_notice
+      contract = FinePrint.get_contract(:privacy_policy)
+      unless contract.signed_by?(current_user)
+        redirect_to pose_term_url(name: contract.name, params: request.params)
+      end
+    end
   end
 end

app/controllers/terms_controller.rb

@@ -1,4 +1,6 @@
 class TermsController < ApplicationController
+  layout 'newflow_layout'
+
   # Allow us to sign terms in an iframe
   # Unlikely that attackers would want to trick our browsers into signing terms
   skip_forgery_protection only: :agree

app/views/terms/index.html.erb

@@ -1,10 +1,10 @@
 <%# Copyright 2011-2016 Rice University. Licensed under the Affero General Public
     License version 3 or later.  See the COPYRIGHT file for details. %>
 
-<%= ox_card(heading: (t :".page_heading")) do %>
+<%= ox_card(heading: (t :".page_heading"), classes: "center") do %>
 
   <% contract_links = @contracts.collect do |contract|
-       link_to contract.title, term_path(contract), remote: true
+       link_to contract.title, term_path(contract)
      end %>
 
   <p><%= t :".notice_html", site_name: SITE_NAME, terms_of_use: contract_links[0], privacy_policy: contract_links[1] %></p>

app/views/terms/pose.html.erb

@@ -1,12 +1,12 @@
-<%= ox_card(heading: @contract.title, classes: "wide") do %>
+<%= ox_card(classes: "wide") do %>
 
   <% if FinePrint.signed_any_version_of_contract?(current_user, @contract) %>
     <p><%= t :".contracts_changed_notice", contract_title: @contract.title %></p>
   <% else %>
     <p><%= t :".contract_acceptance_required" %></p>
   <% end %>
 
-  <div class="well">
+  <div class="policy">
     <%= simple_format @contract.content, {}, sanitize: false %>
   </div>
 
@@ -16,11 +16,16 @@
     method: :post,
     html: { class: 'footer' }
   ) do |f| %>
-    <div class="checkbox">
+
+    <div class="content control-group checkboxes-section center">
+      <div class="terms">
       <label>
-        <%= f.check_box :i_agree, class: 'new-style' %> <%= t :".have_read_terms_and_agree" %>
+        <%= f.check_box :i_agree, style: "height: 1.3rem"%>
+        <span style="font-weight: bold;"><%= t :".have_read_terms_and_agree" %></span>
       </label>
     </div>
+    </div>
+
 
     <%= f.hidden_field :contract_id, value: @contract.id %>
     <%= f.submit (t :".agree"), id: "agreement_submit", class: 'primary new-style' %>

app/views/terms/show.html.erb

@@ -1,5 +1,7 @@
-<%= ox_card(heading: @contract.title) do %>
+<%= ox_card(classes: "center") do %>
 
-  <%= @contract.content.html_safe %>
+  <div class="policy">
+    <%= @contract.content.html_safe %>
+  </div>
 
 <% end %>

spec/features/newflow/require_recent_sign_in_to_change_authentications_spec.rb

@@ -6,7 +6,7 @@
   end
 
   let!(:user) do
-    user = create_newflow_user(email_value)
+    user = create_newflow_user(email_value, 'password', terms_agreed: true)
     user.update(role: User::STUDENT_ROLE)
     user
   end
@@ -108,18 +108,18 @@
 
   scenario 'removing an authentication' do
     with_forgery_protection do
-      FactoryBot.create :authentication, user: user, provider: 'twitter'
+      FactoryBot.create :authentication, user: user, provider: 'facebooknewflow'
 
       newflow_log_in_user(email_value, 'password')
 
       expect(page).to have_no_missing_translations
       Timecop.freeze(Time.now + RequireRecentSignin::REAUTHENTICATE_AFTER) do
-        visit '/profile'
-        expect_newflow_profile_page
-        expect(page).to have_content('Twitter')
+        visit profile_newflow_path
+        expect(page.current_path).to eq(profile_newflow_path)
+        expect(page).to have_content('Facebook')
         screenshot!
 
-        find('.authentication[data-provider="twitter"] .delete--newflow').click
+        find('.authentication[data-provider="facebooknewflow"] .delete--newflow').click
         screenshot!
         click_button 'OK'
         screenshot!
@@ -128,9 +128,9 @@
         expect_newflow_profile_page
         screenshot!
 
-        find('.authentication[data-provider="twitter"] .delete--newflow').click
+        find('.authentication[data-provider="facebooknewflow"] .delete--newflow').click
         click_button 'OK'
-        expect(page).to have_no_content('Twitter')
+        expect(page).to have_no_content('Facebook')
         screenshot!
       end
     end

spec/features/newflow/user_updates_password_spec.rb

@@ -4,20 +4,20 @@
   before(:each) do
     turn_on_student_feature_flag
 
-    @user = create_user('user')
+    @user = create_user('user', 'password', terms_agreed: true)
     @user.update!(role: User::STUDENT_ROLE)
     visit '/'
     newflow_log_in_user('user', 'password')
   end
 
   scenario "adds one" do
     # Get rid of password (have to add another auth first so things don't freak out)
-    FactoryBot.create :authentication, user: @user, provider: 'facebook'
+    FactoryBot.create :authentication, user: @user, provider: 'facebooknewflow'
     @user.authentications.where(provider: 'identity').destroy_all
     @user.identity.destroy
     @user.authentications.reload
     @user.reload.identity
-    visit '/profile'
+    visit profile_newflow_path
 
     screenshot!
     expect(page).not_to have_css('[data-provider=identity]')

spec/features/pose_terms_spec.rb

@@ -11,13 +11,13 @@
     log_in('user','password')
 
     screenshot!
-    expect(page).to have_content("Terms of Use")
+    expect(page).to have_content("To continue, please review and agree to the following site terms")
     expect(page).to have_content(t :"terms.pose.contract_acceptance_required")
     find(:css, '#agreement_i_agree').click
     click_button (t :"terms.pose.agree")
 
     screenshot!
-    expect(page).to have_content("Privacy Policy")
+    expect(page).to have_content("To continue, please review and agree to the following site terms")
     expect(page).to have_content(t :"terms.pose.contract_acceptance_required")
     find(:css, '#agreement_i_agree').click
     click_button (t :"terms.pose.agree")

spec/support/feature_helpers.rb

@@ -430,10 +430,10 @@ def complete_add_password_success_screen
 def complete_terms_screens(without_privacy_policy: false)
 
   check 'agreement_i_agree'
-  expect(page).to have_content('Terms of Use')
+  expect(page).to have_content('To continue, please review and agree to the following site terms')
   click_button (t :"terms.pose.agree")
   unless without_privacy_policy
-    expect(page).to have_content('Privacy Policy')
+    expect(page).to have_content('To continue, please review and agree to the following site terms')
     check 'agreement_i_agree'
     click_button (t :"terms.pose.agree")
   end