From 0b471c74f2cc392a37aa4f8df8ed931bb5969236 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 19 Aug 2019 14:32:03 -0400 Subject: [PATCH] 15529: Support for LoginCluster and RemoteTokenRefresh Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- lib/config/config.default.yml | 13 +- lib/config/export.go | 6 +- lib/config/generated_config.go | 13 +- sdk/go/arvados/config.go | 6 +- .../app/models/api_client_authorization.rb | 150 +++++++++++------- services/api/config/arvados_config.rb | 2 + .../api/test/integration/remote_user_test.rb | 85 ++++++---- 7 files changed, 180 insertions(+), 95 deletions(-) diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml index dfdd031044..3338bec511 100644 --- a/lib/config/config.default.yml +++ b/lib/config/config.default.yml @@ -410,11 +410,20 @@ Clusters: MaxUUIDEntries: 1000 Login: - # These settings are provided by your OAuth2 provider (e.g., - # sso-provider). + # These settings are provided by your OAuth2 provider (eg + # Google) used to perform upstream authentication. ProviderAppSecret: "" ProviderAppID: "" + # The cluster ID to delegate the user database. When set, + # logins on this cluster will be redirected to the login cluster + # (login cluster must appear in RemoteHosts with Proxy: true) + LoginCluster: "" + + # How long a cached token belonging to a remote cluster will + # remain valid before it needs to be revalidated. + RemoteTokenRefresh: 5m + Git: # Git repositories must be readable by api server, or you won't be # able to submit crunch jobs. To pass the test suites, put a clone diff --git a/lib/config/export.go b/lib/config/export.go index a0be827f04..ffeb527c94 100644 --- a/lib/config/export.go +++ b/lib/config/export.go @@ -116,7 +116,11 @@ var whitelist = map[string]bool{ "InstanceTypes": true, "InstanceTypes.*": true, "InstanceTypes.*.*": true, - "Login": false, + "Login": true, + "Login.ProviderAppSecret": false, + "Login.ProviderAppID": false, + "Login.LoginCluster": true, + "Login.RemoteTokenRefresh": true, "Mail": false, "ManagementToken": false, "PostgreSQL": false, diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go index 6cb8bf81ae..ce1d8a1ed8 100644 --- a/lib/config/generated_config.go +++ b/lib/config/generated_config.go @@ -416,11 +416,20 @@ Clusters: MaxUUIDEntries: 1000 Login: - # These settings are provided by your OAuth2 provider (e.g., - # sso-provider). + # These settings are provided by your OAuth2 provider (eg + # Google) used to perform upstream authentication. ProviderAppSecret: "" ProviderAppID: "" + # The cluster ID to delegate the user database. When set, + # logins on this cluster will be redirected to the login cluster + # (login cluster must appear in RemoteHosts with Proxy: true) + LoginCluster: "" + + # How long a cached token belonging to a remote cluster will + # remain valid before it needs to be revalidated. + RemoteTokenRefresh: 5m + Git: # Git repositories must be readable by api server, or you won't be # able to submit crunch jobs. To pass the test suites, put a clone diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go index 02043fb6d1..fdb3293a67 100644 --- a/sdk/go/arvados/config.go +++ b/sdk/go/arvados/config.go @@ -116,8 +116,10 @@ type Cluster struct { Repositories string } Login struct { - ProviderAppSecret string - ProviderAppID string + ProviderAppSecret string + ProviderAppID string + LoginCluster string + RemoteTokenRefresh Duration } Mail struct { MailchimpAPIKey string diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index 7645d1597c..606c3e06f8 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -92,14 +92,28 @@ class ApiClientAuthorization < ArvadosModel uuid_prefix+".arvadosapi.com") end + def self.make_http_client + clnt = HTTPClient.new + if Rails.configuration.TLS.Insecure + clnt.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_NONE + else + # Use system CA certificates + ["/etc/ssl/certs/ca-certificates.crt", + "/etc/pki/tls/certs/ca-bundle.crt"] + .select { |ca_path| File.readable?(ca_path) } + .each { |ca_path| clnt.ssl_config.add_trust_ca(ca_path) } + end + clnt + end + def self.validate(token:, remote: nil) return nil if !token remote ||= Rails.configuration.ClusterID case token[0..2] when 'v2/' - _, uuid, secret, optional = token.split('/') - unless uuid.andand.length == 27 && secret.andand.length.andand > 0 + _, token_uuid, secret, optional = token.split('/') + unless token_uuid.andand.length == 27 && secret.andand.length.andand > 0 return nil end @@ -108,11 +122,11 @@ class ApiClientAuthorization < ArvadosModel # matches expections. c = Container.where(uuid: optional).first if !c.nil? - if !c.auth_uuid.nil? and c.auth_uuid != uuid + if !c.auth_uuid.nil? and c.auth_uuid != token_uuid # token doesn't match the container's token return nil end - if !c.runtime_token.nil? and "v2/#{uuid}/#{secret}" != c.runtime_token + if !c.runtime_token.nil? and "v2/#{token_uuid}/#{secret}" != c.runtime_token # token doesn't match the container's token return nil end @@ -123,45 +137,45 @@ class ApiClientAuthorization < ArvadosModel end end + # fast path: look up the token in the local database auth = ApiClientAuthorization. includes(:user, :api_client). - where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', uuid). + where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token_uuid). first if auth && auth.user && (secret == auth.api_token || secret == OpenSSL::HMAC.hexdigest('sha1', auth.api_token, remote)) + # found it return auth end - uuid_prefix = uuid[0..4] - if uuid_prefix == Rails.configuration.ClusterID - # If the token were valid, we would have validated it above + token_uuid_prefix = token_uuid[0..4] + if token_uuid_prefix == Rails.configuration.ClusterID + # Token is supposedly issued by local cluster, but if the + # token were valid, we would have been found in the database + # in the above query. return nil - elsif uuid_prefix.length != 5 + elsif token_uuid_prefix.length != 5 # malformed return nil end - host = remote_host(uuid_prefix: uuid_prefix) + # Invarient: token_uuid_prefix != Rails.configuration.ClusterID + # + # In other words the remaing code in this method below is the + # case that determines whether to accept a token that was issued + # by a remote cluster when the token absent or expired in our + # database. To begin, we need to ask the cluster that issued + # the token to [re]validate it. + clnt = ApiClientAuthorization.make_http_client + + host = remote_host(uuid_prefix: token_uuid_prefix) if !host - Rails.logger.warn "remote authentication rejected: no host for #{uuid_prefix.inspect}" + Rails.logger.warn "remote authentication rejected: no host for #{token_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 - if Rails.configuration.TLS.Insecure - clnt.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_NONE - else - # Use system CA certificates - ["/etc/ssl/certs/ca-certificates.crt", - "/etc/pki/tls/certs/ca-bundle.crt"] - .select { |ca_path| File.readable?(ca_path) } - .each { |ca_path| clnt.ssl_config.add_trust_ca(ca_path) } - end remote_user = SafeJSON.load( clnt.get_content('https://' + host + '/arvados/v1/users/current', {'remote' => Rails.configuration.ClusterID}, @@ -170,63 +184,91 @@ class ApiClientAuthorization < ArvadosModel 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] + + # Check the response is well formed. + if !remote_user.is_a?(Hash) || !remote_user['uuid'].is_a?(String) 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| - # (this block runs for the "create" case, not for "find") - user.is_admin = false - user.email = remote_user['email'] - if remote_user['username'].andand.length.andand > 0 - user.set_initial_username(requested: remote_user['username']) - end - end + remote_user_prefix = remote_user['uuid'][0..4] + + # Clusters can only authenticate for their own users. + if remote_user_prefix != token_uuid_prefix + Rails.logger.warn "remote authentication rejected: claimed remote user #{remote_user_prefix} but token was issued by #{token_uuid_prefix}" + return nil + end + + # Invarient: remote_user_prefix == token_uuid_prefix + # therefore: remote_user_prefix != Rails.configuration.ClusterID + + # Add or update user and token in local database so we can + # validate subsequent requests faster. + + user = User.find_by_uuid(remote_user['uuid']) + + if !user + # Create a new record for this user. + user = User.new(uuid: remote_user['uuid'], + is_active: false, + is_admin: false, + email: remote_user['email'], + owner_uuid: system_user_uuid) + user.set_initial_username(requested: remote_user['username']) + end + + # Sync user record. + if remote_user_prefix == Rails.configuration.Login.LoginCluster + # Remote cluster controls our user database, copy both + # 'is_active' and 'is_admin' + user.is_active = remote_user['is_active'] + user.is_admin = remote_user['is_admin'] + else if Rails.configuration.Users.NewUsersAreActive || - Rails.configuration.RemoteClusters[remote_user['uuid'][0..4]].andand["ActivateUsers"] - # Update is_active to whatever it is at the remote end + Rails.configuration.RemoteClusters[remote_user_prefix].andand["ActivateUsers"] + # Default policy is to activate users, so match activate + # with the remote record. user.is_active = remote_user['is_active'] elsif !remote_user['is_active'] - # Remote user is inactive; our mirror should be, too. + # Deactivate user if the remote is inactive, otherwise don't + # change 'is_active'. user.is_active = false end + end - %w[first_name last_name email prefs].each do |attr| - user.send(attr+'=', remote_user[attr]) - end + %w[first_name last_name email prefs].each do |attr| + user.send(attr+'=', remote_user[attr]) + end + act_as_system_user do user.save! - auth = ApiClientAuthorization.find_or_create_by(uuid: uuid) do |auth| + # We will accept this token (and avoid reloading the user + # record) for 'RemoteTokenRefresh' (default 5 minutes). + # Possible todo: + # Request the actual api_client_auth record from the remote + # server in case it wants the token to expire sooner. + auth = ApiClientAuthorization.find_or_create_by(uuid: token_uuid) do |auth| auth.user = user - auth.api_token = secret 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!(user: user, api_token: secret, api_client_id: 0, - expires_at: Time.now + 5.minutes) + expires_at: Time.now + Rails.configuration.Login.RemoteTokenRefresh) end return auth else + # token is not a 'v2' token auth = ApiClientAuthorization. - includes(:user, :api_client). - where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token). - first + 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 diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb index 09e54b9d4f..5546e8e406 100644 --- a/services/api/config/arvados_config.rb +++ b/services/api/config/arvados_config.rb @@ -107,6 +107,8 @@ arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_not arvcfg.declare_config "Users.NewInactiveUserNotificationRecipients", Hash, :new_inactive_user_notification_recipients, method(:arrayToHash) arvcfg.declare_config "Login.ProviderAppSecret", NonemptyString, :sso_app_secret arvcfg.declare_config "Login.ProviderAppID", NonemptyString, :sso_app_id +arvcfg.declare_config "Login.LoginCluster", String +arvcfg.declare_config "Login.RemoteTokenRefresh", ActiveSupport::Duration arvcfg.declare_config "TLS.Insecure", Boolean, :sso_insecure arvcfg.declare_config "Services.SSO.ExternalURL", NonemptyString, :sso_provider_url arvcfg.declare_config "AuditLogs.MaxAge", ActiveSupport::Duration, :max_audit_log_age diff --git a/services/api/test/integration/remote_user_test.rb b/services/api/test/integration/remote_user_test.rb index 90a5586539..259d7f554d 100644 --- a/services/api/test/integration/remote_user_test.rb +++ b/services/api/test/integration/remote_user_test.rb @@ -33,39 +33,54 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest @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 + + @remote_server = [] + @remote_host = [] + + ['zbbbb', 'zbork'].each do |clusterid| + 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| + if clusterid == 'zbbbb' and req.header['authorization'][0][10..14] == 'zbork' + # asking zbbbb about zbork should yield an error, zbbbb doesn't trust zbork + res.status = 401 + return + end + res.status = @stub_status + res.body = @stub_content.is_a?(String) ? @stub_content : @stub_content.to_json + end + srv.mount_proc '/arvados/v1/users/register' 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]}" end - ready.pop - @remote_server = srv - @remote_host = "127.0.0.1:#{srv.config[:Port]}" - Rails.configuration.RemoteClusters = Rails.configuration.RemoteClusters.merge({zbbbb: ActiveSupport::InheritableOptions.new({Host: @remote_host}), - zbork: ActiveSupport::InheritableOptions.new({Host: @remote_host})}) - Arvados::V1::SchemaController.any_instance.stubs(:root_url).returns "https://#{@remote_host}" + Rails.configuration.RemoteClusters = Rails.configuration.RemoteClusters.merge({zbbbb: ActiveSupport::InheritableOptions.new({Host: @remote_host[0]}), + zbork: ActiveSupport::InheritableOptions.new({Host: @remote_host[1]})}) + Arvados::V1::SchemaController.any_instance.stubs(:root_url).returns "https://#{@remote_host[0]}" @stub_status = 200 @stub_content = { uuid: 'zbbbb-tpzed-000000000000000', @@ -77,7 +92,9 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest end teardown do - @remote_server.andand.stop + @remote_server.each do |srv| + srv.stop + end end test 'authenticate with remote token' do @@ -148,7 +165,7 @@ class RemoteUsersTest < ActionDispatch::IntegrationTest assert_equal 'foo', json_response['username'] end - test 'authenticate with remote token from misbhehaving remote cluster' do + test 'authenticate with remote token from misbehaving remote cluster' do get '/arvados/v1/users/current', params: {format: 'json'}, headers: auth(remote: 'zbork') -- 2.30.2