Fix: user could steal specific runner

- check if user has manage access to project
- don't cache result of authorized_projects, because it's serialised with User object
- clear user sessions

5 files changed, 24 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 035c7e3..b8fcb24 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+v7.13.1
+  - Fix: user could steal specific runner
+
 v7.13.0
   - Allow to specify image and services in yml that can be used with docker
   - Fix: No runner notification can see managers only
diff --git a/app/models/user.rb b/app/models/user.rb
index 138e5e4..1523577 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -71,7 +71,10 @@ class User
   end
 
   def authorized_projects
-    @authorized_projects ||= Project.where(gitlab_id: gitlab_projects.map(&:id))
+    Project.where(gitlab_id: gitlab_projects.map(&:id)).select do |project|
+      # This is slow: it makes request to GitLab for each project to verify manage permission
+      can_manage_project?(project.gitlab_id)
+    end
   end
 
   private
diff --git a/db/migrate/20150706103229_truncate_sessions.rb b/db/migrate/20150721204649_truncate_sessions.rb
index 32fe651..32fe651 100644
--- a/db/migrate/20150706103229_truncate_sessions.rb
+++ b/db/migrate/20150721204649_truncate_sessions.rb
diff --git a/db/schema.rb b/db/schema.rb
index b42bf0c..1363111 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150710113851) do
+ActiveRecord::Schema.define(version: 20150721204649) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4a1e393..73a7a7d 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -53,16 +53,27 @@ describe User do
   end
 
   describe "authorized_projects" do
-    it "returns projects" do
-      project = FactoryGirl.create :project, gitlab_id: 1
-      project1 = FactoryGirl.create :project, gitlab_id: 2
+    let (:user) { User.new({}) }
+
+    before do
+      FactoryGirl.create :project, gitlab_id: 1
+      FactoryGirl.create :project, gitlab_id: 2
       gitlab_project = OpenStruct.new({id: 1})
       gitlab_project1 = OpenStruct.new({id: 2})
       User.any_instance.stub(:gitlab_projects).and_return([gitlab_project, gitlab_project1])
-      user = User.new({})
+    end
+
+    it "returns projects" do
+      User.any_instance.stub(:can_manage_project?).and_return(true)
 
       user.authorized_projects.count.should == 2
     end
+
+    it "empty list if user miss manage permission" do
+      User.any_instance.stub(:can_manage_project?).and_return(false)
+
+      user.authorized_projects.count.should == 0
+    end
   end
 
   describe "authorized_runners" do
@@ -72,6 +83,7 @@ describe User do
       gitlab_project = OpenStruct.new({id: 1})
       gitlab_project1 = OpenStruct.new({id: 2})
       User.any_instance.stub(:gitlab_projects).and_return([gitlab_project, gitlab_project1])
+      User.any_instance.stub(:can_manage_project?).and_return(true)
       user = User.new({})
 
       runner = FactoryGirl.create :specific_runner