Commit 0189ee97ed34b74cf0f500d678d4435b17ab6a85
1 parent
5ec1ad8b
Exists in
master
and in
4 other branches
Security for online editor. Replace dev_access?, master_access? with can? method usage
Showing
7 changed files
with
56 additions
and
18 deletions
Show diff stats
app/controllers/tree_controller.rb
| @@ -48,5 +48,13 @@ class TreeController < ProjectResourceController | @@ -48,5 +48,13 @@ class TreeController < ProjectResourceController | ||
| 48 | unless @tree.is_blob? && @tree.text? | 48 | unless @tree.is_blob? && @tree.text? |
| 49 | redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" | 49 | redirect_to project_tree_path(@project, @id), notice: "You can only edit text files" |
| 50 | end | 50 | end |
| 51 | + | ||
| 52 | + allowed = if project.protected_branch? @ref | ||
| 53 | + can?(current_user, :push_code_to_protected_branches, project) | ||
| 54 | + else | ||
| 55 | + can?(current_user, :push_code, project) | ||
| 56 | + end | ||
| 57 | + | ||
| 58 | + return access_denied! unless allowed | ||
| 51 | end | 59 | end |
| 52 | end | 60 | end |
app/helpers/tree_helper.rb
| @@ -59,4 +59,12 @@ module TreeHelper | @@ -59,4 +59,12 @@ module TreeHelper | ||
| 59 | def tree_join(*args) | 59 | def tree_join(*args) |
| 60 | File.join(*args) | 60 | File.join(*args) |
| 61 | end | 61 | end |
| 62 | + | ||
| 63 | + def allowed_tree_edit? | ||
| 64 | + if @project.protected_branch? @ref | ||
| 65 | + can?(current_user, :push_code_to_protected_branches, @project) | ||
| 66 | + else | ||
| 67 | + can?(current_user, :push_code, @project) | ||
| 68 | + end | ||
| 69 | + end | ||
| 62 | end | 70 | end |
app/models/ability.rb
| @@ -35,10 +35,15 @@ class Ability | @@ -35,10 +35,15 @@ class Ability | ||
| 35 | ] if project.report_access_for?(user) | 35 | ] if project.report_access_for?(user) |
| 36 | 36 | ||
| 37 | rules << [ | 37 | rules << [ |
| 38 | - :write_wiki | 38 | + :write_wiki, |
| 39 | + :push_code | ||
| 39 | ] if project.dev_access_for?(user) | 40 | ] if project.dev_access_for?(user) |
| 40 | 41 | ||
| 41 | rules << [ | 42 | rules << [ |
| 43 | + :push_code_to_protected_branches | ||
| 44 | + ] if project.master_access_for?(user) | ||
| 45 | + | ||
| 46 | + rules << [ | ||
| 42 | :modify_issue, | 47 | :modify_issue, |
| 43 | :modify_snippet, | 48 | :modify_snippet, |
| 44 | :modify_merge_request, | 49 | :modify_merge_request, |
app/roles/authority.rb
| @@ -53,6 +53,6 @@ module Authority | @@ -53,6 +53,6 @@ module Authority | ||
| 53 | end | 53 | end |
| 54 | 54 | ||
| 55 | def master_access_for?(user) | 55 | def master_access_for?(user) |
| 56 | - !users_projects.where(user_id: user.id, project_access: [UsersProject::MASTER]).empty? || owner_id == user.id | 56 | + !users_projects.where(user_id: user.id, project_access: [UsersProject::MASTER]).empty? |
| 57 | end | 57 | end |
| 58 | end | 58 | end |
app/roles/repository.rb
| @@ -181,4 +181,9 @@ module Repository | @@ -181,4 +181,9 @@ module Repository | ||
| 181 | def http_url_to_repo | 181 | def http_url_to_repo |
| 182 | http_url = [Gitlab.config.url, "/", path, ".git"].join('') | 182 | http_url = [Gitlab.config.url, "/", path, ".git"].join('') |
| 183 | end | 183 | end |
| 184 | + | ||
| 185 | + # Check if current branch name is marked as protected in the system | ||
| 186 | + def protected_branch? branch_name | ||
| 187 | + protected_branches.map(&:name).include?(branch_name) | ||
| 188 | + end | ||
| 184 | end | 189 | end |
app/views/tree/_blob_actions.html.haml
| 1 | .btn-group.tree-btn-group | 1 | .btn-group.tree-btn-group |
| 2 | -# only show edit link for text files | 2 | -# only show edit link for text files |
| 3 | - if @tree.text? | 3 | - if @tree.text? |
| 4 | - = link_to "edit", edit_project_tree_path(@project, @id), class: "btn very_small" | 4 | + = link_to "edit", edit_project_tree_path(@project, @id), class: "btn very_small", disabled: !allowed_tree_edit? |
| 5 | = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" | 5 | = link_to "raw", project_blob_path(@project, @id), class: "btn very_small", target: "_blank" |
| 6 | -# only show normal/blame view links for text files | 6 | -# only show normal/blame view links for text files |
| 7 | - if @tree.text? | 7 | - if @tree.text? |
lib/gitlab/backend/grack_auth.rb
| 1 | module Grack | 1 | module Grack |
| 2 | class Auth < Rack::Auth::Basic | 2 | class Auth < Rack::Auth::Basic |
| 3 | + attr_accessor :user, :project | ||
| 3 | 4 | ||
| 4 | def valid? | 5 | def valid? |
| 5 | # Authentication with username and password | 6 | # Authentication with username and password |
| 6 | email, password = @auth.credentials | 7 | email, password = @auth.credentials |
| 7 | - user = User.find_by_email(email) | 8 | + self.user = User.find_by_email(email) |
| 8 | return false unless user.try(:valid_password?, password) | 9 | return false unless user.try(:valid_password?, password) |
| 9 | 10 | ||
| 10 | # Set GL_USER env variable | 11 | # Set GL_USER env variable |
| @@ -18,28 +19,39 @@ module Grack | @@ -18,28 +19,39 @@ module Grack | ||
| 18 | 19 | ||
| 19 | # Find project by PATH_INFO from env | 20 | # Find project by PATH_INFO from env |
| 20 | if m = /^\/([\w-]+).git/.match(@request.path_info).to_a | 21 | if m = /^\/([\w-]+).git/.match(@request.path_info).to_a |
| 21 | - return false unless project = Project.find_by_path(m.last) | 22 | + self.project = Project.find_by_path(m.last) |
| 23 | + return false unless project | ||
| 22 | end | 24 | end |
| 23 | 25 | ||
| 24 | # Git upload and receive | 26 | # Git upload and receive |
| 25 | if @request.get? | 27 | if @request.get? |
| 26 | - true | 28 | + validate_get_request |
| 27 | elsif @request.post? | 29 | elsif @request.post? |
| 28 | - if @request.path_info.end_with?('git-upload-pack') | ||
| 29 | - return project.dev_access_for?(user) | ||
| 30 | - elsif @request.path_info.end_with?('git-receive-pack') | ||
| 31 | - if project.protected_branches.map(&:name).include?(current_ref) | ||
| 32 | - project.master_access_for?(user) | ||
| 33 | - else | ||
| 34 | - project.dev_access_for?(user) | ||
| 35 | - end | ||
| 36 | - else | ||
| 37 | - false | ||
| 38 | - end | 30 | + validate_post_request |
| 39 | else | 31 | else |
| 40 | false | 32 | false |
| 41 | end | 33 | end |
| 42 | - end# valid? | 34 | + end |
| 35 | + | ||
| 36 | + def validate_get_request | ||
| 37 | + true | ||
| 38 | + end | ||
| 39 | + | ||
| 40 | + def validate_post_request | ||
| 41 | + if @request.path_info.end_with?('git-upload-pack') | ||
| 42 | + can?(user, :push_code, project) | ||
| 43 | + elsif @request.path_info.end_with?('git-receive-pack') | ||
| 44 | + action = if project.protected_branch?(current_ref) | ||
| 45 | + :push_code_to_protected_branches | ||
| 46 | + else | ||
| 47 | + :push_code | ||
| 48 | + end | ||
| 49 | + | ||
| 50 | + can?(user, action, project) | ||
| 51 | + else | ||
| 52 | + false | ||
| 53 | + end | ||
| 54 | + end | ||
| 43 | 55 | ||
| 44 | def current_ref | 56 | def current_ref |
| 45 | if @env["HTTP_CONTENT_ENCODING"] =~ /gzip/ | 57 | if @env["HTTP_CONTENT_ENCODING"] =~ /gzip/ |