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-admintoolsAssignee: Rob Crittenden <rcritten>
Status: CLOSED UPSTREAM QA Contact: Chandrasekar Kannan <ckannan>
Severity: medium Docs Contact:
Priority: low    
Version: unspecifiedCC: 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: ---
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.