Prevent from updating repo settings with wrong user (#1104)

* Prevent from updating repo settings with wrong user

* debug stuff removed

* test fix

Author: piotr-travisci

lib/travis/api/v3/services/user_setting/update.rb

@@ -5,10 +5,12 @@ class Services::UserSetting::Update < Service
 
     def run!
       repository = check_login_and_find(:repository)
-      user_setting = query.update(repository)
       return repo_migrated if migrated?(repository)
-
+      
+      user_setting = query.find(repository)
       access_control.permissions(user_setting).write!
+      
+      user_setting = query.update(repository)      
       result user_setting
     end
   end

spec/v3/services/user_setting/update_spec.rb

@@ -96,7 +96,7 @@
           '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",
           '@representation' => 'minimal',
           'name' => 'build_pushes',
-          'value' => false
+          'value' => true
         }
       )
     end

spec/v3/services/user_settings/for_repository_spec.rb

@@ -4,7 +4,7 @@
   let(:auth_headers) { { 'HTTP_AUTHORIZATION' => "token #{token}" } }
   let(:json_headers) { { 'CONTENT_TYPE' => 'application/json' } }
 
-  before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, pull: true, admin: false) }
+  before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, pull: true, push: true, admin: false) }
 
   describe 'not authenticated' do
     before { get("/v3/repo/#{repo.id}/settings") }
@@ -42,13 +42,13 @@
           '@href' => "/v3/repo/#{repo.id}/settings",
           '@representation' => 'standard',
           'settings' => [
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pushes',                'value' => true },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pull_requests',         'value' => true },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pushes',          'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
-            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'config_validation',           'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pushes',                'value' => true },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pull_requests',         'value' => true },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pushes',          'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
+            { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'config_validation',           'value' => false },
           ]
         )
       end
@@ -60,7 +60,7 @@
 
       example do
         expect(JSON.load(body)['settings']).to include(
-          { '@type' => 'setting', '@permissions' => { 'read' => true, 'write' => false }, '@href' => "/v3/repo/#{repo.id}/setting/allow_config_imports", '@representation' => 'standard', 'name' => 'allow_config_imports', 'value' => false },
+          { '@type' => 'setting', '@permissions' => { 'read' => true, 'write' => true }, '@href' => "/v3/repo/#{repo.id}/setting/allow_config_imports", '@representation' => 'standard', 'name' => 'allow_config_imports', 'value' => false },
         )
       end
     end
@@ -79,13 +79,13 @@
         '@href' => "/v3/repo/#{repo.id}/settings",
         '@representation' => 'standard',
         'settings' => [
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pushes',                'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'build_pull_requests',         'value' => true },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pushes',          'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
-          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => false }, 'name' => 'config_validation',           'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/builds_only_with_travis_yml", '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'builds_only_with_travis_yml', 'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",                '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pushes',                'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/build_pull_requests",         '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'build_pull_requests',         'value' => true },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/maximum_number_of_builds",    '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'maximum_number_of_builds',    'value' => 0 },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pushes",          '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pushes',          'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/auto_cancel_pull_requests",   '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'auto_cancel_pull_requests',   'value' => false },
+          { '@type' => 'setting', '@href' => "/v3/repo/#{repo.id}/setting/config_validation",           '@representation' => 'standard', '@permissions' => { 'read' => true, 'write' => true }, 'name' => 'config_validation',           'value' => false },
         ]
       )
     end
@@ -104,7 +104,7 @@
         '@type' => 'setting',
         '@href' => "/v3/repo/#{repo.id}/setting/build_pushes",
         '@representation' => 'standard',
-        '@permissions' => { 'read' => true, 'write' => false },
+        '@permissions' => { 'read' => true, 'write' => true },
         'name' => 'build_pushes',
         'value' => false
       )