From 14623ed08820ec1760b550b9e791aac2af5df81b Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Tue, 28 Oct 2014 11:10:25 +0100 Subject: [PATCH 1/2] Revert "Revert "track and enforce user-agent"" This reverts commit 2227d0042f166659466b38a1f2db975b735f3638. --- Gemfile | 1 + Gemfile.lock | 6 +- lib/travis/api/app.rb | 1 + lib/travis/api/app/cors.rb | 2 +- .../api/app/middleware/user_agent_tracker.rb | 85 ++++++++++++ spec/unit/cors_spec.rb | 2 +- .../middleware/user_agent_tracker_spec.rb | 123 ++++++++++++++++++ travis-api.gemspec | 40 ++++-- 8 files changed, 242 insertions(+), 18 deletions(-) create mode 100644 lib/travis/api/app/middleware/user_agent_tracker.rb create mode 100644 spec/unit/middleware/user_agent_tracker_spec.rb diff --git a/Gemfile b/Gemfile index 0e4250f8..b53a1b08 100644 --- a/Gemfile +++ b/Gemfile @@ -25,6 +25,7 @@ gem 'pry' gem 'metriks', '0.9.9.6' gem 'metriks-librato_metrics', github: 'eric/metriks-librato_metrics' gem 'micro_migrations' +gem 'useragent' group :test do gem 'rspec', '~> 2.13' diff --git a/Gemfile.lock b/Gemfile.lock index 464628f7..ffb01fae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -36,7 +36,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: 0f487381ecf7f06f759116b60a1e9e8544784286 + revision: 0f7a43a373cd3a7236ed4b5d3ca4a312a4e4c656 specs: travis-core (0.0.1) actionmailer (~> 3.2.19) @@ -74,7 +74,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-yaml.git - revision: 121678eba7280b8269a73c56953cafd20e884bdc + revision: e899680992e31b25ddc0aad33d2189a7e588cc1f specs: travis-yaml (0.1.0) @@ -321,6 +321,7 @@ GEM kgio (~> 2.6) rack raindrops (~> 0.7) + useragent (0.10.0) uuidtools (2.1.5) virtus (1.0.3) axiom-types (~> 0.1) @@ -362,4 +363,5 @@ DEPENDENCIES travis-support! travis-yaml! unicorn + useragent yard-sinatra! diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index 46b2ad93..1934ede2 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -114,6 +114,7 @@ module Travis::Api use Travis::Api::App::Middleware::Logging use Travis::Api::App::Middleware::Metriks use Travis::Api::App::Middleware::Rewrite + use Travis::Api::App::Middleware::UserAgentTracker SettingsEndpoint.subclass :env_vars if Travis.config.endpoints.ssh_key diff --git a/lib/travis/api/app/cors.rb b/lib/travis/api/app/cors.rb index 781efc00..9112300b 100644 --- a/lib/travis/api/app/cors.rb +++ b/lib/travis/api/app/cors.rb @@ -14,7 +14,7 @@ class Travis::Api::App options // do headers['Access-Control-Allow-Methods'] = "HEAD, GET, POST, PATCH, PUT, DELETE" - headers['Access-Control-Allow-Headers'] = "Content-Type, Authorization, Accept, If-None-Match, If-Modified-Since" + headers['Access-Control-Allow-Headers'] = "Content-Type, Authorization, Accept, If-None-Match, If-Modified-Since, X-User-Agent" end end end diff --git a/lib/travis/api/app/middleware/user_agent_tracker.rb b/lib/travis/api/app/middleware/user_agent_tracker.rb new file mode 100644 index 00000000..cba98cf3 --- /dev/null +++ b/lib/travis/api/app/middleware/user_agent_tracker.rb @@ -0,0 +1,85 @@ +require 'travis/api/app' +require 'useragent' + +class Travis::Api::App + class Middleware + class UserAgentTracker < Middleware + WEB_BROWSERS = [ + "Internet Explorer", + "Webkit", "Chrome", "Safari", "Android", + "Firefox", "Camino", "Iceweasel", "Seamonkey", "Android", + "Opera", "Mozilla" + ] + + before(agent: /^$/) do + ::Metriks.meter("api.user_agent.missing").mark + halt(400, "error" => "missing User-Agent header") if Travis::Features.feature_active?(:require_user_agent) + end + + before(agent: /^.+$/) do + agent = UserAgent.parse(request.user_agent) + case agent.browser + when *WEB_BROWSERS then mark_browser + when "curl", "Wget" then mark(:console, agent.browser) + when "travis-api-wrapper" then mark(:script, :node_js, agent.browser) + when "TravisPy" then mark(:script, :python, agent.browser) + when "Ruby", "PHP", "Perl", "Python" then mark(:script, agent.browser, :vanilla) + when "Faraday" then mark(:script, :ruby, :vanilla) + when "Travis" then mark_travis(agent) + else mark_unknown + end + end + + def mark_browser + # allows a JavaScript Client to set X-User-Agent, for instance to "travis-web" in travis-web + x_agent = UserAgent.parse(env['HTTP_X_USER_AGENT'] || 'unknown').browser + mark(:browser, x_agent) + end + + def mark_travis(agent) + os, *rest = agent.application.comment + ruby, rubygems, command = "unknown", "unknown", nil + + rest.each do |comment| + case comment + when /^Ruby (\d\.\d.\d)/ then ruby = $1 + when /^RubyGems (.+)$/ then rubygems = $1 + when /^command (.+)$/ then command = $1 + end + end + + # "Ubuntu 12.04 like Linux" => "linux.ubuntu.12.04" + if os =~ /^(.+) (\S+) like (\S+)$/ + os = "#{$3}.#{$1}.#{$2[/\d+\.\d+/]}" + end + + if command + mark(:cli, version: agent.version, ruby: ruby, rubygems: rubygems, command: command, os: os) + else + # only track ruby version and library version for non-cli usage + mark(:script, :ruby, :travis, version: agent.version, ruby: ruby) + end + end + + def mark_unknown + logger.warn "[user-agent-tracker] Unknown User-Agent: %p" % request.user_agent + mark(:unknown) + end + + def track_key(string) + string.to_s.downcase.gsub(/[^a-z0-9\-\.]+/, '_') + end + + def mark(*keys) + key = "api.user_agent" + keys.each do |subkey| + if subkey.is_a? Hash + subkey.each_pair { |k, v| ::Metriks.meter("#{key}.#{track_key(k)}.#{track_key(v)}").mark } + else + ::Metriks.meter(key << "." << track_key(subkey)).mark + end + end + end + end + end +end diff --git a/spec/unit/cors_spec.rb b/spec/unit/cors_spec.rb index 83e79914..cd90dd6d 100644 --- a/spec/unit/cors_spec.rb +++ b/spec/unit/cors_spec.rb @@ -44,7 +44,7 @@ describe Travis::Api::App::Cors do end it 'sets Access-Control-Allow-Headers' do - headers['Access-Control-Allow-Headers'].should == "Content-Type, Authorization, Accept, If-None-Match, If-Modified-Since" + headers['Access-Control-Allow-Headers'].should == "Content-Type, Authorization, Accept, If-None-Match, If-Modified-Since, X-User-Agent" end end end diff --git a/spec/unit/middleware/user_agent_tracker_spec.rb b/spec/unit/middleware/user_agent_tracker_spec.rb new file mode 100644 index 00000000..7fbe0c29 --- /dev/null +++ b/spec/unit/middleware/user_agent_tracker_spec.rb @@ -0,0 +1,123 @@ +require 'spec_helper' + +describe Travis::Api::App::Middleware::UserAgentTracker do + before do + mock_app do + use Travis::Api::App::Middleware::UserAgentTracker + get('/') { 'ok' } + end + end + + def expect_meter(name) + Metriks.expects(:meter).with(name).returns(stub("meter", mark: nil)) + end + + def get(env = {}) + env['HTTP_USER_AGENT'] ||= agent if agent + super('/', {}, env) + end + + context 'missing User-Agent' do + let(:agent) { } + + it "tracks it" do + expect_meter("api.user_agent.missing") + get.should be_ok + end + + it "denies request if require_user_agent feature is enabled" do + Travis::Features.expects(:feature_active?).with(:require_user_agent).returns(true) + get.status.should be == 400 + end + end + + context 'web browser' do + let(:agent) { "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.36 Safari/537.36" } + + specify 'without X-User-Agent' do + expect_meter("api.user_agent.browser") + expect_meter("api.user_agent.browser.unknown") + get + end + + specify 'with X-User-Agent' do + expect_meter("api.user_agent.browser") + expect_meter("api.user_agent.browser.travis-web") + get('HTTP_X_USER_AGENT' => 'travis-web') + end + end + + context 'console' do + let(:agent) { 'curl' } + specify do + expect_meter("api.user_agent.console") + expect_meter("api.user_agent.console.curl") + get + end + end + + context 'travis-api-wrapper' do + let(:agent) { 'travis-api-wrapper - v0.01 - (cmaujean@gmail.com)' } + specify do + expect_meter("api.user_agent.script") + expect_meter("api.user_agent.script.node_js") + expect_meter("api.user_agent.script.node_js.travis-api-wrapper") + get + end + end + + context 'TravisPy' do + let(:agent) { 'TravisPy' } + specify do + expect_meter("api.user_agent.script") + expect_meter("api.user_agent.script.python") + expect_meter("api.user_agent.script.python.travispy") + get + end + end + + context 'Ruby' do + let(:agent) { 'Ruby' } + specify do + expect_meter("api.user_agent.script") + expect_meter("api.user_agent.script.ruby") + expect_meter("api.user_agent.script.ruby.vanilla") + get + end + end + + context 'Faraday' do + let(:agent) { 'Faraday' } + specify do + expect_meter("api.user_agent.script") + expect_meter("api.user_agent.script.ruby") + expect_meter("api.user_agent.script.ruby.vanilla") + get + end + end + + context 'travis.rb' do + let(:agent) { 'Travis/1.6.8 (Mac OS X 10.9.2 like Darwin; Ruby 2.1.1p42; RubyGems 2.0.14) Faraday/0.8.9 Typhoeus/0.6.7' } + specify do + expect_meter("api.user_agent.script") + expect_meter("api.user_agent.script.ruby") + expect_meter("api.user_agent.script.ruby.travis") + expect_meter("api.user_agent.script.ruby.travis.version.1.6.8") + expect_meter("api.user_agent.script.ruby.travis.ruby.2.1.1") + get + end + end + + context 'Travis CLI' do + let(:agent) { 'Travis/1.6.8 (Mac OS X 10.10.2 like Darwin; Ruby 2.1.1; RubyGems 2.0.14; command whoami) Faraday/0.8.9 Typhoeus/0.6.7' } + specify do + expect_meter("api.user_agent.cli") + expect_meter("api.user_agent.cli.version.1.6.8") + expect_meter("api.user_agent.cli.ruby.2.1.1") + expect_meter("api.user_agent.cli.rubygems.2.0.14") + expect_meter("api.user_agent.cli.command.whoami") + expect_meter("api.user_agent.cli.os.darwin.mac_os_x.10.10") + get + end + end +end diff --git a/travis-api.gemspec b/travis-api.gemspec index 7719fff7..868839c9 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -18,17 +18,18 @@ Gem::Specification.new do |s| "Henrik Hodne", "Andre Arko", "Erik Michaels-Ober", - "Brian Ford", "Steve Richert", - "rainsun", - "Bryan Goldstein", - "James Dennes", - "Nick Schonning", + "Brian Ford", "Patrick Williams", + "Bryan Goldstein", "Puneeth Chaganti", "Thais Camilo and Konstantin Haase", "Tim Carey-Smith", - "Zachary Scott" + "Zachary Scott", + "James Dennes", + "rainsun", + "Dan Rice", + "Nick Schonning" ] s.email = [ @@ -44,17 +45,19 @@ Gem::Specification.new do |s| "andre@arko.net", "svenfuchs@artweb-design.de", "sferik@gmail.com", - "bford@engineyard.com", + "henrik@travis-ci.com", "steve.richert@gmail.com", - "patrick@bittorrent.com", - "punchagan@muse-amuse.in", - "jdennes@gmail.com", + "bford@engineyard.com", "nschonni@gmail.com", + "brysgo@gmail.com", + "punchagan@muse-amuse.in", + "e@zzak.io", + "jdennes@gmail.com", "rainsuner@gmail.com", "dev+narwen+rkh@rkh.im", "tim@spork.in", - "e@zzak.io", - "brysgo@gmail.com" + "dan@zoombody.com", + "patrick@bittorrent.com" ] s.files = [ @@ -65,7 +68,6 @@ Gem::Specification.new do |s| "bin/start-nginx", "config.ru", "config/database.yml", - "config/nginx.conf.erb", "config/puma-config.rb", "config/unicorn.rb", "lib/tasks/build_update_branch.rake", @@ -83,6 +85,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint/builds.rb", "lib/travis/api/app/endpoint/documentation.rb", "lib/travis/api/app/endpoint/endpoints.rb", + "lib/travis/api/app/endpoint/env_vars.rb", "lib/travis/api/app/endpoint/home.rb", "lib/travis/api/app/endpoint/hooks.rb", "lib/travis/api/app/endpoint/jobs.rb", @@ -91,6 +94,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint/repos.rb", "lib/travis/api/app/endpoint/requests.rb", "lib/travis/api/app/endpoint/setting_endpoint.rb", + "lib/travis/api/app/endpoint/singleton_settings_endpoint.rb", "lib/travis/api/app/endpoint/uptime.rb", "lib/travis/api/app/endpoint/users.rb", "lib/travis/api/app/extensions.rb", @@ -119,6 +123,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/responders/plain.rb", "lib/travis/api/app/responders/service.rb", "lib/travis/api/app/responders/xml.rb", + "lib/travis/api/app/services/schedule_request.rb", "lib/travis/api/serializer.rb", "lib/travis/api/v2.rb", "lib/travis/api/v2/http.rb", @@ -143,11 +148,13 @@ Gem::Specification.new do |s| "lib/travis/api/v2/http/request.rb", "lib/travis/api/v2/http/requests.rb", "lib/travis/api/v2/http/ssh_key.rb", - "lib/travis/api/v2/http/ssh_keys.rb", "lib/travis/api/v2/http/ssl_key.rb", "lib/travis/api/v2/http/user.rb", "lib/travis/api/v2/http/validation_error.rb", + "lib/travis/private_key.rb", "public/favicon.ico", + "public/images/result/canceled.png", + "public/images/result/canceled.svg", "public/images/result/error.png", "public/images/result/error.svg", "public/images/result/failing.png", @@ -159,12 +166,14 @@ Gem::Specification.new do |s| "public/images/result/unknown.png", "public/images/result/unknown.svg", "script/console", + "script/repos_stats.rb", "script/server", "spec/integration/formats_handling_spec.rb", "spec/integration/responders_spec.rb", "spec/integration/routes.backup.rb", "spec/integration/scopes_spec.rb", "spec/integration/settings_endpoint_spec.rb", + "spec/integration/singleton_settings_endpoint_spec.rb", "spec/integration/uptime_spec.rb", "spec/integration/v1/branches_spec.rb", "spec/integration/v1/builds_spec.rb", @@ -179,6 +188,7 @@ Gem::Specification.new do |s| "spec/integration/v2/repositories_spec.rb", "spec/integration/v2/requests_spec.rb", "spec/integration/v2/settings/env_vars_spec.rb", + "spec/integration/v2/settings/ssh_key_spec.rb", "spec/integration/v2/users_spec.rb", "spec/integration/v2_spec.backup.rb", "spec/integration/version_spec.rb", @@ -194,6 +204,7 @@ Gem::Specification.new do |s| "spec/unit/api/v2/http/build_spec.rb", "spec/unit/api/v2/http/builds_spec.rb", "spec/unit/api/v2/http/caches_spec.rb", + "spec/unit/api/v2/http/env_var_spec.rb", "spec/unit/api/v2/http/hooks_spec.rb", "spec/unit/api/v2/http/job_spec.rb", "spec/unit/api/v2/http/jobs_spec.rb", @@ -219,6 +230,7 @@ Gem::Specification.new do |s| "spec/unit/endpoint/lint_spec.rb", "spec/unit/endpoint/logs_spec.rb", "spec/unit/endpoint/repos_spec.rb", + "spec/unit/endpoint/requests_spec.rb", "spec/unit/endpoint/users_spec.rb", "spec/unit/endpoint_spec.rb", "spec/unit/extensions/expose_pattern_spec.rb", From 950b8ce4d813c8e63a0f50fbd8ef94f6a59367ad Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Tue, 28 Oct 2014 11:11:52 +0100 Subject: [PATCH 2/2] reduce metrics generated by UA tracker --- .../api/app/middleware/user_agent_tracker.rb | 35 ++++--------------- .../middleware/user_agent_tracker_spec.rb | 19 ---------- 2 files changed, 6 insertions(+), 48 deletions(-) diff --git a/lib/travis/api/app/middleware/user_agent_tracker.rb b/lib/travis/api/app/middleware/user_agent_tracker.rb index cba98cf3..d7d582fa 100644 --- a/lib/travis/api/app/middleware/user_agent_tracker.rb +++ b/lib/travis/api/app/middleware/user_agent_tracker.rb @@ -37,27 +37,14 @@ class Travis::Api::App end def mark_travis(agent) - os, *rest = agent.application.comment - ruby, rubygems, command = "unknown", "unknown", nil - - rest.each do |comment| - case comment - when /^Ruby (\d\.\d.\d)/ then ruby = $1 - when /^RubyGems (.+)$/ then rubygems = $1 - when /^command (.+)$/ then command = $1 - end - end - - # "Ubuntu 12.04 like Linux" => "linux.ubuntu.12.04" - if os =~ /^(.+) (\S+) like (\S+)$/ - os = "#{$3}.#{$1}.#{$2[/\d+\.\d+/]}" - end + command = agent.application.comment.detect { |c| c.start_with? "command " } if command - mark(:cli, version: agent.version, ruby: ruby, rubygems: rubygems, command: command, os: os) + mark(:cli, :version, agent.version) + mark(:cli, command.sub(' ', '.')) else # only track ruby version and library version for non-cli usage - mark(:script, :ruby, :travis, version: agent.version, ruby: ruby) + mark(:script, :ruby, :travis, :version, agent.version) end end @@ -66,19 +53,9 @@ class Travis::Api::App mark(:unknown) end - def track_key(string) - string.to_s.downcase.gsub(/[^a-z0-9\-\.]+/, '_') - end - def mark(*keys) - key = "api.user_agent" - keys.each do |subkey| - if subkey.is_a? Hash - subkey.each_pair { |k, v| ::Metriks.meter("#{key}.#{track_key(k)}.#{track_key(v)}").mark } - else - ::Metriks.meter(key << "." << track_key(subkey)).mark - end - end + key = "api.user_agent." << keys.map { |k| k.to_s.downcase.gsub(/[^a-z0-9\-\.]+/, '_') }.join('.') + ::Metriks.meter(key).mark end end end diff --git a/spec/unit/middleware/user_agent_tracker_spec.rb b/spec/unit/middleware/user_agent_tracker_spec.rb index 7fbe0c29..1e5ebe00 100644 --- a/spec/unit/middleware/user_agent_tracker_spec.rb +++ b/spec/unit/middleware/user_agent_tracker_spec.rb @@ -35,13 +35,11 @@ describe Travis::Api::App::Middleware::UserAgentTracker do let(:agent) { "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.36 Safari/537.36" } specify 'without X-User-Agent' do - expect_meter("api.user_agent.browser") expect_meter("api.user_agent.browser.unknown") get end specify 'with X-User-Agent' do - expect_meter("api.user_agent.browser") expect_meter("api.user_agent.browser.travis-web") get('HTTP_X_USER_AGENT' => 'travis-web') end @@ -50,7 +48,6 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'console' do let(:agent) { 'curl' } specify do - expect_meter("api.user_agent.console") expect_meter("api.user_agent.console.curl") get end @@ -59,8 +56,6 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'travis-api-wrapper' do let(:agent) { 'travis-api-wrapper - v0.01 - (cmaujean@gmail.com)' } specify do - expect_meter("api.user_agent.script") - expect_meter("api.user_agent.script.node_js") expect_meter("api.user_agent.script.node_js.travis-api-wrapper") get end @@ -69,8 +64,6 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'TravisPy' do let(:agent) { 'TravisPy' } specify do - expect_meter("api.user_agent.script") - expect_meter("api.user_agent.script.python") expect_meter("api.user_agent.script.python.travispy") get end @@ -79,8 +72,6 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'Ruby' do let(:agent) { 'Ruby' } specify do - expect_meter("api.user_agent.script") - expect_meter("api.user_agent.script.ruby") expect_meter("api.user_agent.script.ruby.vanilla") get end @@ -89,8 +80,6 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'Faraday' do let(:agent) { 'Faraday' } specify do - expect_meter("api.user_agent.script") - expect_meter("api.user_agent.script.ruby") expect_meter("api.user_agent.script.ruby.vanilla") get end @@ -99,11 +88,7 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'travis.rb' do let(:agent) { 'Travis/1.6.8 (Mac OS X 10.9.2 like Darwin; Ruby 2.1.1p42; RubyGems 2.0.14) Faraday/0.8.9 Typhoeus/0.6.7' } specify do - expect_meter("api.user_agent.script") - expect_meter("api.user_agent.script.ruby") - expect_meter("api.user_agent.script.ruby.travis") expect_meter("api.user_agent.script.ruby.travis.version.1.6.8") - expect_meter("api.user_agent.script.ruby.travis.ruby.2.1.1") get end end @@ -111,12 +96,8 @@ describe Travis::Api::App::Middleware::UserAgentTracker do context 'Travis CLI' do let(:agent) { 'Travis/1.6.8 (Mac OS X 10.10.2 like Darwin; Ruby 2.1.1; RubyGems 2.0.14; command whoami) Faraday/0.8.9 Typhoeus/0.6.7' } specify do - expect_meter("api.user_agent.cli") expect_meter("api.user_agent.cli.version.1.6.8") - expect_meter("api.user_agent.cli.ruby.2.1.1") - expect_meter("api.user_agent.cli.rubygems.2.0.14") expect_meter("api.user_agent.cli.command.whoami") - expect_meter("api.user_agent.cli.os.darwin.mac_os_x.10.10") get end end