On 05/06/2013 07:03 PM, Sumit Bose wrote:
On Mon, May 06, 2013 at 05:55:35PM +0200, Martin Kosek wrote:
On 05/06/2013 01:28 PM, Martin Kosek wrote:
On 05/04/2013 07:13 AM, Alexander Bokovoy wrote:
On Fri, 03 May 2013, Sumit Bose wrote:
On Fri, May 03, 2013 at 09:46:47PM +0300, Alexander Bokovoy wrote:
Hi!
Attached are patches to allow resolving SIDs in Web UI in external
membership panel for groups. Please see more detailed description in the
main patch.
I haven't rebased it yet on top of Petr's Web UI rework, hopefully it
should be simple.
https://fedorahosted.org/freeipa/ticket/3302
Since framework doesn't allow to hide commands from CLI, underlying
command is usable from CLI too:
# ipa trust-resolve
--sids=S-1-5-21-3502988750-125904550-3683905862-{500,512,498}
Name: enterprise read-only domain controll...@ad.lan
SID: S-1-5-21-3502988750-125904550-3683905862-498
Name: administra...@ad.lan
SID: S-1-5-21-3502988750-125904550-3683905862-500
Name: domain adm...@ad.lan
SID: S-1-5-21-3502988750-125904550-3683905862-512
--
/ Alexander Bokovoy
+ try:
+ sids = map(lambda x: str(x), options['sids'])
+ xlate = pysss_nss_idmap.getnamebysid(sids)
The latest version, which is already committed to sssd, return a dict.
The output of ipa trust-resolve now look like:
[root@ipa18-devel ~]# ipa trust-resolve
--sids=S-1-5-21-3090815309-2627318493-3395719201-{498,500,513}
Name: {'type': 3, 'name': u'administrator@ad18.ipa18.devel'}
SID: S-1-5-21-3090815309-2627318493-3395719201-500
Name: {'type': 2, 'name': u'enterprise read-only domain
controllers@ad18.ipa18.devel'}
SID: S-1-5-21-3090815309-2627318493-3395719201-498
Name: {'type': 2, 'name': u'domain users@ad18.ipa18.devel'}
SID: S-1-5-21-3090815309-2627318493-3395719201-513
+ for sid in xlate:
+ entry = dict()
+ entry['sid'] = [unicode(sid)]
+ entry['name'] = [unicode(xlate[sid])]
I think you need entry['name'] =
[unicode(xlate[sid][pysss_nss_idmap.NAME_KEY])]
here.
Fixed, thanks!
I also added type conversion to a text (user, group, both). The type is not
shown by default
in CLI but is available through --all option. We might consider using it
in Web UI for visual hint about the name nature.
I tried with firefox, but the SIDs of the external members are not
resolved. Do I have to clean any firefox cache?
No, you do not. When picking up changes from my development VM, I
omitted one chunk in group.js where sid_facet was actually taken in use.
Without that one nothing is used.
Updated patch 0103 is attached, tested against sssd in ipa-devel repo
which already includes your patches.
Thanks for the patch! Still, I have few comments:
1) Exception should be raised instead of returning empty result:
+ if not _nss_idmap_installed:
+ return dict(result=result)
Otherwise people will be confused what's wrong.
2) Why do we hide error raised in SID processing code?
...
+ except ValueError, e:
+ pass
...
I think that the try-catch should be as localized possible, ideally in the FOR
loop. If processing of the second SID out of 10 fails, just one SID would be
return, with no additional error. People will be confused what's wrong:
# ipa trust-resolve --sids S-1-5-21-3035198329-144811719-1378114514-500
#
This does not really tell me what's wrong.
Could we rather return all requested SIDs either with a proper result or with a
respective error? This is how I would image the translation to look like:
...
try:
sids = map(lambda x: str(x), options['sids'])
xlate = pysss_nss_idmap.getnamebysid(sids)
except SomeError, e:
raise SomeException(e)
for sid in xlate:
entry = dict()
entry['sid'] = ...
try:
name = ...
type = ...
entry['name'], entry['type'] = name, type
except SomeError, e:
entry['failedtranslation'] = unicode(e)
results.append(entry)
...
I filed ticket for SSSD part of the issue:
https://fedorahosted.org/sssd/ticket/1911
3) Tab/Space indentation mix:
+ for sid in xlate:
+ entry = dict()
+ entry['sid'] = [unicode(sid)]
4) Unneeded import:
from ipalib import api, Str, StrEnum, Password, DefaultFrom, _, ngettext,
Object
+from types import NoneType
from ipalib.parameters import Enu
Martin
As Alexander is not here ATM, sending updated patch based on current master
branch (with Web UI refactoring) which also includes few squashes:
- fix for my point 3)
- fix for my point 4)
- squashed Petr Vobornik's Web UI cleanups
I tested it and it worked fine. As for the points 1) and 2) I will file a
ticket, these are not critical.
Martin
Patch is working as expected. So ACK from my side for the functional
part.
bye,
Sumit
Pushed to master.
Martin
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
