Merge pull request #4990 from karlhungus/feature_group_membership_api

Add group membership api

Merge pull request #4990 from karlhungus/feature_group_membership_api
Add group membership api
Dmitriy Zaporozhets
2 parents 9109a207 fadcc251
Showing 4 changed files with 249 additions and 15 deletions Show diff stats
doc/api/groups.md
lib/api/entities.rb
lib/api/groups.rb
spec/requests/api/groups_spec.rb
@@ -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
@@ -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|
@@ -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
@@ -3,11 +3,11 @@ require &#39;spec_helper&#39;
 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
	@@ -55,3 +55,65 @@ POST /groups/:id/projects/:project_id		@@ -55,3 +55,65 @@ POST /groups/:id/projects/:project_id
55	Parameters:	55	Parameters:
56	+ `id` (required) - The ID of a group	56	+ `id` (required) - The ID of a group
57	+ `project_id (required) - The ID of a project	57	+ `project_id (required) - The ID of a project
		58	+
		59	+
		60	+## Group members
		61	+
		62	+### List group members
		63	+
		64	+Get a list of group members viewable by the authenticated user.
		65	+
		66	+```
		67	+GET /groups/:id/members
		68	+```
		69	+
		70	+```json
		71	+[
		72	+ {
		73	+ id: 1,
		74	+ username: "raymond_smith",
		75	+ email: "ray@smith.org",
		76	+ name: "Raymond Smith",
		77	+ state: "active",
		78	+ created_at: "2012-10-22T14:13:35Z",
		79	+ access_level: 30
		80	+ },
		81	+ {
		82	+ id: 2,
		83	+ username: "john_doe",
		84	+ email: "joh@doe.org",
		85	+ name: "John Doe",
		86	+ state: "active",
		87	+ created_at: "2012-10-22T14:13:35Z",
		88	+ access_level: 30
		89	+ }
		90	+]
		91	+```
		92	+
		93	+### Add group member
		94	+
		95	+Adds a user to the list of group members.
		96	+
		97	+```
		98	+POST /groups/:id/members
		99	+```
		100	+
		101	+Parameters:
		102	+
		103	++ `id` (required) - The ID of a group
		104	++ `user_id` (required) - The ID of a user to add
		105	++ `access_level` (required) - Project access level
		106	+
		107	+
		108	+### Remove user team member
		109	+
		110	+Removes user from user team.
		111	+
		112	+```
		113	+DELETE /groups/:id/members/:user_id
		114	+```
		115	+
		116	+Parameters:
		117	+
		118	++ `id` (required) - The ID of a user group
		119	++ `user_id` (required) - The ID of a group member
	@@ -67,6 +67,12 @@ module API		@@ -67,6 +67,12 @@ module API
67	expose :projects, using: Entities::Project	67	expose :projects, using: Entities::Project
68	end	68	end
69		69
		70	+ class GroupMember < UserBasic
		71	+ expose :group_access, as: :access_level do \|user, options\|
		72	+ options[:group].users_groups.find_by_user_id(user.id).group_access
		73	+ end
		74	+ end
		75	+
70	class RepoObject < Grape::Entity	76	class RepoObject < Grape::Entity
71	expose :name, :commit	77	expose :name, :commit
72	expose :protected do \|repo, options\|	78	expose :protected do \|repo, options\|
	@@ -4,6 +4,20 @@ module API		@@ -4,6 +4,20 @@ module API
4	before { authenticate! }	4	before { authenticate! }
5		5
6	resource :groups do	6	resource :groups do
		7	+ helpers do
		8	+ def find_group(id)
		9	+ group = Group.find(id)
		10	+ if current_user.admin or current_user.groups.include? group
		11	+ group
		12	+ else
		13	+ render_api_error!("403 Forbidden - #{current_user.username} lacks sufficient access to #{group.name}", 403)
		14	+ end
		15	+ end
		16	+ def validate_access_level?(level)
		17	+ Gitlab::Access.options_with_owner.values.include? level.to_i
		18	+ end
		19	+ end
		20	+
7	# Get a groups list	21	# Get a groups list
8	#	22	#
9	# Example Request:	23	# Example Request:
	@@ -46,12 +60,8 @@ module API		@@ -46,12 +60,8 @@ module API
46	# Example Request:	60	# Example Request:
47	# GET /groups/:id	61	# GET /groups/:id
48	get ":id" do	62	get ":id" do
49	- @group = Group.find(params[:id])
50	- if current_user.admin or current_user.groups.include? @group
51	- present @group, with: Entities::GroupDetail
52	- else
53	- not_found!
54	- end	63	+ group = find_group(params[:id])
		64	+ present group, with: Entities::GroupDetail
55	end	65	end
56		66
57	# Transfer a project to the Group namespace	67	# Transfer a project to the Group namespace
	@@ -71,6 +81,58 @@ module API		@@ -71,6 +81,58 @@ module API
