diff --git a/CHANGELOG b/CHANGELOG index aa7daa11947cea77563ba54bb287bed424719654..2f9b995f9e1a6e6db112cb5660b7057ad98b1b8c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,7 +53,7 @@ v 7.8.0 - Add a new API function that retrieves all issues assigned to a single milestone (Justin Whear and Hannes Rosenægger) - - - - + - API: Access groups with their path (Julien Bianchi) - - - diff --git a/doc/api/groups.md b/doc/api/groups.md index 9217c7a7f24b00997b2cba3f197b11f5746d5ffb..9f01b5506416de40bfc7cc05ba5d1ce2cac5b2ec 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -32,7 +32,7 @@ GET /groups/:id Parameters: -- `id` (required) - The ID of a group +- `id` (required) - The ID or path of a group ## New group @@ -58,7 +58,7 @@ POST /groups/:id/projects/:project_id Parameters: -- `id` (required) - The ID of a group +- `id` (required) - The ID or path of a group - `project_id` (required) - The ID of a project ## Remove group @@ -71,7 +71,7 @@ DELETE /groups/:id Parameters: -- `id` (required) - The ID of a user group +- `id` (required) - The ID or path of a user group ## Search for group @@ -148,7 +148,7 @@ POST /groups/:id/members Parameters: -- `id` (required) - The ID of a group +- `id` (required) - The ID or path of a group - `user_id` (required) - The ID of a user to add - `access_level` (required) - Project access level @@ -162,5 +162,5 @@ DELETE /groups/:id/members/:user_id Parameters: -- `id` (required) - The ID of a user group +- `id` (required) - The ID or path of a user group - `user_id` (required) - The ID of a group member diff --git a/lib/api/group_members.rb b/lib/api/group_members.rb index d596517c816e5cbc46cab7d08a34e4e763618956..4373070083a4038234c4563c2053de6b6f616caf 100644 --- a/lib/api/group_members.rb +++ b/lib/api/group_members.rb @@ -3,22 +3,6 @@ module API before { authenticate! } resource :groups do - helpers do - def find_group(id) - group = Group.find(id) - - if can?(current_user, :read_group, group) - group - else - render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403) - end - end - - def validate_access_level?(level) - Gitlab::Access.options_with_owner.values.include? level.to_i - end - end - # Get a list of group members viewable by the authenticated user. # # Example Request: diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 730dfad52c89c81bc065a498e1ec7b2bc824f3fd..384a28e41f5735e0ebce37fc047832c33f37f6c9 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -4,22 +4,6 @@ module API before { authenticate! } resource :groups do - helpers do - def find_group(id) - group = Group.find(id) - - if can?(current_user, :read_group, group) - group - else - render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403) - end - end - - def validate_access_level?(level) - Gitlab::Access.options_with_owner.values.include? level.to_i - end - end - # Get a groups list # # Example Request: diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 62c26ef76ceef32200f22d52966cc81976d829df..96249ea8cfe11659696a3292f4141d79dbce4433 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -55,6 +55,21 @@ module API end end + def find_group(id) + begin + group = Group.find(id) + rescue ActiveRecord::RecordNotFound + group = Group.find_by!(path: id) + end + + if can?(current_user, :read_group, group) + group + else + forbidden!("#{current_user.username} lacks sufficient "\ + "access to #{group.name}") + end + end + def paginate(relation) per_page = params[:per_page].to_i paginated = relation.page(params[:page]).per(per_page) @@ -135,10 +150,16 @@ module API errors end + def validate_access_level?(level) + Gitlab::Access.options_with_owner.values.include? level.to_i + end + # error helpers - def forbidden! - render_api_error!('403 Forbidden', 403) + def forbidden!(reason = nil) + message = ['403 Forbidden'] + message << " - #{reason}" if reason + render_api_error!(message.join(' '), 403) end def bad_request!(attribute) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 95f82463367b791d82bb3fc1ce30c5f32e8f9aff..8465d7652942a4e890d21a0fe34c229e17b27109 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -73,6 +73,24 @@ describe API::API, api: true do response.status.should == 404 end end + + context 'when using group path in URL' do + it 'should return any existing group' do + get api("/groups/#{group1.path}", admin) + response.status.should == 200 + json_response['name'] == group2.name + end + + it 'should not return a non existing group' do + get api('/groups/unknown', admin) + response.status.should == 404 + end + + it 'should not return a group not attached to user1' do + get api("/groups/#{group2.path}", user1) + response.status.should == 403 + end + end end describe "POST /groups" do