Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/18/2015 03:42 PM, David Kupka wrote: Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a): On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On Thu, 2015-06-18 at 09:30 +0200, Jan Cholasta wrote: Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a): On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite of --preserve. The attached patch fixes
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite of --preserve. The attached patch fixes both. ... and it also accidentaly changes the
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/18/2015 01:22 PM, Petr Vobornik wrote: On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/18/2015 03:42 PM, David Kupka wrote: Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a): On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite of --preserve. The attached patch fixes both. -- Jan Cholasta From 0a59f9394fc2e8f84142d45b2c588dac19c0c611 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu,
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite of --preserve. The attached patch fixes both. ... and it also accidentaly changes the default behavior. Updated patch attached. -- Jan
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False Simo. -- Simo Sorce * Red Hat, Inc * New York -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. Simo. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a):
Dne 18.5.2015 v 10:33 thierry bordaz napsal(a):
On 05/15/2015 04:44 PM, David Kupka wrote:
Hello Thierry,
thanks for the patch set. Overall functionality of ULC feature looks
good to
me and is definitely alpha ready.
I found following issues but don't insist on fixing it right now:
1) When stageuser-activate fails due to already existent
active/deleted user.
DN is show instead of user name that's used in other commands (user-add,
stageuser-add).
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user
uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
already exists
Hi David, Jan,
Thanks you so much for all those tests and feedback. I agree, some minor
bugs can be fixed separatly from this main patches.
You are right, It should return the user ID not the DN.
2) According to the design there should be '--only-delete' and
'--also-delete'
options for user-find command instead there is '--preserved' option.
Honza proposed adding virtual boolean attribute 'deleted' to user
entry and
filter on it.
The 'deleted' attribute would be useful also in user-show where is no
way to
tell if the displayed user is active or deleted. (Except running with
--all
and looking on the dn).
Yes a bit late to resynch the design.
The final option is 'preserved' for user-find and 'preserve' for
user-del. '--only-delete' or 'also-delete' are old name that I need to
replace in the design.
About the 'deleted' attribute, do you think adding a DS cos virtual
attribute ?
See the attached patch.
3) uidNumber and gidNumber can't be set back to '-1' once set to other
value.
This would be useful when admin changes its mind and want IPA to
assign them.
IIUC, there should be no validation in cn=staged user container. All
validation should be done during stageuser-activate.
Yes that comes from user plugin that enforce the number to be 0.
That is a good point giving the ability to reset uidNumber/gidNumber.
I will check if it is possible, how (give a value or an option to
reset), and also if it would not create other issue.
4) Support for deleted - stage workflow is still missing. But I'm
unsure if we
agreed to finish it now or later.
Yes thanks
5) Twice deleting user with '--preserve' deletes him permanently.
$ ipa user-add tuser --first Test --last User
$ ipa user-del tuser --preserve
$ ipa user-del tuser --preserve
$ ipa user-find --preserved
0 (delete) users matched
Number of entries returned 0
Deleting a deleted (preserved) entry, should permanently remove the
entry.
Now if the second time the preserve option is present, it makes sense to
not delete it.
BTW: I might be stating the obvious here, but it would be better to use
one boolean parameter rather than two mutually exclusive flags in user-del.
thanks
theirry
Overall, LGTM,
Just 2 nitpicks:
1) preserved attribute label: 'Preserved deleted user' - 'Preserved user'
2) 'preserved' attribute should be shown in user-{find,show} when
'--all' is specified
Updated patch attached.
--
David Kupka
From 7c6e3869ceb64177169b360b21b0af5d73e0405c Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 20 May 2015 08:12:07 +
Subject: [PATCH] User life cycle: provide preserved user virtual attribute
https://fedorahosted.org/freeipa/ticket/3813
---
API.txt| 2 +-
VERSION| 4 +--
ipalib/plugins/user.py | 78 +++---
3 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/API.txt b/API.txt
index 9e3f223b7ac338840d7090299f9108e951ea920a..b892eff8bf95c79b7ffdb98738710a7c54000f93 100644
--- a/API.txt
+++ b/API.txt
@@ -5023,7 +5023,7 @@ option: Str('pager', attribute=True, autofill=False, cli_name='pager', multivalu
option: Flag('pkey_only?', autofill=True, default=False)
option: Str('postalcode', attribute=True, autofill=False, cli_name='postalcode', multivalue=False, query=True, required=False)
option: Str('preferredlanguage', attribute=True, autofill=False, cli_name='preferredlanguage', multivalue=False, pattern='^(([a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?(\\s*,\\s*[a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?)*)|(\\*))$', query=True, required=False)
-option: Flag('preserved?', autofill=True, cli_name='preserved', default=False)
+option: Bool('preserved', attribute=False, autofill=False, cli_name='preserved', default=False, multivalue=False, query=True, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
option: Str('sn', attribute=True, autofill=False, cli_name='last', multivalue=False, query=True,
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. Now if the second time the preserve option is present, it makes sense to not delete it. BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 18.5.2015 v 10:33 thierry bordaz napsal(a):
On 05/15/2015 04:44 PM, David Kupka wrote:
Hello Thierry,
thanks for the patch set. Overall functionality of ULC feature looks
good to
me and is definitely alpha ready.
I found following issues but don't insist on fixing it right now:
1) When stageuser-activate fails due to already existent
active/deleted user.
DN is show instead of user name that's used in other commands (user-add,
stageuser-add).
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user
uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
already exists
Hi David, Jan,
Thanks you so much for all those tests and feedback. I agree, some minor
bugs can be fixed separatly from this main patches.
You are right, It should return the user ID not the DN.
2) According to the design there should be '--only-delete' and
'--also-delete'
options for user-find command instead there is '--preserved' option.
Honza proposed adding virtual boolean attribute 'deleted' to user
entry and
filter on it.
The 'deleted' attribute would be useful also in user-show where is no
way to
tell if the displayed user is active or deleted. (Except running with
--all
and looking on the dn).
Yes a bit late to resynch the design.
The final option is 'preserved' for user-find and 'preserve' for
user-del. '--only-delete' or 'also-delete' are old name that I need to
replace in the design.
About the 'deleted' attribute, do you think adding a DS cos virtual
attribute ?
See the attached patch.
3) uidNumber and gidNumber can't be set back to '-1' once set to other
value.
This would be useful when admin changes its mind and want IPA to
assign them.
IIUC, there should be no validation in cn=staged user container. All
validation should be done during stageuser-activate.
Yes that comes from user plugin that enforce the number to be 0.
That is a good point giving the ability to reset uidNumber/gidNumber.
I will check if it is possible, how (give a value or an option to
reset), and also if it would not create other issue.
4) Support for deleted - stage workflow is still missing. But I'm
unsure if we
agreed to finish it now or later.
Yes thanks
5) Twice deleting user with '--preserve' deletes him permanently.
$ ipa user-add tuser --first Test --last User
$ ipa user-del tuser --preserve
$ ipa user-del tuser --preserve
$ ipa user-find --preserved
0 (delete) users matched
Number of entries returned 0
Deleting a deleted (preserved) entry, should permanently remove the entry.
Now if the second time the preserve option is present, it makes sense to
not delete it.
BTW: I might be stating the obvious here, but it would be better to use
one boolean parameter rather than two mutually exclusive flags in user-del.
thanks
theirry
--
Jan Cholasta
From 7151ebe30cac7877b31c3a682730ff3c63561e9f Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 20 May 2015 08:12:07 +
Subject: [PATCH] User life cycle: provide preserved user virtual attribute
https://fedorahosted.org/freeipa/ticket/3813
---
API.txt| 2 +-
VERSION| 4 +--
ipalib/plugins/user.py | 74 ++
3 files changed, 47 insertions(+), 33 deletions(-)
diff --git a/API.txt b/API.txt
index 0808f3c..37eba3f 100644
--- a/API.txt
+++ b/API.txt
@@ -4611,7 +4611,7 @@ option: Str('pager', attribute=True, autofill=False, cli_name='pager', multivalu
option: Flag('pkey_only?', autofill=True, default=False)
option: Str('postalcode', attribute=True, autofill=False, cli_name='postalcode', multivalue=False, query=True, required=False)
option: Str('preferredlanguage', attribute=True, autofill=False, cli_name='preferredlanguage', multivalue=False, pattern='^(([a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?(\\s*,\\s*[a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?)*)|(\\*))$', query=True, required=False)
-option: Flag('preserved?', autofill=True, cli_name='preserved', default=False)
+option: Bool('preserved', attribute=False, autofill=False, cli_name='preserved', default=False, multivalue=False, query=True, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
option: Str('sn', attribute=True, autofill=False, cli_name='last', multivalue=False, query=True, required=False)
diff --git a/VERSION b/VERSION
index c207558..40aa3a6 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=118
-# Last change:
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. Now if the second time the preserve option is present, it makes sense to not delete it. thanks theirry David - Original Message - From: thierry bordaz tbor...@redhat.com To: Jan Cholasta jchol...@redhat.com, David Kupka dku...@redhat.com Cc: freeipa-devel freeipa-devel@redhat.com Sent: Tuesday, May 12, 2015 5:05:29 PM Subject: Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands On 05/12/2015 02:17 PM, thierry bordaz wrote: On 05/05/2015 08:57 AM, Jan Cholasta wrote: Hi, Dne 28.4.2015 v 16:40 thierry bordaz napsal(a): On 04/28/2015 10:40 AM, David Kupka wrote: On 04/28/2015 10:28 AM, thierry bordaz wrote: On 04/28/2015 10:23 AM, David Kupka wrote: On 04/16/2015 01:00 PM, thierry bordaz wrote: Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * -User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds no such user but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: Given we are now only fixing bugs and not doing big structural changes, I would rather like to push what we have and then work on fixing the bugs that are critical for the feature. Some may be before Alpha, some after Alpha or even 4.2.1 or later versions - depending on the impact. 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Please file ticket, can be fixed in 4.2.1 or later IMO. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Please file ticket as well. As I talked to David, it is now difficult to distinguish between active and deleted users, user-show USER shows the user regardless if the user is active or deleted. This is something we should discuss, what is the ideal behavior. Please include this question in the ticket. 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. We may want with filing a ticket unless there is a real demand for this. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. We wanted to do it also, but based on Thierry's availability, it can be done post-Alpha or even 4.2.1. 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 This looks as something we may want to fix before GA. Pushed to master: 273fd057a3be797a05d6c7f34fd619d3dfa09c37 When the UI (on review) is also pushed, we will have the base ULC functionality here and we can close the main RFE. Thanks everyone! Martin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 David - Original Message - From: thierry bordaz tbor...@redhat.com To: Jan Cholasta jchol...@redhat.com, David Kupka dku...@redhat.com Cc: freeipa-devel freeipa-devel@redhat.com Sent: Tuesday, May 12, 2015 5:05:29 PM Subject: Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands On 05/12/2015 02:17 PM, thierry bordaz wrote: On 05/05/2015 08:57 AM, Jan Cholasta wrote: Hi, Dne 28.4.2015 v 16:40 thierry bordaz napsal(a): On 04/28/2015 10:40 AM, David Kupka wrote: On 04/28/2015 10:28 AM, thierry bordaz wrote: On 04/28/2015 10:23 AM, David Kupka wrote: On 04/16/2015 01:00 PM, thierry bordaz wrote: Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * -User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds no such user but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add suser0 --first Stage --last User --password 4. $ kdestroy 5. $ kinit suser0 6. $ ipa user-find Actual: Prints out list of ipa users. Expected: kinit fails with suser0@... not found in Kerberos database Hi David, Thank you so much for having looked at this patch :-) You are right. The Staging users (as well as the Delete users) are not lockout in that patch. The patch 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch will take care of this. Do you prefer that I merged the two patches right now ? thanks thierry Hi Thierry, no, it is not necessary to merge the patches it's ok to have it separated. I'm not sure if the patch should be pushed now or rather wait and push it together with the others. I'm looking forward to next ULC patches from you. Hi David, Here are all the available patches. I also attach a test script that is a kind of regression tests that I am using. Thanks again thierry
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Hi,
Dne 28.4.2015 v 16:40 thierry bordaz napsal(a):
On 04/28/2015 10:40 AM, David Kupka wrote:
On 04/28/2015 10:28 AM, thierry bordaz wrote:
On 04/28/2015 10:23 AM, David Kupka wrote:
On 04/16/2015 01:00 PM, thierry bordaz wrote:
Hello,
Here is the next patch for User life cycle that introduces
del/mod/find and show stageuser plugin commands.
* -User Life Cycle (create containers and scoping DS plugins):
*pushed*
* 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch:
*pushed*
* 0002-User-life-cycle-stageuser-add-verb.patch: *pushed*
* 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed*
* 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under
review *(this one)**
* 0004-User-life-cycle-new-stageuser-commands-activate.patch
* 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch
* 0006-User-life-cycle-user-del-supports-permanently-preser.patch
* 0008-User-life-cycle-user-find-support-finding-delete-use.patch
* 0009-User-life-cycle-support-of-user-undel.patch
* 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch
* 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch
* 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch
* 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch
Thanks
thierry
Hi Thierry,
thanks for the patch, the code looks good to me but there is probably
a bug in ACIs.
After creating a stage user and setting password for him I can kinit
as the stage user. I'm unable to login to the IPA client and id
command for this stage user responds no such user but I can kinit
and invoke ipa commands.
Steps:
0. build freeipa with your patch
1. # ipa-server-install
2. $ kinit admin
3. $ ipa stageuser-add suser0 --first Stage --last User --password
4. $ kdestroy
5. $ kinit suser0
6. $ ipa user-find
Actual:
Prints out list of ipa users.
Expected:
kinit fails with suser0@... not found in Kerberos database
Hi David,
Thank you so much for having looked at this patch :-)
You are right. The Staging users (as well as the Delete users) are not
lockout in that patch.
The patch
0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch will
take care of this.
Do you prefer that I merged the two patches right now ?
thanks
thierry
Hi Thierry,
no, it is not necessary to merge the patches it's ok to have it
separated. I'm not sure if the patch should be pushed now or rather
wait and push it together with the others.
I'm looking forward to next ULC patches from you.
Hi David,
Here are all the available patches.
I also attach a test script that is a kind of regression tests that I am
using.
Thanks again
thierry
Some issues I have found during a quick read of the patches:
Patch 0005:
1) This does nothing and can be safely removed:
+def pre_callback(self, ldap, dn, *keys, **options):
+assert isinstance(dn, DN)
+return dn
2) stageuser_{mod,find,show} have a lot of code that seems to be
copy-pasted from user_{mod,find,show}. I would prefer if the code was
shared instead of copy-pasted, preferably in baseuser_{mod,find,show}.
Patch 0006:
1) These do nothing and can be safely removed:
+def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
**options):
+assert isinstance(dn, DN)
+return dn
+
+def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+assert isinstance(dn, DN)
+return dn
2) You should use output.standard_entry for has_output, as you are only
returning one entry. Or you could add support for activating multiple
users at once.
3) IMO the time to build the new entry and add the active entry
parts should be done by calling user-add, so that the (active) user
creation routine is the same.
4) Please use a single line to separate method definitions in classes.
Patch 008:
1) I guess you forgot to remove these:
+self.log.error( user-add pre_callback 1 %s % dn)
+self.log.error( user-add pre_callback %s % dn)
2)
+takes_options = LDAPDelete.takes_options + (
This should be baseuser_del.takes_options +
Honza
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 04/28/2015 10:28 AM, thierry bordaz wrote: On 04/28/2015 10:23 AM, David Kupka wrote: On 04/16/2015 01:00 PM, thierry bordaz wrote: Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * -User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds no such user but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add suser0 --first Stage --last User --password 4. $ kdestroy 5. $ kinit suser0 6. $ ipa user-find Actual: Prints out list of ipa users. Expected: kinit fails with suser0@... not found in Kerberos database Hi David, Thank you so much for having looked at this patch :-) You are right. The Staging users (as well as the Delete users) are not lockout in that patch. The patch 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch will take care of this. Do you prefer that I merged the two patches right now ? thanks thierry Hi Thierry, no, it is not necessary to merge the patches it's ok to have it separated. I'm not sure if the patch should be pushed now or rather wait and push it together with the others. I'm looking forward to next ULC patches from you. -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 04/28/2015 10:23 AM, David Kupka wrote: On 04/16/2015 01:00 PM, thierry bordaz wrote: Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * -User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds no such user but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add suser0 --first Stage --last User --password 4. $ kdestroy 5. $ kinit suser0 6. $ ipa user-find Actual: Prints out list of ipa users. Expected: kinit fails with suser0@... not found in Kerberos database Hi David, Thank you so much for having looked at this patch :-) You are right. The Staging users (as well as the Delete users) are not lockout in that patch. The patch 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch will take care of this. Do you prefer that I merged the two patches right now ? thanks thierry -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
On 04/16/2015 01:00 PM, thierry bordaz wrote: Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * -User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds no such user but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add suser0 --first Stage --last User --password 4. $ kdestroy 5. $ kinit suser0 6. $ ipa user-find Actual: Prints out list of ipa users. Expected: kinit fails with suser0@... not found in Kerberos database -- David Kupka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
Hello,
Here is the next patch for User life cycle that introduces
del/mod/find and show stageuser plugin commands.
* -User Life Cycle (create containers and scoping DS plugins):
*pushed*
* 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch:
*pushed*
* 0002-User-life-cycle-stageuser-add-verb.patch: *pushed*
* 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed*
* 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under
review *(this one)**
* 0004-User-life-cycle-new-stageuser-commands-activate.patch
* 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch
* 0006-User-life-cycle-user-del-supports-permanently-preser.patch
* 0008-User-life-cycle-user-find-support-finding-delete-use.patch
* 0009-User-life-cycle-support-of-user-undel.patch
* 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch
* 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch
* 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch
* 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch
Thanks
thierry
From 1393b393468584579212e84be30142ae3049a625 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz tbor...@redhat.com
Date: Thu, 16 Apr 2015 10:17:31 +0200
Subject: [PATCH] User life cycle: new stageuser commands del/mod/find/show
Add plugin commands to stageuser plugin:
stageuser_del
stageuser_mod
stageuser_find
stageuser_show
https://fedorahosted.org/freeipa/ticket/3813
---
API.txt | 128 ++
VERSION | 4 +-
ipalib/plugins/stageuser.py | 147 +++-
3 files changed, 276 insertions(+), 3 deletions(-)
diff --git a/API.txt b/API.txt
index f747765d7f9c87761fed0277cd59d1bc3fbd57e9..f2d9af90cc1c0fd373ab9847d2d1327f68671414 100644
--- a/API.txt
+++ b/API.txt
@@ -3740,6 +3740,134 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
+command: stageuser_del
+args: 1,2,3
+arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('version?', exclude='webui')
+output: Output('result', type 'dict', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: ListOfPrimaryKeys('value', None, None)
+command: stageuser_find
+args: 1,52,4
+arg: Str('criteria?', noextrawhitespace=False)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('carlicense', attribute=True, autofill=False, cli_name='carlicense', multivalue=True, query=True, required=False)
+option: Str('cn', attribute=True, autofill=False, cli_name='cn', multivalue=False, query=True, required=False)
+option: Str('departmentnumber', attribute=True, autofill=False, cli_name='departmentnumber', multivalue=True, query=True, required=False)
+option: Str('displayname', attribute=True, autofill=False, cli_name='displayname', multivalue=False, query=True, required=False)
+option: Str('employeenumber', attribute=True, autofill=False, cli_name='employeenumber', multivalue=False, query=True, required=False)
+option: Str('employeetype', attribute=True, autofill=False, cli_name='employeetype', multivalue=False, query=True, required=False)
+option: Str('facsimiletelephonenumber', attribute=True, autofill=False, cli_name='fax', multivalue=True, query=True, required=False)
+option: Str('gecos', attribute=True, autofill=False, cli_name='gecos', multivalue=False, query=True, required=False)
+option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', minvalue=1, multivalue=False, query=True, required=False)
+option: Str('givenname', attribute=True, autofill=False, cli_name='first', multivalue=False, query=True, required=False)
+option: Str('homedirectory', attribute=True, autofill=False, cli_name='homedir', multivalue=False, query=True, required=False)
+option: Str('in_group*', cli_name='in_groups', csv=True)
+option: Str('in_hbacrule*', cli_name='in_hbacrules', csv=True)
+option: Str('in_netgroup*', cli_name='in_netgroups', csv=True)
+option: Str('in_role*', cli_name='in_roles', csv=True)
+option: Str('in_sudorule*', cli_name='in_sudorules', csv=True)
+option: Str('initials', attribute=True, autofill=False, cli_name='initials', multivalue=False, query=True, required=False)
+option: Str('ipatokenradiusconfiglink', attribute=True, autofill=False, cli_name='radius', multivalue=False, query=True, required=False)
+option: Str('ipatokenradiususername', attribute=True, autofill=False, cli_name='radius_username', multivalue=False, query=True, required=False)
+option:
