11453: Merge branch 'master' into 11453-federated-tokens
authorTom Clegg <tclegg@veritasgenetics.com>
Mon, 4 Dec 2017 21:03:38 +0000 (16:03 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Mon, 4 Dec 2017 21:03:38 +0000 (16:03 -0500)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

14 files changed:
build/run-tests.sh
sdk/python/tests/run_test_server.py
services/api/Gemfile
services/api/Gemfile.lock
services/api/app/controllers/application_controller.rb
services/api/app/controllers/arvados/v1/schema_controller.rb
services/api/app/controllers/user_sessions_controller.rb
services/api/app/middlewares/arvados_api_token.rb
services/api/app/models/api_client_authorization.rb
services/api/config/application.default.yml
services/api/test/functional/arvados/v1/groups_controller_test.rb
services/api/test/integration/remote_user_test.rb [new file with mode: 0644]
services/api/test/integration/users_test.rb
services/api/test/test_helper.rb

index 365931d33281d533e88442d2beb7b9f7f454878f..6063779b306411038555604a209b741f78f1fbb6 100755 (executable)
@@ -811,6 +811,16 @@ install_apiserver() {
 
     mkdir -p "$WORKSPACE/services/api/tmp/pids"
 
+    cert="$WORKSPACE/services/api/tmp/self-signed"
+    if ! [[ -e "$cert.pem" ]]; then
+        (
+            dir="$WORKSPACE/services/api/tmp"
+            set -ex
+            openssl req -newkey rsa:2048 -nodes -subj '/C=US/ST=State/L=City/CN=0.0.0.0' -out "$cert.csr" -keyout "$cert.key" </dev/null
+            openssl x509 -req -in "$cert.csr" -signkey "$cert.key" -out "$cert.pem" -days 3650 -extfile <(printf 'subjectAltName=DNS:127.0.0.1,DNS:localhost,DNS:::1')
+        ) || return 1
+    fi
+
     cd "$WORKSPACE/services/api" \
         && RAILS_ENV=test bundle exec rake db:drop \
         && RAILS_ENV=test bundle exec rake db:setup \
index 57efb97c4851ef3b5b678b906bddefb2b05abc83..567b3b3bfaacf693e7147159bff4d3aa9ad71025 100644 (file)
@@ -288,21 +288,6 @@ def run(leave_running_atexit=False):
     if not os.path.exists('tmp/logs'):
         os.makedirs('tmp/logs')
 
-    if not os.path.exists('tmp/self-signed.pem'):
-        # We assume here that either passenger reports its listening
-        # address as https:/0.0.0.0:port/. If it reports "127.0.0.1"
-        # then the certificate won't match the host and reset() will
-        # fail certificate verification. If it reports "localhost",
-        # clients (notably Python SDK's websocket client) might
-        # resolve localhost as ::1 and then fail to connect.
-        subprocess.check_call([
-            'openssl', 'req', '-new', '-x509', '-nodes',
-            '-out', 'tmp/self-signed.pem',
-            '-keyout', 'tmp/self-signed.key',
-            '-days', '3650',
-            '-subj', '/CN=0.0.0.0'],
-        stdout=sys.stderr)
-
     # Install the git repository fixtures.
     gitdir = os.path.join(SERVICES_SRC_DIR, 'api', 'tmp', 'git')
     gittarball = os.path.join(SERVICES_SRC_DIR, 'api', 'test', 'test.git.tar')
index 25e13a51dbbf70444dbd0eaa6c4417fad93b1d15..4cb5671e1801fc75107057da949653482cbf8430 100644 (file)
@@ -57,6 +57,7 @@ gem 'themes_for_rails', git: 'https://github.com/curoverse/themes_for_rails'
 
 gem 'arvados', '>= 0.1.20150615153458'
 gem 'arvados-cli', '>= 0.1.20161017193526'
+gem 'httpclient'
 
 gem 'sshkey'
 gem 'safe_yaml'
index 91a9d04e4ebe26b48b4a85c88a8c40fa1d9ad826..b2de3f51f2851efe374613292ef99f827032a5b3 100644 (file)
@@ -127,6 +127,7 @@ GEM
     hashie (3.5.5)
     highline (1.7.8)
     hike (1.2.3)
+    httpclient (2.8.3)
     i18n (0.9.0)
       concurrent-ruby (~> 1.0)
     jquery-rails (4.2.2)
@@ -295,6 +296,7 @@ DEPENDENCIES
   database_cleaner
   factory_girl_rails
   faye-websocket
+  httpclient
   jquery-rails
   lograge
   logstash-event
index 6bdba7af89d803975faa40f50d6508ab0b25d953..649aa2b0df2a8a72941d399dbd3b5728c3f349db 100644 (file)
@@ -345,7 +345,7 @@ class ApplicationController < ActionController::Base
         .all
     end
     @read_auths.select! { |auth| auth.scopes_allow_request? request }
-    @read_users = @read_auths.map { |auth| auth.user }.uniq
+    @read_users = @read_auths.map(&:user).uniq
   end
 
   def require_login
index 6f893bcc850015b7c682243e1913ac121dbf9551..c3b34112b2d32134a154d8dbc67e2c0eacd6c321 100644 (file)
@@ -17,7 +17,13 @@ class Arvados::V1::SchemaController < ApplicationController
 
   def index
     expires_in 24.hours, public: true
-    discovery = Rails.cache.fetch 'arvados_v1_rest_discovery' do
+    send_json discovery_doc
+  end
+
+  protected
+
+  def discovery_doc
+    Rails.cache.fetch 'arvados_v1_rest_discovery' do
       Rails.application.eager_load!
       discovery = {
         kind: "discovery#restDescription",
@@ -49,6 +55,8 @@ class Arvados::V1::SchemaController < ApplicationController
         crunchLogThrottleLines: Rails.application.config.crunch_log_throttle_lines,
         crunchLimitLogBytesPerJob: Rails.application.config.crunch_limit_log_bytes_per_job,
         crunchLogPartialLineThrottlePeriod: Rails.application.config.crunch_log_partial_line_throttle_period,
+        remoteHosts: Rails.configuration.remote_hosts,
+        remoteHostsViaDNS: Rails.configuration.remote_hosts_via_dns,
         websocketUrl: Rails.application.config.websocket_address,
         workbenchUrl: Rails.application.config.workbench_address,
         parameters: {
@@ -380,6 +388,5 @@ class Arvados::V1::SchemaController < ApplicationController
       end
       discovery
     end
-    send_json discovery
   end
 end
index 5a90f4f8ead61df1bfa5ae791dd0c4354bcebb51..5de85bc98bcbcb1a0051c3ecee355e82292b5a27 100644 (file)
@@ -24,7 +24,11 @@ class UserSessionsController < ApplicationController
       return redirect_to login_failure_url
     end
 
-    user = User.find_by_identity_url(omniauth['info']['identity_url'])
+    # Only local users can create sessions, hence uuid_like_pattern
+    # here.
+    user = User.where('identity_url = ? and uuid like ?',
+                      omniauth['info']['identity_url'],
+                      User.uuid_like_pattern).first
     if not user
       # Check for permission to log in to an existing User record with
       # a different identity_url
index 6a376318271472db857db6b926ba90d4d8262244..4098fd72ca436bdf3ee806b5a0dfb938fe7a5a9b 100644 (file)
@@ -15,57 +15,50 @@ class ArvadosApiToken
   end
 
   def call env
-    # First, clean up just in case we have a multithreaded server and thread
-    # local variables are still set from a prior request.  Also useful for
-    # tests that call this code to set up the environment.
-    Thread.current[:api_client_ip_address] = nil
-    Thread.current[:api_client_authorization] = nil
-    Thread.current[:api_client_uuid] = nil
-    Thread.current[:api_client] = nil
-    Thread.current[:user] = nil
-
     request = Rack::Request.new(env)
     params = request.params
     remote_ip = env["action_dispatch.remote_ip"]
 
     Thread.current[:request_starttime] = Time.now
-    user = nil
-    api_client = nil
-    api_client_auth = nil
-    if request.get? || params["_method"] == 'GET'
+
+    remote = false
+    reader_tokens = nil
+    if params["remote"] && request.get? && (
+         request.path.start_with?('/arvados/v1/groups') ||
+         request.path.start_with?('/arvados/v1/users/current'))
+      # Request from a remote API server, asking to validate a salted
+      # token.
+      remote = params["remote"]
+    elsif request.get? || params["_method"] == 'GET'
       reader_tokens = params["reader_tokens"]
       if reader_tokens.is_a? String
         reader_tokens = SafeJSON.load(reader_tokens)
       end
-    else
-      reader_tokens = nil
     end
 
     # Set current_user etc. based on the primary session token if a
     # valid one is present. Otherwise, use the first valid token in
     # reader_tokens.
+    auth = nil
     [params["api_token"],
      params["oauth_token"],
-     env["HTTP_AUTHORIZATION"].andand.match(/OAuth2 ([a-zA-Z0-9]+)/).andand[1],
+     env["HTTP_AUTHORIZATION"].andand.match(/(OAuth2|Bearer) ([-\/a-zA-Z0-9]+)/).andand[2],
      *reader_tokens,
     ].each do |supplied|
       next if !supplied
       try_auth = ApiClientAuthorization.
-        includes(:api_client, :user).
-        where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', supplied).
-        first
+                 validate(token: supplied, remote: remote)
       if try_auth.andand.user
-        api_client_auth = try_auth
-        user = api_client_auth.user
-        api_client = api_client_auth.api_client
+        auth = try_auth
         break
       end
     end
+
     Thread.current[:api_client_ip_address] = remote_ip
-    Thread.current[:api_client_authorization] = api_client_auth
-    Thread.current[:api_client_uuid] = api_client.andand.uuid
-    Thread.current[:api_client] = api_client
-    Thread.current[:user] = user
+    Thread.current[:api_client_authorization] = auth
+    Thread.current[:api_client_uuid] = auth.andand.api_client.andand.uuid
+    Thread.current[:api_client] = auth.andand.api_client
+    Thread.current[:user] = auth.andand.user
 
     @app.call env if @app
   end
index 10c02cca25a576a113801b07865a75dfa8affa82..542ab8e88df4f4ff21bc4ed304ff151bc91a8bc1 100644 (file)
@@ -6,6 +6,7 @@ class ApiClientAuthorization < ArvadosModel
   include HasUuid
   include KindAndEtag
   include CommonApiTemplate
+  extend CurrentApiClient
 
   belongs_to :api_client
   belongs_to :user
@@ -82,6 +83,109 @@ class ApiClientAuthorization < ArvadosModel
     ["#{table_name}.id desc"]
   end
 
+  def self.remote_host(uuid_prefix:)
+    Rails.configuration.remote_hosts[uuid_prefix] ||
+      (Rails.configuration.remote_hosts_via_dns &&
+       uuid_prefix+".arvadosapi.com")
+  end
+
+  def self.validate(token:, remote:)
+    return nil if !token
+    remote ||= Rails.configuration.uuid_prefix
+
+    case token[0..2]
+    when 'v2/'
+      _, uuid, secret = token.split('/')
+      auth = ApiClientAuthorization.
+             includes(:user, :api_client).
+             where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', uuid).
+             first
+      if auth && auth.user &&
+         (secret == auth.api_token ||
+          secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote))
+        return auth
+      end
+
+      uuid_prefix = uuid[0..4]
+      if uuid_prefix == Rails.configuration.uuid_prefix
+        # If the token were valid, we would have validated it above
+        return nil
+      elsif uuid_prefix.length != 5
+        # malformed
+        return nil
+      end
+
+      host = remote_host(uuid_prefix: uuid_prefix)
+      if !host
+        Rails.logger.warn "remote authentication rejected: no host for #{uuid_prefix.inspect}"
+        return nil
+      end
+
+      # Token was issued by a different cluster. If it's expired or
+      # missing in our database, ask the originating cluster to
+      # [re]validate it.
+      begin
+        clnt = HTTPClient.new
+        remote_user = SafeJSON.load(
+          clnt.get_content('https://' + host + '/arvados/v1/users/current',
+                           {'remote' => Rails.configuration.uuid_prefix},
+                           {'Authorization' => 'Bearer ' + token}))
+      rescue => e
+        Rails.logger.warn "remote authentication with token #{token.inspect} failed: #{e}"
+        return nil
+      end
+      if !remote_user.is_a?(Hash) || !remote_user['uuid'].is_a?(String) || remote_user['uuid'][0..4] != uuid[0..4]
+        Rails.logger.warn "remote authentication rejected: remote_user=#{remote_user.inspect}"
+        return nil
+      end
+      act_as_system_user do
+        # Add/update user and token in our database so we can
+        # validate subsequent requests faster.
+
+        user = User.find_or_create_by(uuid: remote_user['uuid']) do |user|
+          user.is_admin = false
+        end
+
+        updates = {}
+        [:first_name, :last_name, :email, :prefs].each do |attr|
+          updates[attr] = remote_user[attr.to_s]
+        end
+
+        if Rails.configuration.new_users_are_active
+          # Update is_active to whatever it is at the remote end
+          updates[:is_active] = remote_user['is_active']
+        elsif !updates[:is_active]
+          # Remote user is inactive; our mirror should be, too.
+          updates[:is_active] = false
+        end
+
+        user.update_attributes!(updates)
+
+        auth = ApiClientAuthorization.find_or_create_by(uuid: uuid) do |auth|
+          auth.user = user
+          auth.api_token = token
+          auth.api_client_id = 0
+        end
+
+        # Accept this token (and don't reload the user record) for
+        # 5 minutes. TODO: Request the actual api_client_auth
+        # record from the remote server in case it wants the token
+        # to expire sooner.
+        auth.update_attributes!(expires_at: Time.now + 5.minutes)
+      end
+      return auth
+    else
+      auth = ApiClientAuthorization.
+             includes(:user, :api_client).
+             where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token).
+             first
+      if auth && auth.user
+        return auth
+      end
+    end
+    return nil
+  end
+
   protected
 
   def permission_to_create
index 2f32556733b1a8a186bc3ad51a540518c485ac57..89f9892d473a90740cfbe447168b8fe4a2bc94b5 100644 (file)
@@ -382,6 +382,28 @@ common:
   # original job reuse behavior, and is still the default).
   reuse_job_if_outputs_differ: false
 
+  ###
+  ### Federation support.
+  ###
+
+  # You can enable use of this cluster by users who are authenticated
+  # by a remote Arvados site. Control which remote hosts are trusted
+  # to authenticate which user IDs by configuring remote_hosts,
+  # remote_hosts_via_dns, or both. The default configuration disables
+  # remote authentication.
+
+  # Map known prefixes to hosts. For example, if user IDs beginning
+  # with "zzzzz-" should be authenticated by the Arvados server at
+  # "zzzzz.example.com", use:
+  #
+  # remote_hosts:
+  #   zzzzz: zzzzz.example.com
+  remote_hosts: {}
+
+  # Use {prefix}.arvadosapi.com for any prefix not given in
+  # remote_hosts above.
+  remote_hosts_via_dns: false
+
   ###
   ### Remaining assorted configuration options.
   ###
index ddc40a48448f9ab135d54f0f936e133751d4e391..3442eda2447aa1e75ecc254b3ffcfb2392853a8f 100644 (file)
@@ -704,6 +704,5 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
       assert_response :success
       assert_not_nil Group.readable_by(users(auth)).where(uuid: groups(:trashed_subproject).uuid).first
     end
-
   end
 end
diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb
new file mode 100644 (file)
index 0000000..6e5b9e4
--- /dev/null
@@ -0,0 +1,133 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'webrick'
+require 'webrick/https'
+require 'test_helper'
+require 'helpers/users_test_helper'
+
+class RemoteUsersTest < ActionDispatch::IntegrationTest
+  def auth(remote:)
+    token = salt_token(fixture: :active, remote: remote)
+    token.sub!('/zzzzz-', '/'+remote+'-')
+    {"HTTP_AUTHORIZATION" => "Bearer #{token}"}
+  end
+
+  # For remote authentication tests, we bring up a simple stub server
+  # (on a port chosen by webrick) and configure the SUT so the stub is
+  # responsible for clusters "zbbbb" (a well-behaved cluster) and
+  # "zbork" (a misbehaving cluster).
+  #
+  # Test cases can override the stub's default response to
+  # .../users/current by changing @stub_status and @stub_content.
+  setup do
+    clnt = HTTPClient.new
+    clnt.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_NONE
+    HTTPClient.stubs(:new).returns clnt
+
+    @controller = Arvados::V1::UsersController.new
+    ready = Thread::Queue.new
+    srv = WEBrick::HTTPServer.new(
+      Port: 0,
+      Logger: WEBrick::Log.new(
+        Rails.root.join("log", "webrick.log").to_s,
+        WEBrick::Log::INFO),
+      AccessLog: [[File.open(Rails.root.join(
+                              "log", "webrick_access.log").to_s, 'a+'),
+                   WEBrick::AccessLog::COMBINED_LOG_FORMAT]],
+      SSLEnable: true,
+      SSLVerifyClient: OpenSSL::SSL::VERIFY_NONE,
+      SSLPrivateKey: OpenSSL::PKey::RSA.new(
+        File.open(Rails.root.join("tmp", "self-signed.key")).read),
+      SSLCertificate: OpenSSL::X509::Certificate.new(
+        File.open(Rails.root.join("tmp", "self-signed.pem")).read),
+      SSLCertName: [["CN", WEBrick::Utils::getservername]],
+      StartCallback: lambda { ready.push(true) })
+    srv.mount_proc '/discovery/v1/apis/arvados/v1/rest' do |req, res|
+      Rails.cache.delete 'arvados_v1_rest_discovery'
+      res.body = Arvados::V1::SchemaController.new.send(:discovery_doc).to_json
+    end
+    srv.mount_proc '/arvados/v1/users/current' do |req, res|
+      res.status = @stub_status
+      res.body = @stub_content.is_a?(String) ? @stub_content : @stub_content.to_json
+    end
+    Thread.new do
+      srv.start
+    end
+    ready.pop
+    @remote_server = srv
+    @remote_host = "127.0.0.1:#{srv.config[:Port]}"
+    Rails.configuration.remote_hosts['zbbbb'] = @remote_host
+    Rails.configuration.remote_hosts['zbork'] = @remote_host
+    Arvados::V1::SchemaController.any_instance.stubs(:root_url).returns "https://#{@remote_host}"
+    @stub_status = 200
+    @stub_content = {
+      uuid: 'zbbbb-tpzed-000000000000000',
+      is_admin: true,
+      is_active: true,
+    }
+  end
+
+  teardown do
+    @remote_server.andand.stop
+  end
+
+  test 'authenticate with remote token' do
+    get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb')
+    assert_response :success
+    assert_equal 'zbbbb-tpzed-000000000000000', json_response['uuid']
+    assert_equal false, json_response['is_admin']
+  end
+
+  test 'authenticate with remote token from misbhehaving remote cluster' do
+    get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbork')
+    assert_response 401
+  end
+
+  test 'authenticate with remote token that fails validate' do
+    @stub_status = 401
+    @stub_content = {
+      error: 'not authorized',
+    }
+    get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb')
+    assert_response 401
+  end
+
+  test 'remote api server is not an api server' do
+    @stub_status = 200
+    @stub_content = '<html>bad</html>'
+    get '/arvados/v1/users/current', {format: 'json'}, auth(remote: 'zbbbb')
+    assert_response 401
+  end
+
+  ['zbbbb', 'z0000'].each do |token_valid_for|
+    test "validate #{token_valid_for}-salted token for remote cluster zbbbb" do
+      salted_token = salt_token(fixture: :active, remote: token_valid_for)
+      get '/arvados/v1/users/current', {format: 'json', remote: 'zbbbb'}, {
+            "HTTP_AUTHORIZATION" => "Bearer #{salted_token}"
+          }
+      if token_valid_for == 'zbbbb'
+        assert_response 200
+        assert_equal(users(:active).uuid, json_response['uuid'])
+      else
+        assert_response 401
+      end
+    end
+  end
+
+  test "list readable groups with salted token" do
+    salted_token = salt_token(fixture: :active, remote: 'zbbbb')
+    get '/arvados/v1/groups', {
+          format: 'json',
+          remote: 'zbbbb',
+          limit: 10000,
+        }, {
+          "HTTP_AUTHORIZATION" => "Bearer #{salted_token}"
+        }
+    assert_response 200
+    group_uuids = json_response['items'].collect { |i| i['uuid'] }
+    assert_includes(group_uuids, 'zzzzz-j7d0g-fffffffffffffff')
+    refute_includes(group_uuids, 'zzzzz-j7d0g-000000000000000')
+  end
+end
index 0288e887f94e83361e59f60a7beba6a7cf62a493..8ddab3fee1eb6963dff5c34b3f2788fa09bcef1e 100644 (file)
@@ -216,5 +216,4 @@ class UsersTest < ActionDispatch::IntegrationTest
     end
     nil
   end
-
 end
index d874bc4f1ffebd2777cdc9e7ec392e907eceb7d7..c834250cb6caa89c28ff25ad942978dd14399949 100644 (file)
@@ -127,6 +127,14 @@ class ActiveSupport::TestCase
                              "HTTP_AUTHORIZATION" => "OAuth2 #{t}")
   end
 
+  def salt_token(fixture:, remote:)
+    auth = api_client_authorizations(fixture)
+    uuid = auth.uuid
+    token = auth.api_token
+    hmac = OpenSSL::HMAC.hexdigest('sha1', token, remote)
+    return "v2/#{uuid}/#{hmac}"
+  end
+
   def self.skip_slow_tests?
     !(ENV['RAILS_TEST_SHORT'] || '').empty?
   end