diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2015-07-21 22:48:34 +0200 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2015-07-22 16:52:19 +0200 |
commit | b9c551302253f86a8a85e8288099696b1d8ccdd6 (patch) | |
tree | 5308f2a13fe26929e9df717d56f7f39a157d2d20 | |
parent | 7728125c3e3474fef153c0037355c20ec72868b0 (diff) | |
download | gitlab-ci-b9c551302253f86a8a85e8288099696b1d8ccdd6.tar.gz |
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
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | app/models/user.rb | 5 | ||||
-rw-r--r-- | db/migrate/20150721204649_truncate_sessions.rb (renamed from db/migrate/20150706103229_truncate_sessions.rb) | 0 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 20 |
5 files changed, 24 insertions, 6 deletions
@@ -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 |