From 18bfd8b829b6e2ce5f2c926e44e291986af1bfff Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Thu, 9 Mar 2023 17:23:47 -0600
Subject: [PATCH 1/9] Added signed contract info to the user API as private
 data

---
 app/representers/api/v1/user_representer.rb   |  7 ++++++
 .../api/v1/user_representer_spec.rb           | 22 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/app/representers/api/v1/user_representer.rb b/app/representers/api/v1/user_representer.rb
index 9f9e53d239..e7a9059a53 100644
--- a/app/representers/api/v1/user_representer.rb
+++ b/app/representers/api/v1/user_representer.rb
@@ -191,5 +191,12 @@ class UserRepresenter < Roar::Decorator
                  description: "A list of the applications the user has accessed",
                  required: false
                }
+
+    collection :signed_contract_names,
+               type: String,
+               readable: true,
+               writeable: false,
+               if: ->(user_options:, **) { user_options.try(:fetch, :include_private_data, false) },
+               getter: ->(*) { FinePrint::Contract.published.latest.signed_by(self).pluck(:name) }
   end
 end
diff --git a/spec/representers/api/v1/user_representer_spec.rb b/spec/representers/api/v1/user_representer_spec.rb
index af2d1e60e3..f2fc6a45f5 100644
--- a/spec/representers/api/v1/user_representer_spec.rb
+++ b/spec/representers/api/v1/user_representer_spec.rb
@@ -2,9 +2,11 @@
 
 RSpec.describe Api::V1::UserRepresenter, type: :representer do
   let(:user)            { FactoryBot.create :user  }
-  let(:school)            { FactoryBot.create :school  }
+  let(:school)          { FactoryBot.create :school  }
   subject(:representer) { described_class.new(user) }
 
+  let(:contract)  { FactoryBot.create :fine_print_contract, :published }
+
   context 'uuid' do
     it 'can be read' do
       expect(representer.to_hash['uuid']).to eq user.uuid
@@ -158,4 +160,22 @@
       expect { representer.from_hash(hash) }.not_to change { user.reload.grant_tutor_access }
     end
   end
+
+  context 'signed_contract_names' do
+    it 'can be read' do
+      FactoryBot.create :fine_print_signature, contract: contract, user: user
+
+      expect(
+        representer.to_hash(user_options: { include_private_data: true })['signed_contract_names']
+      ).to eq [ contract.name ]
+    end
+
+    it 'cannot be written (attempts are silently ignored)' do
+      hash = { 'signed_contract_names' => [ contract.name ] }
+
+      expect do
+        representer.from_hash(hash, user_options: { include_private_data: true })
+      end.not_to change { FinePrint::Signature.count }
+    end
+  end
 end

From 21395f40b13ceff0e66cd9b54aba4d89c6310d37 Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Mon, 13 Mar 2023 09:05:28 -0500
Subject: [PATCH 2/9] Added signed_contract_names to user_matcher

---
 spec/support/user_hash.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/support/user_hash.rb b/spec/support/user_hash.rb
index e86982c04d..cd7c4e01ea 100644
--- a/spec/support/user_hash.rb
+++ b/spec/support/user_hash.rb
@@ -37,6 +37,7 @@ def user_matcher(user, include_private_data: false)
             Api::V1::ContactInfoRepresenter.new(ci).to_hash.symbolize_keys
           end
         )
+    base_hash[:signed_contract_names] = FinePrint::Contract.published.latest.signed_by(user).pluck(:name)
   end
 
   base_hash.delete_if { |k,v| v.nil? }

From 79977aa20dd4aa6fd099448a7c1f3898ef721621 Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Mon, 13 Mar 2023 14:48:54 -0500
Subject: [PATCH 3/9] Added new route to sign latest fine print contracts by
 name

