integration with gitlab auth

21 files changed, 51 insertions, 170 deletions

diff --git a/app/controllers/ci/application_controller.rb b/app/controllers/ci/application_controller.rb
index 95390d09737..e5c99066a68 100644
--- a/app/controllers/ci/application_controller.rb
+++ b/app/controllers/ci/application_controller.rb
@@ -1,5 +1,5 @@
 module Ci
-  class ApplicationController < ActionController::Base
+  class ApplicationController < ::ApplicationController
     def self.railtie_helpers_paths
       "app/helpers/ci"
     end
@@ -9,49 +9,19 @@ module Ci
     rescue_from Ci::Network::UnauthorizedError, with: :invalid_token
     before_filter :default_headers
     #before_filter :check_config
+    helper_method :gl_project
 
     protect_from_forgery
 
-    helper_method :current_user
-    before_filter :reset_cache
-
     private
 
-    def current_user
-      @current_user ||= session[:ci_current_user]
-    end
-
-    def sign_in(user)
-      session[:ci_current_user] = user
-    end
-
-    def sign_out
-      reset_session
-    end
-
-    def authenticate_user!
-      unless current_user
-        redirect_to new_ci_user_sessions_path
-        return
-      end
-    end
-
-    def authenticate_admin!
-      unless current_user && current_user.is_admin
-        redirect_to new_ci_user_sessions_path
-        return
-      end
-    end
-
     def authenticate_public_page!
       unless project.public
         unless current_user
-          redirect_to(new_ci_user_sessions_path(state: generate_oauth_state(request.fullpath))) and return
+          redirect_to(new_user_sessions_path) and return
         end
 
-        unless current_user.can_access_project?(project.gitlab_id)
-          page_404 and return
-        end
+        return access_denied! unless can?(current_user, :read_project, gl_project)
       end
     end
 
@@ -62,19 +32,23 @@ module Ci
     end
 
     def authorize_access_project!
-      unless current_user.can_access_project?(@project.gitlab_id)
+      unless can?(current_user, :read_project, gl_project)
         return page_404
       end
     end
 
-    def authorize_project_developer!
-      unless current_user.has_developer_access?(@project.gitlab_id)
+    def authorize_manage_builds!
+      unless can?(current_user, :manage_builds, gl_project)
         return page_404
       end
     end
 
+    def authenticate_admin!
+      return render_404 unless current_user.is_admin?
+    end
+
     def authorize_manage_project!
-      unless current_user.can_manage_project?(@project.gitlab_id)
+      unless can?(current_user, :manage_project, gl_project)
         return page_404
       end
     end
@@ -83,13 +57,6 @@ module Ci
       render file: "#{Rails.root}/public/404.html", status: 404, layout: false
     end
 
-    # Reset user cache every day for security purposes
-    def reset_cache
-      if current_user && current_user.sync_at < (Time.zone.now - 24.hours)
-        current_user.reset_cache
-      end
-    end
-
     def default_headers
       headers['X-Frame-Options'] = 'DENY'
       headers['X-XSS-Protection'] = '1; mode=block'
@@ -129,5 +96,9 @@ module Ci
       reset_session
       redirect_to ci_root_path
     end
+
+    def gl_project
+      ::Project.find(@project.gitlab_id)
+    end
   end
 end
diff --git a/app/controllers/ci/builds_controller.rb b/app/controllers/ci/builds_controller.rb
index eeff3f1e0a0..28fad3671f7 100644
--- a/app/controllers/ci/builds_controller.rb
+++ b/app/controllers/ci/builds_controller.rb
@@ -5,7 +5,7 @@ module Ci
     before_filter :project
     before_filter :authorize_access_project!, except: [:status, :show]
     before_filter :authorize_manage_project!, except: [:status, :show, :retry, :cancel]
-    before_filter :authorize_project_developer!, only: [:retry, :cancel]
+    before_filter :authorize_manage_builds!, only: [:retry, :cancel]
     before_filter :build, except: [:show]
 
     def show
diff --git a/app/controllers/ci/commits_controller.rb b/app/controllers/ci/commits_controller.rb
index 9f74a2fd807..bad9075dde6 100644
--- a/app/controllers/ci/commits_controller.rb
+++ b/app/controllers/ci/commits_controller.rb
@@ -4,7 +4,7 @@ module Ci
     before_filter :authenticate_public_page!, only: :show
     before_filter :project
     before_filter :authorize_access_project!, except: [:status, :show, :cancel]
-    before_filter :authorize_project_developer!, only: [:cancel]
+    before_filter :authorize_manage_builds!, only: [:cancel]
     before_filter :commit, only: :show
 
     def show
