From 4e97f26649a7756bef843fca74e3c58eadd117e1 Mon Sep 17 00:00:00 2001
From: jubianchi <contact@jubianchi.fr>
Date: Fri, 30 Jan 2015 10:46:08 +0100
Subject: [PATCH] Acces groups with their path in API

---
 CHANGELOG                        |  2 +-
 doc/api/groups.md                | 10 +++++-----
 lib/api/group_members.rb         | 16 ----------------
 lib/api/groups.rb                | 16 ----------------
 lib/api/helpers.rb               | 25 +++++++++++++++++++++++--
 spec/requests/api/groups_spec.rb | 18 ++++++++++++++++++
 6 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index aa7daa11947..2f9b995f9e1 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 9217c7a7f24..9f01b550641 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 d596517c816..4373070083a 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 730dfad52c8..384a28e41f5 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 62c26ef76ce..96249ea8cfe 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 95f82463367..8465d765294 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