From f52a03655e64fc8e4b908b540d1c8f4523ee8187 Mon Sep 17 00:00:00 2001 From: Stanislav K Date: Thu, 25 Feb 2021 14:28:53 +0200 Subject: [PATCH 1/2] Switch to VCS instead of direct GH calls (#1168) --- Gemfile | 2 +- Gemfile.lock | 18 +- bin/migrate-hooks | 7 +- config.ru | 6 +- lib/travis.rb | 1 - lib/travis/api/app.rb | 1 - lib/travis/api/app/endpoint/authorization.rb | 208 +----------------- lib/travis/api/app/endpoint/home.rb | 1 - .../api/app/services/schedule_request.rb | 2 +- .../api/serialize/v0/worker/job/test.rb | 2 +- lib/travis/api/serialize/v2/http/user.rb | 3 - lib/travis/api/v3.rb | 2 + lib/travis/api/v3/access_control/signature.rb | 2 +- lib/travis/api/v3/github.rb | 132 ----------- lib/travis/api/v3/models/cron.rb | 2 +- .../api/v3/models/repository_migration.rb | 2 +- lib/travis/api/v3/queries/caches.rb | 2 +- lib/travis/api/v3/queries/organization.rb | 4 +- lib/travis/api/v3/queries/owner.rb | 10 +- lib/travis/api/v3/queries/request.rb | 4 +- lib/travis/api/v3/queries/request_preview.rb | 2 +- lib/travis/api/v3/queries/user.rb | 16 +- lib/travis/api/v3/service.rb | 4 - lib/travis/api/v3/service_index.rb | 1 - .../api/v3/services/repository/activate.rb | 26 +-- .../api/v3/services/repository/deactivate.rb | 14 +- lib/travis/config/defaults.rb | 2 +- lib/travis/errors.rb | 2 + lib/travis/github.rb | 20 -- lib/travis/github/education.rb | 24 -- lib/travis/github/oauth.rb | 51 ----- lib/travis/github/services/set_hook.rb | 19 +- lib/travis/github/services/set_key.rb | 56 ++--- lib/travis/model/organization.rb | 1 - lib/travis/model/owner_group.rb | 1 - lib/travis/model/request.rb | 2 +- lib/travis/model/user.rb | 4 +- lib/travis/model/user/oauth.rb | 3 +- lib/travis/services.rb | 1 - lib/travis/services/find_admin.rb | 94 -------- lib/travis/services/find_caches.rb | 2 +- lib/travis/testing/factories.rb | 3 +- lib/travis/testing/stubs.rb | 6 +- spec/auth/helpers.rb | 2 +- spec/auth/v2.1/users_spec.rb | 12 + spec/auth/v2/users_spec.rb | 10 + spec/integration/v2/hooks_spec.rb | 19 +- spec/integration/v2/pusher_spec.rb | 4 +- spec/integration/v2/users_spec.rb | 12 + spec/lib/github/oauth_spec.rb | 65 ------ spec/lib/github/services/set_key_spec.rb | 86 ++++---- spec/lib/model/repository_spec.rb | 4 +- spec/lib/model/request_spec.rb | 14 -- spec/lib/services/find_admin_spec.rb | 88 -------- spec/lib/services/find_caches_spec.rb | 6 +- spec/spec_helper.rb | 1 - spec/support/payloads.rb | 1 + spec/unit/endpoint/accounts_spec.rb | 2 +- .../authorization/user_manager_spec.rb | 110 --------- spec/unit/endpoint/authorization_spec.rb | 169 ++++---------- spec/unit/endpoint/users_spec.rb | 10 +- spec/unit/serialize/v2/http/user_spec.rb | 9 + spec/v3/services/caches/delete_spec.rb | 2 +- spec/v3/services/caches/find_spec.rb | 6 +- spec/v3/services/owner/find_spec.rb | 8 +- .../repositories/for_current_user_spec.rb | 2 +- .../services/repositories/for_owner_spec.rb | 39 ++-- spec/v3/services/repository/activate_spec.rb | 184 ++-------------- .../v3/services/repository/deactivate_spec.rb | 132 ++--------- spec/v3/services/repository/find_spec.rb | 9 +- spec/v3/services/repository/migrate_spec.rb | 2 +- spec/v3/services/request/preview_spec.rb | 2 +- spec/v3/services/requests/create_spec.rb | 2 +- spec/v3/services/user/current_spec.rb | 6 +- spec/v3/services/user/find_spec.rb | 12 +- spec/v3/services/user/sync_spec.rb | 15 +- .../v2_subscription/executions_spec.rb | 2 +- .../services/v2_subscriptions/create_spec.rb | 1 - 78 files changed, 332 insertions(+), 1481 deletions(-) delete mode 100644 lib/travis/api/v3/github.rb delete mode 100644 lib/travis/github/oauth.rb delete mode 100644 lib/travis/services/find_admin.rb delete mode 100644 spec/lib/github/oauth_spec.rb delete mode 100644 spec/lib/services/find_admin_spec.rb delete mode 100644 spec/unit/endpoint/authorization/user_manager_spec.rb diff --git a/Gemfile b/Gemfile index ee92245a24..9010abcb90 100644 --- a/Gemfile +++ b/Gemfile @@ -28,7 +28,6 @@ gem 'rack', '>= 2.1.4' gem 'rack-contrib' gem 'rack-cache', git: 'https://github.com/rtomayko/rack-cache' gem 'rack-attack', '~> 5.0' -gem 'gh', git: 'https://github.com/travis-ci/gh' gem 'bunny', '~> 2.9.2' gem 'dalli' gem 'pry' @@ -56,6 +55,7 @@ gem 'opencensus-stackdriver' gem 'faraday' gem 'faraday_middleware' +gem 'net-http-persistent' gem 'knapsack' diff --git a/Gemfile.lock b/Gemfile.lock index 2589b5bf32..fd9778b0d4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -19,18 +19,6 @@ GIT rack-cache (1.11.0) rack (>= 0.4) -GIT - remote: https://github.com/travis-ci/gh - revision: 66449dc14db5eee0a24a4dbcb85e52dd5b63513f - specs: - gh (0.15.2) - addressable (~> 2.4) - backports - faraday (~> 0.8) - multi_json (~> 1.0) - net-http-persistent (~> 2.9) - net-http-pipeline - GIT remote: https://github.com/travis-ci/marginalia revision: 46bbe301640efb5ea81828cb7deeaf874516de78 @@ -283,8 +271,8 @@ GEM mustermann (1.1.1) ruby2_keywords (~> 0.0.1) nakayoshi_fork (0.0.4) - net-http-persistent (2.9.4) - net-http-pipeline (1.0.1) + net-http-persistent (4.0.0) + connection_pool (~> 2.2) nokogiri (1.10.9) mini_portile2 (~> 2.4.0) opencensus (0.5.0) @@ -439,7 +427,6 @@ DEPENDENCIES fog-aws (~> 0.12.0) fog-google (~> 0.4.2) foreman - gh! google-api-client (~> 0.9.4) hashdiff hashr @@ -457,6 +444,7 @@ DEPENDENCIES multi_json mustermann nakayoshi_fork + net-http-persistent opencensus opencensus-stackdriver pg (~> 0.21) diff --git a/bin/migrate-hooks b/bin/migrate-hooks index 21e1020f5c..346f9715f9 100755 --- a/bin/migrate-hooks +++ b/bin/migrate-hooks @@ -13,7 +13,6 @@ require 'date' require 'stringio' require 'thread' require 'travis' -require 'travis/api/v3/github' require 'travis/config/defaults' require 'travis/model/repository' @@ -55,7 +54,7 @@ class Migrate ids = query.ids ids.sort.reverse.each { |id| queue.push(id) } puts "Queued #{ids.size} repos" - + sleep 5 # Start num_workers threads and start running off the queue @@ -115,13 +114,13 @@ class Migrate puts "No webhook found but service hook found, syncing repo #{repo.slug} active attr" puts "Save failed" unless repo.update(active: service_hook['active']) end - + # Only set webhook for repos with an active service hook if (service_hook && service_hook['active']) puts "Found active service hook for #{repo.slug}, setting webhook" gh.set_hook(repo, true) end - + migrated = true break rescue => e diff --git a/config.ru b/config.ru index 01ba8ceb9b..f0d8ac5f15 100644 --- a/config.ru +++ b/config.ru @@ -10,10 +10,10 @@ require 'travis/api/app' require 'core_ext/module/load_constants' models = Travis::Model.constants.map(&:to_s) -only = [/^(ActiveRecord|ActiveModel|Travis|GH|#{models.join('|')})/] -skip = ['Travis::Memory', 'GH::ResponseWrapper', 'Travis::Helpers::Legacy', 'GH::FaradayAdapter::EMSynchrony'] +only = [/^(ActiveRecord|ActiveModel|Travis|#{models.join('|')})/] +skip = ['Travis::Memory', 'Travis::Helpers::Legacy'] -[Travis::Api, Travis, GH].each do |target| +[Travis::Api, Travis].each do |target| target.load_constants! :only => only, :skip => skip, :debug => false end diff --git a/lib/travis.rb b/lib/travis.rb index bcc7f6a25b..c133806bc8 100644 --- a/lib/travis.rb +++ b/lib/travis.rb @@ -42,7 +42,6 @@ def setup(options = {}) Travis.logger.info("Setting up module Travis") - Github.setup Services.register Github::Services.register end diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index 935f7651a9..9d1678ed3a 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -17,7 +17,6 @@ require 'travis/api/attack' require 'active_record' require 'redis' -require 'gh' require 'raven' require 'raven/integrations/rack' require 'sidekiq' diff --git a/lib/travis/api/app/endpoint/authorization.rb b/lib/travis/api/app/endpoint/authorization.rb index d344299b06..67fb964808 100644 --- a/lib/travis/api/app/endpoint/authorization.rb +++ b/lib/travis/api/app/endpoint/authorization.rb @@ -3,8 +3,6 @@ require 'faraday_middleware' require 'securerandom' require 'travis/api/app' -require 'travis/github/education' -require 'travis/github/oauth' require 'travis/remote_vcs/user' require 'travis/remote_vcs/response_error' require 'uri' @@ -92,11 +90,10 @@ class Authorization < Endpoint end # For new provider method - # renew_access_token(token: params[:github_token], app_id: 1, provider: :github) - { 'access_token' => github_to_travis(params[:github_token], app_id: 1, drop_token: true) } + renew_access_token(token: params[:github_token], app_id: 1, provider: :github) end - # Endpoint for making sure user authorized Travis CI to access GitHub. + # Endpoint for making sure user authorized Travis CI to access VCS provider. # There are no restrictions on where to redirect to after handshake. # However, no information whatsoever is being sent with the redirect. # @@ -104,10 +101,9 @@ class Authorization < Endpoint # # * **redirect_uri**: URI to redirect to after handshake. get '/handshake/?:provider?' do - method = org? ? :handshake : :vcs_handshake params[:provider] ||= 'github' - send(method) do |user, token, redirect_uri| - if target_ok? redirect_uri + vcs_handshake do |user, token, redirect_uri| + if target_ok?(redirect_uri) content_type :html data = { user: user, token: token, uri: redirect_uri } erb(:post_payload, locals: data) @@ -151,42 +147,6 @@ def log_with_request_id(line) Travis.logger.info "#{line} " end - def handshake - config = Travis.config.oauth2.to_h - endpoint = Addressable::URI.parse(config[:authorization_server]) - values = { - client_id: config[:client_id], - scope: config[:scope], - redirect_uri: oauth_endpoint - } - - log_with_request_id("[handshake] Starting handshake") - - if params[:code] - unless state_ok?(params[:state]) - log_with_request_id("[handshake] Handshake failed (state mismatch)") - handle_invalid_response - return - end - - endpoint.path = config[:access_token_path] - values[:state] = params[:state] - values[:code] = params[:code] - values[:client_secret] = config[:client_secret] - github_token = get_token(endpoint.to_s, values) - user = user_for_github_token(github_token) - token = generate_token(user: user, app_id: 0) - payload = params[:state].split(":::", 2)[1] - update_first_login(user) - yield serialize_user(user), token, payload - else - values[:state] = create_state - endpoint.path = config[:authorize_path] - endpoint.query_values = values - redirect to(endpoint.to_s) - end - end - # VCS HANDSHAKE START def remote_vcs_user @@ -274,171 +234,11 @@ def handle_invalid_response redirect to("https://#{Travis.config.host}/") end - def create_state - state = SecureRandom.urlsafe_base64(16) - redis.sadd('github:states', state) - redis.expire('github:states', 1800) - payload = params[:origin] || params[:redirect_uri] - state << ":::" << payload if payload - response.set_cookie(cookie_name, state) - state - end - def state_ok?(state, provider = :github) cookie_state = request.cookies[cookie_name(provider)] state == cookie_state and redis.srem('github:states', state.to_s.split(":::", 1)) end - def github_to_travis(token, options = {}) - drop_token = options.delete(:drop_token) - generate_token options.merge(user: user_for_github_token(token, drop_token)) - end - - class UserManager < Struct.new(:data, :token, :drop_token) - include User::Renaming - - attr_accessor :user - - def initialize(*) - super - - @user = ::User.find_by_github_id(data['id']) - end - - def info(attributes = {}) - info = data.to_hash.slice('name', 'login', 'gravatar_id') - info.merge! attributes.stringify_keys - if Travis::Features.feature_active?(:education_data_sync) || - (user && Travis::Features.owner_active?(:education_data_sync, user)) - info['education'] = education - end - info['github_id'] ||= data['id'] - info['vcs_id'] ||= data['id'] - info - end - - def user_exists? - user - end - - def education - Travis::Github::Education.new(token.to_s).student? - end - - def fetch - retried ||= false - info = drop_token ? self.info : self.info(github_oauth_token: token) - - ActiveRecord::Base.transaction do - if user - ensure_token_is_available - rename_repos_owner(user.login, info['login']) - user.update_attributes info - else - self.user = ::User.create! info - end - - Travis::Github::Oauth.update_scopes(user) # unless Travis.env == 'test' - - nullify_logins(user.github_id, user.login) - end - - user - rescue ActiveRecord::RecordNotUnique - unless retried - retried = true - retry - end - end - - def ensure_token_is_available - unless user.tokens.first - user.create_a_token - end - end - end - - def user_for_github_token(token, drop_token = false) - data = GH.with(token: token.to_s, client_id: nil) { GH['user'] } - scopes = parse_scopes data.headers['x-oauth-scopes'] - manager = UserManager.new(data, token, drop_token) - - unless acceptable?(scopes, drop_token) - # TODO: we should probably only redirect if this is a web - # oauth request, are there any other possibilities to - # consider? - url = Travis.config.oauth2.insufficient_access_redirect_url - url += "#existing-user" if manager.user_exists? - redirect to(url) - end - - user = manager.fetch - if user.nil? - log_with_request_id("[handshake] Fetching user failed") - halt 403, 'not a Travis user' - end - - Travis.run_service(:sync_user, user) - - user - rescue GH::Error - # not a valid token actually, but we don't want to expose that info - halt 403, 'not a Travis user' - end - - def get_token(endpoint, values) - # Get base URL for when we setup Faraday since otherwise it'll ignore no_proxy - url = URI.parse(endpoint) - base_url = "#{url.scheme}://#{url.host}" - http_options = {url: base_url, ssl: Travis.config.ssl.to_h.merge(Travis.config.github.ssl || {}).compact} - - conn = Faraday.new(http_options) do |conn| - conn.request :json - conn.use :instrumentation - conn.use OpenCensus::Trace::Integrations::FaradayMiddleware if Travis::Api::App::Middleware::OpenCensus.enabled? - conn.adapter :net_http_persistent - end - response = conn.post(endpoint, values) - - parameters = Addressable::URI.form_unencode(response.body) - token_info = parameters.assoc("access_token") - - unless token_info - log_with_request_id("[handshake] Could not fetch token, github's response: status=#{response.status}, body=#{parameters.inspect} headers=#{response.headers.inspect}") - halt 401, 'could not resolve github token' - end - token_info.last - end - - def parse_scopes(data) - data.gsub(/\s/,'').split(',') if data - end - - def generate_token(options) - AccessToken.create(options).token - end - - def acceptable?(scopes, lossy = false) - Travis::Github::Oauth.wanted_scopes.all? do |scope| - acceptable_scopes_for(scope, lossy).any? { |s| scopes.include? s } - end - end - - def acceptable_scopes_for(scope, lossy = false) - scopes = case scope = scope.to_s - when /^(.+):/ then [$1, scope] - when 'public_repo' then [scope, 'repo'] - else [scope] - end - - if lossy - scopes << 'repo' - scopes << 'public_repo' if lossy and scope != 'repo' - end - - scopes - end - def post_message(payload) content_type :html erb(:post_message, locals: payload) diff --git a/lib/travis/api/app/endpoint/home.rb b/lib/travis/api/app/endpoint/home.rb index 26ebc68a38..212eea3dd7 100644 --- a/lib/travis/api/app/endpoint/home.rb +++ b/lib/travis/api/app/endpoint/home.rb @@ -12,7 +12,6 @@ class Home < Endpoint shorten_host: Travis.config.shorten_host, assets: Travis.config.assets.to_h, pusher: (Travis.config.pusher_ws || Travis.config.pusher.to_h || {}).to_hash.slice(:scheme, :host, :port, :path, :key, :secure, :private), - github: { api_url: GH.current.api_host.to_s, scopes: Travis.config.oauth2.try(:scope).to_s.split(?,) }, notifications: { webhook: { public_key: Travis.config.webhook.public_key } } set :check_auth, false diff --git a/lib/travis/api/app/services/schedule_request.rb b/lib/travis/api/app/services/schedule_request.rb index f4fe89982d..2beaf5660e 100644 --- a/lib/travis/api/app/services/schedule_request.rb +++ b/lib/travis/api/app/services/schedule_request.rb @@ -60,7 +60,7 @@ def enqueue_build_request def payload data = params.merge(user: { id: current_user.id }) - data[:repository][:id] = repo.vcs_id || repo.github_id + data[:repository][:id] = repo.vcs_id data[:repository][:vcs_type] = repo.vcs_type MultiJson.encode(data) end diff --git a/lib/travis/api/serialize/v0/worker/job/test.rb b/lib/travis/api/serialize/v0/worker/job/test.rb index 7f24284795..75bba9a0a9 100644 --- a/lib/travis/api/serialize/v0/worker/job/test.rb +++ b/lib/travis/api/serialize/v0/worker/job/test.rb @@ -53,7 +53,7 @@ def repository_data { 'id' => repository.id, 'slug' => repository.slug, - 'github_id' => repository.github_id, + 'github_id' => repository.vcs_id, 'vcs_id' => repository.vcs_id, 'vcs_type' => repository.vcs_type, 'source_url' => repository.source_url, diff --git a/lib/travis/api/serialize/v2/http/user.rb b/lib/travis/api/serialize/v2/http/user.rb index ff9ef654f2..bab8283c68 100644 --- a/lib/travis/api/serialize/v2/http/user.rb +++ b/lib/travis/api/serialize/v2/http/user.rb @@ -1,5 +1,4 @@ require 'travis/api/serialize/formats' -require 'travis/github/oauth' require 'travis/remote_vcs/user' require 'travis/remote_vcs/response_error' @@ -55,8 +54,6 @@ def user_data end def check_scopes - return Github::Oauth.correct_scopes?(user) if user.github? - ::Travis::RemoteVCS::User.new.check_scopes(user_id: user.id) rescue ::Travis::RemoteVCS::ResponseError false diff --git a/lib/travis/api/v3.rb b/lib/travis/api/v3.rb index a4503c2bfa..738bc06272 100644 --- a/lib/travis/api/v3.rb +++ b/lib/travis/api/v3.rb @@ -1,5 +1,7 @@ module Travis module API + GITHUB_EVENTS = %i(push pull_request issue_comment public member create delete repository) + module V3 V3 = self diff --git a/lib/travis/api/v3/access_control/signature.rb b/lib/travis/api/v3/access_control/signature.rb index e8dce5ec76..091801afa6 100644 --- a/lib/travis/api/v3/access_control/signature.rb +++ b/lib/travis/api/v3/access_control/signature.rb @@ -16,7 +16,7 @@ def self.for_request(type, payload, env) if vcs_id = options[?u.freeze] - return unless user = ::User.find_by(vcs_id: vcs_id) || ::User.find_by(github_id: vcs_id) + return unless user = ::User.find_by(vcs_id: vcs_id) end if application = options[?a.freeze] diff --git a/lib/travis/api/v3/github.rb b/lib/travis/api/v3/github.rb deleted file mode 100644 index db3ca48774..0000000000 --- a/lib/travis/api/v3/github.rb +++ /dev/null @@ -1,132 +0,0 @@ -require 'gh' -require 'uri' - -module Travis::API::V3 - class GitHub - def self.config - @config ||= Travis::Config.load - end - - EVENTS = %i(push pull_request issue_comment public member create delete repository) - - DEFAULT_OPTIONS = { - client_id: config.oauth2.try(:client_id), - client_secret: config.oauth2.try(:client_secret), - scopes: config.oauth2.try(:scope).to_s.split(?,), - user_agent: "Travis-API/3 Travis-CI/0.0.1 GH/#{GH::VERSION}", - origin: config.host, - api_url: config.github.api_url, - web_url: config.github.api_url.gsub(%r{\A(https?://)(?:api\.)?([^/]+)(?:/.*)?\Z}, '\1\2'), - ssl: config.ssl.to_h.merge(config.github.ssl || {}).compact - } - private_constant :DEFAULT_OPTIONS - - HOOKS_URL = "repositories/%i/hooks" - private_constant :HOOKS_URL - - def self.client_config - { - api_url: DEFAULT_OPTIONS[:api_url], - web_url: DEFAULT_OPTIONS[:web_url], - scopes: DEFAULT_OPTIONS[:scopes] - } - end - - attr_reader :gh, :user - - def initialize(user = nil, token = nil) - if user.respond_to? :github_oauth_token - raise ServerError, 'no GitHub token for user' if user.github_oauth_token.blank? - token = user.github_oauth_token - end - - @user = user - @gh = GH.with(token: token, **DEFAULT_OPTIONS) - end - - def set_hook(repo, active) - set_webhook(repo, active) - deactivate_service_hook(repo) if Travis.config.enterprise - end - - def upload_key(repository) - keys_path = "repositories/#{repository.vcs_id || repository.github_id}/keys" - key = gh[keys_path].detect { |e| e['key'] == repository.key.encoded_public_key } - - unless key - gh.post keys_path, { - title: Travis.config.host.to_s, - key: repository.key.encoded_public_key, - read_only: !Travis::Features.owner_active?(:read_write_github_keys, repository.owner) - } - end - end - - def set_webhook(repo, active) - payload = { - name: 'web'.freeze, - events: EVENTS, - active: active, - config: { url: service_hook_url.to_s, insecure_ssl: insecure_ssl? } - } - if url = webhook_url?(repo) - info("Updating webhook repo=%s github_id=%i active=%s" % [repo.slug, repo.vcs_id || repo.github_id, active]) - gh.patch(url, payload) - else - hooks_url = HOOKS_URL % [repo.vcs_id || repo.github_id] - info("Creating webhook repo=%s github_id=%i active=%s" % [repo.slug, repo.vcs_id || repo.github_id, active]) - gh.post(hooks_url, payload) - end - end - - def deactivate_service_hook(repo) - if url = service_hook_url?(repo) - info("Deactivating service hook repo=%s github_id=%i" % [repo.slug, repo.vcs_id || repo.github_id]) - # Have to update events here too, to avoid old hooks failing validation - gh.patch(url, { events: EVENTS, active: false }) - end - end - - def service_hook(repo) - hooks(repo).detect { |h| h['name'] == 'travis' && h.dig('config', 'domain') == service_hook_url.to_s } - end - - def service_hook_url?(repo) - if hook = service_hook(repo) - hook.dig('_links', 'self', 'href') - end - end - - def webhook(repo) - hooks(repo).detect do |h| - h['name'] == 'web' && URI(h.dig('config', 'url')) == service_hook_url - end - end - - def webhook_url?(repo) - if hook = webhook(repo) - hook.dig('_links', 'self', 'href') - end - end - - def hooks(repo) - gh[HOOKS_URL % [repo.vcs_id || repo.github_id]] - end - - def service_hook_url - url = Travis.config.service_hook_url || '' - url.prepend('https://') unless url.starts_with?('https://', 'http://') - URI(url) - end - - def info(msg) - Travis.logger.info(msg) - end - - private - - def insecure_ssl? - Travis.config.ssl.to_h.key?(:verify) && Travis.config.ssl.to_h[:verify] == false - end - end -end diff --git a/lib/travis/api/v3/models/cron.rb b/lib/travis/api/v3/models/cron.rb index 2552cccada..b8b0d97122 100644 --- a/lib/travis/api/v3/models/cron.rb +++ b/lib/travis/api/v3/models/cron.rb @@ -54,7 +54,7 @@ def enqueue payload = { repository: { - id: branch.repository.vcs_id || branch.repository.github_id, + id: branch.repository.vcs_id, vcs_type: branch.repository.vcs_type, owner_name: branch.repository.owner_name, name: branch.repository.name }, diff --git a/lib/travis/api/v3/models/repository_migration.rb b/lib/travis/api/v3/models/repository_migration.rb index 45a3981436..9c2b2248a0 100644 --- a/lib/travis/api/v3/models/repository_migration.rb +++ b/lib/travis/api/v3/models/repository_migration.rb @@ -22,7 +22,7 @@ def migrate! c.use OpenCensus::Trace::Integrations::FaradayMiddleware if Travis::Api::App::Middleware::OpenCensus.enabled? c.adapter Faraday.default_adapter end - response = connection.post("/api/repo/by_github_id/#{repository.vcs_id || repository.github_id}/migrate") + response = connection.post("/api/repo/by_github_id/#{repository.vcs_id}/migrate") unless response.success? raise MigrationRequestFailed diff --git a/lib/travis/api/v3/queries/caches.rb b/lib/travis/api/v3/queries/caches.rb index e2f37412a3..9cf2cf98bd 100644 --- a/lib/travis/api/v3/queries/caches.rb +++ b/lib/travis/api/v3/queries/caches.rb @@ -25,7 +25,7 @@ def main_type private def prefix - prefix = "#{@repo.vcs_id || @repo.github_id}/#{branch}" + prefix = "#{@repo.vcs_id}/#{branch}" prefix << '/' unless prefix.last == '/' prefix end diff --git a/lib/travis/api/v3/queries/organization.rb b/lib/travis/api/v3/queries/organization.rb index cb8152097a..f2e53c224e 100644 --- a/lib/travis/api/v3/queries/organization.rb +++ b/lib/travis/api/v3/queries/organization.rb @@ -1,10 +1,10 @@ module Travis::API::V3 class Queries::Organization < Query - params :id, :login, :github_id, :provider + params :id, :login, :vcs_id, :provider def find return Models::Organization.find_by_id(id) if id - return Models::Organization.find_by(vcs_id: github_id) || Models::Organization.find_by(github_id: github_id) if github_id + return Models::Organization.find_by(vcs_id: vcs_id) if vcs_id return Models::Organization.where( 'lower(login) = ? and lower(vcs_type) = ?'.freeze, login.downcase, diff --git a/lib/travis/api/v3/queries/owner.rb b/lib/travis/api/v3/queries/owner.rb index d5e4031491..1ce3f38bb4 100644 --- a/lib/travis/api/v3/queries/owner.rb +++ b/lib/travis/api/v3/queries/owner.rb @@ -10,10 +10,12 @@ def find def query(type, main_type: self.main_type, params: self.params) main_type = type if main_type == :owner - params['provider'] ||= 'github' - params = params.merge("#{type}.login" => params['login']) if params['login'] - params = params.merge("#{type}.github_id" => params['github_id']) if params['github_id'] - Queries[type].new(params, main_type, service: @service) + new_params = params.dup + new_params.delete('github_id') + new_params['provider'] ||= 'github' + new_params["#{type}.login"] = params['login'] if params['login'] + new_params["#{type}.vcs_id"] = params['github_id'] if params['github_id'] + Queries[type].new(new_params, main_type, service: @service) end def find!(type) diff --git a/lib/travis/api/v3/queries/request.rb b/lib/travis/api/v3/queries/request.rb index 5b80b00d76..289593cc96 100644 --- a/lib/travis/api/v3/queries/request.rb +++ b/lib/travis/api/v3/queries/request.rb @@ -18,14 +18,14 @@ def messages(request) end def schedule(repository, user) - raise ServerError, 'repository does not have a provider id'.freeze unless repository.vcs_id || repository.github_id + raise ServerError, 'repository does not have a provider id'.freeze unless repository.vcs_id raise WrongParams, 'missing user'.freeze unless user and user.id request = create_request(repository) payload = { repository: { - id: repository.github_id || repository.vcs_id, + id: repository.vcs_id, vcs_type: repository.vcs_type, owner_name: repository.owner_name, name: repository.name , diff --git a/lib/travis/api/v3/queries/request_preview.rb b/lib/travis/api/v3/queries/request_preview.rb index e17087014e..932494f069 100644 --- a/lib/travis/api/v3/queries/request_preview.rb +++ b/lib/travis/api/v3/queries/request_preview.rb @@ -39,7 +39,7 @@ def env def repo_data { - github_id: repo.github_id, + vcs_id: repo.vcs_id, slug: repo.slug, token: repo_token, private: repo.private?, diff --git a/lib/travis/api/v3/queries/user.rb b/lib/travis/api/v3/queries/user.rb index 7d6a38bef9..e09613db24 100644 --- a/lib/travis/api/v3/queries/user.rb +++ b/lib/travis/api/v3/queries/user.rb @@ -1,11 +1,10 @@ module Travis::API::V3 class Queries::User < Query - setup_sidekiq(:user_sync, queue: :sync, class_name: "Travis::GithubSync::Worker") - params :id, :login, :email, :github_id, :vcs_id, :vcs_type, :is_syncing + params :id, :login, :email, :vcs_id, :vcs_type, :is_syncing def find return Models::User.find_by_id(id) if id - return Models::User.find_by(vcs_id: github_id) || Models::User.find_by(github_id: github_id) if github_id + return Models::User.find_by(vcs_id: vcs_id) if vcs_id return Models::User.where( 'lower(login) = ? and lower(vcs_type) = ?'.freeze, login.downcase, @@ -25,14 +24,9 @@ def find_by_email(email) def sync(user) raise AlreadySyncing if user.is_syncing? - if Travis::Features.user_active?(:use_vcs, user) || !user.github? - Travis::RemoteVCS::User.new.sync(user_id: user.id) - user.reload - else - perform_async(:user_sync, :sync_user, user_id: user.id) - user.update_column(:is_syncing, true) - user - end + + Travis::RemoteVCS::User.new.sync(user_id: user.id) + user.reload end private diff --git a/lib/travis/api/v3/service.rb b/lib/travis/api/v3/service.rb index 3bbb9b6f3f..3c04fb1e4b 100644 --- a/lib/travis/api/v3/service.rb +++ b/lib/travis/api/v3/service.rb @@ -67,10 +67,6 @@ def query(type = result_type) @queries[type] ||= Queries[type].new(params, result_type, service: self) end - def github(user = nil) - @github[user] ||= GitHub.new(user) - end - def find(type = result_type, *args) not_found(true, type) unless object = query(type).find(*args) not_found(false, type) unless access_control.visible? object diff --git a/lib/travis/api/v3/service_index.rb b/lib/travis/api/v3/service_index.rb index 779efd4424..2748e2e42a 100644 --- a/lib/travis/api/v3/service_index.rb +++ b/lib/travis/api/v3/service_index.rb @@ -23,7 +23,6 @@ def config pusher_config = (Travis.config.pusher_ws || Travis.config.pusher.to_h || {}).to_hash.slice(:scheme, :host, :port, :path, :key, :secure, :private) { host: Travis.config.client_domain || Travis.config.host, - github: V3::GitHub.client_config, pusher: pusher_config } end diff --git a/lib/travis/api/v3/services/repository/activate.rb b/lib/travis/api/v3/services/repository/activate.rb index 2263d58de2..a2e13fb435 100644 --- a/lib/travis/api/v3/services/repository/activate.rb +++ b/lib/travis/api/v3/services/repository/activate.rb @@ -9,26 +9,18 @@ def run! return repo_migrated if migrated?(repository) admin = access_control.admin_for(repository) - if Travis::Features.user_active?(:use_vcs, admin) || !admin.github? - remote_vcs_repository.set_hook( - repository_id: repository.id, - user_id: admin.id - ) - else - github(admin).set_hook(repository, true) - end + remote_vcs_repository.set_hook( + repository_id: repository.id, + user_id: admin.id + ) repository.update_attributes(active: true) if repository.private? || access_control.enterprise? - if Travis::Features.deactivate_owner(:use_vcs, admin) || !admin.github? - remote_vcs_repository.upload_key( - repository_id: repository.id, - user_id: admin.id, - read_only: !Travis::Features.owner_active?(:read_write_github_keys, repository.owner) - ) - else - github(admin).upload_key(repository) - end + remote_vcs_repository.upload_key( + repository_id: repository.id, + user_id: admin.id, + read_only: !Travis::Features.owner_active?(:read_write_github_keys, repository.owner) + ) end query.sync(access_control.user || access_control.admin_for(repository)) diff --git a/lib/travis/api/v3/services/repository/deactivate.rb b/lib/travis/api/v3/services/repository/deactivate.rb index 86d3b24368..c99be6feec 100644 --- a/lib/travis/api/v3/services/repository/deactivate.rb +++ b/lib/travis/api/v3/services/repository/deactivate.rb @@ -7,15 +7,11 @@ def run!(activate = false) admin = access_control.admin_for(repository) - if Travis::Features.user_active?(:use_vcs, admin) || !admin.github? - remote_vcs_repository.set_hook( - repository_id: repository.id, - user_id: admin.id, - activate: activate - ) - else - github(admin).set_hook(repository, activate) - end + remote_vcs_repository.set_hook( + repository_id: repository.id, + user_id: admin.id, + activate: activate + ) repository.update_attributes(active: activate) result repository diff --git a/lib/travis/config/defaults.rb b/lib/travis/config/defaults.rb index 72128f5c53..96ee3973c0 100644 --- a/lib/travis/config/defaults.rb +++ b/lib/travis/config/defaults.rb @@ -55,7 +55,7 @@ def fallback_logs_api_auth_token sidekiq: { namespace: 'sidekiq', pool_size: 1 }, smtp: {}, email: {}, - github: { api_url: 'https://api.github.com', token: 'travisbot-token' }, + github: { api_url: 'https://api.github.com' }, github_apps: { id: nil, private_pem: nil }, async: {}, notifications: [], # TODO rename to event.handlers diff --git a/lib/travis/errors.rb b/lib/travis/errors.rb index 939590c858..b7fa7858e4 100644 --- a/lib/travis/errors.rb +++ b/lib/travis/errors.rb @@ -7,6 +7,8 @@ def initialize(params) details = "with id=#{params[:repository_id] || params[:id]} " elsif params[:github_id] details = "with github_id=#{params[:github_id]} " + elsif params[:vcs_id] + details = "with vcs_id=#{params[:vcs_id]} " elsif params.key?(:slug) details = "with slug=#{params[:slug]} " elsif params.key?(:name) && params.key?(:owner_name) diff --git a/lib/travis/github.rb b/lib/travis/github.rb index a2fd9ad2eb..e2c0490055 100644 --- a/lib/travis/github.rb +++ b/lib/travis/github.rb @@ -1,28 +1,8 @@ -require 'gh' require 'core_ext/hash/compact' module Travis module Github - class << self - def setup - GH.set( - client_id: Travis.config.oauth2.client_id, - client_secret: Travis.config.oauth2.client_secret, - user_agent: "GH/#{GH::VERSION}", - origin: Travis.config.host, - api_url: Travis.config.github.api_url, - ssl: Travis.config.ssl.to_h.merge(Travis.config.github.ssl.to_h || {}).to_h.compact - ) - end - - def authenticated(user, &block) - fail "we don't have a github token for #{user.inspect}" if user.github_oauth_token.blank? - GH.with(:token => user.github_oauth_token, &block) - end - end - require 'travis/github/education' - require 'travis/github/oauth' require 'travis/github/services' end end diff --git a/lib/travis/github/education.rb b/lib/travis/github/education.rb index 765ecf3015..561e2eb1c7 100644 --- a/lib/travis/github/education.rb +++ b/lib/travis/github/education.rb @@ -16,30 +16,6 @@ def self.education_queue?(owner) end include Travis::Logging - - def student? - data['student'] - end - - def data - @data ||= fetch - end - - def fetch - Timeout::timeout(timeout) do - remote = GH::Remote.new - remote.setup('https://education.github.com/api', token: github_oauth_token) - response = remote.fetch_resource('/user') - JSON.parse(response.body) - end - rescue GH::Error, JSON::ParserError, Timeout::Error => e - log_exception(e) unless e.is_a? GH::Error and e.info[:response_status] == 401 - {} - end - - def timeout - Travis.config.education_endpoint_timeout || 2 - end end end end diff --git a/lib/travis/github/oauth.rb b/lib/travis/github/oauth.rb deleted file mode 100644 index 59348ab184..0000000000 --- a/lib/travis/github/oauth.rb +++ /dev/null @@ -1,51 +0,0 @@ -module Travis - module Github - module Oauth - class UpdateScopes < Struct.new(:user) - def run - update if update? - end - - private - - def update - user.github_scopes = Oauth.scopes_for(user) - user.save! - end - - def update? - token_changed?(user) or not Oauth.correct_scopes?(user) - end - - def token_changed?(user) - user.github_oauth_token_changed? or user.previous_changes.key?('github_oauth_token') - end - end - - class << self - def update_scopes(user) - UpdateScopes.new(user).run - end - - def correct_scopes?(user) - missing = wanted_scopes - user.github_scopes - missing.empty? - end - - def wanted_scopes - Travis.config.oauth2.scope.to_s.split(',').sort - end - - # TODO: Maybe this should move to gh? - def scopes_for(token) - token = token.github_oauth_token if token.respond_to? :github_oauth_token - scopes = GH.with(token: token.to_s) { GH.head('user') }.headers['x-oauth-scopes'] if token.present? - scopes &&= scopes.gsub(/\s/,'').split(',') - Array(scopes).sort - rescue GH::Error - [] - end - end - end - end -end diff --git a/lib/travis/github/services/set_hook.rb b/lib/travis/github/services/set_hook.rb index 32bd3b8116..1d97856a27 100644 --- a/lib/travis/github/services/set_hook.rb +++ b/lib/travis/github/services/set_hook.rb @@ -3,7 +3,6 @@ module Travis module API; end # Load-order issue - require 'travis/api/v3/github' module Github module Services @@ -11,15 +10,11 @@ class SetHook < Travis::Services::Base register :github_set_hook def run - if Travis::Features.user_active?(:use_vcs, current_user) || !current_user.github? - remote_vcs_repository.set_hook( - repository_id: repo.id, - user_id: current_user.id, - activate: active? - ) - else - v3_github.set_hook(repo, active?) - end + remote_vcs_repository.set_hook( + repository_id: repo.id, + user_id: current_user.id, + activate: active? + ) end private @@ -28,10 +23,6 @@ def active? params[:active] end - def v3_github - @v3_github ||= Travis::API::V3::GitHub.new(current_user, current_user.github_oauth_token) - end - def repo @repo ||= run_service(:find_repo, id: params[:id]) end diff --git a/lib/travis/github/services/set_key.rb b/lib/travis/github/services/set_key.rb index 04f896d7a5..a4d6e8acb1 100644 --- a/lib/travis/github/services/set_key.rb +++ b/lib/travis/github/services/set_key.rb @@ -25,16 +25,10 @@ def has_key? private def keys - if Travis::Features.user_active?(:use_vcs, current_user) || !current_user.github? - @keys ||= remote_vcs_repository.keys( - repository_id: repo.id, - user_id: current_user.id - ) - else - @keys ||= authenticated do - GH[keys_path] - end - end + @keys ||= remote_vcs_repository.keys( + repository_id: repo.id, + user_id: current_user.id + ) end def key @@ -42,47 +36,25 @@ def key end def set_key - read_only = !Travis::Features.owner_active?(:read_write_github_keys, repo.owner) - if Travis::Features.user_active?(:use_vcs, current_user) || !current_user.github? - remote_vcs_repository.upload_key( - repository_id: repo.id, - user_id: current_user.id, - read_only: read_only - ) - else - authenticated do - GH.post keys_path, { - title: Travis.config.host.to_s, - key: repo.key.encoded_public_key, - read_only: read_only - } - end - end + remote_vcs_repository.upload_key( + repository_id: repo.id, + user_id: current_user.id, + read_only: !Travis::Features.owner_active?(:read_write_github_keys, repo.owner) + ) end def delete_key - if Travis::Features.user_active?(:use_vcs, current_user) || !current_user.github? - remote_vcs_repository.delete_key( - repository_id: repo.id, - user_id: current_user.id, - id: key['id'] - ) - else - authenticated do - GH.delete "#{keys_path}/#{key['id']}" #key['_links']['self']['href'] - @keys = [] - end - end + remote_vcs_repository.delete_key( + repository_id: repo.id, + user_id: current_user.id, + id: key['id'] + ) end def keys_path "repos/#{repo.slug}/keys" end - def authenticated(&block) - Travis::Github.authenticated(current_user, &block) - end - class Instrument < Notification::Instrument def run_completed publish(:msg => "for #{target.repo.slug}", :result => result) diff --git a/lib/travis/model/organization.rb b/lib/travis/model/organization.rb index 50f1c33687..37ca365e3d 100644 --- a/lib/travis/model/organization.rb +++ b/lib/travis/model/organization.rb @@ -1,4 +1,3 @@ -require 'gh' require 'travis/model' class Organization < Travis::Model diff --git a/lib/travis/model/owner_group.rb b/lib/travis/model/owner_group.rb index 89b6f075ff..cf9fcf1dcc 100644 --- a/lib/travis/model/owner_group.rb +++ b/lib/travis/model/owner_group.rb @@ -1,4 +1,3 @@ -require 'gh' require 'travis/model' class OwnerGroup < Travis::Model diff --git a/lib/travis/model/request.rb b/lib/travis/model/request.rb index 68e5d68055..98f4ddfb17 100644 --- a/lib/travis/model/request.rb +++ b/lib/travis/model/request.rb @@ -102,7 +102,7 @@ def base_branch end def config_url - GH.full_url("repos/#{repository.slug}/contents/.travis.yml?ref=#{commit.commit}").to_s + "#{Travis.config.github.api_url}/repos/#{repository.slug}/contents/.travis.yml?ref=#{commit.commit}" end def same_repo_pull_request? diff --git a/lib/travis/model/user.rb b/lib/travis/model/user.rb index c878857f08..36d73cf904 100644 --- a/lib/travis/model/user.rb +++ b/lib/travis/model/user.rb @@ -1,6 +1,4 @@ -require 'gh' require 'travis/model' -require 'travis/github/oauth' class User < Travis::Model require 'travis/model/user/oauth' @@ -45,7 +43,7 @@ def token end def to_json - keys = %w/id login email name locale github_id gravatar_id is_syncing synced_at updated_at created_at/ + keys = %w/id login email name locale github_id vcs_id gravatar_id is_syncing synced_at updated_at created_at/ { 'user' => attributes.slice(*keys) }.to_json end diff --git a/lib/travis/model/user/oauth.rb b/lib/travis/model/user/oauth.rb index d8a9e5ab82..cbcafc012b 100644 --- a/lib/travis/model/user/oauth.rb +++ b/lib/travis/model/user/oauth.rb @@ -3,7 +3,7 @@ module Oauth class << self def find_or_create_by(payload) attrs = attributes_from(payload) - user = User.find_by_github_id(attrs['github_id']) + user = User.find_by_vcs_id(attrs['vcs_id']) user ? user.update_attributes(attrs) : user = User.create!(attrs) user end @@ -13,6 +13,7 @@ def attributes_from(payload) 'name' => payload['info']['name'], 'email' => payload['info']['email'], 'login' => payload['info']['nickname'], + 'vcs_id' => payload['uid'], 'github_id' => payload['uid'].to_i, 'github_oauth_token' => payload['credentials']['token'], 'gravatar_id' => payload['extra']['raw_info']['gravatar_id'] diff --git a/lib/travis/services.rb b/lib/travis/services.rb index ddae48158f..ba5ac87ca9 100644 --- a/lib/travis/services.rb +++ b/lib/travis/services.rb @@ -38,7 +38,6 @@ module Travis require 'travis/services/base' require 'travis/services/delete_caches' -require 'travis/services/find_admin' require 'travis/services/find_branch' require 'travis/services/find_branches' require 'travis/services/find_build' diff --git a/lib/travis/services/find_admin.rb b/lib/travis/services/find_admin.rb deleted file mode 100644 index ea4eac4f77..0000000000 --- a/lib/travis/services/find_admin.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'faraday/error' -require 'travis/services/base' - -# TODO extract github specific stuff to a separate service - -module Travis - module Services - class FindAdmin < Base - extend Travis::Instrumentation - include Travis::Logging - - register :find_admin - - def run - if repository - admin = candidates.first - admin || raise_admin_missing - else - error "[github-admin] repository is nil: #{params.inspect}" - raise Travis::RepositoryMissing, "no repository given" - end - end - instrument :run - - def repository - params[:repository] - end - - private - - def candidates - User.with_github_token.with_permissions(:repository_id => repository.id, :admin => true) - end - - def validate(user) - Timeout.timeout(2) do - data = Github.authenticated(user) { repository_data } - if data['permissions'] && data['permissions']['admin'] - user - else - info "[github-admin] #{user.login} no longer has admin access to #{repository.slug}" - update(user, data['permissions']) - false - end - end - rescue Timeout::Error => error - handle_error(user, error) - false - rescue GH::Error => error - handle_error(user, error) - false - end - - def handle_error(user, error) - status = error.info[:response_status] - case status - when 401 - error "[github-admin] token for #{user.login} no longer valid" - user.update_attributes!(:github_oauth_token => "") - when 404 - info "[github-admin] #{user.login} no longer has any access to #{repository.slug}" - update(user, {}) - else - error "[github-admin] error retrieving repository info for #{repository.slug} for #{user.login}: #{error.message}" - end - end - - # TODO should this not be memoized? - def repository_data - data = GH["repos/#{repository.slug}"] - info "[github-admin] could not retrieve data for #{repository.slug}" unless data - data || { 'permissions' => {} } - end - - def update(user, permissions) - user.update_attributes!(:permissions => permissions) - end - - def raise_admin_missing - raise Travis::AdminMissing.new("no admin available for #{repository.slug}") - end - - class Instrument < Notification::Instrument - def run_completed - publish( - msg: "for #{target.repository.slug}: #{result.login}", - result: result - ) - end - end - Instrument.attach_to(self) - end - end -end diff --git a/lib/travis/services/find_caches.rb b/lib/travis/services/find_caches.rb index 9f37b1284a..9528a60682 100644 --- a/lib/travis/services/find_caches.rb +++ b/lib/travis/services/find_caches.rb @@ -124,7 +124,7 @@ def branch end def prefix - prefix = "#{repo.vcs_id || repo.github_id}/" + prefix = "#{repo.vcs_id}/" prefix << branch << '/' if branch prefix end diff --git a/lib/travis/testing/factories.rb b/lib/travis/testing/factories.rb index 21b62e0c37..ea5091d5b4 100644 --- a/lib/travis/testing/factories.rb +++ b/lib/travis/testing/factories.rb @@ -81,7 +81,8 @@ last_build_number { '2' } last_build_started_at { Time.now.utc } last_build_finished_at { Time.now.utc } - sequence(:github_id) {|n| n } + sequence(:vcs_id) {|n| n } + github_id { |r| r.vcs_id } private { false } end diff --git a/lib/travis/testing/stubs.rb b/lib/travis/testing/stubs.rb index 89dda3ba1f..e658a4b934 100644 --- a/lib/travis/testing/stubs.rb +++ b/lib/travis/testing/stubs.rb @@ -55,7 +55,7 @@ def stub_repo(attributes = {}) last_build_state: :passed, last_build_duration: 60, github_language: 'ruby', - github_id: 549743, + vcs_id: 549743, builds_only_with_travis_yml?: false, settings: stub_settings, users_with_permission: [], @@ -251,7 +251,7 @@ def stub_worker(attributes = {}) def stub_user(attributes = {}) Stubs.stub 'user', attributes.reverse_merge( id: 1, - github_id: 1, + vcs_id: 1, organizations: [org], name: 'Sven Fuchs', login: 'svenfuchs', @@ -264,7 +264,7 @@ def stub_user(attributes = {}) is_syncing: false, synced_at: Time.now.utc - 3600, tokens: [double('token', token: 'token')], - github_scopes: Travis.config.oauth2.scopes.to_s.split(','), + github_scopes: Travis.config.oauth2.scope.to_s.split(','), created_at: Time.now.utc - 7200, first_logged_in_at: Time.now.utc - 5400, subscribed?: false, diff --git a/spec/auth/helpers.rb b/spec/auth/helpers.rb index b7709c2b83..2cebb3a219 100644 --- a/spec/auth/helpers.rb +++ b/spec/auth/helpers.rb @@ -4,7 +4,7 @@ RSpec.shared_context 'cache setup' do include Support::S3 before { Travis.config.cache_options = { s3: { bucket_name: '' , access_key_id: '', secret_access_key: ''} } } - before { s3_bucket << "#{repo.github_id}/master/cache--example1.tbz" } + before { s3_bucket << "#{repo.vcs_id}/master/cache--example1.tbz" } end RSpec::Matchers.define :auth do |expected| diff --git a/spec/auth/v2.1/users_spec.rb b/spec/auth/v2.1/users_spec.rb index 4544fcaa1c..ac1a0bc29b 100644 --- a/spec/auth/v2.1/users_spec.rb +++ b/spec/auth/v2.1/users_spec.rb @@ -1,6 +1,18 @@ describe 'v2.1 users', auth_helpers: true, api_version: :'v2.1', set_app: true do let(:user) { User.first } let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + let!(:request) do + WebMock.stub_request(:post, 'http://vcsfake.travis-ci.com/users/1/check_scopes') + .to_return( + status: 200, + body: nil, + ) + end + + before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' + end # TODO put /users/ # TODO put /users/:id ? diff --git a/spec/auth/v2/users_spec.rb b/spec/auth/v2/users_spec.rb index 0db97498a1..a15512b248 100644 --- a/spec/auth/v2/users_spec.rb +++ b/spec/auth/v2/users_spec.rb @@ -2,6 +2,16 @@ let(:user) { User.first } let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' + stub_request(:post, "http://vcsfake.travis-ci.com/users/#{user.id}/check_scopes") + .to_return( + status: 200, + body: nil, + ) + end + # TODO put /users/ # TODO put /users/:id ? # TODO post /users/sync diff --git a/spec/integration/v2/hooks_spec.rb b/spec/integration/v2/hooks_spec.rb index 6005db6d52..fea65c0619 100644 --- a/spec/integration/v2/hooks_spec.rb +++ b/spec/integration/v2/hooks_spec.rb @@ -1,8 +1,9 @@ require 'travis/testing/payloads' -require 'travis/api/v3/github' describe 'Hooks', set_app: true do - before(:each) do + before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' user.permissions.create repository: repo, admin: true end @@ -19,23 +20,29 @@ describe 'PUT /hooks' do # TODO really should be /hooks/1 let(:hook) { user.service_hooks.first } - let :payload do + let(:payload) do { :name => 'web', - :events => Travis::API::V3::GitHub::EVENTS, + :events => Travis::API::GITHUB_EVENTS, :active => true, :config => { url: 'notify.travis-ci.org' } } end + let!(:request) do + stub_request(:post, "http://vcsfake.travis-ci.com/repos/#{repo.id}/hook?user_id=#{repo.owner_id}") + .to_return( + status: 200, + body: nil, + ) + end before(:each) do Travis.config.service_hook_url = 'notify.travis-ci.org' - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(status: 200, body: '[]') - stub_request(:post, "https://api.github.com/repositories/#{repo.github_id}/hooks") end it 'sets the hook' do response = put 'hooks', { hook: { id: hook.id, active: 'true' } }, headers + expect(request).to have_been_made expect(repo.reload.active?).to eq(true) expect(response).to be_successful end diff --git a/spec/integration/v2/pusher_spec.rb b/spec/integration/v2/pusher_spec.rb index 096b0dc9a4..c40560d7da 100644 --- a/spec/integration/v2/pusher_spec.rb +++ b/spec/integration/v2/pusher_spec.rb @@ -1,8 +1,8 @@ describe Travis::Api::App::Endpoint::Pusher, set_app: true do let(:sven) { FactoryBot.create(:user) } let(:rkh) { FactoryBot.create(:user, login: 'rkh', name: 'Konstantin Haase') } - let(:travis) { FactoryBot.create(:repository, github_id: 200) } - let(:sinatra) { FactoryBot.create(:repository, name: 'sinatra', owner_name: 'sinatra', github_id: 300) } + let(:travis) { FactoryBot.create(:repository, vcs_id: 200) } + let(:sinatra) { FactoryBot.create(:repository, name: 'sinatra', owner_name: 'sinatra', vcs_id: 300) } let(:build) { FactoryBot.create(:build, request: request) } let(:commit) { FactoryBot.create(:commit) } let(:request) { FactoryBot.create(:request, owner: travis) } diff --git a/spec/integration/v2/users_spec.rb b/spec/integration/v2/users_spec.rb index 1d27ad538e..7eaa295921 100644 --- a/spec/integration/v2/users_spec.rb +++ b/spec/integration/v2/users_spec.rb @@ -3,10 +3,22 @@ let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json', 'HTTP_AUTHORIZATION' => "token #{token}" } } + before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' + end + context 'GET /users/:id' do let(:repo1) { FactoryBot.create(:repository, owner: user) } let(:org) { FactoryBot.create(:org) } let(:repo2) { FactoryBot.create(:repository, owner: org) } + let!(:request) do + WebMock.stub_request(:post, 'http://vcsfake.travis-ci.com/users/1/check_scopes') + .to_return( + status: 200, + body: nil, + ) + end before do user.permissions.create!(repository: repo1) diff --git a/spec/lib/github/oauth_spec.rb b/spec/lib/github/oauth_spec.rb deleted file mode 100644 index af416b4bea..0000000000 --- a/spec/lib/github/oauth_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -describe Travis::Github::Oauth do - let(:user) { FactoryBot.create(:user, github_oauth_token: 'token', github_scopes: scopes) } - - describe 'correct_scopes?' do - let(:scopes) { ['public_repo', 'user:email'] } - subject { described_class.correct_scopes?(user) } - - it 'accepts correct scopes' do - should eq true - end - - it 'complains about missing scopes' do - user.github_scopes.pop - should eq false - end - - it 'accepts additional scopes' do - user.github_scopes << 'foo' - should eq true - end - end - - describe 'update_scopes' do - before { user.reload } - - describe 'the token did not change' do - let(:scopes) { ['public_repo', 'user:email'] } - - it 'does not resolve github scopes' do - expect(Travis::Github::Oauth).not_to receive(:scopes_for) - described_class.update_scopes(user) - end - end - - describe 'the token has changed' do - let(:scopes) { ['public_repo', 'user:email'] } - - before do - user.github_oauth_token = 'changed' - user.save! - end - - it 'updates github scopes' do - expect(Travis::Github::Oauth).to receive(:scopes_for).and_return(['foo', 'bar']) - described_class.update_scopes(user) - expect(user.reload.github_scopes).to eq ['foo', 'bar'] - end - end - - describe 'no scopes have been set so far' do - let(:scopes) { nil } - - before do - user.github_oauth_token = 'changed' - user.save! - end - - it 'updates github scopes' do - expect(Travis::Github::Oauth).to receive(:scopes_for).and_return(['foo', 'bar']) - described_class.update_scopes(user) - expect(user.reload.github_scopes).to eq ['foo', 'bar'] - end - end - end -end diff --git a/spec/lib/github/services/set_key_spec.rb b/spec/lib/github/services/set_key_spec.rb index 292ebda0b5..0b2cad54e2 100644 --- a/spec/lib/github/services/set_key_spec.rb +++ b/spec/lib/github/services/set_key_spec.rb @@ -5,73 +5,83 @@ let(:key_path) { "#{keys_path}/1" } let(:keys) { [{ 'id' => 1, 'key' => SSL_KEYS[:public_base64], '_links' => { 'self' => { 'href' => key_path } } }] } let(:key) { SslKey.new(SSL_KEYS.slice(:private_key, :public_key)) } - let(:owner) { User.new(login: 'travis-ci') } - let(:repo) { Repository.new(owner_name: 'travis-ci', name: 'travis-core', key: key, owner: owner) } + let(:owner) { User.new(id: 1, login: 'travis-ci') } + let(:repo) { Repository.new(id: 1, owner_name: 'travis-ci', name: 'travis-core', key: key, owner: owner) } let(:params) { { id: repo.id } } let(:service) { described_class.new(user, params) } let(:publisher) { Travis::Notification::Publisher::Memory.new } let(:event) { publisher.events.last } + let!(:keys_request) do + WebMock.stub_request(:get, "http://vcsfake.travis-ci.com/repos/#{repo.id}/keys?user_id=#{owner.id}") + .to_return( + status: 200, + body: JSON.dump( + data: keys, + ) + ) + end + let!(:delete_request) do + WebMock.stub_request(:delete, "http://vcsfake.travis-ci.com/repos/#{repo.id}/keys/1?user_id=#{owner.id}") + .to_return( + status: 200, + body: nil, + ) + end + let!(:create_request) do + WebMock.stub_request(:post, "http://vcsfake.travis-ci.com/repos/#{repo.id}/keys?user_id=#{owner.id}&read_only=true") + .to_return( + status: 201, + body: nil, + ) + end - before :each do - allow(GH).to receive(:[]).and_return([]) - allow(GH).to receive(:post) - allow(GH).to receive(:delete) + before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' Travis::Notification.publishers.replace([publisher]) allow_any_instance_of(Travis::Services::FindRepo).to receive(:run).and_return(repo) end - it 'authenticates with the current user' do - expect(Travis::Github).to receive(:authenticated).with(user).at_least(:once).and_return([]) - service.run - end - describe 'given force: false' do - before :each do - params.update force: false + before do + params[:force] = false end it 'does not try to delete an existing key on github' do - allow(GH).to receive(:[]).with('repos/travis-ci/travis-core/keys').and_return(keys) - expect(GH).not_to receive(:delete) service.run + expect(delete_request).not_to have_been_made + expect(create_request).not_to have_been_made end - it 'sets the encoded public repository key to github if github does not have it' do - allow(GH).to receive(:[]).with(keys_path).and_return([]) - expect(GH).to receive(:post).with(keys_path, title: 'travis-ci.org', key: SSL_KEYS[:public_base64], read_only: true) - service.run - end + context 'when there are no keys' do + let(:keys) { [] } - it 'does not set anything to github if github already has the encoded public repository key' do - allow(GH).to receive(:[]).with('repos/travis-ci/travis-core/keys').and_return(keys) - expect(GH).not_to receive(:post) - service.run + it 'sets the encoded public repository key to github' do + service.run + expect(create_request).to have_been_made + end end end describe 'given force: true' do - before :each do - params.update force: true + before do + params[:force] = true end - it 'does not try to delete a key on github when no one exists' do - allow(GH).to receive(:[]).with('repos/travis-ci/travis-core/keys').and_return([]) - expect(GH).not_to receive(:delete) - service.run - end + context 'when no keys exist' do + let(:keys) { [] } - it 'deletes an existing key on github' do - allow(GH).to receive(:[]).with('repos/travis-ci/travis-core/keys').and_return(keys) - expect(GH).to receive(:delete).with(key_path) - service.run + it 'does not try to delete a key on github' do + service.run + expect(delete_request).not_to have_been_made + end end - it 'sets the encoded public repository key to github' do - allow(GH).to receive(:[]).with('repos/travis-ci/travis-core/keys').and_return(keys) - expect(GH).to receive(:post).with(keys_path, title: 'travis-ci.org', key: SSL_KEYS[:public_base64], read_only: true) + it 'deletes an existing key on github' do service.run + expect(delete_request).to have_been_made end end diff --git a/spec/lib/model/repository_spec.rb b/spec/lib/model/repository_spec.rb index 3b943563ee..fdee236e9d 100644 --- a/spec/lib/model/repository_spec.rb +++ b/spec/lib/model/repository_spec.rb @@ -49,8 +49,8 @@ describe 'by_params' do let(:minimal) { FactoryBot.create(:repository) } - it "should find a repository by it's github_id" do - expect(Repository.by_params(github_id: minimal.github_id).to_a.first).to eq(minimal) + it "should find a repository by its vcs_id" do + expect(Repository.by_params(vcs_id: minimal.vcs_id).to_a.first).to eq(minimal) end it "should find a repository by it's id" do diff --git a/spec/lib/model/request_spec.rb b/spec/lib/model/request_spec.rb index 1eba99401a..ee12a0adb9 100644 --- a/spec/lib/model/request_spec.rb +++ b/spec/lib/model/request_spec.rb @@ -6,23 +6,9 @@ let(:pull_request) { nil } describe 'config_url' do - before :each do - GH.options.delete(:api_url) - GH.current = nil - end - - after :each do - GH.set api_url: nil - end - it 'returns the api url to the .travis.yml file on github' do expect(request.config_url).to eq('https://api.github.com/repos/travis-ci/travis-core/contents/.travis.yml?ref=12345678') end - - it 'returns the api url to the .travis.yml file on github with a gh endpoint given' do - GH.set api_url: 'http://localhost/api/v3' - expect(request.config_url).to eq('http://localhost/api/v3/repos/travis-ci/travis-core/contents/.travis.yml?ref=12345678') - end end describe 'api_request?' do diff --git a/spec/lib/services/find_admin_spec.rb b/spec/lib/services/find_admin_spec.rb deleted file mode 100644 index 3d83b89645..0000000000 --- a/spec/lib/services/find_admin_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -describe Travis::Services::FindAdmin do - include Travis::Testing::Stubs - - describe 'find' do - let(:result) { described_class.new(nil, repository: repository).run } - - before :each do - allow(User).to receive(:with_permissions).with(:repository_id => repository.id, :admin => true).and_return [user] - end - - describe 'given a user has admin access to a repository (as seen by github)' do - before :each do - allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => true }) - end - - it 'returns that user' do - expect(result).to eq(user) - end - end - - describe 'given a user does not have access to a repository' do - before :each do - allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => false }) - allow(user).to receive(:update_attributes!) - end - - xit 'raises an exception' do - expect { result }.to raise_error(Travis::AdminMissing, 'no admin available for svenfuchs/minimal') - end - - xit 'revokes admin permissions for that user on our side' do - expect(user).to receive(:update_attributes!).with(:permissions => { 'admin' => false }) - ignore_exception { result } - end - end - - describe 'given an error occurs while retrieving the repository info' do - let(:error) { double('error', :backtrace => [], :response => double('response')) } - - before :each do - allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_raise(GH::Error.new(error)) - end - - xit 'raises an exception' do - expect { result }.to raise_error(Travis::AdminMissing, 'no admin available for svenfuchs/minimal') - end - - it 'does not revoke permissions' do - expect(user).not_to receive(:update_permissions!) - ignore_exception { result } - end - end - - describe 'missing repository' do - it 'raises Travis::RepositoryMissing' do - expect { described_class.new.run }.to raise_error(Travis::RepositoryMissing) - end - end - - def ignore_exception(&block) - block.call - rescue Travis::AdminMissing - end - end -end - -describe Travis::Services::FindAdmin::Instrument do - include Travis::Testing::Stubs - - let(:publisher) { Travis::Notification::Publisher::Memory.new } - let(:event) { publisher.events[1] } - let(:service) { Travis::Services::FindAdmin.new(nil, repository: repository) } - - before :each do - Travis::Notification.publishers.replace([publisher]) - allow(User).to receive(:with_permissions).with(repository_id: repository.id, admin: true).and_return [user] - allow(GH).to receive(:[]).with("repos/#{repository.slug}").and_return('permissions' => { 'admin' => true }) - service.run - end - - it 'publishes a event' do - expect(event).to publish_instrumentation_event( - event: 'travis.services.find_admin.run:completed', - message: 'Travis::Services::FindAdmin#run:completed for svenfuchs/minimal: svenfuchs', - result: user, - ) - end -end diff --git a/spec/lib/services/find_caches_spec.rb b/spec/lib/services/find_caches_spec.rb index 436168d81e..34deb0619f 100644 --- a/spec/lib/services/find_caches_spec.rb +++ b/spec/lib/services/find_caches_spec.rb @@ -23,9 +23,9 @@ describe 'with caches' do before do - s3_bucket << "#{repo.github_id}/master/cache--example1.tbz" - s3_bucket << "#{repo.github_id}/other/cache--example2.tbz" - s3_bucket << "#{repo.github_id.succ}/master/cache--example3.tbz" + s3_bucket << "#{repo.vcs_id}/master/cache--example1.tbz" + s3_bucket << "#{repo.vcs_id}/other/cache--example2.tbz" + s3_bucket << "#{repo.vcs_id.succ}/master/cache--example3.tbz" end its(:size) { is_expected.to eq 2 } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ccf1ddc656..7c72bc82ec 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,7 +13,6 @@ require 'rspec/its' require 'database_cleaner' require 'logger' -require 'gh' require 'multi_json' require 'pry' require 'stackprof' diff --git a/spec/support/payloads.rb b/spec/support/payloads.rb index 93140bf804..f402fe210a 100644 --- a/spec/support/payloads.rb +++ b/spec/support/payloads.rb @@ -525,6 +525,7 @@ 'name' => 'John', 'email' => 'john@email.example.com', 'login' => 'john', + 'vcs_id' => '234423', 'github_id' => 234423, 'github_oauth_token' => '1234567890abcdefg', 'gravatar_id' => '41193cdbffbf06be0cdf231b28c54b18' diff --git a/spec/unit/endpoint/accounts_spec.rb b/spec/unit/endpoint/accounts_spec.rb index 9b5a81474c..07b862e029 100644 --- a/spec/unit/endpoint/accounts_spec.rb +++ b/spec/unit/endpoint/accounts_spec.rb @@ -3,7 +3,7 @@ let(:access_token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } before do - allow(User).to receive(:find_by_github_id).and_return(user) + allow(User).to receive(:find_by_vcs_id).and_return(user) allow(User).to receive(:find).and_return(user) allow_any_instance_of(Travis::Services::FindUserAccounts).to receive(:owners_with_counts).and_return([{ 'owner_id' => user.id, 'owner_type' => 'User', 'repos_count' => 1}]) allow(user).to receive(:attributes).and_return(:id => user.id, :login => user.login, :name => user.name) diff --git a/spec/unit/endpoint/authorization/user_manager_spec.rb b/spec/unit/endpoint/authorization/user_manager_spec.rb deleted file mode 100644 index f6776c9933..0000000000 --- a/spec/unit/endpoint/authorization/user_manager_spec.rb +++ /dev/null @@ -1,110 +0,0 @@ -describe Travis::Api::App::Endpoint::Authorization::UserManager, billing_spec_helper: true do - let(:manager) { described_class.new(data, 'abc123') } - - before do - Travis::Features.enable_for_all(:education_data_sync) - allow(Travis::Github::Oauth).to receive(:update_scopes) # TODO test that scopes are being updated - end - - describe '#info' do - let(:data) { - { - name: 'Piotr Sarnacki', login: 'drogus', gravatar_id: '123', id: 456, foo: 'bar' - }.stringify_keys - } - - before { allow(manager).to receive(:education).and_return(false) } - - it 'gets data from github payload' do - expect(manager.info).to eq({ - name: 'Piotr Sarnacki', login: 'drogus', gravatar_id: '123', github_id: 456, education: false, vcs_id: 456 - }.stringify_keys) - end - - it 'allows to overwrite existing keys' do - expect(manager.info({login: 'piotr.sarnacki', bar: 'baz'}.stringify_keys)).to eq({ - name: 'Piotr Sarnacki', login: 'piotr.sarnacki', gravatar_id: '123', - github_id: 456, bar: 'baz', education: false, vcs_id: 456 - }.stringify_keys) - end - end - - describe '#fetch' do - let(:data) { - { login: 'drogus', id: 456 }.stringify_keys - } - - it 'drops the token when drop_token is set to true' do - user = double('user', login: 'drogus', github_id: 456, previous_changes: {}, recently_signed_up?: false, tokens: [double('token')]) - expect(User).to receive(:find_by_github_id).with(456).and_return(user) - - manager = described_class.new(data, 'abc123', true) - allow(manager).to receive(:education).and_return(false) - - attributes = { login: 'drogus', github_id: 456, education: false, vcs_id: 456 }.stringify_keys - - expect(user).to receive(:update_attributes).with(attributes) - - expect(manager.fetch).to eq(user) - end - - context 'with existing user' do - let!(:user) { FactoryBot.create(:user, login: 'drogus', github_id: 456, github_oauth_token: token) } - let(:token) { nil } - let(:billing_url) { 'http://billingfake.travis-ci.com' } - let(:billing_auth_key) { 'secret' } - - before do - allow(manager).to receive(:education).and_return(false) - Travis.config.billing.url = billing_url - Travis.config.billing.auth_key = billing_auth_key - stubbed_request = stub_billing_request(:post, "/v2/initial_subscription", auth_key: billing_auth_key, user_id: user.id) - .to_return(status: 201, body: JSON.dump(billing_v2_subscription_response_body('id' => 456, 'owner' => { 'type' => 'User', 'id' => user.id }))) - end - - context 'without any User#tokens record' do - before do - user.tokens.destroy_all - end - - it 'creates a User#tokens record' do - expect_any_instance_of(User).to receive(:create_a_token) - expect_any_instance_of(User).to receive(:tokens).and_return([]) - expect(manager.fetch).to eq(user) - end - end - - it 'updates user data' do - attributes = { login: 'drogus', github_id: 456, github_oauth_token: 'abc123', education: false, vcs_id: 456 }.stringify_keys - expect_any_instance_of(User).to receive(:update_attributes).with(attributes) - expect(manager.fetch).to eq(user) - end - end - - context 'without existing user' do - let(:user) { User.create(login: 'drogus', github_id: 456, vcs_id: 456) } - let(:attrs) { { login: 'drogus', github_id: 456, github_oauth_token: 'abc123', education: false, vcs_id: 456 }.stringify_keys } - - before do - allow(manager).to receive(:education).and_return(false) - allow(User).to receive(:create!).with(attrs).and_return(user) - end - - it 'creates new user' do - expect(User).to receive(:create!).with(attrs).and_return(user) - expect(manager.fetch).to eq(user) - end - end - end - - describe '#education' do - let(:data) { {} } - it 'runs students check with token' do - education = double(:education => nil) - expect(education).to receive(:student?).and_return(true) - expect(Travis::Github::Education).to receive(:new).with('abc123').and_return(education) - - expect(manager.education).to be_truthy - end - end -end diff --git a/spec/unit/endpoint/authorization_spec.rb b/spec/unit/endpoint/authorization_spec.rb index 39ff7723d2..535b60cf0e 100644 --- a/spec/unit/endpoint/authorization_spec.rb +++ b/spec/unit/endpoint/authorization_spec.rb @@ -3,6 +3,7 @@ let(:billing_url) { 'http://billingfake.travis-ci.com' } let(:billing_auth_key) { 'secret' } + let(:vcs_url) { 'http://vcsfake.travis-ci.com' } before do add_endpoint '/info' do @@ -11,29 +12,17 @@ end end - allow(user).to receive(:github_id).and_return(42) - allow(User).to receive(:find_github_id).and_return(user) + allow(user).to receive(:vcs_id).and_return(42) allow(User).to receive(:find).and_return(user) - @original_config = Travis.config.oauth2 - Travis.config.oauth2 = { - authorization_server: 'https://foobar.com', - access_token_path: '/access_token_path', - client_id: 'client-id', - client_secret: 'client-secret', - scope: 'public_repo,user:email,new_scope', - insufficient_access_redirect_url: 'https://travis-ci.org/insufficient_access' - } Travis.config.billing.url = billing_url Travis.config.billing.auth_key = billing_auth_key + Travis.config.vcs.url = vcs_url + Travis.config.vcs.token = 'vcs-token' WebMock.stub_request(:post, 'http://billingfake.travis-ci.com/v2/initial_subscription') .to_return(status: 200, body: JSON.dump(billing_v2_subscription_response_body('id' => 456, 'owner' => { 'type' => 'User', 'id' => user.id }))) end - after do - Travis.config.oauth2 = @original_config - end - it 'does not check auth' do expect(subject.settings.check_auth?).to eq false end @@ -47,13 +36,6 @@ end describe "GET /auth/handshake" do - before do - ENV['TRAVIS_SITE'] = 'org' - end - after do - ENV['TRAVIS_SITE'] = nil - end - describe 'evil hackers messing with the state' do before do WebMock.stub_request(:post, "https://foobar.com/access_token_path"). @@ -72,80 +54,30 @@ it 'does not succeed if state cookie mismatches (redirects)' do Travis.redis.sadd('github:states', 'github-state') - response = get '/auth/handshake?state=github-state&code=oauth-code' + response = get('/auth/handshake?state=github-state&code=oauth-code') expect(response.status).to eq(302) Travis.redis.srem('github:states', 'github-state') end end - describe 'On org, evil hackers messing with redirection' do + describe 'On com and enterprise, evil hackers messing with redirection' do before do - WebMock.stub_request(:post, "https://foobar.com/access_token_path") - .to_return(status: 200, body: 'access_token=token&token_type=bearer') - - WebMock.stub_request(:get, "https://api.github.com/user?per_page=100") + WebMock.stub_request(:post, 'http://vcsfake.travis-ci.com/users/session?code=1234&provider=github&redirect_uri=http://example.org/auth/handshake/github') .to_return( status: 200, - body: JSON.dump(name: 'Piotr Sarnacki', login: 'drogus', gravatar_id: '123', id: 456, foo: 'bar'), headers: {'X-OAuth-Scopes' => 'repo, user, new_scope'} + body: JSON.dump( + data: { + user: { + id: user.id, + } + } + ), ) - cookie_jar['travis.state-github'] = state - Travis.redis.sadd('github:states', state) - end - - after do - Travis.redis.srem('github:states', state) - end - - context 'when redirect uri is correct' do - let(:state) { 'github-state:::https://travis-ci.com/?any=params' } - - it 'it does allow redirect' do - response = get "/auth/handshake?code=1234&state=#{URI.encode(state)}" - expect(response.status).to eq(200) - end - end - - context 'when redirect uri is not allowed' do - let(:state) { 'github-state:::https://dark-corner-of-web.com/' } - - it 'does not allow redirect' do - response = get "/auth/handshake?code=1234&state=#{URI.encode(state)}" - expect(response.status).to eq(401) - expect(response.body).to eq("target URI not allowed") - end - end - - context 'when script tag is injected into redirect uri' do - let(:state) { 'github-state:::https://travis-ci.com/ 'repo, user, new_scope'} + body: nil, ) cookie_jar['travis.state-github'] = state @@ -160,7 +92,7 @@ context 'when redirect uri is correct' do let(:state) { 'github-state:::https://travis-ci.com/?any=params' } - it 'it does allow redirect' do + it 'does allow redirect' do response = get "/auth/handshake/github?code=1234&state=#{URI.encode(state)}" expect(response.status).to eq(200) end @@ -214,7 +146,6 @@ data = { 'id' => 111 } expect(data).to receive(:headers).and_return('x-oauth-scopes' => 'public_repo,user:email') - expect(GH).to receive(:with).with(token: 'foobarbaz-token', client_id: nil).and_return(data) end after do @@ -232,7 +163,7 @@ # TODO disabling this as per @rkh's advice xit 'redirects to insufficient access page for existing user' do user = double('user') - expect(User).to receive(:find_by_github_id).with(111).and_return(user) + expect(User).to receive(:find_by_vcs_id).with(111).and_return(user) expect { response = get '/auth/handshake?state=github-state&code=oauth-code' expect(response).to redirect_to('https://travis-ci.org/insufficient_access#existing-user') @@ -242,12 +173,23 @@ end describe 'POST /auth/github' do + let(:token) { 'private repos' } + let(:access_token) { 'access_123' } + let(:user_data) do + { 'id' => user.vcs_id, 'name' => user.name, 'login' => user.login, 'gravatar_id' => user.gravatar_id } + end + before do - data = { 'id' => user.github_id, 'name' => user.name, 'login' => user.login, 'gravatar_id' => user.gravatar_id } - allow(GH).to receive(:with).with(token: 'private repos', client_id: nil).and_return double(:[] => user.login, :headers => {'x-oauth-scopes' => 'repo'}, :to_hash => data) - allow(GH).to receive(:with).with(token: 'public repos', client_id: nil).and_return double(:[] => user.login, :headers => {'x-oauth-scopes' => 'public_repo'}, :to_hash => data) - allow(GH).to receive(:with).with(token: 'no repos', client_id: nil).and_return double(:[] => user.login, :headers => {'x-oauth-scopes' => 'user'}, :to_hash => data) - allow(GH).to receive(:with).with(token: 'invalid token', client_id: nil).and_raise(Faraday::Error::ClientError, 'CLIENT ERROR!') + WebMock.stub_request(:post, "http://vcsfake.travis-ci.com/users/session/generate_token?app_id=1&provider=github&token=#{CGI.escape(token)}"). + to_return( + status: 200, + body: JSON.dump( + data: { + user: user_data, + token: access_token, + }, + ), + ) end def get_token(github_token) @@ -255,52 +197,15 @@ def get_token(github_token) parsed_body['access_token'] end - def user_for(github_token) - get '/info/login', access_token: get_token(github_token) - expect(last_response.status).to eq(200) - user if user.login == body - end - - it 'accepts tokens with repo scope' do - expect(user_for('private repos').name).to eq(user.name) - end - - it 'accepts tokens with public_repo scope' do - expect(user_for('public repos').name).to eq(user.name) - end - - it 'rejects tokens with user scope' do - expect(post('/auth/github', github_token: 'no repos')).not_to be_ok - expect(body).not_to include('access_token') - end - - it 'rejects tokens with user scope' do - expect(post('/auth/github', github_token: 'invalid token')).not_to be_ok - expect(body).not_to include('access_token') - end - - it 'does not store the token' do - expect(user_for('public repos').github_oauth_token).not_to eq('public repos') + it 'returns an access token' do + expect(get_token(token)).to eq(access_token) end it "errors if no token is given" do - allow(User).to receive(:find_by_github_id).with(111).and_return(user) + allow(User).to receive(:find_by_vcs_id).with(111).and_return(user) expect(post("/auth/github")).not_to be_ok expect(last_response.status).to eq(422) expect(body).not_to include("access_token") end - - it "errors if github throws an error" do - allow(GH).to receive(:with).and_raise(GH::Error) - expect(post("/auth/github", github_token: 'foo bar')).not_to be_ok - expect(last_response.status).to eq(403) - expect(body).not_to include("access_token") - expect(body).to include("not a Travis user") - end - - it 'syncs the user' do - expect(Travis).to receive(:run_service).with(:sync_user, instance_of(User)) - expect(post('/auth/github', github_token: 'public repos')).to be_ok - end end end diff --git a/spec/unit/endpoint/users_spec.rb b/spec/unit/endpoint/users_spec.rb index b865c1179a..fdfe455abf 100644 --- a/spec/unit/endpoint/users_spec.rb +++ b/spec/unit/endpoint/users_spec.rb @@ -3,9 +3,17 @@ let(:access_token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } before do - allow(User).to receive(:find_by_github_id).and_return(user) + allow(User).to receive(:find_by_vcs_id).and_return(user) allow(User).to receive(:find).and_return(user) allow(user).to receive(:github_scopes).and_return(['public_repo', 'user:email']) + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' + + WebMock.stub_request(:post, 'http://vcsfake.travis-ci.com/users/1/check_scopes') + .to_return( + status: 200, + body: nil, + ) end it 'needs to be authenticated' do diff --git a/spec/unit/serialize/v2/http/user_spec.rb b/spec/unit/serialize/v2/http/user_spec.rb index d938ba728d..f5ea9a1e84 100644 --- a/spec/unit/serialize/v2/http/user_spec.rb +++ b/spec/unit/serialize/v2/http/user_spec.rb @@ -22,8 +22,17 @@ 'vcs_type' => 'GithubUser' } } + let!(:request) do + WebMock.stub_request(:post, 'http://vcsfake.travis-ci.com/users/1/check_scopes') + .to_return( + status: 200, + body: nil, + ) + end before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' allow(user).to receive(:github_scopes).and_return(['public_repo', 'user:email']) end diff --git a/spec/v3/services/caches/delete_spec.rb b/spec/v3/services/caches/delete_spec.rb index 164c15639e..8f1a59177d 100644 --- a/spec/v3/services/caches/delete_spec.rb +++ b/spec/v3/services/caches/delete_spec.rb @@ -111,7 +111,7 @@ 1000 false - #{repo.github_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz + #{repo.vcs_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz 2009-10-12T17:50:30.000Z "hgb9dede5f27731c9771645a39863328" 20308738 diff --git a/spec/v3/services/caches/find_spec.rb b/spec/v3/services/caches/find_spec.rb index 5071dcf2d0..fa27abdeff 100644 --- a/spec/v3/services/caches/find_spec.rb +++ b/spec/v3/services/caches/find_spec.rb @@ -130,7 +130,7 @@ 1000 false - #{repo.github_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz + #{repo.vcs_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz 2009-10-12T17:50:30.000Z "hgb9dede5f27731c9771645a39863328" 20308738 @@ -141,7 +141,7 @@ - #{repo.github_id}/master/cache-linux-precise-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-default--gemfile-Gemfile.tgz + #{repo.vcs_id}/master/cache-linux-precise-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-default--gemfile-Gemfile.tgz 2009-10-12T17:50:30.000Z "1b2cf535f27731c974343645a3985328" 64994 @@ -165,7 +165,7 @@ 1000 false - #{repo.github_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz + #{repo.vcs_id}/ha-bug-rm_rf/cache-linux-precise-lkjdhfsod8fu4tc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855--rvm-2.2.5--gemfile-Gemfile.tgz 2009-10-12T17:50:30.000Z "hgb9dede5f27731c9771645a39863328" 20308738 diff --git a/spec/v3/services/owner/find_spec.rb b/spec/v3/services/owner/find_spec.rb index 5f277a96f1..93fee48ac4 100644 --- a/spec/v3/services/owner/find_spec.rb +++ b/spec/v3/services/owner/find_spec.rb @@ -1,7 +1,7 @@ describe Travis::API::V3::Services::Owner::Find, set_app: true do describe "organization" do - let(:org) { Travis::API::V3::Models::Organization.new(login: 'example-org', github_id: 1234) } + let(:org) { Travis::API::V3::Models::Organization.new(login: 'example-org', github_id: 1234, vcs_id: 1234) } before { org.save! } after { org.delete } @@ -105,7 +105,7 @@ "name" => "example-repo", "slug" => "example-org/example-repo", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "example-org", @@ -179,7 +179,7 @@ "name" => "example-repo", "slug" => "example-org/example-repo", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "example-org", @@ -265,7 +265,7 @@ end describe "user" do - let(:user) { Travis::API::V3::Models::User.new(login: 'example-user', github_id: 5678) } + let(:user) { Travis::API::V3::Models::User.new(login: 'example-user', github_id: 5678, vcs_id: 5678) } before { user.save! } after { user.delete } diff --git a/spec/v3/services/repositories/for_current_user_spec.rb b/spec/v3/services/repositories/for_current_user_spec.rb index a9d3817920..3e33cd6e51 100644 --- a/spec/v3/services/repositories/for_current_user_spec.rb +++ b/spec/v3/services/repositories/for_current_user_spec.rb @@ -55,7 +55,7 @@ "name" => "minimal", "slug" => "svenfuchs/minimal", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", diff --git a/spec/v3/services/repositories/for_owner_spec.rb b/spec/v3/services/repositories/for_owner_spec.rb index 9a3c3bfbfb..1445a58425 100644 --- a/spec/v3/services/repositories/for_owner_spec.rb +++ b/spec/v3/services/repositories/for_owner_spec.rb @@ -17,6 +17,13 @@ before { repo.update_attribute(:current_build, build) } after { repo.update_attribute(:private, false) } before { RSpec::Support::ObjectFormatter.default_instance.max_formatted_output_length = 1024*1024 } + before do + stub_billing_request(:get, "/usage/users/1/allowance", auth_key: billing_auth_key, user_id: 1) + .to_return(body: JSON.dump({ 'public_repos': true, 'private_repos': true, 'user_usage': true, 'pending_user_licenses': false, 'concurrency_limit': 666 })) + + Travis.config.billing.url = billing_url + Travis.config.billing.auth_key = billing_auth_key + end describe "sorting by default_branch.last_build" do let!(:repo2) { Travis::API::V3::Models::Repository.create!(owner_name: 'svenfuchs', owner: repo.owner, name: 'second-repo', default_branch_name: 'other-branch') } @@ -82,7 +89,7 @@ "name" => "minimal", "slug" => "svenfuchs/minimal", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", @@ -135,7 +142,7 @@ "name" =>"minimal", "slug" =>"svenfuchs/minimal", "description" =>nil, - "github_id" =>repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", @@ -250,7 +257,7 @@ "name" => "minimal", "slug" => "svenfuchs/minimal", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", @@ -411,7 +418,7 @@ "name" => "minimal", "slug" => "svenfuchs/minimal", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", @@ -457,7 +464,7 @@ "name" => "maximal", "slug" => "svenfuchs/maximal", "description" => nil, - "github_id" => repo2.github_id, + "github_id" => repo2.vcs_id, "vcs_id" => repo2.vcs_id, "vcs_type" => repo2.vcs_type, "owner_name" => "svenfuchs", @@ -529,7 +536,7 @@ "name" => "sharedrepo", "slug" => "sharedrepoowner/sharedrepo", "description" => nil, - "github_id" => sharedrepo.github_id, + "github_id" => sharedrepo.vcs_id.to_i, "vcs_id" => sharedrepo.vcs_id, "vcs_type" => sharedrepo.vcs_type, "owner_name" => "sharedrepoowner", @@ -557,29 +564,9 @@ }]}} end - describe "allowance, org" do - before { get("/v3/owner/svenfuchs/allowance", {}, headers) } - example { expect(last_response).to be_ok } - example { expect(JSON.load(body)).to be == { - "@representation" => "standard", - "@type" => "allowance", - "concurrency_limit" => 1, - "private_repos" => false, - "public_repos" => true, - "subscription_type" => 1, - "user_usage" => false, - "pending_user_licenses" => false, - "id" => 0 - }} - end - describe "allowance, com" do before do Travis.config.host = 'travis-ci.com' - Travis.config.billing.url = billing_url - Travis.config.billing.auth_key = billing_auth_key - stub_billing_request(:get, "/usage/users/1/allowance", auth_key: billing_auth_key, user_id: 1) - .to_return(body: JSON.dump({ 'public_repos': true, 'private_repos': true, 'user_usage': true, 'pending_user_licenses': false, 'concurrency_limit': 666 })) get("/v3/owner/svenfuchs/allowance", {}, headers) end example { expect(last_response).to be_ok } diff --git a/spec/v3/services/repository/activate_spec.rb b/spec/v3/services/repository/activate_spec.rb index aa2aafe0c8..057f7666e4 100644 --- a/spec/v3/services/repository/activate_spec.rb +++ b/spec/v3/services/repository/activate_spec.rb @@ -1,17 +1,10 @@ describe Travis::API::V3::Services::Repository::Activate, set_app: true do - let(:sidekiq_job) { Sidekiq::Client.last.deep_symbolize_keys } let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first } before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' repo.update_attributes!(active: false) - @original_sidekiq = Sidekiq::Client - Sidekiq.send(:remove_const, :Client) # to avoid a warning - Sidekiq::Client = [] - end - - after do - Sidekiq.send(:remove_const, :Client) # to avoid a warning - Sidekiq::Client = @original_sidekiq end describe "not authenticated" do @@ -38,12 +31,7 @@ end describe "existing repository, push access" do - let(:webhook_payload) { JSON.dump(name: 'web', events: Travis::API::V3::GitHub::EVENTS, active: true, config: { url: Travis.config.service_hook_url || '', insecure_ssl: false }) } - let(:service_hook_payload) { JSON.dump(events: Travis::API::V3::GitHub::EVENTS, active: false) } - before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, admin: true, pull: true, push: true) } - before { allow_any_instance_of(Travis::API::V3::GitHub).to receive(:upload_key) } - before { stub_request(:any, %r(https://api.github.com/repositories/#{repo.github_id}/hooks(/\d+)?)) } around do |ex| Travis.config.service_hook_url = 'https://url.of.listener.something' @@ -51,159 +39,18 @@ Travis.config.service_hook_url = nil end - context 'queues sidekiq job' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(status: 200, body: '[]') - post("/v3/repo/#{repo.id}/activate", {}, headers) - end - - example do - expect(sidekiq_job).to eq( - queue: :sync, - class: 'Travis::GithubSync::Worker', - args: [:sync_repo, repo_id: 1, user_id: 1] - ) - end - end - - context 'when both service hook and webhook exist' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return( - status: 200, body: JSON.dump( - [ - { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } }, - { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } } - ] - ) - ) - post("/v3/repo/#{repo.id}/activate", {}, headers) - end - - context 'enterprise' do - around do |ex| - Travis.config.enterprise = true - ex.run - Travis.config.enterprise = false - end - - example 'deactivates service hook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload).once - end - end - - example 'no longer deactivates service hook' do - expect(WebMock).not_to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload) - end - - example 'updates webhook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/456").with(body: webhook_payload).once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => true - ) - end - end - - context 'when webhook exists' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return( - status: 200, body: JSON.dump( - [ - { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } } - ] - ) - ) - post("/v3/repo/#{repo.id}/activate", {}, headers) - end - - example 'updates webhook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/456").with(body: webhook_payload).once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => true - ) - end - - context 'when ssl verification has been disabled' do - let(:webhook_payload) { JSON.dump(name: 'web', events: Travis::API::V3::GitHub::EVENTS, active: true, config: { url: Travis.config.service_hook_url || '', insecure_ssl: true }) } - - around do |ex| - Travis.config.ssl = { verify: false } - ex.run - Travis.config.ssl = {} - end - - example 'updates webhook with ssl disabled' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/456").with(body: webhook_payload).once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => true + context 'request' do + let!(:request) do + stub_request(:post, "http://vcsfake.travis-ci.com/repos/#{repo.id}/hook?user_id=#{repo.owner_id}") + .to_return( + status: 200, + body: nil, ) - end - end - end - - context 'when webhook does not exist' do - let(:webhook_payload) { JSON.dump(name: 'web', events: Travis::API::V3::GitHub::EVENTS, active: true, config: { url: Travis.config.service_hook_url || '', insecure_ssl: false }) } - - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return(status: 200, body: '[]') - post("/v3/repo/#{repo.id}/activate", {}, headers) - end - - example 'creates webhook' do - expect(WebMock).to have_requested(:post, "https://api.github.com/repositories/#{repo.github_id}/hooks").with(body: webhook_payload).once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => true - ) end - context 'when ssl verification has been disabled' do - let(:webhook_payload) { JSON.dump(name: 'web', events: Travis::API::V3::GitHub::EVENTS, active: true, config: { url: Travis.config.service_hook_url || '', insecure_ssl: true}) } - - around do |ex| - Travis.config.ssl = { verify: false } - ex.run - Travis.config.ssl = {} - end - - example 'requests webhooks without ssl verification' do - expect(WebMock).to have_requested(:post, "https://api.github.com/repositories/#{repo.github_id}/hooks").with(body: webhook_payload).once - end - end - end - - context 'when the repo ssh key does not exist' do - before do - repo.key.destroy - post("/v3/repo/#{repo.id}/activate", {}, headers) - end - - example { expect(last_response.status).to eq 409 } - example do - expect(JSON.load(body)).to eq( - '@type' => 'error', - 'error_type' => 'repo_ssh_key_missing', - 'error_message' => 'request cannot be completed because the repo ssh key is still pending to be created. please retry in a bit, or try syncing the repository if this condition does not resolve' - ) + post("/v3/repo/#{repo.id}/activate", {}, headers) + expect(request).to have_been_made end end end @@ -264,7 +111,14 @@ context do let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } let(:headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } } - before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, admin: true, push: true, pull: true) } + before do + Travis.config.host = 'http://travis-ci.org' + Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, admin: true, push: true, pull: true) + end + + after do + Travis.config.host = 'http://travis-ci.com' + end describe "repo migrating" do before { repo.update_attributes(migration_status: "migrating") } @@ -278,7 +132,7 @@ }} end - describe "repo migrating" do + describe "repo migrated" do before { repo.update_attributes(migration_status: "migrated") } before { post("/v3/repo/#{repo.id}/activate", {}, headers) } diff --git a/spec/v3/services/repository/deactivate_spec.rb b/spec/v3/services/repository/deactivate_spec.rb index a7479dc285..676188640a 100644 --- a/spec/v3/services/repository/deactivate_spec.rb +++ b/spec/v3/services/repository/deactivate_spec.rb @@ -2,6 +2,8 @@ let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first } before do + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' repo.update_attributes!(active: true) end @@ -32,7 +34,6 @@ shared_examples 'repository deactivation' do describe 'existing repository, wrong access' do before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true) } - before { stub_request(:any, %r(api.github.com/repositories/#{repo.github_id}/hooks(/\d+)?)) } before { post("/v3/repo/#{repo.id}/deactivate", {}, headers) } example 'is success' do @@ -45,11 +46,15 @@ end describe "existing repository, admin and push access" do - let(:webhook_payload) { JSON.dump(name: 'web', events: Travis::API::V3::GitHub::EVENTS, active: false, config: { url: Travis.config.service_hook_url || '', insecure_ssl: false }) } - let(:service_hook_payload) { JSON.dump(events: Travis::API::V3::GitHub::EVENTS, active: false) } + let!(:request) do + stub_request(:delete, "http://vcsfake.travis-ci.com/repos/#{repo.id}/hook?user_id=#{repo.owner_id}") + .to_return( + status: 200, + body: nil, + ) + end before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, admin: true, push: true) } - before { stub_request(:any, %r(api.github.com/repositories/#{repo.github_id}/hooks(/\d+)?)) } around do |ex| Travis.config.service_hook_url = 'https://url.of.listener.something' @@ -57,116 +62,15 @@ Travis.config.service_hook_url = nil end - context 'when repo has service hook and webhook' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return( - status: 200, body: JSON.dump( - [ - { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } }, - { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } } - ] - ) - ) - post("/v3/repo/#{repo.id}/deactivate", {}, headers) - end - - context 'enterprise' do - around do |ex| - Travis.config.enterprise = true - ex.run - Travis.config.enterprise = false - end - - example 'deactivates service hook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload).once - end - end - - example 'no longer deactivates service hook' do - expect(WebMock).not_to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload) - end - - example 'updates webhook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/456").with(body: webhook_payload).once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => false - ) - end - end - - context 'when repo has only service hook' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return( - status: 200, body: JSON.dump( - [ - { name: 'travis', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/123", config: { domain: 'https://url.of.listener.something' } } - ] - ) - ) - post("/v3/repo/#{repo.id}/deactivate", {}, headers) - end - - context 'enterprise' do - around do |ex| - Travis.config.enterprise = true - ex.run - Travis.config.enterprise = false - end - - example 'deactivates service hook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload).once - end - end + example 'creates webhook' do + post("/v3/repo/#{repo.id}/deactivate", {}, headers) + expect(request).to have_been_made - example 'no longer deactivates service hook' do - expect(WebMock).not_to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123").with(body: service_hook_payload) - end - - example 'creates webhook' do - expect(WebMock).to have_requested(:post, "https://api.github.com/repositories/#{repo.github_id}/hooks").once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => false - ) - end - end - - context 'when repo has only webhook' do - before do - stub_request(:get, "https://api.github.com/repositories/#{repo.github_id}/hooks?per_page=100").to_return( - status: 200, body: JSON.dump( - [ - { name: 'web', url: "https://api.github.com/repositories/#{repo.github_id}/hooks/456", config: { url: Travis.config.service_hook_url } } - ] - ) - ) - post("/v3/repo/#{repo.id}/deactivate", {}, headers) - end - - example 'does nothing with service hook' do - expect(WebMock).not_to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/123") - end - - example 'updates webhook' do - expect(WebMock).to have_requested(:patch, "https://api.github.com/repositories/#{repo.github_id}/hooks/456").once - end - - example 'is success' do - expect(last_response.status).to eq 200 - expect(JSON.load(body)).to include( - '@type' => 'repository', - 'active' => false - ) - end + expect(last_response.status).to eq 200 + expect(JSON.load(body)).to include( + '@type' => 'repository', + 'active' => false + ) end end end @@ -244,7 +148,7 @@ }} end - describe "repo migrating" do + describe "repo migrated" do before { repo.update_attributes(migration_status: "migrated") } before { post("/v3/repo/#{repo.id}/deactivate", {}, headers) } diff --git a/spec/v3/services/repository/find_spec.rb b/spec/v3/services/repository/find_spec.rb index 1ffb9c4c26..86e3d973c2 100644 --- a/spec/v3/services/repository/find_spec.rb +++ b/spec/v3/services/repository/find_spec.rb @@ -77,7 +77,7 @@ "name" => "minimal", "slug" => "svenfuchs/minimal", "description" => nil, - "github_id" => repo.github_id, + "github_id" => repo.vcs_id.to_i, "vcs_id" => repo.vcs_id, "vcs_type" => repo.vcs_type, "owner_name" => "svenfuchs", @@ -143,7 +143,8 @@ owner_id: 1, owner_type: "User", last_build_state: "passed", - github_id: 12345 + vcs_id: 12345, + github_id: 12345, ) get("/v3/repo/svenfuchs%2FMinimal") } @@ -365,7 +366,7 @@ describe "including full owner" do before { get("/v3/repo/#{repo.id}?include=repository.owner") } example { expect(last_response).to be_ok } - example { expect(parsed_body['owner']).to include("github_id", "is_syncing", "synced_at", + example { expect(parsed_body['owner']).to include("vcs_id", "github_id", "is_syncing", "synced_at", "@type" => "user", "id" => repo.owner_id, "login" => "svenfuchs", @@ -422,7 +423,7 @@ describe "including full owner" do before { get("/v3/repo/#{repo.id}?include=repository.owner") } example { expect(last_response).to be_ok } - example { expect(parsed_body['owner']).to include("github_id", "is_syncing", "synced_at")} + example { expect(parsed_body['owner']).to include("vcs_id", "github_id", "is_syncing", "synced_at")} end describe "when owner is missing" do diff --git a/spec/v3/services/repository/migrate_spec.rb b/spec/v3/services/repository/migrate_spec.rb index 7120a2a4ec..ef311dd618 100644 --- a/spec/v3/services/repository/migrate_spec.rb +++ b/spec/v3/services/repository/migrate_spec.rb @@ -13,7 +13,7 @@ context "has admin permissions to the repo" do before { Travis::API::V3::Models::Permission.create(repository: repo, user: user, admin: true) } before do - stub_request(:post, "https://merge.localhost/api/repo/by_github_id/#{repo.github_id}/migrate") + stub_request(:post, "https://merge.localhost/api/repo/by_github_id/#{repo.vcs_id}/migrate") end it "makes a request to the merge app" do diff --git a/spec/v3/services/request/preview_spec.rb b/spec/v3/services/request/preview_spec.rb index 4b2bc63cb3..6801766b67 100644 --- a/spec/v3/services/request/preview_spec.rb +++ b/spec/v3/services/request/preview_spec.rb @@ -98,7 +98,7 @@ def parse(str) expect(WebMock).to have_requested(:post, %r(/configs)).with { |req| expect(parse(req.body)).to eq( repo: { - github_id: repo.github_id, + vcs_id: repo.vcs_id, slug: repo.slug, token: 'github_oauth_token', private: false, diff --git a/spec/v3/services/requests/create_spec.rb b/spec/v3/services/requests/create_spec.rb index 601993b6d2..c0bd4dff7f 100644 --- a/spec/v3/services/requests/create_spec.rb +++ b/spec/v3/services/requests/create_spec.rb @@ -123,7 +123,7 @@ let(:payload) do { repository: { - id: repo.github_id, + id: repo.vcs_id, vcs_type: repo.vcs_type, owner_name: 'svenfuchs', name: 'minimal' diff --git a/spec/v3/services/user/current_spec.rb b/spec/v3/services/user/current_spec.rb index e9241e8194..bc1e665684 100644 --- a/spec/v3/services/user/current_spec.rb +++ b/spec/v3/services/user/current_spec.rb @@ -25,9 +25,9 @@ "education" => nil, "allow_migration" => false, "allowance" => { - "@type" => "allowance", - "@representation" => "minimal", - "id" => user.id + "@type" => "allowance", + "@representation" => "minimal", + "id" => user.id, }, "recently_signed_up"=>false, "secure_user_hash" => nil diff --git a/spec/v3/services/user/find_spec.rb b/spec/v3/services/user/find_spec.rb index ff4a200170..559e8708a9 100644 --- a/spec/v3/services/user/find_spec.rb +++ b/spec/v3/services/user/find_spec.rb @@ -3,7 +3,7 @@ let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: 1) } let(:headers) {{ 'HTTP_AUTHORIZATION' => "token #{token}" }} - + let(:billing_url) { 'http://billingfake.travis-ci.com' } let(:billing_auth_key) { 'secret' } @@ -14,7 +14,7 @@ Travis.config.billing.url = billing_url Travis.config.billing.auth_key = billing_auth_key stub_billing_request(:get, "/usage/users/#{user.id}/allowance", auth_key: billing_auth_key, user_id: user.id) - .to_return(body: JSON.dump({ 'public_repos': true, 'private_repos': true, 'user_usage': true, 'pending_user_licenses': false, 'concurrency_limit': 666 })) + .to_return(body: JSON.dump('public_repos' => true, 'private_repos' => true, 'concurrency_limit' => 666)) end describe "authenticated as user with access" do @@ -29,7 +29,7 @@ "login" => "svenfuchs", "name" => "Sven Fuchs", "email" => "sven@fuchs.com", - "github_id" => user.github_id, + "github_id" => user.vcs_id, "vcs_id" => user.vcs_id, "vcs_type" => user.vcs_type, "avatar_url" => "https://0.gravatar.com/avatar/07fb84848e68b96b69022d333ca8a3e2", @@ -38,9 +38,9 @@ "education" => true, "allow_migration" => false, "allowance" => { - "@type" => "allowance", - "@representation" => "minimal", - "id" => user.id + "@type" => "allowance", + "@representation" => "minimal", + "id" => user.id, }, "recently_signed_up"=>false, "secure_user_hash" => nil, diff --git a/spec/v3/services/user/sync_spec.rb b/spec/v3/services/user/sync_spec.rb index c27d2aa504..d7cc6993d1 100644 --- a/spec/v3/services/user/sync_spec.rb +++ b/spec/v3/services/user/sync_spec.rb @@ -3,6 +3,13 @@ let(:user2) { Travis::API::V3::Models::User.create(login: 'carlad', is_syncing: true) } let(:sidekiq_payload) { JSON.load(Sidekiq::Client.last['args'].to_json) } let(:sidekiq_params) { Sidekiq::Client.last['args'].last.deep_symbolize_keys } + let!(:request) do + stub_request(:post, 'http://vcsfake.travis-ci.com/users/1/sync_data') + .to_return( + status: 200, + body: nil, + ) + end before do user.update_attribute(:is_syncing, false) @@ -10,6 +17,9 @@ @original_sidekiq = Sidekiq::Client Sidekiq.send(:remove_const, :Client) # to avoid a warning Sidekiq::Client = [] + Travis.config.vcs.url = 'http://vcsfake.travis-ci.com' + Travis.config.vcs.token = 'vcs-token' + end after do @@ -60,10 +70,7 @@ "true") } - example { expect(sidekiq_payload).to be == ['sync_user', 'user_id' => 1] } - - example { expect(Sidekiq::Client.last['queue']).to be == :sync } - example { expect(Sidekiq::Client.last['class']).to be == 'Travis::GithubSync::Worker' } + example { expect(request).to have_been_made } end describe "existing user, current user does not have sync access " do diff --git a/spec/v3/services/v2_subscription/executions_spec.rb b/spec/v3/services/v2_subscription/executions_spec.rb index 6c1b57bcec..c68eb2e6b0 100644 --- a/spec/v3/services/v2_subscription/executions_spec.rb +++ b/spec/v3/services/v2_subscription/executions_spec.rb @@ -113,7 +113,7 @@ "slug"=>"svenfuchs/minimal", "description"=>nil, "github_id"=>1, - "vcs_id"=>nil, + "vcs_id"=>'1', "vcs_type"=>"GithubRepository", "github_language"=>nil, "active"=>true, diff --git a/spec/v3/services/v2_subscriptions/create_spec.rb b/spec/v3/services/v2_subscriptions/create_spec.rb index 56cf1984a0..3109fb3259 100644 --- a/spec/v3/services/v2_subscriptions/create_spec.rb +++ b/spec/v3/services/v2_subscriptions/create_spec.rb @@ -252,7 +252,6 @@ 'addon_id' => 7, 'addon_quantity' => 40_000, 'addon_usage' => 0, - 'remaining' => 40000, 'purchase_date' => '2017-11-28T00:09:59.502Z', 'valid_to' => '2017-11-28T00:09:59.502Z', 'remaining' => 40_000, From a3e5693d591129506c6b3d983bc06fee15e32e87 Mon Sep 17 00:00:00 2001 From: Artur Kremens Date: Fri, 26 Feb 2021 17:18:59 +0100 Subject: [PATCH 2/2] Update readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 7d62a5c0bc..0533d14c85 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ https://api.travis-ci.org ## WARNING!!!!! Master branch is designed for .com only. If you would like to deploy changes for .org please use org-only branch + ## Requirements You will need the following packages to get travis-api to work: