Merge pull request #4990 from karlhungus/feature_group_membership_api

Add group membership api

4 files changed, 249 insertions, 15 deletions

diff --git a/doc/api/groups.md b/doc/api/groups.md
index e9702ea2cd1..9c551fff83a 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -55,3 +55,65 @@ POST  /groups/:id/projects/:project_id
 Parameters:
 + `id` (required) - The ID of a group
 + `project_id (required) - The ID of a project
+
+
+## Group members
+
+### List group members
+
+Get a list of group members viewable by the authenticated user.
+
+```
+GET /groups/:id/members
+```
+
+```json
+[
+  {
+    id: 1,
+    username: "raymond_smith",
+    email: "ray@smith.org",
+    name: "Raymond Smith",
+    state: "active",
+    created_at: "2012-10-22T14:13:35Z",
+    access_level: 30
+  },
+  {
+    id: 2,
+    username: "john_doe",
+    email: "joh@doe.org",
+    name: "John Doe",
+    state: "active",
+    created_at: "2012-10-22T14:13:35Z",
+    access_level: 30
+  }
+]
+```
+
+### Add group member
+
+Adds a user to the list of group members.
+
+```
+POST /groups/:id/members
+```
+
+Parameters:
+
++ `id` (required) - The ID of a group
++ `user_id` (required) - The ID of a user to add
++ `access_level` (required) - Project access level
+
+
+### Remove user team member
+
+Removes user from user team.
+
+```
+DELETE /groups/:id/members/:user_id
+```
+
+Parameters:
+
++ `id` (required) - The ID of a user group
++ `user_id` (required) - The ID of a group member
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index f15ca35e954..1f35e9ec5fc 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -67,6 +67,12 @@ module API
       expose :projects, using: Entities::Project
     end
 
+    class GroupMember < UserBasic
+      expose :group_access, as: :access_level do |user, options|
+        options[:group].users_groups.find_by_user_id(user.id).group_access
+      end
+    end
+
     class RepoObject < Grape::Entity
       expose :name, :commit
       expose :protected do |repo, options|
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 701f6777b77..396554404af 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -4,6 +4,20 @@ module API
     before { authenticate! }
 
     resource :groups do
+      helpers do
+        def find_group(id)
+          group = Group.find(id)
+          if current_user.admin or current_user.groups.include? 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:
@@ -46,12 +60,8 @@ module API
       # Example Request:
       #   GET /groups/:id
       get ":id" do
-        @group = Group.find(params[:id])
-        if current_user.admin or current_user.groups.include? @group
-          present @group, with: Entities::GroupDetail
-        else
-          not_found!
-        end
+        group = find_group(params[:id])
+        present group, with: Entities::GroupDetail
       end
 
       # Transfer a project to the Group namespace
@@ -71,6 +81,58 @@ module API
           not_found!
         end
       end
+
+      # Get a list of group members viewable by the authenticated user.
+      #
+      # Example Request:
+      #  GET /groups/:id/members
+      get ":id/members" do
+        group = find_group(params[:id])
+        members = group.users_groups
+        users = (paginate members).collect(&:user)
+        present users, with: Entities::GroupMember, group: group
+      end
+
+      # Add a user to the list of group members
+      #
+      # Parameters:
+      #   id (required) - group id
+      #   user_id (required) - the users id
+      #   access_level (required) - Project access level
+      # Example Request:
+      #  POST /groups/:id/members
+      post ":id/members" do
+        required_attributes! [:user_id, :access_level]
+        unless validate_access_level?(params[:access_level])
+          render_api_error!("Wrong access level", 422)
+        end
+        group = find_group(params[:id])
+        if group.users_groups.find_by_user_id(params[:user_id])
+          render_api_error!("Already exists", 409)
+        end
+        group.add_users([params[:user_id]], params[:access_level])
+        member = group.users_groups.find_by_user_id(params[:user_id])
+        present member.user, with: Entities::GroupMember, group: group
+      end
+
+      # Remove member.
+      #
+      # Parameters:
+      #   id (required) - group id
+      #   user_id (required) - the users id
+      #
+      # Example Request:
+      #   DELETE /groups/:id/members/:user_id
+      delete ":id/members/:user_id" do
+        group = find_group(params[:id])
+        member =  group.users_groups.find_by_user_id(params[:user_id])
+        if member.nil?
+          render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}",404)
+        else
+          member.destroy
+        end
+      end
+
     end
   end
 end
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index d2e25fb9e23..f7fd27523b0 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -3,11 +3,11 @@ require 'spec_helper'
 describe API::API do
   include ApiHelpers
 
-  let(:user1)  { create(:user) }
-  let(:user2)  { create(:user) }
+  let(:user1) { create(:user) }
+  let(:user2) { create(:user) }
   let(:admin) { create(:admin) }
-  let!(:group1)  { create(:group, owner: user1) }
-  let!(:group2)  { create(:group, owner: user2) }
+  let!(:group1) { create(:group, owner: user1) }
+  let!(:group2) { create(:group, owner: user2) }
 
   describe "GET /groups" do
     context "when unauthenticated" do
@@ -52,7 +52,7 @@ describe API::API do
 
       it "should not return a group not attached to user1" do
         get api("/groups/#{group2.id}", user1)
-        response.status.should == 404
+        response.status.should == 403
       end
     end
 
@@ -90,7 +90,7 @@ describe API::API do
       end
 
       it "should return 400 bad request error if name not given" do
-        post api("/groups", admin), { path: group2.path }
+        post api("/groups", admin), {path: group2.path}
         response.status.should == 400
       end
 
@@ -104,11 +104,10 @@ describe API::API do
   describe "POST /groups/:id/projects/:project_id" do
     let(:project) { create(:project) }
     before(:each) do
-       project.stub!(:transfer).and_return(true)
-       Project.stub(:find).and_return(project)
+      project.stub!(:transfer).and_return(true)
+      Project.stub(:find).and_return(project)
     end
 
-
     context "when authenticated as user" do
       it "should not transfer project to group" do
         post api("/groups/#{group1.id}/projects/#{project.id}", user2)
@@ -123,4 +122,109 @@ describe API::API do
       end
     end
   end
+
+  describe "members" do
+    let(:owner) { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:developer) { create(:user) }
+    let(:master) { create(:user) }
+    let(:guest) { create(:user) }
+    let!(:group_with_members) do
+      group = create(:group, owner: owner)
+      group.add_users([reporter.id], UsersGroup::REPORTER)
+      group.add_users([developer.id], UsersGroup::DEVELOPER)
+      group.add_users([master.id], UsersGroup::MASTER)
+      group.add_users([guest.id], UsersGroup::GUEST)
+      group
+    end
+    let!(:group_no_members) { create(:group, owner: owner) }
+
+    describe "GET /groups/:id/members" do
+      context "when authenticated as user that is part or the group" do
+        it "each user: should return an array of members groups of group3" do
+          [owner, master, developer, reporter, guest].each do |user|
+            get api("/groups/#{group_with_members.id}/members", user)
+            response.status.should == 200
+            json_response.should be_an Array
+            json_response.size.should == 5
+            json_response.find { |e| e['id']==owner.id }['access_level'].should == UsersGroup::OWNER
+            json_response.find { |e| e['id']==reporter.id }['access_level'].should == UsersGroup::REPORTER
+            json_response.find { |e| e['id']==developer.id }['access_level'].should == UsersGroup::DEVELOPER
+            json_response.find { |e| e['id']==master.id }['access_level'].should == UsersGroup::MASTER
+            json_response.find { |e| e['id']==guest.id }['access_level'].should == UsersGroup::GUEST
+          end
+        end
+
+        it "users not part of the group should get access error" do
+          get api("/groups/#{group_with_members.id}/members", user1)
+          response.status.should == 403
+        end
+      end
+    end
+
+    describe "POST /groups/:id/members" do
+      context "when not a member of the group" do
+        it "should not add guest as member of group_no_members when adding being done by person outside the group" do
+          post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: UsersGroup::MASTER
+          response.status.should == 403
+        end
+      end
+
+      context "when a member of the group" do
+        it "should return ok and add new member" do
+          count_before=group_no_members.users_groups.count
+          new_user = create(:user)
+          post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: UsersGroup::MASTER
+          response.status.should == 201
+          json_response['name'].should == new_user.name
+          json_response['access_level'].should == UsersGroup::MASTER
+          group_no_members.users_groups.count.should == count_before + 1
+        end
+
+        it "should return error if member already exists" do
+          post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: UsersGroup::MASTER
+          response.status.should == 409
+        end
+
+        it "should return a 400 error when user id is not given" do
+          post api("/groups/#{group_no_members.id}/members", owner), access_level: UsersGroup::MASTER
+          response.status.should == 400
+        end
+
+        it "should return a 400 error when access level is not given" do
+          post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id
+          response.status.should == 400
+        end
+
+        it "should return a 422 error when access level is not known" do
+          post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id, access_level: 1234
+          response.status.should == 422
+        end
+      end
+    end
+
+    describe "DELETE /groups/:id/members/:user_id" do
+      context "when not a member of the group" do
+        it "should not delete guest's membership of group_with_members" do
+          random_user = create(:user)
+          delete api("/groups/#{group_with_members.id}/members/#{owner.id}", random_user)
+          response.status.should == 403
+        end
+      end
+
+      context "when a member of the group" do
+        it "should delete guest's membership of group" do
+          count_before=group_with_members.users_groups.count
+          delete api("/groups/#{group_with_members.id}/members/#{guest.id}", owner)
+          response.status.should == 200
+          group_with_members.users_groups.count.should == count_before - 1
+        end
+
+        it "should return a 404 error when user id is not known" do
+          delete api("/groups/#{group_with_members.id}/members/1328", owner)
+          response.status.should == 404
+        end
+      end
+    end
+  end
 end