Adding user access checking per project

  * Made :per_page and :page on Project.from_gitlab optional for API
    calls

  * Added call to retrieve Gitlab CI projects that are not only
    authorized, but "owned"

4 files changed, 90 insertions, 23 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index b9de85c..e324dc5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -27,7 +27,6 @@ class Project < ActiveRecord::Base
   has_many :runner_projects, dependent: :destroy
   has_many :runners, through: :runner_projects
 
-
   #
   # Validations
   #
@@ -45,11 +44,9 @@ class Project < ActiveRecord::Base
   before_validation :set_default_values
 
   def self.from_gitlab(user, page, per_page, scope = :owned)
-    opts = {
-      private_token: user.private_token,
-      per_page: per_page,
-      page: page,
-    }
+    opts = { private_token: user.private_token }
+    opts[:per_page] = per_page if per_page.present?
+    opts[:page]     = page     if page.present?
 
     projects = Network.new.projects(user.url, opts, scope)
 
diff --git a/app/models/user_session.rb b/app/models/user_session.rb
index d964cf3..a29c705 100644
--- a/app/models/user_session.rb
+++ b/app/models/user_session.rb
@@ -29,7 +29,12 @@ class UserSession
     user = block.call(url, Network.new, options)
 
     if user
-      return User.new(user.merge("url" => url))
+      verified_options = user.merge({
+        "url" => url,
+        "private_token" => options[:private_token]
+      })
+
+      return User.new(verified_options)
     else
       nil
     end
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 7bd497d..c482f4a 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -12,15 +12,36 @@ module API
       #   GET /projects/:id
       get ":id" do
         project = Project.find(params[:id])
-        present project, with: Entities::Project
+
+        if current_user.can_access_project?(project.gitlab_id)
+          present project, with: Entities::Project
+        else
+          unauthorized!
+        end
       end
 
-      # Retrieve info for aall Gitlab CI projects
+      # Retrieve all Gitlab CI projects that the user has access to
       #
       # Example Request:
       #   GET /projects
       get do
-        projects = Project.all
+        gitlab_projects = Project.from_gitlab(current_user, nil, nil, :authorized)
+        ids = gitlab_projects.map { |project| project.id }
+
+        projects = Project.where("gitlab_id IN (?)", ids).all
+        present projects, with: Entities::Project
+      end
+
+      # Retrieve all Gitlab CI projects that the user owns
+      #
+      # Example Request:
+      #   GET /projects/owned
+      get "owned" do
+        byebug
+        gitlab_projects = Project.from_gitlab(current_user, nil, nil, :owned)
+        ids = gitlab_projects.map { |project| project.id }
+
+        projects = Project.where("gitlab_id IN (?)", ids).all
         present projects, with: Entities::Project
       end
 
@@ -38,9 +59,6 @@ module API
       post do
         required_attributes! [:name, :gitlab_id, :gitlab_url, :ssh_url_to_repo]
 
-        # TODO: Check if this has automatic filtering of attributes
-        # via the Grape API
-
         filtered_params = {
           :name            => params[:name],
           :gitlab_id       => params[:gitlab_id],
@@ -75,7 +93,7 @@ module API
       put ":id" do
         project = Project.find(params[:id])
 
-        if project.present?
+        if project.present? && current_user.can_access_project?(project.gitlab_id)
           attrs = attributes_for_keys [:name, :gitlab_id, :gitlab_url, :scripts, :default_ref, :ssh_url_to_repo]
 
           if project.update_attributes(attrs)
@@ -98,7 +116,7 @@ module API
       delete ":id" do
         project = Project.find(params[:id])
 
-        if project.present?
+        if project.present? && current_user.can_access_project?(project.gitlab_id)
           project.destroy
         else
           not_found!
@@ -113,6 +131,8 @@ module API
       # Example Request:
       #   POST /projects/:id/runners/:runner_id
       post ":id/runners/:runner_id" do
+        unauthorized! unless current_user.can_access_project?(params[:id])
+
         project_exists = Project.exists?(params[:id])
         runner_exists = Runner.exists?(params[:runner_id])
 
@@ -141,6 +161,8 @@ module API
       # Example Request:
       #   DELETE /projects/:id/runners/:runner_id
       delete ":id/runners/:runner_id" do
+        unauthorized! unless current_user.can_access_project?(params[:id])
+
         project_exists = Project.exists?(params[:id])
         runner_exists = Runner.exists?(params[:runner_id])
 
diff --git a/spec/requests/projects_spec.rb b/spec/requests/projects_spec.rb
index 851b42d..dd55938 100644
--- a/spec/requests/projects_spec.rb
+++ b/spec/requests/projects_spec.rb
@@ -6,6 +6,8 @@ describe API::API do
   let(:private_token) { "A" }
   let(:gitlab_url) { GitlabCi.config.allowed_gitlab_urls.first }
   let(:auth_url) { File.join(gitlab_url, Network::API_PREFIX, "user.json") }
+  let(:projects_authorized_url) { File.join(gitlab_url, Network::API_PREFIX, "projects.json") }
+  let(:projects_owned_url) { File.join(gitlab_url, Network::API_PREFIX, "projects/owned.json") }
   let(:options) {
     {
       :private_token => private_token,
@@ -23,16 +25,57 @@ describe API::API do
       :body => { :url => "http://myurl" }.to_json)
   end
 
-  describe "GET /projects" do
-    let(:project) { FactoryGirl.create(:project) }
+  context "requests for scoped projects" do
+    let(:project1) { FactoryGirl.create(:project, :name => "Project 1") }
+    let(:project2) { FactoryGirl.create(:project, :name => "Project 2") }
 
-    before { project }
+    before do
+      # NOTE: Making project ids the same as the gitlab id for ease of matching
+      projects.map { |project| project.update_attribute(:gitlab_id, project.id) }
+    end
 
-    it "should return all projects on the CI instance" do
-      get api("/projects"), options
-      response.status.should == 200
-      json_response.count.should == 1
-      json_response.first["id"].should == project.id
+    describe "GET /projects" do
+      let(:projects) { [project1, project2] }
+
+      before do
+        # stub authentication endpoint for authorized projects
+        stub_request(:get, projects_authorized_url).
+          with(:query => { :private_token => private_token }).
+          to_return(
+          :status => 200,
+          :headers => {"Content-Type" => "application/json"},
+          :body => projects.to_json)
+      end
+
+      it "should return all projects on the CI instance" do
+        byebug
+        get api("/projects"), options
+        response.status.should == 200
+        json_response.count.should == 2
+        json_response.first["id"].should == project1.id
+        json_response.last["id"].should == project2.id
+      end
+    end
+
+    describe "GET /projects/owned" do
+      let(:projects) { [project2] }
+
+      before do
+        # stub authentication endpoint for owned projects
+        stub_request(:get, projects_owned_url).
+          with(:query => { :private_token => private_token }).
+          to_return(
+          :status => 200,
+          :headers => {"Content-Type" => "application/json"},
+          :body => projects.to_json)
+      end
+
+      it "should return all projects on the CI instance" do
+        get api("/projects/owned"), options
+        response.status.should == 200
+        json_response.count.should == 1
+        json_response.first["id"].should == project2.id
+      end
     end
   end