71	not_found!	81	not_found!
72	end	82	end
73	end	83	end
		84	+
		85	+ # Get a list of group members viewable by the authenticated user.
		86	+ #
		87	+ # Example Request:
		88	+ # GET /groups/:id/members
		89	+ get ":id/members" do
		90	+ group = find_group(params[:id])
		91	+ members = group.users_groups
		92	+ users = (paginate members).collect(&:user)
		93	+ present users, with: Entities::GroupMember, group: group
		94	+ end
		95	+
		96	+ # Add a user to the list of group members
		97	+ #
		98	+ # Parameters:
		99	+ # id (required) - group id
		100	+ # user_id (required) - the users id
		101	+ # access_level (required) - Project access level
		102	+ # Example Request:
		103	+ # POST /groups/:id/members
		104	+ post ":id/members" do
		105	+ required_attributes! [:user_id, :access_level]
		106	+ unless validate_access_level?(params[:access_level])
		107	+ render_api_error!("Wrong access level", 422)
		108	+ end
		109	+ group = find_group(params[:id])
		110	+ if group.users_groups.find_by_user_id(params[:user_id])
		111	+ render_api_error!("Already exists", 409)
		112	+ end
		113	+ group.add_users([params[:user_id]], params[:access_level])
		114	+ member = group.users_groups.find_by_user_id(params[:user_id])
		115	+ present member.user, with: Entities::GroupMember, group: group
		116	+ end
		117	+
		118	+ # Remove member.
		119	+ #
		120	+ # Parameters:
		121	+ # id (required) - group id
		122	+ # user_id (required) - the users id
		123	+ #
		124	+ # Example Request:
		125	+ # DELETE /groups/:id/members/:user_id
		126	+ delete ":id/members/:user_id" do
		127	+ group = find_group(params[:id])
		128	+ member = group.users_groups.find_by_user_id(params[:user_id])
		129	+ if member.nil?
		130	+ render_api_error!("404 Not Found - user_id:#{params[:user_id]} not a member of group #{group.name}",404)
		131	+ else
		132	+ member.destroy
		133	+ end
		134	+ end
		135	+
74	end	136	end
75	end	137	end
76	end	138	end
	@@ -3,11 +3,11 @@ require 'spec_helper'		@@ -3,11 +3,11 @@ require 'spec_helper'
3	describe API::API do	3	describe API::API do
4	include ApiHelpers	4	include ApiHelpers
5		5
6	- let(:user1) { create(:user) }
7	- let(:user2) { create(:user) }	6	+ let(:user1) { create(:user) }
		7	+ let(:user2) { create(:user) }
8	let(:admin) { create(:admin) }	8	let(:admin) { create(:admin) }
9	- let!(:group1) { create(:group, owner: user1) }
10	- let!(:group2) { create(:group, owner: user2) }	9	+ let!(:group1) { create(:group, owner: user1) }
		10	+ let!(:group2) { create(:group, owner: user2) }
11		11
12	describe "GET /groups" do	12	describe "GET /groups" do
13	context "when unauthenticated" do	13	context "when unauthenticated" do
	@@ -52,7 +52,7 @@ describe API::API do		@@ -52,7 +52,7 @@ describe API::API do
52		52
53	it "should not return a group not attached to user1" do	53	it "should not return a group not attached to user1" do
54	get api("/groups/#{group2.id}", user1)	54	get api("/groups/#{group2.id}", user1)
55	- response.status.should == 404	55	+ response.status.should == 403
56	end	56	end
57	end	57	end
58		58
	@@ -90,7 +90,7 @@ describe API::API do		@@ -90,7 +90,7 @@ describe API::API do
90	end	90	end
91		91
92	it "should return 400 bad request error if name not given" do	92	it "should return 400 bad request error if name not given" do
93	- post api("/groups", admin), { path: group2.path }	93	+ post api("/groups", admin), {path: group2.path}
94	response.status.should == 400	94	response.status.should == 400
95	end	95	end
96		96
	@@ -104,11 +104,10 @@ describe API::API do		@@ -104,11 +104,10 @@ describe API::API do
104	describe "POST /groups/:id/projects/:project_id" do	104	describe "POST /groups/:id/projects/:project_id" do
105	let(:project) { create(:project) }	105	let(:project) { create(:project) }
106	before(:each) do	106	before(:each) do
107	- project.stub!(:transfer).and_return(true)
108	- Project.stub(:find).and_return(project)	107	+ project.stub!(:transfer).and_return(true)
		108	+ Project.stub(:find).and_return(project)
109	end	109	end
110		110
111	-
112	context "when authenticated as user" do	111	context "when authenticated as user" do
113	it "should not transfer project to group" do	112	it "should not transfer project to group" do
114	post api("/groups/#{group1.id}/projects/#{project.id}", user2)	113	post api("/groups/#{group1.id}/projects/#{project.id}", user2)
	@@ -123,4 +122,109 @@ describe API::API do		@@ -123,4 +122,109 @@ describe API::API do
