From a64b03d2042f832f839ad43e4c503855199f23c6 Mon Sep 17 00:00:00 2001
From: Maciej Szlosarczyk <maciej.szlosarczyk@eestiinternet.ee>
Date: Fri, 24 Aug 2018 12:54:05 +0300
Subject: [PATCH] Make sure that only admin contacts and registrants can lock a
 domain

---
 .../registrant/registry_locks_controller.rb   | 12 ++++++++-
 app/models/registrant_user.rb                 | 26 ++++++++++++-------
 .../registrant_api_registry_locks_test.rb     | 10 +++++++
 test/models/registrant_user_test.rb           | 15 ++++++++---
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/app/controllers/api/v1/registrant/registry_locks_controller.rb b/app/controllers/api/v1/registrant/registry_locks_controller.rb
index 509ee4c23..2309cac8b 100644
--- a/app/controllers/api/v1/registrant/registry_locks_controller.rb
+++ b/app/controllers/api/v1/registrant/registry_locks_controller.rb
@@ -3,6 +3,7 @@ module Api
     module Registrant
       class RegistryLocksController < BaseController
         before_action :set_domain
+        before_action :authorized_to_manage_locks?
 
         def create
           if @domain.apply_registry_lock
@@ -25,13 +26,22 @@ module Api
         private
 
         def set_domain
-          domain_pool = current_user.administrated_domains
+          domain_pool = current_user.domains
           @domain = domain_pool.find_by(uuid: params[:domain_uuid])
 
           return if @domain
           render json: { errors: [{ base: ['Domain not found'] }] },
                  status: :not_found and return
         end
+
+        def authorized_to_manage_locks?
+          return if current_user.administrated_domains.include?(@domain)
+
+          render json: { errors: [
+                           { base: ['Only administrative contacts can manage registry locks'] }
+                         ] },
+                 status: :unauthorized and return
+        end
       end
     end
   end
diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb
index 8d8cfdeb8..c6effe2d4 100644
--- a/app/models/registrant_user.rb
+++ b/app/models/registrant_user.rb
@@ -16,22 +16,30 @@ class RegistrantUser < User
   end
 
   def domains
-    Domain.includes(:registrar, :registrant).where(
-      contacts: {
-        ident_type: 'priv',
-        ident: ident,
-        ident_country_code: country_code
-      }
-    )
+    Domain.uniq
+      .joins(:contacts)
+      .where(contacts: { ident_type: 'priv', ident: ident, ident_country_code: country_code })
   end
 
   def contacts
     Contact.where(ident_type: 'priv', ident: ident, ident_country_code: country_code)
   end
 
+  # In Rails 5, can be replaced with a much simpler `or` query method and the raw SQL parts can be
+  # removed.
+  # https://guides.rubyonrails.org/active_record_querying.html#or-conditions
   def administrated_domains
-    Domain.joins(:domain_contacts)
-      .where(domain_contacts: { contact_id: contacts, type: AdminDomainContact })
+    domains_where_is_administrative_contact = begin
+      Domain.joins(:domain_contacts)
+        .where(domain_contacts: { contact_id: contacts, type: [AdminDomainContact] })
+    end
+
+    domains_where_is_registrar = Domain.where(registrant_id: contacts)
+
+    Domain.from(
+      "(#{domains_where_is_registrar.to_sql} UNION " \
+      "#{domains_where_is_administrative_contact.to_sql}) AS domains"
+    )
   end
 
   def to_s
diff --git a/test/integration/api/registrant/registrant_api_registry_locks_test.rb b/test/integration/api/registrant/registrant_api_registry_locks_test.rb
index 0ff677b1d..0b7f31aad 100644
--- a/test/integration/api/registrant/registrant_api_registry_locks_test.rb
+++ b/test/integration/api/registrant/registrant_api_registry_locks_test.rb
@@ -99,6 +99,16 @@ class RegistrantApiRegistryLocksTest < ApplicationIntegrationTest
     assert_equal({ errors: [{ base: ['Domain not found'] }] }, response_json)
   end
 
+  def test_technical_contact_cannot_lock_a_domain
+    post '/api/v1/registrant/domains/647bcc48-8d5e-4a04-8ce5-2a3cd17b6eab/registry_lock',
+         {}, @auth_headers
+
+    response_json = JSON.parse(response.body, symbolize_names: true)
+    assert_equal(401, response.status)
+    assert_equal({ errors: [{ base: ['Only administrative contacts can manage registry locks'] }] },
+                 response_json)
+  end
+
   private
 
   def auth_token
diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb
index 5af663dd9..b04829999 100644
--- a/test/models/registrant_user_test.rb
+++ b/test/models/registrant_user_test.rb
@@ -4,20 +4,27 @@ class RegistrantUserTest < ActiveSupport::TestCase
   def setup
     super
 
-    @user = RegistrantUser.new(registrant_ident: 'US-1234')
+    @user = users(:registrant)
   end
 
   def teardown
     super
   end
 
-  def test_domains_returns_an_list_of_domains_associated_with_a_specific_id_code
+  def test_domains_returns_an_list_of_distinct_domains_associated_with_a_specific_id_code
     domain_names = @user.domains.pluck(:name)
     assert_equal(3, domain_names.length)
+
+    # User is a registrant, but not a contact for the domain.
+    refute(domain_names.include?('shop.test'))
   end
 
-  def test_administrated_domains_returns_a_list_of_domains_that_is_smaller_than_domains
-    assert_equal(2, @user.administrated_domains.count)
+  def test_administrated_domains_returns_a_list_of_domains
+    domain_names = @user.administrated_domains.pluck(:name)
+    assert_equal(3, domain_names.length)
+
+    # User is a tech contact for the domain.
+    refute(domain_names.include?('library.test'))
   end
 
   def test_contacts_returns_an_list_of_contacts_associated_with_a_specific_id_code