diff --git a/lib/travis/api/app/endpoint/authorization.rb b/lib/travis/api/app/endpoint/authorization.rb index ce48fd49..fc735379 100644 --- a/lib/travis/api/app/endpoint/authorization.rb +++ b/lib/travis/api/app/endpoint/authorization.rb @@ -250,7 +250,7 @@ class Travis::Api::App scopes = parse_scopes data.headers['x-oauth-scopes'] manager = UserManager.new(data, token, drop_token) - unless acceptable? scopes + 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? @@ -280,19 +280,25 @@ class Travis::Api::App AccessToken.create(options).token end - def acceptable?(scopes) + def acceptable?(scopes, lossy = false) User::Oauth.wanted_scopes.all? do |scope| - acceptable_scopes_for(scope).any? { |s| scopes.include? s } + acceptable_scopes_for(scope, lossy).any? { |s| scopes.include? s } end end - def acceptable_scopes_for(scope) - case scope = scope.to_s - when /^user/ then ['user', scope, 'public_repo', 'repo'] - when /^(.+):/ then [$1, scope] - when 'public_repo' then [scope, 'repo'] - else [scope] + 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) diff --git a/spec/unit/endpoint/authorization_spec.rb b/spec/unit/endpoint/authorization_spec.rb index e8425082..bb91f4bc 100644 --- a/spec/unit/endpoint/authorization_spec.rb +++ b/spec/unit/endpoint/authorization_spec.rb @@ -13,6 +13,20 @@ describe Travis::Api::App::Endpoint::Authorization do user.stubs(:github_id).returns(42) User.stubs(:find_github_id).returns(user) User.stubs(:find).returns(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' + } + end + + after do + Travis.config.oauth2 = @original_config end describe 'GET /auth/authorize' do @@ -26,18 +40,6 @@ describe Travis::Api::App::Endpoint::Authorization do describe "GET /auth/handshake" do describe 'with insufficient oauth permissions' do before do - # TODO: this is a first try of writing tests for authorization, it does not look pretty - # but I wanted to have something to start with, later on I would like to extract - # this flow to a class, which would be easier to test. - @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.redis.sadd('github:states', 'github-state') response = mock('response') @@ -56,7 +58,6 @@ describe Travis::Api::App::Endpoint::Authorization do end after do - Travis.config.oauth2 = @original_config Travis.redis.srem('github:states', 'github-state') # this is cached after first run, so if we change scopes, it will stay # like that for the rest of the run