---
 app/controllers/api/v1/users_controller.rb | 16 ++++++++++++++++
 app/controllers/terms_controller.rb        |  6 ++++++
 config/initializers/access_policies.rb     |  2 +-
 config/routes.rb                           |  3 +++
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb
index d68e4c197a..b5ae85a3e7 100644
--- a/app/controllers/api/v1/users_controller.rb
+++ b/app/controllers/api/v1/users_controller.rb
@@ -222,6 +222,22 @@ def find_or_create
     end
   end
 
+  ###############################################################
+  # external_id
+  ###############################################################
+
+  api :POST, '/user/external_id', 'Creates an external id for a given user.'
+  description <<-EOS
+    Creates an external id for a given user.
+
+    Both external_id and user_id must be supplied.
+
+    Only trusted apps may call this API. All others will receive a 403 error.
+
+    If the external_id does not exist, it is created and 201 is returned.
+
+    #{json_schema(Api::V1::ExternalIdRepresenter, include: [:readable, :writable])}
+  EOS
   def create_external_id
     standard_create ExternalId.new
   end
diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index b92a00f096..d51c6756c0 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -3,6 +3,7 @@ class TermsController < ApplicationController
 
   fine_print_skip :general_terms_of_use, :privacy_policy
 
+  before_action :allow_iframe_access, only: [:sign]
   before_action :get_contract, only: [:show]
 
   def index
@@ -29,6 +30,11 @@ def pose
     @contract = FinePrint.get_contract(params[:terms].first)
   end
 
+  def sign
+    @contract = FinePrint::Contract.published.latest.find_by! params[:name]
+    render :pose
+  end
+
   def agree
     handle_with(TermsAgree, complete: lambda { fine_print_return })
   end
diff --git a/config/initializers/access_policies.rb b/config/initializers/access_policies.rb
index 24f3a82f48..bda01c1018 100644
--- a/config/initializers/access_policies.rb
+++ b/config/initializers/access_policies.rb
@@ -15,4 +15,4 @@
 OSU::AccessPolicy.register(GroupOwner, GroupOwnerAccessPolicy)
 OSU::AccessPolicy.register(GroupNesting, GroupNestingAccessPolicy)
 OSU::AccessPolicy.register(ApplicationGroup, ApplicationGroupAccessPolicy)
-OSU::AccessPolicy.register(ExternalId, ExternalIdAccessPolicy)
\ No newline at end of file
+OSU::AccessPolicy.register(ExternalId, ExternalIdAccessPolicy)
diff --git a/config/routes.rb b/config/routes.rb
index 8c2d0f9a3e..acdfd7b98f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -210,6 +210,9 @@
       post 'agree', as: 'agree_to'
     end
   end
+  resources :terms, param: :name, only: [] do
+    get :sign, on: :member
+  end
 
   scope controller: 'static_pages' do
     get 'copyright'

From 5746a9faaf49481aff55563761231d13c95bcce9 Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Wed, 15 Mar 2023 12:27:30 -0500
Subject: [PATCH 4/9] Disable contract content sanitization and redirect back
 to trusted host after signing

---
 app/controllers/terms_controller.rb | 7 ++++++-
 app/views/terms/pose.html.erb       | 4 ++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index d51c6756c0..cae44a5b5b 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -36,7 +36,12 @@ def sign
   end
 
   def agree
-    handle_with(TermsAgree, complete: lambda { fine_print_return })
+    handle_with(
+      TermsAgree, complete: -> do
+        params[:r].present? && Host.trusted?(params[:r]) ?
+          redirect_to(params[:r]) : fine_print_return
+      end
+    )
   end
 
   protected
diff --git a/app/views/terms/pose.html.erb b/app/views/terms/pose.html.erb
index 219a7a38e4..eaf268e23b 100644
--- a/app/views/terms/pose.html.erb
+++ b/app/views/terms/pose.html.erb
@@ -7,10 +7,10 @@
   <% end %>
 
   <div class="well">
-    <%= simple_format(@contract.content) %>
+    <%= simple_format @contract.content, {}, sanitize: false %>
   </div>
 