diff --git a/app/controllers/ci/projects_controller.rb b/app/controllers/ci/projects_controller.rb
index 6ff7fc9f77a..80a5e602171 100644
--- a/app/controllers/ci/projects_controller.rb
+++ b/app/controllers/ci/projects_controller.rb
@@ -21,12 +21,15 @@ module Ci
       @limit, @offset = (params[:limit] || PROJECTS_BATCH).to_i, (params[:offset] || 0).to_i
       @page = @offset == 0 ? 1 : (@offset / @limit + 1)
 
-      current_user.reset_cache if params[:reset_cache]
+      @gl_projects = current_user.authorized_projects
+      @gl_projects = @gl_projects.where("name LIKE %?%", params[:search]) if params[:search]
+      @gl_projects = @gl_projects.page(@page).per(@limit)
 
-      @gl_projects = current_user.gitlab_projects(params[:search], @page, @limit)
       @projects = Ci::Project.where(gitlab_id: @gl_projects.map(&:id)).ordered_by_last_commit_date
       @total_count = @gl_projects.size
-      @gl_projects.reject! { |gl_project| @projects.map(&:gitlab_id).include?(gl_project.id) }
+      
+      @gl_projects = @gl_projects.where.not(id: @projects.map(&:gitlab_id))
+
       respond_to do |format|
         format.json do
           pager_json("ci/projects/gitlab", @total_count)
@@ -52,7 +55,7 @@ module Ci
     def create
       project_data = OpenStruct.new(JSON.parse(params["project"]))
 
-      unless current_user.can_manage_project?(project_data.id)
+      unless can?(current_user, :manage_project, ::Project.find(project_data.id))
         return redirect_to ci_root_path, alert: 'You have to have at least master role to enable CI for this project'
       end
 
diff --git a/app/controllers/ci/user_sessions_controller.rb b/app/controllers/ci/user_sessions_controller.rb
index 82134c1f7ba..818e1fcdea1 100644
--- a/app/controllers/ci/user_sessions_controller.rb
+++ b/app/controllers/ci/user_sessions_controller.rb
@@ -10,11 +10,6 @@ module Ci
     end
 
     def auth
