Commit 612a909eac58b8ebb6a2d9f68bbe2b8998411b89
Exists in
spb-stable
and in
3 other branches
Merge branch 'git-refactoring' into 'master'
Git Refactoring
Showing
5 changed files
with
101 additions
and
84 deletions
Show diff stats
Procfile
1 | -web: bundle exec unicorn_rails -p $PORT -E development | 1 | +web: bundle exec unicorn_rails -p $PORT -E development -c config/unicorn_development.rb |
2 | worker: bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default,gitlab_shell | 2 | worker: bundle exec sidekiq -q post_receive,mailer,system_hook,project_web_hook,common,default,gitlab_shell |
lib/api/internal.rb
1 | module API | 1 | module API |
2 | # Internal access API | 2 | # Internal access API |
3 | class Internal < Grape::API | 3 | class Internal < Grape::API |
4 | - | ||
5 | - DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive } | ||
6 | - PUSH_COMMANDS = %w{ git-receive-pack } | ||
7 | - | ||
8 | namespace 'internal' do | 4 | namespace 'internal' do |
9 | - # | ||
10 | - # Check if ssh key has access to project code | 5 | + # Check if git command is allowed to project |
11 | # | 6 | # |
12 | # Params: | 7 | # Params: |
13 | - # key_id - SSH Key id | 8 | + # key_id - ssh key id for Git over SSH |
9 | + # user_id - user id for Git over HTTP | ||
14 | # project - project path with namespace | 10 | # project - project path with namespace |
15 | # action - git action (git-upload-pack or git-receive-pack) | 11 | # action - git action (git-upload-pack or git-receive-pack) |
16 | # ref - branch name | 12 | # ref - branch name |
@@ -22,43 +18,25 @@ module API | @@ -22,43 +18,25 @@ module API | ||
22 | # the wiki repository as well. | 18 | # the wiki repository as well. |
23 | project_path = params[:project] | 19 | project_path = params[:project] |
24 | project_path.gsub!(/\.wiki/,'') if project_path =~ /\.wiki/ | 20 | project_path.gsub!(/\.wiki/,'') if project_path =~ /\.wiki/ |
25 | - | ||
26 | - key = Key.find(params[:key_id]) | ||
27 | project = Project.find_with_namespace(project_path) | 21 | project = Project.find_with_namespace(project_path) |
28 | - git_cmd = params[:action] | ||
29 | return false unless project | 22 | return false unless project |
30 | 23 | ||
31 | - | ||
32 | - if key.is_a? DeployKey | ||
33 | - key.projects.include?(project) && DOWNLOAD_COMMANDS.include?(git_cmd) | ||
34 | - else | ||
35 | - user = key.user | ||
36 | - | ||
37 | - return false if user.blocked? | ||
38 | - | ||
39 | - if Gitlab.config.ldap.enabled | ||
40 | - if user.ldap_user? | ||
41 | - # Check if LDAP user exists and match LDAP user_filter | ||
42 | - unless Gitlab::LDAP::Access.new.allowed?(user) | ||
43 | - return false | ||
44 | - end | ||
45 | - end | ||
46 | - end | ||
47 | - | ||
48 | - action = case git_cmd | ||
49 | - when *DOWNLOAD_COMMANDS | ||
50 | - then :download_code | ||
51 | - when *PUSH_COMMANDS | ||
52 | - then | ||
53 | - if project.protected_branch?(params[:ref]) | ||
54 | - :push_code_to_protected_branches | ||
55 | - else | ||
56 | - :push_code | ||
57 | - end | ||
58 | - end | ||
59 | - | ||
60 | - user.can?(action, project) | ||
61 | - end | 24 | + actor = if params[:key_id] |
25 | + Key.find(params[:key_id]) | ||
26 | + elsif params[:user_id] | ||
27 | + User.find(params[:user_id]) | ||
28 | + end | ||
29 | + | ||
30 | + return false unless actor | ||
31 | + | ||
32 | + Gitlab::GitAccess.new.allowed?( | ||
33 | + actor, | ||
34 | + params[:action], | ||
35 | + project, | ||
36 | + params[:ref], | ||
37 | + params[:oldrev], | ||
38 | + params[:newrev] | ||
39 | + ) | ||
62 | end | 40 | end |
63 | 41 | ||
64 | # | 42 | # |
lib/gitlab/backend/grack_auth.rb
@@ -5,7 +5,7 @@ module Grack | @@ -5,7 +5,7 @@ module Grack | ||
5 | class Auth < Rack::Auth::Basic | 5 | class Auth < Rack::Auth::Basic |
6 | include Helpers | 6 | include Helpers |
7 | 7 | ||
8 | - attr_accessor :user, :project, :ref, :env | 8 | + attr_accessor :user, :project, :env |
9 | 9 | ||
10 | def call(env) | 10 | def call(env) |
11 | @env = env | 11 | @env = env |
@@ -80,24 +80,11 @@ module Grack | @@ -80,24 +80,11 @@ module Grack | ||
80 | def authorize_request(service) | 80 | def authorize_request(service) |
81 | case service | 81 | case service |
82 | when 'git-upload-pack' | 82 | when 'git-upload-pack' |
83 | - can?(user, :download_code, project) | ||
84 | - when'git-receive-pack' | ||
85 | - refs.each do |ref| | ||
86 | - action = if project.protected_branch?(ref) | ||
87 | - :push_code_to_protected_branches | ||
88 | - else | ||
89 | - :push_code | ||
90 | - end | ||
91 | - | ||
92 | - return false unless can?(user, action, project) | ||
93 | - end | ||
94 | - | ||
95 | - # Never let git-receive-pack trough unauthenticated; it's | ||
96 | - # harmless but git < 1.8 doesn't like it | ||
97 | - return false if user.nil? | ||
98 | - true | 83 | + # Serve only upload request. |
84 | + # Authorization on push will be serverd by update hook in repository | ||
85 | + Gitlab::GitAccess.new.download_allowed?(user, project) | ||
99 | else | 86 | else |
100 | - false | 87 | + true |
101 | end | 88 | end |
102 | end | 89 | end |
103 | 90 | ||
@@ -114,29 +101,5 @@ module Grack | @@ -114,29 +101,5 @@ module Grack | ||
114 | def project | 101 | def project |
115 | @project ||= project_by_path(@request.path_info) | 102 | @project ||= project_by_path(@request.path_info) |
116 | end | 103 | end |
117 | - | ||
118 | - def refs | ||
119 | - @refs ||= parse_refs | ||
120 | - end | ||
121 | - | ||
122 | - def parse_refs | ||
123 | - input = if @env["HTTP_CONTENT_ENCODING"] =~ /gzip/ | ||
124 | - Zlib::GzipReader.new(@request.body).read | ||
125 | - else | ||
126 | - @request.body.read | ||
127 | - end | ||
128 | - | ||
129 | - # Need to reset seek point | ||
130 | - @request.body.rewind | ||
131 | - | ||
132 | - # Parse refs | ||
133 | - refs = input.force_encoding('ascii-8bit').scan(/refs\/heads\/([\/\w\.-]+)/n).flatten.compact | ||
134 | - | ||
135 | - # Cleanup grabare from refs | ||
136 | - # if push to multiple branches | ||
137 | - refs.map do |ref| | ||
138 | - ref.gsub(/00.*/, "") | ||
139 | - end | ||
140 | - end | ||
141 | end | 104 | end |
142 | end | 105 | end |
@@ -0,0 +1,74 @@ | @@ -0,0 +1,74 @@ | ||
1 | +module Gitlab | ||
2 | + class GitAccess | ||
3 | + DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive } | ||
4 | + PUSH_COMMANDS = %w{ git-receive-pack } | ||
5 | + | ||
6 | + attr_reader :params, :project, :git_cmd, :user | ||
7 | + | ||
8 | + def allowed?(actor, cmd, project, ref = nil, oldrev = nil, newrev = nil) | ||
9 | + case cmd | ||
10 | + when *DOWNLOAD_COMMANDS | ||
11 | + if actor.is_a? User | ||
12 | + download_allowed?(actor, project) | ||
13 | + elsif actor.is_a? DeployKey | ||
14 | + actor.projects.include?(project) | ||
15 | + elsif actor.is_a? Key | ||
16 | + download_allowed?(actor.user, project) | ||
17 | + else | ||
18 | + raise 'Wrong actor' | ||
19 | + end | ||
20 | + when *PUSH_COMMANDS | ||
21 | + if actor.is_a? User | ||
22 | + push_allowed?(actor, project, ref, oldrev, newrev) | ||
23 | + elsif actor.is_a? DeployKey | ||
24 | + # Deploy key not allowed to push | ||
25 | + return false | ||
26 | + elsif actor.is_a? Key | ||
27 | + push_allowed?(actor.user, project, ref, oldrev, newrev) | ||
28 | + else | ||
29 | + raise 'Wrong actor' | ||
30 | + end | ||
31 | + else | ||
32 | + false | ||
33 | + end | ||
34 | + end | ||
35 | + | ||
36 | + def download_allowed?(user, project) | ||
37 | + if user_allowed?(user) | ||
38 | + user.can?(:download_code, project) | ||
39 | + else | ||
40 | + false | ||
41 | + end | ||
42 | + end | ||
43 | + | ||
44 | + def push_allowed?(user, project, ref, oldrev, newrev) | ||
45 | + if user_allowed?(user) | ||
46 | + action = if project.protected_branch?(ref) | ||
47 | + :push_code_to_protected_branches | ||
48 | + else | ||
49 | + :push_code | ||
50 | + end | ||
51 | + user.can?(action, project) | ||
52 | + else | ||
53 | + false | ||
54 | + end | ||
55 | + end | ||
56 | + | ||
57 | + private | ||
58 | + | ||
59 | + def user_allowed?(user) | ||
60 | + return false if user.blocked? | ||
61 | + | ||
62 | + if Gitlab.config.ldap.enabled | ||
63 | + if user.ldap_user? | ||
64 | + # Check if LDAP user exists and match LDAP user_filter | ||
65 | + unless Gitlab::LDAP::Access.new.allowed?(user) | ||
66 | + return false | ||
67 | + end | ||
68 | + end | ||
69 | + end | ||
70 | + | ||
71 | + true | ||
72 | + end | ||
73 | + end | ||
74 | +end |