-  <%= lev_form_for :agreement, url: agree_to_terms_path, method: :post do |f| %>
+  <%= lev_form_for :agreement, url: agree_to_terms_path(r: params[:r]), method: :post do |f| %>
     <div class="checkbox">
       <label>
         <%= f.check_box :i_agree %> <%= t :".have_read_terms_and_agree" %>

From 0c5bc1b5e1b9286eecc70c359a74548b7b70cd1a Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Wed, 15 Mar 2023 12:56:48 -0500
Subject: [PATCH 5/9] Allow agree action to be done in iframe Renamed action
 that poses contracts by name

---
 app/controllers/terms_controller.rb | 4 ++--
 config/routes.rb                    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index cae44a5b5b..d4220d0497 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -3,7 +3,7 @@ class TermsController < ApplicationController
 
   fine_print_skip :general_terms_of_use, :privacy_policy
 
-  before_action :allow_iframe_access, only: [:sign]
+  before_action :allow_iframe_access, only: [:pose_by_name, :agree]
   before_action :get_contract, only: [:show]
 
   def index
@@ -30,7 +30,7 @@ def pose
     @contract = FinePrint.get_contract(params[:terms].first)
   end
 
-  def sign
+  def pose_by_name
     @contract = FinePrint::Contract.published.latest.find_by! params[:name]
     render :pose
   end
diff --git a/config/routes.rb b/config/routes.rb
index acdfd7b98f..8c66278e0b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -211,7 +211,7 @@
     end
   end
   resources :terms, param: :name, only: [] do
-    get :sign, on: :member
+    get :pose, action: :pose_by_name, on: :member
   end
 
   scope controller: 'static_pages' do

From 2faf6faf0417993d10a724846196b6f625f72a93 Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Wed, 15 Mar 2023 13:13:53 -0500
Subject: [PATCH 6/9] Allow contracts to be posed and signed with an access
 token

---
 app/controllers/terms_controller.rb | 19 +++++++++++++++----
 app/handlers/terms_agree.rb         |  1 -
 app/views/terms/pose.html.erb       |  6 +++++-
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index d4220d0497..284ec1d352 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -1,11 +1,12 @@
 class TermsController < ApplicationController
-  skip_before_action :authenticate_user!, :complete_signup_profile, only: [:index, :show]
+  skip_before_action :authenticate_user!, only: [:index, :show, :pose_by_name, :agree]
+  skip_before_action :complete_signup_profile, only: [:index, :show]
 
-  fine_print_skip :general_terms_of_use, :privacy_policy
-
-  before_action :allow_iframe_access, only: [:pose_by_name, :agree]
+  before_action :authenticate_user_with_token!, :allow_iframe_access, only: [:pose_by_name, :agree]
   before_action :get_contract, only: [:show]
 
+  fine_print_skip :general_terms_of_use, :privacy_policy
+
   def index
     @contracts = [FinePrint.get_contract(:general_terms_of_use),
                   FinePrint.get_contract(:privacy_policy)].compact
@@ -46,6 +47,16 @@ def agree
 
   protected
 
+  def authenticate_user_with_token!
+    if params[:token].present?
+      token = Doorkeeper::AccessToken.find_by token: params[:token]
+      return head(:forbidden) if token.nil?
+      @current_user = token.user
+    else
+      authenticate_user!
+    end
+  end
+
   def get_contract
     @contract = FinePrint.get_contract(params[:id])
   end
diff --git a/app/handlers/terms_agree.rb b/app/handlers/terms_agree.rb
index 60b4dd1462..842daabd16 100644
--- a/app/handlers/terms_agree.rb
+++ b/app/handlers/terms_agree.rb
@@ -1,5 +1,4 @@
 class TermsAgree
-
   lev_handler
 
   paramify :agreement do