-      unless is_oauth_state_valid?(params[:state])
-        redirect_to new_ci_user_sessions_path
-        return
-      end
-
       redirect_to client.auth_code.authorize_url({
         redirect_uri: callback_ci_user_sessions_url,
         state: params[:state]
@@ -22,11 +17,6 @@ module Ci
     end
 
     def callback
-      unless is_oauth_state_valid?(params[:state])
-        redirect_to new_ci_user_sessions_path
-        return
-      end
-
       token = client.auth_code.get_token(params[:code], redirect_uri: callback_ci_user_sessions_url).token
       
       @user_session = Ci::UserSession.new
diff --git a/app/helpers/ci/commits_helper.rb b/app/helpers/ci/commits_helper.rb
index 0479bc10594..748d12138b1 100644
--- a/app/helpers/ci/commits_helper.rb
+++ b/app/helpers/ci/commits_helper.rb
@@ -16,7 +16,7 @@ module Ci
     end
 
     def commit_link(commit)
-      link_to(commit.short_sha, ci_project_ref_commit_path(commit.project, commit.ref, commit.sha))
+      link_to(commit.short_sha, ci_project_ref_commits_path(commit.project, commit.ref, commit.sha))
     end
 
     def truncate_first_line(message, length = 50)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index f8e5afa9b01..a020b24a550 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -149,6 +149,7 @@ class Ability
         :admin_merge_request,
         :create_merge_request,
         :create_wiki,
+        :manage_builds,
         :push_code
       ]
     end
diff --git a/app/models/ci/user.rb b/app/models/ci/user.rb
index 49ec7126fc1..f7e2eaae1f1 100644
--- a/app/models/ci/user.rb
+++ b/app/models/ci/user.rb
@@ -39,43 +39,13 @@ module Ci
       @sync_at = Time.now
     end
 
-    def can_access_project?(project_gitlab_id)
-      !!project_info(project_gitlab_id)
-    end
-
-    # Indicate if user has developer access or higher
-    def has_developer_access?(project_gitlab_id)
-      data = project_info(project_gitlab_id)
-
-      return false unless data && data["permissions"]
-
-      permissions = data["permissions"]
-
-      if permissions["project_access"] && permissions["project_access"]["access_level"] >= DEVELOPER_ACCESS
-        return true
-      end
-
-      if permissions["group_access"] && permissions["group_access"]["access_level"] >= DEVELOPER_ACCESS
-        return true
-      end
-    end
-
-    def can_manage_project?(project_gitlab_id)
-      Rails.cache.fetch(cache_key('manage', project_gitlab_id, sync_at)) do
-        !!Ci::Network.new.project_hooks(authenticate_options, project_gitlab_id)
-      end
-    end
-
     def authorized_runners
       Ci::Runner.specific.includes(:runner_projects).
         where(Ci::RunnerProject.table_name => { project_id: authorized_projects } )
     end
 
     def authorized_projects
-      Ci::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
+      Ci::Project.where(gitlab_id: current_user.authorized_projects.map(&:id))
     end
 
     def authenticate_options
diff --git a/app/views/ci/builds/_build.html.haml b/app/views/ci/builds/_build.html.haml
index 54ca1102b5c..da306c9f020 100644
--- a/app/views/ci/builds/_build.html.haml
+++ b/app/views/ci/builds/_build.html.haml
@@ -35,7 +35,7 @@
         #{build.coverage}%
 
   %td
-    - if defined?(controls) && current_user && current_user.has_developer_access?(@project.gitlab_id)
+    - if defined?(controls) && current_user && can?(current_user, :manage_builds, gl_project)
       .pull-right
         - if build.active?
           = link_to cancel_ci_project_build_path(build.project, build, return_to: request.original_url), title: 'Cancel build' do
diff --git a/app/views/ci/builds/show.html.haml b/app/views/ci/builds/show.html.haml
index 0bef67d8a20..a698176c3ed 100644
--- a/app/views/ci/builds/show.html.haml
+++ b/app/views/ci/builds/show.html.haml
@@ -1,10 +1,10 @@
 %h4.page-title
-  = link_to @project.name, @project
+  = link_to ci_project_path(@project)
   @
   = @commit.short_sha
   
 %p
-  = link_to ci_project_ref_commit_path(@project, @commit.ref, @commit.sha) do
+  = link_to ci_project_ref_commits_path(@project, @commit.ref, @commit.sha) do
     &larr; Back to project commit
 %hr
 #up-build-trace
@@ -12,7 +12,7 @@
   %ul.nav.nav-tabs.append-bottom-10
     - @commit.builds_without_retry_sorted.each do |build|
       %li{class: ('active' if build == @build) }
-        = link_to ci_build_url(build) do
+        = link_to ci_project_build_url(@project, build) do
           %i{class: build_icon_css_class(build)}
           %span
             Build ##{build.id}
@@ -84,7 +84,7 @@
     .build-widget
       %h4.title
         Build
-        - if current_user && current_user.has_developer_access?(@project.gitlab_id)
+        - if current_user && can?(current_user, :manage_builds, gl_project)
           .pull-right
             - if @build.active?
               = link_to "Cancel", cancel_ci_project_build_path(@project, @build), class: 'btn btn-sm btn-danger'
@@ -161,7 +161,7 @@
           - @builds.each_with_index do |build, i|
             %tr.build.alert{class: build_status_alert_class(build)}
               %td
-                = link_to ci_build_url(build) do
+                = link_to ci_project_build_url(@project, build) do
                   %span ##{build.id}
               %td
                 - if build.name
@@ -173,4 +173,4 @@
 
 
 :javascript
-  new CiBuild("#{ci_build_url(@build)}", "#{@build.status}")
+  new CiBuild("#{ci_project_build_url(@project, @build)}", "#{@build.status}")
diff --git a/app/views/ci/commits/_commit.html.haml b/app/views/ci/commits/_commit.html.haml
index a955a5b6479..c1b1988d147 100644
--- a/app/views/ci/commits/_commit.html.haml
+++ b/app/views/ci/commits/_commit.html.haml
@@ -7,7 +7,7 @@
 
 
   %td.build-link
-    = link_to ci_project_ref_commit_path(commit.project, commit.ref, commit.sha) do
+    = link_to ci_project_ref_commits_path(commit.project, commit.ref, commit.sha) do
       %strong #{commit.short_sha}
 
   %td.build-message
diff --git a/app/views/ci/commits/show.html.haml b/app/views/ci/commits/show.html.haml
index 832cc6a1bae..9b597b45aa5 100644
--- a/app/views/ci/commits/show.html.haml
+++ b/app/views/ci/commits/show.html.haml
@@ -33,10 +33,10 @@
           %span.attr-name Created at:
           #{@commit.created_at.to_s(:short)}
 
-- if current_user && current_user.has_developer_access?(@project.gitlab_id)
+- if current_user && can?(current_user, :manage_builds, gl_project)
   .pull-right
     - if @commit.builds.running_or_pending.any?
-      = link_to "Cancel", cancel_ci_project_ref_commit_path(@project, @commit.ref, @commit.sha), class: 'btn btn-sm btn-danger'
+      = link_to "Cancel", cancel_ci_project_ref_commits_path(@project, @commit.ref, @commit.sha), class: 'btn btn-sm btn-danger'
 
 
 - if @commit.yaml_errors.present?
diff --git a/app/views/ci/projects/_gl_projects.html.haml b/app/views/ci/projects/_gl_projects.html.haml
index 6ed19e13887..f63d8246e96 100644
--- a/app/views/ci/projects/_gl_projects.html.haml
+++ b/app/views/ci/projects/_gl_projects.html.haml
@@ -11,5 +11,5 @@
           Added
       - else
         = form_tag ci_projects_path do
-          = hidden_field_tag :project, project.to_h.to_json
+          = hidden_field_tag :project, project.to_json
           = submit_tag 'Add project to CI', class: 'btn btn-default btn-sm'
diff --git a/app/views/ci/projects/gitlab.html.haml b/app/views/ci/projects/gitlab.html.haml
index bd55b1f12e7..0344676680e 100644
--- a/app/views/ci/projects/gitlab.html.haml
+++ b/app/views/ci/projects/gitlab.html.haml
@@ -4,10 +4,6 @@
       Fetched from GitLab (#{link_to GitlabCi.config.gitlab_server.url, GitlabCi.config.gitlab_server.url, no_turbolink})
       - if params[:search].present?
         by keyword: "#{params[:search]}",
-      #{time_ago_in_words(current_user.sync_at)} ago.
-      = link_to gitlab_ci_projects_path(reset_cache: true, search: params[:search]), class: 'sync-now btn btn-sm btn-default reset-cache' do
-        %i.fa.fa-refresh
-        Sync now
       %br
 
     .pull-right
diff --git a/app/views/ci/projects/show.html.haml b/app/views/ci/projects/show.html.haml
index 27899591391..b79ab957ba8 100644
--- a/app/views/ci/projects/show.html.haml
+++ b/app/views/ci/projects/show.html.haml
@@ -1,6 +1,6 @@
 = render 'ci/shared/guide' unless @project.setup_finished?
 
-- if current_user && current_user.can_manage_project?(@project.gitlab_id) && !@project.any_runners?
+- if current_user && can?(current_user, :manage_project, gl_project) && !@project.any_runners?
   .alert.alert-danger
     Builds for this project wont be served unless you configure runners on
     = link_to "Runners page", ci_project_runners_path(@project)
diff --git a/app/views/ci/user_sessions/show.html.haml b/app/views/ci/user_sessions/show.html.haml
index 43f64a429b2..0710e205618 100644
--- a/app/views/ci/user_sessions/show.html.haml
+++ b/app/views/ci/user_sessions/show.html.haml
@@ -2,7 +2,7 @@
 %h3
   Hi, #{@user.name}
 
-  - if @user.is_admin
+  - if @user.is_admin?
     %span.label.label-success Admin
 
 .profile-block
diff --git a/app/views/layouts/ci/_info.html.haml b/app/views/layouts/ci/_info.html.haml
index bce3ce77031..9bc23cfad05 100644
--- a/app/views/layouts/ci/_info.html.haml
+++ b/app/views/layouts/ci/_info.html.haml
@@ -5,5 +5,5 @@
     - if notice
       .alert.alert-info= notice
 
-  - if current_user && current_user.is_admin && Ci::Runner.count.zero?
+  - if current_user && current_user.is_admin? && Ci::Runner.count.zero?
     = render 'ci/shared/no_runners'
diff --git a/app/views/layouts/ci/_nav.html.haml b/app/views/layouts/ci/_nav.html.haml
index babd14ca2d3..4a0e45a0217 100644
--- a/app/views/layouts/ci/_nav.html.haml
+++ b/app/views/layouts/ci/_nav.html.haml
@@ -9,7 +9,7 @@
 
     .collapse.navbar-collapse
       %ul.nav.navbar-nav
-        - if current_user && current_user.is_admin
+        - if current_user && current_user.is_admin?
           %li
             = link_to ci_admin_projects_path do
               Admin
@@ -19,12 +19,12 @@
       %ul.nav.navbar-nav.pull-right
         - if current_user
           %li
-            = link_to ci_user_sessions_path do
+            = link_to new_user_session_path do
               .profile-holder
                 = image_tag user_avatar_url(current_user, 64), class: 'avatar s32', alt: ''
                 %span= current_user.name
           %li
-            = link_to ci_user_sessions_path, class: "logout", method: :delete do
+            = link_to destroy_user_session_path, class: "logout", method: :delete do
               %i.fa.fa-signout
               Logout
         - else
diff --git a/app/views/layouts/ci/project.html.haml b/app/views/layouts/ci/project.html.haml
index 763a7fc0b02..9549485d095 100644
--- a/app/views/layouts/ci/project.html.haml
+++ b/app/views/layouts/ci/project.html.haml
@@ -16,7 +16,7 @@
           = link_to 'View on GitLab', @project.gitlab_url, no_turbolink.merge( class: 'btn btn-sm' )
     %hr
     .container
-      - if current_user && current_user.can_manage_project?(@project.gitlab_id)
+      - if current_user && can?(current_user, :manage_project, gl_project)
         .row
           .col-md-2.append-bottom-20
             = render 'layouts/ci/nav_project'
diff --git a/lib/ci/api/projects.rb b/lib/ci/api/projects.rb
index f9b4937c033..bdcacecf6ab 100644
--- a/lib/ci/api/projects.rb
+++ b/lib/ci/api/projects.rb
@@ -66,7 +66,7 @@ module Ci
         get ":id" do
           project = Ci::Project.find(params[:id])
 
-          unauthorized! unless current_user.can_access_project?(project.gitlab_id)
+          unauthorized! unless can?(current_user, :read_project, gl_project)
 
           present project, with: Entities::Project
         end
@@ -118,7 +118,7 @@ module Ci
         put ":id" do
           project = Ci::Project.find(params[:id])
 
-          unauthorized! unless current_user.can_manage_project?(project.gitlab_id)
+          unauthorized! unless can?(current_user, :manage_project, gl_project)
 
           attrs = attributes_for_keys [:name, :gitlab_id, :path, :gitlab_url, :default_ref, :ssh_url_to_repo]
 
@@ -144,7 +144,7 @@ module Ci
         delete ":id" do
           project = Ci::Project.find(params[:id])
 
-          unauthorized! unless current_user.can_manage_project?(project.gitlab_id)
+          unauthorized! unless can?(current_user, :manage_project, gl_project)
 
           project.destroy
         end
@@ -160,7 +160,7 @@ module Ci
           project = Ci::Project.find(params[:id])
           runner  = Ci::Runner.find(params[:runner_id])
 
-          unauthorized! unless current_user.can_manage_project?(project.gitlab_id)
+          unauthorized! unless can?(current_user, :manage_project, gl_project)
 
           options = {
             project_id: project.id,
@@ -188,7 +188,7 @@ module Ci
           project = Ci::Project.find(params[:id])
           runner  = Ci::Runner.find(params[:runner_id])
 
-          unauthorized! unless current_user.can_manage_project?(project.gitlab_id)
+          unauthorized! unless can?(current_user, :manage_project, gl_project)
 
           options = {
             project_id: project.id,
diff --git a/spec/models/ci/user_spec.rb b/spec/models/ci/user_spec.rb
index d1b87988b74..c4d7b3ccae5 100644
--- a/spec/models/ci/user_spec.rb
+++ b/spec/models/ci/user_spec.rb
@@ -2,56 +2,6 @@ require 'spec_helper'
 
 describe Ci::User do
 
-  describe "has_developer_access?" do
-    before do
-      @user = User.new({})
-    end
-
-    let(:project_with_owner_access) do
-      {
-        "name" => "gitlab-shell",
-        "permissions" => {
-          "project_access" => {
-            "access_level"=> 10,
-            "notification_level" => 3
-          },
-          "group_access" => {
-            "access_level" => 50,
-            "notification_level" => 3
-          }
-        }
-      }
-    end
-
-    let(:project_with_reporter_access) do
-      {
-        "name" => "gitlab-shell",
-        "permissions" => {
-          "project_access" => {
-            "access_level" => 20,
-            "notification_level" => 3
-          },
-          "group_access" => {
-            "access_level" => 10,
-            "notification_level" => 3
-          }
-        }
-      }
-    end
-
-    it "returns false for reporter" do
-      @user.stub(:project_info).and_return(project_with_reporter_access)
-
-      @user.has_developer_access?(1).should be_false
-    end
-
-    it "returns true for owner" do
-      @user.stub(:project_info).and_return(project_with_owner_access)
-
-      @user.has_developer_access?(1).should be_true
-    end
-  end
-
   describe "authorized_projects" do
     let (:user) { User.new({}) }