From 6d62b613cd6c74f2673a08c0eb6797b7d31375e8 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Wed, 5 Jul 2023 21:48:55 +0200 Subject: [PATCH] poc --- .../config_schemas/base/api_schema.rb | 3 + lib/cloud_controller/dependency_locator.rb | 16 ++ lib/cloud_controller/security_context.rb | 4 + lib/cloud_controller/uaa/uaa_client.rb | 92 ++++++++-- lib/cloud_controller/uaa/uaa_token_decoder.rb | 58 +++--- lib/services/sso/uaa/uaa_client_manager.rb | 1 + .../dependency_locator_spec.rb | 29 ++- spec/unit/lib/uaa/uaa_client_spec.rb | 172 +++++++++++++----- spec/unit/lib/uaa/uaa_token_decoder_spec.rb | 68 +++---- 9 files changed, 304 insertions(+), 139 deletions(-) diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index 35e751011ba..90a44012a50 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -194,6 +194,9 @@ class ApiSchema < VCAP::Config optional(:uaa_client_secret) => String, optional(:uaa_client_scope) => String, + optional(:cc_zone_lookup_client_name) => String, + optional(:cc_zone_lookup_client_secret) => String, + cloud_controller_username_lookup_client_name: String, cloud_controller_username_lookup_client_secret: String, diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb index c6843c96982..dbe026f0e7e 100644 --- a/lib/cloud_controller/dependency_locator.rb +++ b/lib/cloud_controller/dependency_locator.rb @@ -274,9 +274,23 @@ def router_group_type_populating_collection_renderer create_paginated_collection_renderer(collection_transformer: RouterGroupTypePopulator.new(routing_api_client)) end + def uaa_zone_lookup_client + unless config.get(:cc_zone_lookup_client_name).nil? + # TODO: [UAA ZONES] Cache client? + UaaClient.new( + uaa_target: config.get(:uaa, :internal_url), + client_id: config.get(:cc_zone_lookup_client_name), + secret: config.get(:cc_zone_lookup_client_secret), + ca_file: config.get(:uaa, :ca_file), + ) + end + end + def uaa_username_lookup_client + # TODO: [UAA ZONES] Cache client per zone id? Is subdomain change allowed/supported? UaaClient.new( uaa_target: config.get(:uaa, :internal_url), + zone: UaaZones.get_subdomain(uaa_zone_lookup_client, VCAP::CloudController::SecurityContext.zone_id), client_id: config.get(:cloud_controller_username_lookup_client_name), secret: config.get(:cloud_controller_username_lookup_client_secret), ca_file: config.get(:uaa, :ca_file), @@ -286,6 +300,7 @@ def uaa_username_lookup_client def routing_api_client return RoutingApi::DisabledClient.new if config.get(:routing_api).nil? + # TODO: [UAA ZONES] Is this use case relevant for zones? uaa_client = UaaClient.new( uaa_target: config.get(:uaa, :internal_url), client_id: config.get(:routing_api, :routing_client_name), @@ -299,6 +314,7 @@ def routing_api_client end def credhub_client + # TODO: [UAA ZONES] Is this use case relevant for zones? uaa_client = UaaClient.new( uaa_target: config.get(:uaa, :internal_url), client_id: config.get(:cc_service_key_client_name), diff --git a/lib/cloud_controller/security_context.rb b/lib/cloud_controller/security_context.rb index 494b3b81d9c..603bf62d9b9 100644 --- a/lib/cloud_controller/security_context.rb +++ b/lib/cloud_controller/security_context.rb @@ -81,5 +81,9 @@ def self.issuer '' end + + def self.zone_id + token['zid'] if valid_token? + end end end diff --git a/lib/cloud_controller/uaa/uaa_client.rb b/lib/cloud_controller/uaa/uaa_client.rb index a48b08394ac..e72e791accc 100644 --- a/lib/cloud_controller/uaa/uaa_client.rb +++ b/lib/cloud_controller/uaa/uaa_client.rb @@ -1,27 +1,60 @@ module VCAP::CloudController + class UaaHttpClient + include CF::UAA::Http + + def initialize(target, auth_header, options={}) + @target = target + @auth_header = auth_header + initialize_http_options(options) + end + + def get(path) + json_get(@target, path, nil, headers) + end + + private + + def headers + @auth_header.empty? ? {} : { 'authorization' => @auth_header } + end + end + class UaaClient - attr_reader :uaa_target, :client_id, :secret, :ca_file, :http_timeout + attr_reader :subdomain, :zone, :client_id, :secret, :ca_file, :http_timeout def self.default_http_timeout @default_http_timeout ||= VCAP::CloudController::Config.config.get(:uaa, :client_timeout) end def auth_header - token = UaaTokenCache.get_token(client_id) - return token if token + return '' if client_id.empty? - UaaTokenCache.set_token(client_id, token_info.auth_header, expires_in: token_info.info['expires_in']) + # TODO: [UAA ZONES] Cache token per client_id + subdomain. + # token = UaaTokenCache.get_token(client_id) + # return token if token + # + # UaaTokenCache.set_token(client_id, token_info.auth_header, expires_in: token_info.info['expires_in']) token_info.auth_header end - def initialize(uaa_target:, client_id:, secret:, ca_file:) + def initialize(uaa_target:, subdomain: '', zone: '', client_id: '', secret: '', ca_file:) @uaa_target = uaa_target + @subdomain = subdomain + @zone = zone @client_id = client_id @secret = secret @ca_file = ca_file @http_timeout = self.class.default_http_timeout end + def uaa_target + return @uaa_target if subdomain.empty? + + uri = Addressable::URI.parse(@uaa_target) + uri.host = "#{subdomain}.#{uri.host}" + uri.to_s + end + def get_clients(client_ids) client_ids.map do |id| get(:client, id) @@ -54,7 +87,8 @@ def usernames_for_ids(user_ids) def id_for_username(username, origin: nil) filter_string = %(username eq "#{username}") filter_string = %/origin eq "#{origin}" and #{filter_string}/ if origin.present? - results = query(:user_id, includeInactive: true, filter: filter_string) + # TODO: [UAA ZONES] Is the changed query semantically identical? + results = query(:user, filter: filter_string, sort_by: 'username', attributes: 'id') user = results['resources'].first user && user['id'] @@ -69,12 +103,8 @@ def ids_for_usernames_and_origins(usernames, origins, precise_username_match=tru origin_filter_string = origins&.map { |o| "origin eq \"#{o}\"" }&.join(' or ') filter_string = construct_filter_string(username_filter_string, origin_filter_string) - - if precise_username_match - results = query(:user_id, includeInactive: true, filter: filter_string) - else - results = query(:user, filter: filter_string, attributes: 'id') - end + # TODO: [UAA ZONES] Is the changed query semantically identical? + results = query(:user, filter: filter_string, sort_by: 'username', attributes: 'id') results['resources'].map { |r| r['id'] } rescue CF::UAA::UAAError => e @@ -92,7 +122,8 @@ def construct_filter_string(username_filter_string, origin_filter_string) def origins_for_username(username) filter_string = %(username eq "#{username}") - results = query(:user_id, includeInactive: true, filter: filter_string) + # TODO: [UAA ZONES] Is the changed query semantically identical? + results = query(:user, filter: filter_string, sort_by: 'username', attributes: 'id,origin') results['resources'].map { |resource| resource['origin'] } rescue UaaUnavailable, CF::UAA::UAAError => e @@ -104,6 +135,10 @@ def info CF::UAA::Info.new(uaa_target, uaa_connection_opts) end + def http_get(path) + http_client.get(path) + end + private def query(type, **opts) @@ -121,10 +156,6 @@ def with_cache_retry yield end - def scim - CF::UAA::Scim.new(uaa_target, auth_header, uaa_connection_opts) - end - def fetch_users(user_ids) return {} unless user_ids.present? @@ -132,7 +163,9 @@ def fetch_users(user_ids) user_ids.each_slice(200) do |batch| filter_string = batch.map { |user_id| %(id eq "#{user_id}") }.join(' or ') - results = query(:user_id, filter: filter_string, count: batch.length) + filter_string = %/active eq true and ( #{filter_string} )/ + # TODO: [UAA ZONES] Is the changed query semantically identical? + results = query(:user, filter: filter_string, count: batch.length, sort_by: 'username', attributes: 'id,username,origin') results['resources'].each do |user| results_hash[user['id']] = user end @@ -141,10 +174,22 @@ def fetch_users(user_ids) results_hash end + def scim + opts = uaa_connection_opts + opts.merge!({ zone: zone }) unless zone.empty? + CF::UAA::Scim.new(uaa_target, auth_header, opts) + end + def token_issuer + raise ArgumentError.new('TokenIssuer requires client_id') if client_id.empty? + CF::UAA::TokenIssuer.new(uaa_target, client_id, secret, uaa_connection_opts) end + def http_client + UaaHttpClient.new(uaa_target, auth_header, uaa_connection_opts) + end + def uaa_connection_opts { skip_ssl_validation: false, @@ -157,4 +202,15 @@ def logger @logger ||= Steno.logger('cc.uaa_client') end end + + class UaaZones + def self.get_subdomain(uaa_client, zone_id) + return '' if uaa_client.nil? || zone_id.nil? + + zone = uaa_client.http_get("/identity-zones/#{zone_id}") + zone['subdomain'] + rescue CF::UAA::NotFound + raise 'invalid zone id' + end + end end diff --git a/lib/cloud_controller/uaa/uaa_token_decoder.rb b/lib/cloud_controller/uaa/uaa_token_decoder.rb index 3f173d3a87f..4517b9f4410 100644 --- a/lib/cloud_controller/uaa/uaa_token_decoder.rb +++ b/lib/cloud_controller/uaa/uaa_token_decoder.rb @@ -64,12 +64,12 @@ def decode_token_with_asymmetric_key(auth_token) tries -= 1 # If we uncover issues due to attempting to decode with every # key, we can revisit: https://www.pivotaltracker.com/story/show/132270761 - asymmetric_key.value.each do |key| + asymmetric_key(decode_token_zone_id(auth_token)).value.each do |key| return decode_token_with_key(auth_token, pkey: key) rescue CF::UAA::InvalidSignature => e last_error = e end - asymmetric_key.refresh + # asymmetric_key.refresh - TODO: [UAA ZONES] Enable once keys are cached. end raise last_error end @@ -90,14 +90,21 @@ def decode_token_with_key(auth_token, options) raise BadToken.new('Incorrect token') unless access_token?(token) - if token['iss'] != uaa_issuer - @uaa_issuer = nil - raise BadToken.new('Incorrect issuer') if token['iss'] != uaa_issuer + if token['iss'] != uaa_issuer(token['zid']) + # TODO: [UAA ZONES] Clear cached issuer for this zone id. + raise BadToken.new('Incorrect issuer') # if token['iss'] != uaa_issuer(token['zid']) - TODO: [UAA ZONES] Enable once issuers are cached. end token end + def decode_token_zone_id(token) + segments = token.split('.') + raise CF::UAA::InvalidTokenFormat.new('Not enough or too many segments') if segments.length < 2 || segments.length > 3 + + CF::UAA::Util.json_decode64(segments[1], :sym)[:zid] + end + def symmetric_key config[:symmetric_secret] end @@ -106,37 +113,30 @@ def symmetric_key2 config[:symmetric_secret2] end - def asymmetric_key - @asymmetric_key ||= UaaVerificationKeys.new(uaa_username_lookup_client.info) - end - - def uaa_username_lookup_client - ::CloudController::DependencyLocator.instance.uaa_username_lookup_client + def asymmetric_key(zone_id) + # TODO: [UAA ZONES] Cache keys per zone id. + UaaVerificationKeys.new(uaa_client(zone_id).info) end - def uaa_issuer - @uaa_issuer ||= with_request_error_handling do - fetch_uaa_issuer + def uaa_issuer(zone_id) + # TODO: [UAA ZONES] Cache issuer per zone id. + with_request_error_handling do + fetch_uaa_issuer(zone_id) end end - def fetch_uaa_issuer - response = http_client.get('.well-known/openid-configuration') - raise "Could not retrieve issuer information from UAA: #{response.status}" unless response.status == 200 - - JSON.parse(response.body).fetch('issuer') + def fetch_uaa_issuer(zone_id) + uaa_client(zone_id).http_get('/.well-known/openid-configuration')['issuer'] + rescue CF::UAA::UAAError + raise 'Could not retrieve issuer information from UAA' end - def http_client - uaa_target = config[:internal_url] - uaa_ca = config[:ca_file] - client = HTTPClient.new(base_url: uaa_target) - client.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_PEER - if !uaa_ca.nil? && !uaa_ca.empty? - client.ssl_config.set_trust_ca(uaa_ca) - end - - client + def uaa_client(zone_id) + UaaClient.new( + uaa_target: config[:internal_url], + subdomain: UaaZones.get_subdomain(CloudController::DependencyLocator.instance.uaa_zone_lookup_client, zone_id), + ca_file: config[:ca_file], + ) end def with_request_error_handling(&blk) diff --git a/lib/services/sso/uaa/uaa_client_manager.rb b/lib/services/sso/uaa/uaa_client_manager.rb index 635eda9d9b7..933bae7dbdf 100644 --- a/lib/services/sso/uaa/uaa_client_manager.rb +++ b/lib/services/sso/uaa/uaa_client_manager.rb @@ -104,6 +104,7 @@ def filter_uaa_client_scope end def create_uaa_client + # TODO: [UAA ZONES] Is this use case relevant for zones? VCAP::CloudController::UaaClient.new( uaa_target: uaa_target, client_id: VCAP::CloudController::Config.config.get(:uaa_client_name), diff --git a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb index e36f339a238..0ffea65f479 100644 --- a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb +++ b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb @@ -294,25 +294,36 @@ end describe '#uaa_username_lookup_client' do - context 'when a CA file is not configured for the UAA' do - before do - TestConfig.override(uaa: { ca_file: nil, internal_url: TestConfig.config_instance.get(:uaa, :internal_url) }) - end - + context 'when a CA file is configured for the UAA' do it 'returns a uaa client with credentials for looking up usernames' do uaa_username_lookup_client = locator.uaa_username_lookup_client expect(uaa_username_lookup_client.client_id).to eq(config.get(:cloud_controller_username_lookup_client_name)) expect(uaa_username_lookup_client.secret).to eq(config.get(:cloud_controller_username_lookup_client_secret)) expect(uaa_username_lookup_client.uaa_target).to eq(config.get(:uaa, :internal_url)) + expect(uaa_username_lookup_client.ca_file).to eq(config.get(:uaa, :ca_file)) + expect(uaa_username_lookup_client.zone).to eq('') end end - context 'when a CA file is configured for the UAA' do + context 'when a CA file is not configured for the UAA' do + before do + TestConfig.override(uaa: { ca_file: nil, internal_url: config.get(:uaa, :internal_url) }) + end + it 'returns a uaa client with credentials for looking up usernames' do uaa_username_lookup_client = locator.uaa_username_lookup_client - expect(uaa_username_lookup_client.client_id).to eq(config.get(:cloud_controller_username_lookup_client_name)) - expect(uaa_username_lookup_client.secret).to eq(config.get(:cloud_controller_username_lookup_client_secret)) - expect(uaa_username_lookup_client.uaa_target).to eq(config.get(:uaa, :internal_url)) + expect(uaa_username_lookup_client.ca_file).to be_nil + end + end + + context 'when a UAA zone is used' do + before do + allow(VCAP::CloudController::UaaZones).to receive(:get_subdomain).and_return('zone') + end + + it 'adapts the UAA URL accordingly' do + uaa_username_lookup_client = locator.uaa_username_lookup_client + expect(uaa_username_lookup_client.zone).to eq('zone') end end end diff --git a/spec/unit/lib/uaa/uaa_client_spec.rb b/spec/unit/lib/uaa/uaa_client_spec.rb index b2411bb92da..2c4d3f26966 100644 --- a/spec/unit/lib/uaa/uaa_client_spec.rb +++ b/spec/unit/lib/uaa/uaa_client_spec.rb @@ -51,6 +51,8 @@ module VCAP::CloudController end it 'fetches a new token after the cached token expires' do + skip # TODO: [UAA ZONES] Remove once UaaTokenCache is fixed. + expect(uaa_client.auth_header).to eq 'bearer STUFF' Timecop.travel(2399.seconds.from_now) allow(token_info).to receive(:auth_header).and_return('bearer OTHERTOKEN') @@ -144,14 +146,16 @@ module VCAP::CloudController UaaTokenCache.set_token(client_id, 'bearer invalid') WebMock::API.stub_request(:get, "#{url}/oauth/clients/client_id"). - with(headers: { 'Authorization' => 'bearer STUFF' }). + with( + headers: { 'Authorization' => 'bearer STUFF' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, body: { 'client_id' => client_id, name: 'My Client Name' }.to_json) WebMock::API.stub_request(:get, "#{url}/oauth/clients/client_id"). - with(headers: { 'Authorization' => 'bearer invalid' }). + with( + headers: { 'Authorization' => 'bearer invalid' }). to_return( status: 403, headers: { 'content-type' => 'application/json' }, @@ -179,8 +183,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 2 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -240,15 +247,23 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 2 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }, headers: { 'Authorization' => 'bearer STUFF' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }, + headers: { 'Authorization' => 'bearer STUFF' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, body: response_body.to_json) - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }, headers: { 'Authorization' => 'bearer invalid' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }, + headers: { 'Authorization' => 'bearer invalid' }). to_return( status: 403, headers: { 'content-type' => 'application/json' }, @@ -288,8 +303,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 2 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -341,23 +359,25 @@ module VCAP::CloudController end before do - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { - 'filter' => user_ids.slice(0, 200).map { |user_id| %(id eq "#{user_id}") }.join(' or '), - 'count' => 200 - }).to_return( - status: 200, - headers: { 'content-type' => 'application/json' }, - body: response_body1.to_json) + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( ' + user_ids.slice(0, 200).map { |user_id| %(id eq "#{user_id}") }.join(' or ') + ' )', + 'count' => 200, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }). + to_return( + status: 200, + headers: { 'content-type' => 'application/json' }, + body: response_body1.to_json) - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { - 'filter' => user_ids.slice(200, 100).map { |user_id| %(id eq "#{user_id}") }.join(' or '), - 'count' => 100 - }).to_return( - status: 200, - headers: { 'content-type' => 'application/json' }, - body: response_body2.to_json) + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( ' + user_ids.slice(200, 100).map { |user_id| %(id eq "#{user_id}") }.join(' or ') + ' )', + 'count' => 100, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }). + to_return( + status: 200, + headers: { 'content-type' => 'application/json' }, + body: response_body2.to_json) end it 'returns the list of users after making batch requests' do @@ -401,15 +421,23 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 2 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }, headers: { 'Authorization' => 'bearer STUFF' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }, + headers: { 'Authorization' => 'bearer STUFF' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, body: response_body.to_json) - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'filter' => 'id eq "111" or id eq "222"', 'count' => 2 }, headers: { 'Authorization' => 'bearer invalid' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'active eq true and ( id eq "111" or id eq "222" )', + 'count' => 2, 'sort_by' => 'username', 'attributes' => 'id,userName,origin' }, + headers: { 'Authorization' => 'bearer invalid' }). to_return( status: 403, headers: { 'content-type' => 'application/json' }, @@ -447,8 +475,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 1 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'origin eq "ldap" and username eq "user@example.com"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'origin eq "ldap" and username eq "user@example.com"', + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -468,8 +499,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 1 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'username eq "user@example.com"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'username eq "user@example.com"', + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -487,8 +521,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 0 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'username eq "user@example.com"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'username eq "user@example.com"', + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -542,8 +579,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 1 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). # 'id eq "111" or id eq "222"' - with(query: { 'includeInactive' => true, 'filter' => "username eq \"#{username1}\" or username eq \"#{username2}\"" }). + WebMock::API.stub_request(:get, "#{url}/Users"). # 'id eq "111" or id eq "222"' + with( + query: { + 'filter' => "username eq \"#{username1}\" or username eq \"#{username2}\"", + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -567,8 +607,10 @@ module VCAP::CloudController 'totalresults' => 1 } WebMock::API.stub_request(:get, "#{url}/Users"). - with(query: { 'filter' => "username co \"#{partial_username}\"", - 'attributes' => 'id' }). + with( + query: { + 'filter' => "username co \"#{partial_username}\"", + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -590,8 +632,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 1 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). # 'id eq "111" or id eq "222"' - with(query: { 'includeInactive' => true, 'filter' => "( username eq \"#{username1}\" or username eq \"#{username2}\" ) and ( origin eq \"Okta\" )" }). + WebMock::API.stub_request(:get, "#{url}/Users"). # 'id eq "111" or id eq "222"' + with( + query: { + 'filter' => "( username eq \"#{username1}\" or username eq \"#{username2}\" ) and ( origin eq \"Okta\" )", + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -614,8 +659,10 @@ module VCAP::CloudController 'totalresults' => 1 } WebMock::API.stub_request(:get, "#{url}/Users"). - with(query: { 'filter' => "( username co \"#{partial_username}\" ) and ( origin eq \"Okta\" )", - 'attributes' => 'id' }). + with( + query: { + 'filter' => "( username co \"#{partial_username}\" ) and ( origin eq \"Okta\" )", + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -633,8 +680,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 0 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'username eq "non-existent-user"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'username eq "non-existent-user"', + 'sort_by' => 'username', 'attributes' => 'id' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -718,8 +768,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 2 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'username eq "user_1"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'username eq "user_1"', + 'sort_by' => 'username', 'attributes' => 'id,origin' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -738,8 +791,11 @@ module VCAP::CloudController 'itemsperpage' => 100, 'totalresults' => 0 } - WebMock::API.stub_request(:get, "#{url}/ids/Users"). - with(query: { 'includeInactive' => true, 'filter' => 'username eq "user_1"' }). + WebMock::API.stub_request(:get, "#{url}/Users"). + with( + query: { + 'filter' => 'username eq "user_1"', + 'sort_by' => 'username', 'attributes' => 'id,origin' }). to_return( status: 200, headers: { 'content-type' => 'application/json' }, @@ -780,5 +836,21 @@ module VCAP::CloudController end end end + + describe '#http_get' do + it 'returns the parsed data' do + response_body = { 'field' => 'value' } + + WebMock::API.stub_request(:get, "#{url}/path"). + with( + headers: { 'Authorization' => 'bearer STUFF' }). + to_return( + status: 200, + headers: { 'content-type' => 'application/json' }, + body: response_body.to_json) + + expect(uaa_client.http_get('/path')['field']).to eq('value') + end + end end end diff --git a/spec/unit/lib/uaa/uaa_token_decoder_spec.rb b/spec/unit/lib/uaa/uaa_token_decoder_spec.rb index f2a30561cad..6151f2b688b 100644 --- a/spec/unit/lib/uaa/uaa_token_decoder_spec.rb +++ b/spec/unit/lib/uaa/uaa_token_decoder_spec.rb @@ -16,12 +16,10 @@ module VCAP::CloudController end let(:uaa_info) { double(CF::UAA::Info) } - let(:uaa_client) { instance_double(VCAP::CloudController::UaaClient) } let(:logger) { double(Steno::Logger) } before do - allow(::CloudController::DependencyLocator.instance).to receive(:uaa_username_lookup_client).and_return(uaa_client) - allow(uaa_client).to receive(:info).and_return(uaa_info) + allow_any_instance_of(VCAP::CloudController::UaaClient).to receive(:info).and_return(uaa_info) allow(Steno).to receive(:logger).with('cc.uaa_token_decoder').and_return(logger) # undo global stubbing in spec_helper.rb allow_any_instance_of(VCAP::CloudController::UaaTokenDecoder).to receive(:uaa_issuer).and_call_original @@ -53,7 +51,7 @@ module VCAP::CloudController describe '#decode_token' do before do Timecop.freeze(Time.now.utc) - stub_request(:get, uaa_issuer_info_url).to_return(body: { 'issuer' => uaa_issuer_string }.to_json) + stub_request(:get, uaa_issuer_info_url).to_return(headers: { 'content-type' => 'application/json' }, body: { 'issuer' => uaa_issuer_string }.to_json) end after { Timecop.return } @@ -87,13 +85,14 @@ module VCAP::CloudController expect(subject.decode_token("bearer #{token}")).to eq(token_content) end - it 'caches the issuer info from UAA' do - token = CF::UAA::TokenCoder.encode(token_content, { skey: 'symmetric-key' }) - subject.decode_token("bearer #{token}") - subject.decode_token("bearer #{token}") - - expect(WebMock).to have_requested(:get, uaa_issuer_info_url).once - end + # TODO: [UAA ZONES] Enable once issuers are cached. + # it 'caches the issuer info from UAA' do + # token = CF::UAA::TokenCoder.encode(token_content, { skey: 'symmetric-key' }) + # subject.decode_token("bearer #{token}") + # subject.decode_token("bearer #{token}") + # + # expect(WebMock).to have_requested(:get, uaa_issuer_info_url).once + # end end context 'when the second key decodes the token' do @@ -103,13 +102,14 @@ module VCAP::CloudController expect(subject.decode_token("bearer #{token}")).to eq(token_content) end - it 'caches the issuer info from UAA' do - token = CF::UAA::TokenCoder.encode(token_content, { skey: 'symmetric-key2' }) - subject.decode_token("bearer #{token}") - subject.decode_token("bearer #{token}") - - expect(WebMock).to have_requested(:get, uaa_issuer_info_url).once - end + # TODO: [UAA ZONES] Enable once issuers are cached. + # it 'caches the issuer info from UAA' do + # token = CF::UAA::TokenCoder.encode(token_content, { skey: 'symmetric-key2' }) + # subject.decode_token("bearer #{token}") + # subject.decode_token("bearer #{token}") + # + # expect(WebMock).to have_requested(:get, uaa_issuer_info_url).once + # end end context 'when no CA certificate is configured to use with the interal UAA URL' do @@ -157,7 +157,7 @@ module VCAP::CloudController stub_request(:get, uaa_issuer_info_url). to_return(status: 404).then. to_return(status: 404).then. - to_return(body: { 'issuer' => uaa_issuer_string }.to_json) + to_return(headers: { 'content-type' => 'application/json' }, body: { 'issuer' => uaa_issuer_string }.to_json) end it 'eventually decodes the token' do @@ -222,8 +222,9 @@ module VCAP::CloudController expect(uaa_info).to receive(:validation_keys_hash) expect(subject.decode_token("bearer #{token}")).to eq(token_content) - expect(uaa_info).not_to receive(:validation_keys_hash) - expect(subject.decode_token("bearer #{token}")).to eq(token_content) + # TODO: [UAA ZONES] Enable once UaaVerificationKeys are cached. + # expect(uaa_info).not_to receive(:validation_keys_hash) + # expect(subject.decode_token("bearer #{token}")).to eq(token_content) end describe 're-fetching key' do @@ -260,17 +261,18 @@ module VCAP::CloudController end end - context 'when the token issuer is out of date' do - let(:token_issuer_string) { 'https://totally.different.issuer/uaa' } - let(:invalid_issuer) { 'oops' } - - it 'calls uaa_issuer twice' do - token = generate_token(rsa_key, token_content) - allow_any_instance_of(UaaTokenDecoder).to receive(:fetch_uaa_issuer).and_return(invalid_issuer, token_issuer_string) - - expect(subject.decode_token("bearer #{token}")).to eq(token_content) - end - end + # TODO: [UAA ZONES] Enable once issuers are cached. + # context 'when the token issuer is out of date' do + # let(:token_issuer_string) { 'https://totally.different.issuer/uaa' } + # let(:invalid_issuer) { 'oops' } + # + # it 'calls uaa_issuer twice' do + # token = generate_token(rsa_key, token_content) + # allow_any_instance_of(UaaTokenDecoder).to receive(:fetch_uaa_issuer).and_return(invalid_issuer, token_issuer_string) + # + # expect(subject.decode_token("bearer #{token}")).to eq(token_content) + # end + # end context 'when UAA responds with a non-200 while fetching the issuer' do let(:token_issuer_string) { uaa_issuer_string } @@ -280,7 +282,7 @@ module VCAP::CloudController stub_request(:get, uaa_issuer_info_url). to_return(status: 404).then. to_return(status: 404).then. - to_return(body: { 'issuer' => uaa_issuer_string }.to_json) + to_return(headers: { 'content-type' => 'application/json' }, body: { 'issuer' => uaa_issuer_string }.to_json) end it 'eventually decodes the token' do