diff --git a/app/views/terms/pose.html.erb b/app/views/terms/pose.html.erb
index eaf268e23b..16032c39ac 100644
--- a/app/views/terms/pose.html.erb
+++ b/app/views/terms/pose.html.erb
@@ -10,7 +10,11 @@
     <%= simple_format @contract.content, {}, sanitize: false %>
   </div>
 
-  <%= lev_form_for :agreement, url: agree_to_terms_path(r: params[:r]), method: :post do |f| %>
+  <%= lev_form_for(
+    :agreement,
+    url: agree_to_terms_path(r: params[:r], token: params[:token]),
+    method: :post
+  ) do |f| %>
     <div class="checkbox">
       <label>
         <%= f.check_box :i_agree %> <%= t :".have_read_terms_and_agree" %>

From 2d1ebfdaadb61818c212805b2c57d35056e4c88b Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Wed, 15 Mar 2023 14:51:13 -0500
Subject: [PATCH 7/9] Added tests for the terms controller and fixed some
 mistakes

---
 app/controllers/terms_controller.rb       |   7 +-
 spec/controllers/terms_controller_spec.rb | 104 ++++++++++++++++++++++
 2 files changed, 107 insertions(+), 4 deletions(-)
 create mode 100644 spec/controllers/terms_controller_spec.rb

diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index 284ec1d352..bd4f63e309 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -32,7 +32,7 @@ def pose
   end
 
   def pose_by_name
-    @contract = FinePrint::Contract.published.latest.find_by! params[:name]
+    @contract = FinePrint::Contract.published.latest.find_by! name: params[:name]
     render :pose
   end
 
@@ -49,9 +49,8 @@ def agree
 
   def authenticate_user_with_token!
     if params[:token].present?
-      token = Doorkeeper::AccessToken.find_by token: params[:token]
-      return head(:forbidden) if token.nil?
-      @current_user = token.user
+      token = Doorkeeper::AccessToken.select(:resource_owner_id).find_by! token: params[:token]
+      @current_user = User.find token.resource_owner_id
     else
       authenticate_user!
     end
diff --git a/spec/controllers/terms_controller_spec.rb b/spec/controllers/terms_controller_spec.rb
new file mode 100644
index 0000000000..d90df7c590
--- /dev/null
+++ b/spec/controllers/terms_controller_spec.rb
@@ -0,0 +1,104 @@
+require 'rails_helper'
+require 'byebug'
+
+RSpec.describe TermsController, type: :controller do
+  let(:contract) { FactoryBot.create :fine_print_contract, :published }
+  let!(:user_1)  { create_user 'user1' }
+  let!(:user_2)  { create_user 'user2' }
+
+  let(:trusted_return_url)   { 'https://openstax.org/example' }
+  let(:untrusted_return_url) { 'https://www.example.com' }
+
+  let(:token)    { FactoryBot.create(:doorkeeper_access_token, resource_owner_id: user_2.id).token }
+
+  before         { controller.sign_in! user_1 }
+
+  context 'pose_by_name' do
+    context 'no params' do
+      it 'renders pose form' do
+        get :pose_by_name, params: { name: contract.name }
+        expect(response).to render_template(:pose)
+      end
+    end
+
+    context 'redirect and token params' do
+      it 'passes params to form url' do
+        get :pose_by_name, params: { name: contract.name, r: trusted_return_url, token: token }
+        expect(response).to render_template(:pose)
+      end
+    end
+
+    context 'invalid token' do
+      it 'fails with 404 Not Found' do
+        get :pose_by_name, params: { name: contract.name, token: SecureRandom.hex }
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+  end
+
+  context 'agree' do
+    context 'agreed' do
+      context 'no token or return url' do
+        it 'records the signature and redirects back' do
+          expect do
+            post :agree, params: { agreement: { contract_id: contract.id, i_agree: true } }
+          end.to change { FinePrint::Signature.count }.by(1)
+          signature = FinePrint::Signature.order(:created_at).last
+          expect(signature.user).to eq user_1
+          expect(response).to redirect_to('/')
+        end
+      end
+
+      context 'token and trusted return url' do
+        it 'records the signature for the token user and redirects back to the trusted url' do
+          expect do
+            post :agree, params: {
+              agreement: { contract_id: contract.id, i_agree: true },
+              r: trusted_return_url,
+              token: token
+            }
+          end.to change { FinePrint::Signature.count }.by(1)
+          signature = FinePrint::Signature.order(:created_at).last
+          expect(signature.user).to eq user_2
+          expect(response).to redirect_to(trusted_return_url)
+        end
+      end
+
+      context 'invalid token' do
+        it 'fails with 404 Not Found' do
+          expect do
+            post :agree, params: {
+              agreement: { contract_id: contract.id, i_agree: true },
+              token: SecureRandom.hex
+            }
+          end.not_to change { FinePrint::Signature.count }
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+
+      context 'untrusted return url' do
+        it 'ignores the untrusted url' do
+          expect do
+            post :agree, params: {
+              agreement: { contract_id: contract.id, i_agree: true },
+              r: untrusted_return_url
+            }
+          end.to change { FinePrint::Signature.count }.by(1)
+          signature = FinePrint::Signature.order(:created_at).last
+          expect(signature.user).to eq user_1
+          expect(response).to redirect_to('/')
+        end
+      end
+    end
+
+    context 'did not agree' do
+      it 'does not record a signature' do
+        expect do
+          post :agree, params: { agreement: { contract_id: contract.id } }
+        end.not_to change { FinePrint::Signature.count }
+        expect(response).to redirect_to('/')
+      end
+    end
+
+  end
+end

