From cbfc2739835c0b7dab5484479933c3113f5a329f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 30 Jan 2024 12:22:27 -0500 Subject: [PATCH 1/3] Browser support: Check iOS Safari engine for all iOS browsers changelog: Internal, Browser Support, Improve browser detection for iOS browsers --- app/services/browser_support.rb | 29 +++++++++++------ spec/services/browser_support_spec.rb | 45 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/app/services/browser_support.rb b/app/services/browser_support.rb index ee74ad3f604..d4dcf5fc6c6 100644 --- a/app/services/browser_support.rb +++ b/app/services/browser_support.rb @@ -1,22 +1,20 @@ class BrowserSupport @cache = LruRedux::Cache.new(1_000) - # rubocop:disable Layout/LineLength BROWSERSLIST_TO_BROWSER_MAP = { - and_chr: ->(browser, version) { browser.android? && !browser.platform.android_webview? && browser.chrome?(version) }, + and_chr: ->(browser, version) { browser.android? && browser.chrome?(version) }, and_uc: ->(browser, version) { browser.android? && browser.uc_browser?(version) }, - android: ->(browser, version) { browser.platform.android_webview? && browser.send(:detect_version?, browser.version, version) }, - chrome: ->(browser, version) { !browser.platform.android_webview? && browser.chrome?(version) }, + android: ->(browser, version) { browser.send(:detect_version?, browser.version, version) }, + chrome: ->(browser, version) { browser.chrome?(version) }, edge: ->(browser, version) { browser.edge?(version) }, firefox: ->(browser, version) { browser.firefox?(version) }, - ios_saf: ->(browser, version) { browser.ios? && browser.safari?(version) }, + ios_saf: ->(browser, version) { browser.ios?(version) }, op_mini: ->(browser, version) { browser.opera_mini?(version) }, op_mob: ->(browser, version) { browser.platform.android? && browser.opera?(version) }, opera: ->(browser, version) { browser.opera?(version) }, safari: ->(browser, version) { browser.safari?(version) }, samsung: ->(browser, version) { browser.samsung_browser?(version) }, - }.with_indifferent_access.freeze - # rubocop:enable Layout/LineLength + }.freeze class << self def supported?(user_agent) @@ -24,7 +22,8 @@ def supported?(user_agent) return true if browser_support_config.nil? cache.getset(user_agent) do - matchers.any? { |matcher| matcher.call(BrowserCache.parse(user_agent)) } + browser = BrowserCache.parse(user_agent) + matchers(browserslist_to_browser_map(browser)).any? { |matcher| matcher.call(browser) } end end @@ -38,10 +37,20 @@ def clear_cache! attr_reader :cache - def matchers + def browserslist_to_browser_map(browser) + if browser.ios? + BROWSERSLIST_TO_BROWSER_MAP.slice(:ios_saf) + elsif browser.platform.android_webview? + BROWSERSLIST_TO_BROWSER_MAP.slice(:android) + else + BROWSERSLIST_TO_BROWSER_MAP + end + end + + def matchers(mapping) @matchers ||= browser_support_config.flat_map do |config_entry| key, version = config_entry.split(' ', 2) - browser_matcher = BROWSERSLIST_TO_BROWSER_MAP[key] + browser_matcher = mapping[key.to_sym] next [] if !browser_matcher low_version, _high_version = version.split('-', 2) diff --git a/spec/services/browser_support_spec.rb b/spec/services/browser_support_spec.rb index e53b1d43376..5fb5f0bf541 100644 --- a/spec/services/browser_support_spec.rb +++ b/spec/services/browser_support_spec.rb @@ -147,6 +147,51 @@ end end + context 'with user agent for chrome on ios' do + # All browsers on iOS use the Safari browser engine, so we expect the specific browser + # version to be disregarded in favor of the iOS version. + + context 'with both chrome version and ios version below supported range' do + let(:user_agent) do + # Chrome 100 on iOS 14.0 + 'Mozilla/5.0 (iPhone; CPU iPhone OS 14_0 like Mac OS X) AppleWebKit/605.1.15 ' \ + '(KHTML, like Gecko) CriOS/100.0.0000.000 Mobile/15E148 Safari/604.1' + end + + it { expect(supported).to eq(false) } + end + + context 'with chrome version above supported range, ios version below supported range' do + let(:user_agent) do + # Chrome 120 on iOS 14.0 + 'Mozilla/5.0 (iPhone; CPU iPhone OS 14_0 like Mac OS X) AppleWebKit/605.1.15 ' \ + '(KHTML, like Gecko) CriOS/120.0.0000.000 Mobile/15E148 Safari/604.1' + end + + it { expect(supported).to eq(false) } + end + + context 'with chrome version below supported range, ios version above supported range' do + let(:user_agent) do + # Chrome 100 on iOS 15.0 + 'Mozilla/5.0 (iPhone; CPU iPhone OS 15_0 like Mac OS X) AppleWebKit/605.1.15 ' \ + '(KHTML, like Gecko) CriOS/120.0.0000.000 Mobile/15E148 Safari/604.1' + end + + it { expect(supported).to eq(true) } + end + + context 'with both chrome version and ios version above supported range' do + let(:user_agent) do + # Chrome 120 on iOS 17.3 + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_3 like Mac OS X) AppleWebKit/605.1.15 ' \ + '(KHTML, like Gecko) CriOS/120.0.0000.000 Mobile/15E148 Safari/604.1' + end + + it { expect(supported).to eq(true) } + end + end + context 'with opera desktop user agent' do let(:user_agent) do # Opera 83 From 7511389264d561bb8a20182f7b1293acc53f7636 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 30 Jan 2024 15:03:23 -0500 Subject: [PATCH 2/3] Filter browser matchers after memoization See: https://github.com/18F/identity-idp/pull/10002/files#r1471722417 --- app/services/browser_support.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/services/browser_support.rb b/app/services/browser_support.rb index d4dcf5fc6c6..65983341e23 100644 --- a/app/services/browser_support.rb +++ b/app/services/browser_support.rb @@ -23,7 +23,7 @@ def supported?(user_agent) cache.getset(user_agent) do browser = BrowserCache.parse(user_agent) - matchers(browserslist_to_browser_map(browser)).any? { |matcher| matcher.call(browser) } + matchers_for_browser(browser).any? { |_key, matcher| matcher.call(browser) } end end @@ -37,26 +37,29 @@ def clear_cache! attr_reader :cache - def browserslist_to_browser_map(browser) + def matchers_for_browser(browser) if browser.ios? - BROWSERSLIST_TO_BROWSER_MAP.slice(:ios_saf) + matchers.slice(:ios_saf) elsif browser.platform.android_webview? - BROWSERSLIST_TO_BROWSER_MAP.slice(:android) + matchers.slice(:android) else - BROWSERSLIST_TO_BROWSER_MAP + matchers end end - def matchers(mapping) + def matchers @matchers ||= browser_support_config.flat_map do |config_entry| key, version = config_entry.split(' ', 2) - browser_matcher = mapping[key.to_sym] + key = key.to_sym + browser_matcher = BROWSERSLIST_TO_BROWSER_MAP[key] next [] if !browser_matcher low_version, _high_version = version.split('-', 2) low_version = nil if !numeric?(low_version) - proc { |browser| browser_matcher.call(browser, low_version && ">= #{low_version}") } - end + version_test = low_version && ">= #{low_version}" + matcher = proc { |browser| browser_matcher.call(browser, version_test) } + [[key, matcher]] + end.to_h end def numeric?(value) From 1ed647bbc35845ee18519c997e854aa2fc3cf013 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 30 Jan 2024 15:17:42 -0500 Subject: [PATCH 3/3] Build hash with each_with_object --- app/services/browser_support.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/services/browser_support.rb b/app/services/browser_support.rb index 65983341e23..83ea8482bfe 100644 --- a/app/services/browser_support.rb +++ b/app/services/browser_support.rb @@ -48,18 +48,18 @@ def matchers_for_browser(browser) end def matchers - @matchers ||= browser_support_config.flat_map do |config_entry| + @matchers ||= browser_support_config.each_with_object({}) do |config_entry, result| key, version = config_entry.split(' ', 2) key = key.to_sym browser_matcher = BROWSERSLIST_TO_BROWSER_MAP[key] - next [] if !browser_matcher + next if !browser_matcher low_version, _high_version = version.split('-', 2) low_version = nil if !numeric?(low_version) version_test = low_version && ">= #{low_version}" matcher = proc { |browser| browser_matcher.call(browser, version_test) } - [[key, matcher]] - end.to_h + result[key] = matcher + end end def numeric?(value)