123	end	122	end
124	end	123	end
125	end	124	end
		125	+
		126	+ describe "members" do
		127	+ let(:owner) { create(:user) }
		128	+ let(:reporter) { create(:user) }
		129	+ let(:developer) { create(:user) }
		130	+ let(:master) { create(:user) }
		131	+ let(:guest) { create(:user) }
		132	+ let!(:group_with_members) do
		133	+ group = create(:group, owner: owner)
		134	+ group.add_users([reporter.id], UsersGroup::REPORTER)
		135	+ group.add_users([developer.id], UsersGroup::DEVELOPER)
		136	+ group.add_users([master.id], UsersGroup::MASTER)
		137	+ group.add_users([guest.id], UsersGroup::GUEST)
		138	+ group
		139	+ end
		140	+ let!(:group_no_members) { create(:group, owner: owner) }
		141	+
		142	+ describe "GET /groups/:id/members" do
		143	+ context "when authenticated as user that is part or the group" do
		144	+ it "each user: should return an array of members groups of group3" do
		145	+ [owner, master, developer, reporter, guest].each do \|user\|
		146	+ get api("/groups/#{group_with_members.id}/members", user)
		147	+ response.status.should == 200
		148	+ json_response.should be_an Array
		149	+ json_response.size.should == 5
		150	+ json_response.find { \|e\| e['id']==owner.id }['access_level'].should == UsersGroup::OWNER
		151	+ json_response.find { \|e\| e['id']==reporter.id }['access_level'].should == UsersGroup::REPORTER
		152	+ json_response.find { \|e\| e['id']==developer.id }['access_level'].should == UsersGroup::DEVELOPER
		153	+ json_response.find { \|e\| e['id']==master.id }['access_level'].should == UsersGroup::MASTER
		154	+ json_response.find { \|e\| e['id']==guest.id }['access_level'].should == UsersGroup::GUEST
		155	+ end
		156	+ end
		157	+
		158	+ it "users not part of the group should get access error" do
		159	+ get api("/groups/#{group_with_members.id}/members", user1)
		160	+ response.status.should == 403
		161	+ end
		162	+ end
		163	+ end
		164	+
		165	+ describe "POST /groups/:id/members" do
		166	+ context "when not a member of the group" do
		167	+ it "should not add guest as member of group_no_members when adding being done by person outside the group" do
		168	+ post api("/groups/#{group_no_members.id}/members", reporter), user_id: guest.id, access_level: UsersGroup::MASTER
		169	+ response.status.should == 403
		170	+ end
		171	+ end
		172	+
		173	+ context "when a member of the group" do
		174	+ it "should return ok and add new member" do
		175	+ count_before=group_no_members.users_groups.count
		176	+ new_user = create(:user)
		177	+ post api("/groups/#{group_no_members.id}/members", owner), user_id: new_user.id, access_level: UsersGroup::MASTER
		178	+ response.status.should == 201
		179	+ json_response['name'].should == new_user.name
		180	+ json_response['access_level'].should == UsersGroup::MASTER
		181	+ group_no_members.users_groups.count.should == count_before + 1
		182	+ end
		183	+
		184	+ it "should return error if member already exists" do
		185	+ post api("/groups/#{group_with_members.id}/members", owner), user_id: master.id, access_level: UsersGroup::MASTER
		186	+ response.status.should == 409
		187	+ end
		188	+
		189	+ it "should return a 400 error when user id is not given" do
		190	+ post api("/groups/#{group_no_members.id}/members", owner), access_level: UsersGroup::MASTER
		191	+ response.status.should == 400
		192	+ end
		193	+
		194	+ it "should return a 400 error when access level is not given" do
		195	+ post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id
		196	+ response.status.should == 400
		197	+ end
		198	+
		199	+ it "should return a 422 error when access level is not known" do
		200	+ post api("/groups/#{group_no_members.id}/members", owner), user_id: master.id, access_level: 1234
		201	+ response.status.should == 422
		202	+ end
		203	+ end
		204	+ end
		205	+
		206	+ describe "DELETE /groups/:id/members/:user_id" do
		207	+ context "when not a member of the group" do
		208	+ it "should not delete guest's membership of group_with_members" do
		209	+ random_user = create(:user)
		210	+ delete api("/groups/#{group_with_members.id}/members/#{owner.id}", random_user)
		211	+ response.status.should == 403
		212	+ end
		213	+ end
		214	+
		215	+ context "when a member of the group" do
		216	+ it "should delete guest's membership of group" do
		217	+ count_before=group_with_members.users_groups.count
		218	+ delete api("/groups/#{group_with_members.id}/members/#{guest.id}", owner)
		219	+ response.status.should == 200
		220	+ group_with_members.users_groups.count.should == count_before - 1
		221	+ end
		222	+
		223	+ it "should return a 404 error when user id is not known" do
		224	+ delete api("/groups/#{group_with_members.id}/members/1328", owner)
		225	+ response.status.should == 404
		226	+ end
		227	+ end
		228	+ end
		229	+ end
126	end	230	end