From 9cb52bdfe5932094b63fc6c90442dd41ce33eabc Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Tue, 21 Mar 2023 11:30:19 -0500
Subject: [PATCH 8/9] Disable CSRF protection for terms signing so they can be
 signed in iframes

---
 app/controllers/terms_controller.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/controllers/terms_controller.rb b/app/controllers/terms_controller.rb
index bd4f63e309..49cea19c43 100644
--- a/app/controllers/terms_controller.rb
+++ b/app/controllers/terms_controller.rb
@@ -1,4 +1,8 @@
 class TermsController < ApplicationController
+  # 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
+
   skip_before_action :authenticate_user!, only: [:index, :show, :pose_by_name, :agree]
   skip_before_action :complete_signup_profile, only: [:index, :show]
 

From a13da4638473ac777e56b6494d473687cc0f4d03 Mon Sep 17 00:00:00 2001
From: Dante Soares <dante.m.soares@rice.edu>
Date: Tue, 21 Mar 2023 13:10:02 -0500
Subject: [PATCH 9/9] Updated FinePrint to fix timeout in FinePrint admin
 screen

---
 Gemfile.lock                      |  4 ++--
 config/initializers/fine_print.rb | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 704b603e31..a5e63f3a71 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -307,10 +307,10 @@ GEM
       eventmachine (>= 0.12.0)
       websocket-driver (>= 0.5.1)
     ffi (1.15.4)
-    fine_print (5.0.0)
+    fine_print (6.0.1)
       action_interceptor
       jquery-rails
-      rails
+      rails (< 7)
       responders
     font-awesome-rails (4.7.0.5)
       railties (>= 3.2, < 6.1)
diff --git a/config/initializers/fine_print.rb b/config/initializers/fine_print.rb
index b7aef6b63f..3b8dd35e5c 100644
--- a/config/initializers/fine_print.rb
+++ b/config/initializers/fine_print.rb
@@ -59,3 +59,14 @@
   }
 
 end
+
+# Patch that allows us to use FinePrint 6 on Rails 5
+Rails.application.config.to_prepare do
+  FinePrint::Contract.class_exec do
+    scope :signed_by, ->(user) do
+      joins(:signatures).where(
+        fine_print_signatures: { user_id: user.id, user_type: class_name_for(user) }
+      )
+    end
+  end
+end