Commit 9bb1d8fc authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge pull request #7382 from Razer6/git_ref_validation

Validate branch/tag-names and references WebUI, API
parents 640a3c5c 39211391
...@@ -17,9 +17,17 @@ class Projects::BranchesController < Projects::ApplicationController ...@@ -17,9 +17,17 @@ class Projects::BranchesController < Projects::ApplicationController
end end
def create def create
@branch = CreateBranchService.new.execute(project, params[:branch_name], params[:ref], current_user) result = CreateBranchService.new.execute(project,
params[:branch_name],
params[:ref],
current_user)
if result[:status] == :success
@branch = result[:branch]
redirect_to project_tree_path(@project, @branch.name) redirect_to project_tree_path(@project, @branch.name)
else
@error = result[:message]
render action: 'new'
end
end end
def destroy def destroy
......
...@@ -13,10 +13,15 @@ class Projects::TagsController < Projects::ApplicationController ...@@ -13,10 +13,15 @@ class Projects::TagsController < Projects::ApplicationController
end end
def create def create
@tag = CreateTagService.new.execute(@project, params[:tag_name], result = CreateTagService.new.execute(@project, params[:tag_name],
params[:ref], current_user) params[:ref], current_user)
if result[:status] == :success
@tag = result[:tag]
redirect_to project_tags_path(@project) redirect_to project_tags_path(@project)
else
@error = result[:message]
render action: 'new'
end
end end
def destroy def destroy
......
class CreateBranchService class CreateBranchService
def execute(project, branch_name, ref, current_user) def execute(project, branch_name, ref, current_user)
valid_branch = Gitlab::GitRefValidator.validate(branch_name)
if valid_branch == false
return error('Branch name invalid')
end
repository = project.repository repository = project.repository
existing_branch = repository.find_branch(branch_name)
if existing_branch
return error('Branch already exists')
end
repository.add_branch(branch_name, ref) repository.add_branch(branch_name, ref)
new_branch = repository.find_branch(branch_name) new_branch = repository.find_branch(branch_name)
if new_branch if new_branch
Event.create_ref_event(project, current_user, new_branch, 'add') Event.create_ref_event(project, current_user, new_branch, 'add')
return success(new_branch)
else
return error('Invalid reference name')
end
end
def error(message)
{
message: message,
status: :error
}
end end
new_branch def success(branch)
{
branch: branch,
status: :success
}
end end
end end
class CreateTagService class CreateTagService
def execute(project, tag_name, ref, current_user) def execute(project, tag_name, ref, current_user)
valid_tag = Gitlab::GitRefValidator.validate(tag_name)
if valid_tag == false
return error('Tag name invalid')
end
repository = project.repository repository = project.repository
existing_tag = repository.find_tag(tag_name)
if existing_tag
return error('Tag already exists')
end
repository.add_tag(tag_name, ref) repository.add_tag(tag_name, ref)
new_tag = repository.find_tag(tag_name) new_tag = repository.find_tag(tag_name)
if new_tag if new_tag
Event.create_ref_event(project, current_user, new_tag, 'add', 'refs/tags') Event.create_ref_event(project, current_user, new_tag, 'add', 'refs/tags')
return success(new_tag)
else
return error('Invalid reference name')
end
end
def error(message)
{
message: message,
status: :error
}
end end
new_tag def success(branch)
{
tag: branch,
status: :success
}
end end
end end
...@@ -5,21 +5,21 @@ class DeleteBranchService ...@@ -5,21 +5,21 @@ class DeleteBranchService
# No such branch # No such branch
unless branch unless branch
return error('No such branch') return error('No such branch', 404)
end end
if branch_name == repository.root_ref if branch_name == repository.root_ref
return error('Cannot remove HEAD branch') return error('Cannot remove HEAD branch', 405)
end end
# Dont allow remove of protected branch # Dont allow remove of protected branch
if project.protected_branch?(branch_name) if project.protected_branch?(branch_name)
return error('Protected branch cant be removed') return error('Protected branch cant be removed', 405)
end end
# Dont allow user to remove branch if he is not allowed to push # Dont allow user to remove branch if he is not allowed to push
unless current_user.can?(:push_code, project) unless current_user.can?(:push_code, project)
return error('You dont have push access to repo') return error('You dont have push access to repo', 405)
end end
if repository.rm_branch(branch_name) if repository.rm_branch(branch_name)
...@@ -30,9 +30,10 @@ class DeleteBranchService ...@@ -30,9 +30,10 @@ class DeleteBranchService
end end
end end
def error(message) def error(message, return_code = 400)
{ {
message: message, message: message,
return_code: return_code,
state: :error state: :error
} }
end end
......
- if @error
.alert.alert-danger
%button{ type: "button", class: "close", "data-dismiss" => "alert"} &times;
= @error
%h3.page-title %h3.page-title
%i.icon-code-fork %i.icon-code-fork
New branch New branch
...@@ -5,11 +9,11 @@ ...@@ -5,11 +9,11 @@
.form-group .form-group
= label_tag :branch_name, 'Name for new branch', class: 'control-label' = label_tag :branch_name, 'Name for new branch', class: 'control-label'
.col-sm-10 .col-sm-10
= text_field_tag :branch_name, nil, placeholder: 'enter new branch name', required: true, tabindex: 1, class: 'form-control' = text_field_tag :branch_name, params[:branch_name], placeholder: 'enter new branch name', required: true, tabindex: 1, class: 'form-control'
.form-group .form-group
= label_tag :ref, 'Create from', class: 'control-label' = label_tag :ref, 'Create from', class: 'control-label'
.col-sm-10 .col-sm-10
= text_field_tag :ref, nil, placeholder: 'existing branch name, tag or commit SHA', required: true, tabindex: 2, class: 'form-control' = text_field_tag :ref, params[:ref], placeholder: 'existing branch name, tag or commit SHA', required: true, tabindex: 2, class: 'form-control'
.form-actions .form-actions
= submit_tag 'Create branch', class: 'btn btn-create', tabindex: 3 = submit_tag 'Create branch', class: 'btn btn-create', tabindex: 3
= link_to 'Cancel', project_branches_path(@project), class: 'btn btn-cancel' = link_to 'Cancel', project_branches_path(@project), class: 'btn btn-cancel'
......
- if @error
.alert.alert-danger
%button{ type: "button", class: "close", "data-dismiss" => "alert"} &times;
= @error
%h3.page-title %h3.page-title
%i.icon-code-fork %i.icon-code-fork
New tag New tag
...@@ -5,11 +9,11 @@ ...@@ -5,11 +9,11 @@
.form-group .form-group
= label_tag :tag_name, 'Name for new tag', class: 'control-label' = label_tag :tag_name, 'Name for new tag', class: 'control-label'
.col-sm-10 .col-sm-10
= text_field_tag :tag_name, nil, placeholder: 'v3.0.1', required: true, tabindex: 1, class: 'form-control' = text_field_tag :tag_name, params[:tag_name], placeholder: 'v3.0.1', required: true, tabindex: 1, class: 'form-control'
.form-group .form-group
= label_tag :ref, 'Create from', class: 'control-label' = label_tag :ref, 'Create from', class: 'control-label'
.col-sm-10 .col-sm-10
= text_field_tag :ref, nil, placeholder: 'master', required: true, tabindex: 2, class: 'form-control' = text_field_tag :ref, params[:ref], placeholder: 'master', required: true, tabindex: 2, class: 'form-control'
.light Branch name or commit SHA .light Branch name or commit SHA
.form-actions .form-actions
= submit_tag 'Create tag', class: 'btn btn-create', tabindex: 3 = submit_tag 'Create tag', class: 'btn btn-create', tabindex: 3
......
...@@ -196,6 +196,8 @@ Parameters: ...@@ -196,6 +196,8 @@ Parameters:
} }
``` ```
It return 200 if succeed or 400 if failed with error message explaining reason.
## Delete repository branch ## Delete repository branch
``` ```
...@@ -207,4 +209,5 @@ Parameters: ...@@ -207,4 +209,5 @@ Parameters:
- `id` (required) - The ID of a project - `id` (required) - The ID of a project
- `branch` (required) - The name of the branch - `branch` (required) - The name of the branch
It return 200 if succeed or 405 if failed with error message explaining reason. It return 200 if succeed, 404 if the branch to be deleted does not exist
or 400 for other reasons. In case of an error, an explaining message is provided.
...@@ -71,6 +71,9 @@ Parameters: ...@@ -71,6 +71,9 @@ Parameters:
] ]
``` ```
It returns 200 if the operation succeed. In case of an error,
405 with an explaining error message is returned.
## List repository tree ## List repository tree
Get a list of repository files and directories in a project. Get a list of repository files and directories in a project.
......
...@@ -15,7 +15,7 @@ Feature: Project Browse branches ...@@ -15,7 +15,7 @@ Feature: Project Browse branches
Scenario: I create a branch Scenario: I create a branch
Given I visit project branches page Given I visit project branches page
And I click new branch link And I click new branch link
When I submit new branch form And I submit new branch form
Then I should see new branch created Then I should see new branch created
@javascript @javascript
...@@ -23,3 +23,21 @@ Feature: Project Browse branches ...@@ -23,3 +23,21 @@ Feature: Project Browse branches
Given I visit project branches page Given I visit project branches page
And I click branch 'improve/awesome' delete link And I click branch 'improve/awesome' delete link
Then I should not see branch 'improve/awesome' Then I should not see branch 'improve/awesome'
Scenario: I create a branch with invalid name
Given I visit project branches page
And I click new branch link
And I submit new branch form with invalid name
Then I should see new an error that branch is invalid
Scenario: I create a branch with invalid reference
Given I visit project branches page
And I click new branch link
And I submit new branch form with invalid reference
Then I should see new an error that ref is invalid
Scenario: I create a branch that already exists
Given I visit project branches page
And I click new branch link
And I submit new branch form with branch that already exists
Then I should see new an error that branch already exists
...@@ -7,5 +7,25 @@ Feature: Project Browse tags ...@@ -7,5 +7,25 @@ Feature: Project Browse tags
Scenario: I can see all git tags Scenario: I can see all git tags
Then I should see "Shop" all tags list Then I should see "Shop" all tags list
Scenario: I create a tag
And I click new tag link
And I submit new tag form
Then I should see new tag created
Scenario: I create a tag with invalid name
And I click new tag link
And I submit new tag form with invalid name
Then I should see new an error that tag is invalid
Scenario: I create a tag with invalid reference
And I click new tag link
And I submit new tag form with invalid reference
Then I should see new an error that tag ref is invalid
Scenario: I create a tag that already exists
And I click new tag link
And I submit new tag form with tag that already exists
Then I should see new an error that tag already exists
# @wip # @wip
# Scenario: I can download project by tag # Scenario: I can download project by tag
...@@ -38,10 +38,38 @@ class ProjectBrowseBranches < Spinach::FeatureSteps ...@@ -38,10 +38,38 @@ class ProjectBrowseBranches < Spinach::FeatureSteps
click_button 'Create branch' click_button 'Create branch'
end end
step 'I submit new branch form with invalid name' do
fill_in 'branch_name', with: '1.0 stable'
fill_in 'ref', with: 'master'
click_button 'Create branch'
end
step 'I submit new branch form with invalid reference' do
fill_in 'branch_name', with: 'foo'
fill_in 'ref', with: 'foo'
click_button 'Create branch'
end
step 'I submit new branch form with branch that already exists' do
fill_in 'branch_name', with: 'master'
fill_in 'ref', with: 'master'
click_button 'Create branch'
end
step 'I should see new branch created' do step 'I should see new branch created' do
within '.tree-ref-holder' do
page.should have_content 'deploy_keys' page.should have_content 'deploy_keys'
end end
step 'I should see new an error that branch is invalid' do
page.should have_content 'Branch name invalid'
end
step 'I should see new an error that ref is invalid' do
page.should have_content 'Invalid reference name'
end
step 'I should see new an error that branch already exists' do
page.should have_content 'Branch already exists'
end end
step "I click branch 'improve/awesome' delete link" do step "I click branch 'improve/awesome' delete link" do
......
...@@ -3,8 +3,52 @@ class ProjectBrowseTags < Spinach::FeatureSteps ...@@ -3,8 +3,52 @@ class ProjectBrowseTags < Spinach::FeatureSteps
include SharedProject include SharedProject
include SharedPaths include SharedPaths
Then 'I should see "Shop" all tags list' do step 'I should see "Shop" all tags list' do
page.should have_content "Tags" page.should have_content "Tags"
page.should have_content "v1.0.0" page.should have_content "v1.0.0"
end end
step 'I click new tag link' do
click_link 'New tag'
end
step 'I submit new tag form' do
fill_in 'tag_name', with: 'v7.0'
fill_in 'ref', with: 'master'
click_button 'Create tag'
end
step 'I submit new tag form with invalid name' do
fill_in 'tag_name', with: 'v 1.0'
fill_in 'ref', with: 'master'
click_button 'Create tag'
end
step 'I submit new tag form with invalid reference' do
fill_in 'tag_name', with: 'foo'
fill_in 'ref', with: 'foo'
click_button 'Create tag'
end
step 'I submit new tag form with tag that already exists' do
fill_in 'tag_name', with: 'v1.0.0'
fill_in 'ref', with: 'master'
click_button 'Create tag'
end
step 'I should see new tag created' do
page.should have_content 'v7.0'
end
step 'I should see new an error that tag is invalid' do
page.should have_content 'Tag name invalid'
end
step 'I should see new an error that tag ref is invalid' do
page.should have_content 'Invalid reference name'
end
step 'I should see new an error that tag already exists' do
page.should have_content 'Tag already exists'
end
end end
...@@ -80,9 +80,17 @@ module API ...@@ -80,9 +80,17 @@ module API
# POST /projects/:id/repository/branches # POST /projects/:id/repository/branches
post ":id/repository/branches" do post ":id/repository/branches" do
authorize_push_project authorize_push_project
@branch = CreateBranchService.new.execute(user_project, params[:branch_name], params[:ref], current_user) result = CreateBranchService.new.execute(user_project,
params[:branch_name],
present @branch, with: Entities::RepoObject, project: user_project params[:ref],
current_user)
if result[:status] == :success
present result[:branch],
with: Entities::RepoObject,
project: user_project
else
render_api_error!(result[:message], 400)
end
end end
# Delete branch # Delete branch
...@@ -99,7 +107,7 @@ module API ...@@ -99,7 +107,7 @@ module API
if result[:state] == :success if result[:state] == :success
true true
else else
render_api_error!(result[:message], 405) render_api_error!(result[:message], result[:return_code])
end end
end end
end end
......
...@@ -36,10 +36,15 @@ module API ...@@ -36,10 +36,15 @@ module API
# POST /projects/:id/repository/tags # POST /projects/:id/repository/tags
post ':id/repository/tags' do post ':id/repository/tags' do
authorize_push_project authorize_push_project
@tag = CreateTagService.new.execute(user_project, params[:tag_name], result = CreateTagService.new.execute(user_project, params[:tag_name],
params[:ref], current_user) params[:ref], current_user)
if result[:status] == :success
present @tag, with: Entities::RepoObject, project: user_project present result[:tag],
with: Entities::RepoObject,
project: user_project
else
render_api_error!(result[:message], 400)
end
end end
# Get a project repository tree # Get a project repository tree
......
module Gitlab
module GitRefValidator
extend self
# Validates a given name against the git reference specification
#
# Returns true for a valid reference name, false otherwise
def validate(ref_name)
system *%W(git check-ref-format refs/#{ref_name})
end
end
end
require 'spec_helper'
describe Gitlab::GitRefValidator do
it { Gitlab::GitRefValidator.validate('feature/new').should be_true }
it { Gitlab::GitRefValidator.validate('implement_@all').should be_true }
it { Gitlab::GitRefValidator.validate('my_new_feature').should be_true }
it { Gitlab::GitRefValidator.validate('#1').should be_true }
it { Gitlab::GitRefValidator.validate('feature/~new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/^new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/:new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/?new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/*new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/[new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/new/').should be_false }
it { Gitlab::GitRefValidator.validate('feature/new.').should be_false }
it { Gitlab::GitRefValidator.validate('feature\@{').should be_false }
it { Gitlab::GitRefValidator.validate('feature\new').should be_false }
it { Gitlab::GitRefValidator.validate('feature//new').should be_false }
it { Gitlab::GitRefValidator.validate('feature new').should be_false }
end
...@@ -94,12 +94,12 @@ describe API::API, api: true do ...@@ -94,12 +94,12 @@ describe API::API, api: true do
describe "POST /projects/:id/repository/branches" do describe "POST /projects/:id/repository/branches" do
it "should create a new branch" do it "should create a new branch" do
post api("/projects/#{project.id}/repository/branches", user), post api("/projects/#{project.id}/repository/branches", user),
branch_name: branch_name, branch_name: 'feature1',
ref: branch_sha ref: branch_sha
response.status.should == 201 response.status.should == 201
json_response['name'].should == branch_name json_response['name'].should == 'feature1'
json_response['commit']['id'].should == branch_sha json_response['commit']['id'].should == branch_sha
end end
...@@ -107,9 +107,37 @@ describe API::API, api: true do ...@@ -107,9 +107,37 @@ describe API::API, api: true do
post api("/projects/#{project.id}/repository/branches", user2), post api("/projects/#{project.id}/repository/branches", user2),
branch_name: branch_name, branch_name: branch_name,
ref: branch_sha ref: branch_sha
response.status.should == 403 response.status.should == 403
end end
it 'should return 400 if branch name is invalid' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new design',
ref: branch_sha
response.status.should == 400
json_response['message'].should == 'Branch name invalid'
end
it 'should return 400 if branch already exists' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design1',
ref: branch_sha
response.status.should == 201
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design1',
ref: branch_sha
response.status.should == 400
json_response['message'].should == 'Branch already exists'
end
it 'should return 400 if ref name is invalid' do
post api("/projects/#{project.id}/repository/branches", user),
branch_name: 'new_design3',
ref: 'foo'
response.status.should == 400
json_response['message'].should == 'Invalid reference name'
end
end end
describe "DELETE /projects/:id/repository/branches/:branch" do describe "DELETE /projects/:id/repository/branches/:branch" do
...@@ -120,6 +148,11 @@ describe API::API, api: true do ...@@ -120,6 +148,11 @@ describe API::API, api: true do
response.status.should == 200 response.status.should == 200
end end
it 'should return 404 if branch not exists' do
delete api("/projects/#{project.id}/repository/branches/foobar", user)
response.status.should == 404
end
it "should remove protected branch" do it "should remove protected branch" do
project.protected_branches.create(name: branch_name) project.protected_branches.create(name: branch_name)
delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user)
......
...@@ -25,20 +25,46 @@ describe API::API, api: true do ...@@ -25,20 +25,46 @@ describe API::API, api: true do
describe 'POST /projects/:id/repository/tags' do describe 'POST /projects/:id/repository/tags' do
it 'should create a new tag' do it 'should create a new tag' do
post api("/projects/#{project.id}/repository/tags", user), post api("/projects/#{project.id}/repository/tags", user),
tag_name: 'v1.0.0', tag_name: 'v2.0.0',
ref: 'master' ref: 'master'
response.status.should == 201 response.status.should == 201
json_response['name'].should == 'v1.0.0' json_response['name'].should == 'v2.0.0'
end end
it 'should deny for user without push access' do it 'should deny for user without push access' do
post api("/projects/#{project.id}/repository/tags", user2), post api("/projects/#{project.id}/repository/tags", user2),
tag_name: 'v1.0.0', tag_name: 'v1.0.0',
ref: '621491c677087aa243f165eab467bfdfbee00be1' ref: '621491c677087aa243f165eab467bfdfbee00be1'
response.status.should == 403 response.status.should == 403
end end
it 'should return 400 if tag name is invalid' do
post api("/projects/#{project.id}/repository/tags", user),
tag_name: 'v 1.0.0',
ref: 'master'
response.status.should == 400
json_response['message'].should == 'Tag name invalid'
end
it 'should return 400 if tag already exists' do
post api("/projects/#{project.id}/repository/tags", user),
tag_name: 'v8.0.0',
ref: 'master'
response.status.should == 201
post api("/projects/#{project.id}/repository/tags", user),
tag_name: 'v8.0.0',
ref: 'master'
response.status.should == 400
json_response['message'].should == 'Tag already exists'
end
it 'should return 400 if ref name is invalid' do
post api("/projects/#{project.id}/repository/tags", user),
tag_name: 'mytag',
ref: 'foo'
response.status.should == 400
json_response['message'].should == 'Invalid reference name'
end
end end
describe "GET /projects/:id/repository/tree" do describe "GET /projects/:id/repository/tree" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment