From 0371c7348509369fe338951b0df86325819622d5 Mon Sep 17 00:00:00 2001
From: Lucas Di Pentima <lucas@di-pentima.com.ar>
Date: Fri, 21 Aug 2020 11:59:01 -0300
Subject: [PATCH] 16678: Don't trust API clients from trusted URLs when
 TokenLifetime is set.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>
---
 services/api/app/models/api_client.rb     |  9 +++--
 services/api/test/unit/api_client_test.rb | 41 +++++++++++++----------
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/services/api/app/models/api_client.rb b/services/api/app/models/api_client.rb
index 8ed693f820..c6c48a5b6b 100644
--- a/services/api/app/models/api_client.rb
+++ b/services/api/app/models/api_client.rb
@@ -15,13 +15,16 @@ class ApiClient < ArvadosModel
   end
 
   def is_trusted
-    norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench1.ExternalURL) ||
-      norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench2.ExternalURL) ||
-      super
+    (from_trusted_url && Rails.configuration.Login.TokenLifetime == 0) || super
   end
 
   protected
 
+  def from_trusted_url
+    norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench1.ExternalURL) ||
+      norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench2.ExternalURL)
+  end
+
   def norm url
     # normalize URL for comparison
     url = URI(url)
diff --git a/services/api/test/unit/api_client_test.rb b/services/api/test/unit/api_client_test.rb
index df082c27fd..93e4c51abf 100644
--- a/services/api/test/unit/api_client_test.rb
+++ b/services/api/test/unit/api_client_test.rb
@@ -7,25 +7,32 @@ require 'test_helper'
 class ApiClientTest < ActiveSupport::TestCase
   include CurrentApiClient
 
-  test "configured workbench is trusted" do
-    Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
-    Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
+  [true, false].each do |token_lifetime_enabled|
+    test "configured workbench is trusted when token lifetime is#{token_lifetime_enabled ? '': ' not'} enabled" do
+      Rails.configuration.Login.TokenLifetime = token_lifetime_enabled ? 8.hours : 0
+      Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
+      Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
 
-    act_as_system_user do
-      [["http://wb0.example.com", false],
-       ["http://wb1.example.com", true],
-       ["http://wb2.example.com", false],
-       ["https://wb2.example.com", true],
-       ["https://wb2.example.com/", true],
-      ].each do |pfx, result|
-        a = ApiClient.create(url_prefix: pfx, is_trusted: false)
-        assert_equal result, a.is_trusted
-      end
+      act_as_system_user do
+        [["http://wb0.example.com", false],
+        ["http://wb1.example.com", true],
+        ["http://wb2.example.com", false],
+        ["https://wb2.example.com", true],
+        ["https://wb2.example.com/", true],
+        ].each do |pfx, result|
+          a = ApiClient.create(url_prefix: pfx, is_trusted: false)
+          if token_lifetime_enabled
+            assert_equal false, a.is_trusted, "API client with url prefix '#{pfx}' shouldn't be trusted"
+          else
+            assert_equal result, a.is_trusted
+          end
+        end
 
-      a = ApiClient.create(url_prefix: "http://example.com", is_trusted: true)
-      a.save!
-      a.reload
-      assert a.is_trusted
+        a = ApiClient.create(url_prefix: "http://example.com", is_trusted: true)
+        a.save!
+        a.reload
+        assert a.is_trusted
+      end
     end
   end
 end
-- 
2.30.2