16678: Don't trust API clients from trusted URLs when TokenLifetime is set.
authorLucas Di Pentima <lucas@di-pentima.com.ar>
Fri, 21 Aug 2020 14:59:01 +0000 (11:59 -0300)
committerLucas Di Pentima <lucas@di-pentima.com.ar>
Fri, 21 Aug 2020 14:59:01 +0000 (11:59 -0300)
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

services/api/app/models/api_client.rb
services/api/test/unit/api_client_test.rb

index 8ed693f820d5eac0eff9389ac851166e800d6516..c6c48a5b6b13c803d8d54d660a2d8fbd2a265740 100644 (file)
@@ -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)
index df082c27fd8c35f7a8d1011bcd3faeba3d4bd4d8..93e4c51abf0e9266a1883ab18bf5634ea45722b0 100644 (file)
@@ -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