Bug 524977

Summary:	using ipa group-add-member listing users already in the group causes a internal error
Product:	[Retired] freeIPA	Reporter:	Michael Gregg <mgregg>
Component:	ipa-admintools	Assignee:	Rob Crittenden <rcritten>
Status:	CLOSED UPSTREAM	QA Contact:	Chandrasekar Kannan <ckannan>
Severity:	medium	Docs Contact:
Priority:	low
Version:	unspecified	CC:	benl, dpal
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-03-28 09:33:49 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	431020

Description Michael Gregg 2009-09-22 20:53:46 UTC

Description of problem:
calling ipa group-add-member to add a member to a group causes a internal error if the user already exists in the group.
I admit that this is not something that you would normally do, A better error message would be nice.

Version-Release number of selected component (if applicable):
ipa-server-2.0-4.20090911.el5ipa

How reproducible:
always

Steps to Reproduce:
1. create user1
2. create group1
3. add user1 to group 1 with "ipa group-add-member --users user1 group1"
4. try "ipa group-add-member --users user1 group1" again.
  
Actual results:
ipa group-add-member --users user1 group1
ipa: ERROR: an internal error has occurred

Expected results:
ipa group-add-member --users user1 group1
ipa: ERROR: user1 already exists in group1

Comment 1 Pavel Zuna 2009-11-20 15:41:58 UTC

Can't reproduce anymore. If someone is trying to add a user that is already present, the command will just ignore this and report the number of users actually added (in this case zero).

Comment 2 Jenny Severance 2009-11-20 15:55:31 UTC

Does it report back that the user is already a member and return a non-zero return code?  If not, than it is not user friendly to the admin.

Comment 3 Rob Crittenden 2009-11-20 16:16:46 UTC

Pavel, I think this one might be a candidate for an additional attribute in the return values when Jason converts it to a dict. Not sure exactly the format we'd return things in, we'd have to think about the best way for the client to handle it.

Comment 4 Pavel Zuna 2009-11-23 12:17:55 UTC

It does report objects that failed to add/remove themselves:

# ./ipa group-add-member group1 --users=admin,pzuna --groups=ahoj
-----------------
group-add-member:
-----------------
Group: group1
  member groups: ahoj
  member users: pzuna, admin
----------------
3 members added.
----------------

# ./ipa group-add-member group1 --users=admin,mnagy --groups=ahoj
groups failed: ahoj
users failed: admin
-----------------
group-add-member:
-----------------
Group: group1
  member groups: ahoj
  member users: pzuna, admin, mnagy
---------------
1 member added.
---------------

It doesn't exit with non-zero however. Maybe we should discuss exit codes again, because last time (just checked), we didn't reach any conclusive agreement. I think that we should at least make a distinction between success/partial-success/epic-fail. Partial success might be a truncated search result and only some members added/removed.