Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 05/03/2013 07:35 PM, Endi Sukma Dewata wrote: Hi, Sorry for the delay, I have some questions comments. Registry: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? Ideally a class should have complete methods to manage the objects it stores (e.g. unregister(), remove()). How is reg.entity created? Are there others beside 'entity'? How is Registries_registry in reg.js used? It doesn't seem to be used anywhere else. Plugins: In plugins.py the list of plugins is generated using os.listdir(). Then each plugin also has a list of dependencies which I suppose can include other plugins. Then when registering the plugin task, it will have a priority as well. So there seem to be several factors that determine the execution order of the plugins. There should be a document explaining how this will work, so plugin writers can be sure that the code will be executed at the right time. In general I'd avoid using task priority because it doesn't guarantee the correct execution order unless the priorities of all tasks are well coordinated (which might be challenging if there are multiple plugins owned by different people). Could you add more examples of simple plugins for various scenarios including custom entity, custom facet, custom field, custom menu? They can be included in the RPM for reference. Writing a plugin seems to still require programming skills, reliance on good docs, and probably even some source code familiarity. What do you think about simplifying this a little further? So we'll have 2 ways to define a plugin: one is programatically using the current framework already implemented (e.g. simpleuser.js), and the other is completely declaratively using a plain json data (e.g. simpleuser.json). The declarative plugin will obviously be more limited, but much simpler to use. Builder: b) Second big issue was build of objects. Entities and facets have complex build logic. It can be simplified into three steps: 1) modifications of spec 2) creation of object and class inheritance 3) init logic Yes, creating an object has become very complicated now with the builders, factories, constructors, preops, postops, inits, overrides, diff, etc. I think the problem is that we're trying to create/modify the spec before creating the object and we need a whole set of mechanisms to do that. Maybe we can simplify it into two basic steps: 1. Create an empty/simple object. 2. Initialize the object. The initialization process could be split further into smaller operations such as: * Load the spec and modify it if necessary * Creating dependent objects and initializing them * Other initialization steps The builder, factory, preops, and postops can be included as part of the initialization step. They can be normal class methods rather than loosely defined functions and can be overridden by subclasses. There's probably a lot more details that need to be discussed. 1. Move ./_base/metadata_provider to ./metadata? Might simplify stuff. This seems to be IPA-specific, so yes. 2. Move actions/buttons spec from factories to pre_ops associated with the factories. Example of stuff to be moved (search.js): spec.actions = spec.actions || []; spec.actions.unshift( 'refresh', 'batch_remove', 'add'); It may simplify/allow removal of those items in spec or pre_ops of child factories. Currently there is no way how to intercept them. Sure, I don't see any problem with that. In general there is no major issue that would warrant a NACK. As long as the API is well documented for plugin writers it should be sufficient. Thanks for review Endi! Since we do not have a NACK, lets do small changes we can do now for 3.2 GA and create tickets for the rest (can be done in 3.2 stabilization phase). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 403 Update pki proxy configuration
Replicas with Dogtag pki-ca 10.0.2 CA require access to additional Dogtag REST API calls. Update pki proxy configuration to allow that. https://fedorahosted.org/freeipa/ticket/3601 From 5c88f2b9d385b80bd6b5b49c15c29592e1b2372c Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Mon, 6 May 2013 09:22:27 +0200 Subject: [PATCH] Update pki proxy configuration Replicas with Dogtag pki-ca 10.0.2 CA require access to additional Dogtag REST API calls. Update pki proxy configuration to allow that. https://fedorahosted.org/freeipa/ticket/3601 --- install/conf/ipa-pki-proxy.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/install/conf/ipa-pki-proxy.conf b/install/conf/ipa-pki-proxy.conf index 8c4f3a9b68f2f16036310bb28661159e7acb06d0..6f0463242b75a58cf63a38e62c23fa372aeacf64 100644 --- a/install/conf/ipa-pki-proxy.conf +++ b/install/conf/ipa-pki-proxy.conf @@ -1,4 +1,4 @@ -# VERSION 2 - DO NOT REMOVE THIS LINE +# VERSION 3 - DO NOT REMOVE THIS LINE ProxyRequests Off @@ -11,7 +11,7 @@ ProxyRequests Off /LocationMatch # matches for admin port and installer -LocationMatch ^/ca/admin/ca/getCertChain|^/ca/admin/ca/getConfigEntries|^/ca/admin/ca/getCookie|^/ca/admin/ca/getStatus|^/ca/admin/ca/securityDomainLogin|^/ca/admin/ca/getDomainXML|^/ca/rest/installer/installToken +LocationMatch ^/ca/admin/ca/getCertChain|^/ca/admin/ca/getConfigEntries|^/ca/admin/ca/getCookie|^/ca/admin/ca/getStatus|^/ca/admin/ca/securityDomainLogin|^/ca/admin/ca/getDomainXML|^/ca/rest/installer/installToken|^/ca/admin/ca/updateNumberRange|^/ca/rest/securityDomain/domainInfo|^/ca/rest/account/login|^/ca/admin/ca/tokenAuthenticate|^/ca/admin/ca/updateNumberRange|^/ca/admin/ca/updateDomainXML|^/ca/rest/account/logout|^/ca/rest/securityDomain/installToken NSSOptions +StdEnvVars +ExportCertData +StrictRequire +OptRenegotiate NSSVerifyClient none ProxyPassMatch ajp://localhost:$DOGTAG_PORT -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
On Sat, May 04, 2013 at 08:24:52PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 06:02:27PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 08:13:17AM +0300, 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.
I'm sorry, it still does not work for me in firefox on F18 32bits. Can
you give me some hints where to look what the WebUI is trying to do?
sorry, I meant how to debug the WebUI.
Petr wrote these notes:
http://pvoborni.fedorapeople.org/doc/debugging_web_ui.html
You'd need to put breakpoint in association.js, in
sidxlate_command.on_success(), once you used sync.sh to
copy over non-compiled version of the UI javascript code.
Thank you for the hints. Now I see the following, it is working
as expected when I just call
./sync.sh -fc
but the SIDs are not translated when I call
./sync.sh -fcC
I didn't change anything at the *.js source files, just called sync.sh.
Any ideas what might be wrong here?
bye,
Sumit
'ipa trust-resolve' on the command line is working well.
Navigate from top /ipa/ui to:
- Identity|User groups
- select specific group
- select 'External' tab
I recorded small animated sequence that shows how it looks in new Web
UI: http://abbra.fedorapeople.org/.paste/freeipa-sid-resolve-new-web-ui.gif
I only see the SIDs with your patches applied. I used master with your git
patches. Do I need the patches for the new WebUI and your additional
patch for that as well?
GIT master with my patches should be enough -- if you used 0103 revision 1
Additional patch part is only for new Web UI rebase for
instal/ui/src/freeipa/*
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
On 05/06/2013 10:47 AM, Sumit Bose wrote:
On Sat, May 04, 2013 at 08:24:52PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 06:02:27PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 08:13:17AM +0300, 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.
I'm sorry, it still does not work for me in firefox on F18 32bits. Can
you give me some hints where to look what the WebUI is trying to do?
sorry, I meant how to debug the WebUI.
Petr wrote these notes:
http://pvoborni.fedorapeople.org/doc/debugging_web_ui.html
You'd need to put breakpoint in association.js, in
sidxlate_command.on_success(), once you used sync.sh to
copy over non-compiled version of the UI javascript code.
Thank you for the hints. Now I see the following, it is working
as expected when I just call
./sync.sh -fc
but the SIDs are not translated when I call
./sync.sh -fcC
I didn't change anything at the *.js source files, just called sync.sh.
Any ideas what might be wrong here?
-C stands for: send a built/compiled version. Usually you have to create
it first - it's not in git.
so all ./make-ui.sh before ./sync.sh -fcC
I will send comments for the patch later today - mostly nitpicks.
bye,
Sumit
'ipa trust-resolve' on the command line is working well.
Navigate from top /ipa/ui to:
- Identity|User groups
- select specific group
- select 'External' tab
I recorded small animated sequence that shows how it looks in new Web
UI: http://abbra.fedorapeople.org/.paste/freeipa-sid-resolve-new-web-ui.gif
I only see the SIDs with your patches applied. I used master with your git
patches. Do I need the patches for the new WebUI and your additional
patch for that as well?
GIT master with my patches should be enough -- if you used 0103 revision 1
Additional patch part is only for new Web UI rebase for
instal/ui/src/freeipa/*
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
On Mon, May 06, 2013 at 11:05:40AM +0200, Petr Vobornik wrote:
On 05/06/2013 10:47 AM, Sumit Bose wrote:
On Sat, May 04, 2013 at 08:24:52PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 06:02:27PM +0300, Alexander Bokovoy wrote:
On Sat, 04 May 2013, Sumit Bose wrote:
On Sat, May 04, 2013 at 08:13:17AM +0300, 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.
I'm sorry, it still does not work for me in firefox on F18 32bits. Can
you give me some hints where to look what the WebUI is trying to do?
sorry, I meant how to debug the WebUI.
Petr wrote these notes:
http://pvoborni.fedorapeople.org/doc/debugging_web_ui.html
You'd need to put breakpoint in association.js, in
sidxlate_command.on_success(), once you used sync.sh to
copy over non-compiled version of the UI javascript code.
Thank you for the hints. Now I see the following, it is working
as expected when I just call
./sync.sh -fc
but the SIDs are not translated when I call
./sync.sh -fcC
I didn't change anything at the *.js source files, just called sync.sh.
Any ideas what might be wrong here?
-C stands for: send a built/compiled version. Usually you have to
create it first - it's not in git.
so all ./make-ui.sh before ./sync.sh -fcC
thanks, that did the trick and also told me why the packages created
with 'make rpms' didn't work for me, because I forgot to add
export BASE_OPTIONS=-Xss512k
to my build environment, because otherwise I see a StackOverflowError
while uglifying.
bye,
Sumit
I will send comments for the patch later today - mostly nitpicks.
bye,
Sumit
'ipa trust-resolve' on the command line is working well.
Navigate from top /ipa/ui to:
- Identity|User groups
- select specific group
- select 'External' tab
I recorded small animated sequence that shows how it looks in new Web
UI:
http://abbra.fedorapeople.org/.paste/freeipa-sid-resolve-new-web-ui.gif
I only see the SIDs with your patches applied. I used master with your git
patches. Do I need the patches for the new WebUI and your additional
patch for that as well?
GIT master with my patches should be enough -- if you used 0103 revision 1
Additional patch part is only for new Web UI rebase for
instal/ui/src/freeipa/*
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0146] Disallow all dynamic updates if update policy configuration failed
On 04/16/2013 10:40 AM, Petr Spacek wrote: Hello, Disallow all dynamic updates if update policy configuration failed. Without this patch the old update policy stays in effect when re-configuration failed. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, works as expected. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Possible fix for CA install bug?
On 05/03/2013 12:43 PM, Martin Kosek wrote: On 05/02/2013 07:51 PM, Rob Crittenden wrote: Rob Crittenden wrote: Nathaniel McCallum wrote: When installing beta1, I encountered a bug where the CA install would fail. This may have already been fixed in dogtag or elsewhere, but if not, this patch WorksForMe. I have no idea if it is the right fix. Good catch. This change apparently was added during the last week of 10.0.2 development and I'm not sure how I missed it. I did at least one successful install using those bits. Maybe either my test was bogus or I had left-over kruft. In any case, we can specify the location directly to pkispawn and not have to move the file. BTW, My patch 1098 bumps up the minimum version of dogtag to 10.0.2. rob I tested 1100 and it works great on master server. However when I am on replica, it always fails: # ipa-ca-install replica-info-vm-024.idm.lab.bos.redhat.com.gpg Directory Manager (existing master) password: ... Connection check OK Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/16]: creating certificate server user [2/16]: configuring certificate server instance ipa : CRITICAL failed to configure ca instance Command '/usr/sbin/pkispawn -s CA -f /tmp/tmpRR0ic3' returned non-zero exit status 1 Your system may be partly configured. Run /usr/sbin/ipa-server-install --uninstall to clean up. Configuration of CA failed CA installation log including pkispawn error attached. Martin The bug Martin found was unrelated, and will be fixed with https://fedorahosted.org/freeipa/ticket/3601. ACK for rcrit-1100. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 403 Update pki proxy configuration
On 05/06/2013 10:13 AM, Martin Kosek wrote: Replicas with Dogtag pki-ca 10.0.2 CA require access to additional Dogtag REST API calls. Update pki proxy configuration to allow that. https://fedorahosted.org/freeipa/ticket/3601 With this, Rob's patch 1100 and the latest Dogtag (10.0.2-4), the CA replica installs and works, both with a new and upgraded server. ACK -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 403 Update pki proxy configuration
On 05/06/2013 01:05 PM, Petr Viktorin wrote: On 05/06/2013 10:13 AM, Martin Kosek wrote: Replicas with Dogtag pki-ca 10.0.2 CA require access to additional Dogtag REST API calls. Update pki proxy configuration to allow that. https://fedorahosted.org/freeipa/ticket/3601 With this, Rob's patch 1100 and the latest Dogtag (10.0.2-4), the CA replica installs and works, both with a new and upgraded server. ACK Thanks. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Possible fix for CA install bug?
On 05/06/2013 01:05 PM, Petr Viktorin wrote: On 05/03/2013 12:43 PM, Martin Kosek wrote: On 05/02/2013 07:51 PM, Rob Crittenden wrote: Rob Crittenden wrote: Nathaniel McCallum wrote: When installing beta1, I encountered a bug where the CA install would fail. This may have already been fixed in dogtag or elsewhere, but if not, this patch WorksForMe. I have no idea if it is the right fix. Good catch. This change apparently was added during the last week of 10.0.2 development and I'm not sure how I missed it. I did at least one successful install using those bits. Maybe either my test was bogus or I had left-over kruft. In any case, we can specify the location directly to pkispawn and not have to move the file. BTW, My patch 1098 bumps up the minimum version of dogtag to 10.0.2. rob I tested 1100 and it works great on master server. However when I am on replica, it always fails: # ipa-ca-install replica-info-vm-024.idm.lab.bos.redhat.com.gpg Directory Manager (existing master) password: ... Connection check OK Configuring certificate server (pki-tomcatd): Estimated time 3 minutes 30 seconds [1/16]: creating certificate server user [2/16]: configuring certificate server instance ipa : CRITICAL failed to configure ca instance Command '/usr/sbin/pkispawn -s CA -f /tmp/tmpRR0ic3' returned non-zero exit status 1 Your system may be partly configured. Run /usr/sbin/ipa-server-install --uninstall to clean up. Configuration of CA failed CA installation log including pkispawn error attached. Martin The bug Martin found was unrelated, and will be fixed with https://fedorahosted.org/freeipa/ticket/3601. Right. ACK for rcrit-1100. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
Hello,
here is review of Web UI part.
ACK for abbra-102, it's a fix of error (typo), but it won't probably
have any impact. Because links associated with 'a' elements are
different than the resulting ones.
NACK for abbra-103:
NACK for abbra-pvoborni... (is included in new abbra-103)
Are CLI and IPA-API parts of old abbra-103 in some different patch? We
should make proper patches from the API/CLI part and WebUI part. Because
of this I didn't test following fixes by using installed IPA with
established trust.
Attaching a diff with fixes for following errors of abbra-103:
1) There are jslint errors (missing semicolons, extra semicolons,
trailing commas) (run `jslint -conf jsl.conf` in install/ui dir)
Funny thing, I do the exact opposite mistakes when writing python code.
2) Do not use deferred directly as a value, use promise instead:
value[i][that.attribute] = {
promise: deferred.promise,
temp: sid
};
Latter is better because promise can't be changed by consumer component.
Its resolution is still controlled by deferred.
3) We should not call trust-resolve when there are no sids. It's
pointless and trust-resolve requires at least one sid (`Str('sids+',`)
4) I see that you copied attribute facet preop as sid preop but omitted
adding of facet update policy. Is there a reason for it? IMO it's better
just to copy the whole or don't do it at all and just specified the
facet as:
$type: 'attribute',
$factory: IPA.sid_facet,
This change is in fix2.diff.
Nitpicks:
a) Use `[]` instead of `new Array()`
b) add space before `for`
c) specifying sids: '' in command construction is not needed - it's set
later
--
Petr
On 05/04/2013 08:04 AM, Alexander Bokovoy wrote:
On Sat, 04 May 2013, 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.
... and here is rebase of install/ui/src/freeipa to Web UI refactoring
branch, to help testing on top of Petr's changes. With this patch SID
resolving works in new Web UI.
There are probably some changes that could further be removed, I haven't
looked into greater detail.
Please note that attached patch only covers parts in
install/ui/src/freeipa, you'd still need to add plugin changes from
ipalib/plugins/trust.py.
--
Petr Vobornik
diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index cd76b7e..71ee71d 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -1372,17 +1372,17 @@ IPA.sid_facet = function(spec,
Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
On 18.4.2013 11:04, Petr Spacek wrote:
Hello,
Clean up PTR record synchronization code and make it more robust.
PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.
Tbabej's GCC cries about uninitialized variable 'ptr_a_equal', but we weren't
able to find any real error.
This version of the patch contains a workaround for the GCC oddities.
--
Petr^2 Spacek
From 5e6abb29df58ce00ecf7045254dfc7fb09fc4650 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
robust.
PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 507 --
1 file changed, 342 insertions(+), 165 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 8448412b7a1a9150bd24d9ca46575c0402be0c9f..6c5cf2e79d762251954e3bb099dbef98a0b2d805 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,360 @@ cleanup:
#undef SET_LDAP_MOD
}
+
+#define SYNCPTR_PREFPTR record synchronization
+#define SYNCPTR_FMTPRE SYNCPTR_PREF (%s) for A/ '%s'
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+ static const char add[] = addition;
+ static const char del[] = deletion;
+
+ switch (mod_op) {
+ case LDAP_MOD_ADD:
+ return add;
+
+ case LDAP_MOD_DELETE:
+ return del;
+
+ default:
+ INSIST(unsupported LDAP mod_op == NULL);
+ return NULL;
+ }
+}
+
+static void
+append_trailing_dot(char *str, unsigned int size) {
+ unsigned int length = strlen(str);
+ if (str[length] != '.') {
+ REQUIRE(length + 1 size);
+ str[length] = '.';
+ str[length+1] = '\0';
+ }
+}
+
+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+ dns_name_t *ptr_name, ld_string_t *ptr_dn,
+ dns_name_t *zone_name) {
+ isc_result_t result;
+ isc_mem_t *mctx = ldap_inst-mctx;
+
+ in_addr_t ip;
+
+ /* Get string with IP address from change request
+ * and convert it to in_addr structure. */
+ if ((ip = inet_addr(ip_str)) == 0) {
+ log_bug(SYNCPTR_PREF could not convert IP address
+ from string '%s', ip_str);
+ CLEANUP_WITH(ISC_R_UNEXPECTED);
+ }
+
+ /* Use internal net address representation. */
+ isc_netaddr_t isc_ip;
+ /* Only copy data to isc_ip stucture. */
+ isc_netaddr_fromin(isc_ip,(struct in_addr *) ip);
+
+ /*
+ * Convert IP address to PTR record.
+ *
+ * @example
+ * 192.168.0.1 - 1.0.168.192.in-addr.arpa
+ *
+ * @todo Check if it works for IPv6 correctly.
+ */
+ CHECK(dns_byaddr_createptrname2(isc_ip, 0, ptr_name));
+
+ /* Get LDAP entry indentifier. */
+ CHECK(dnsname_to_dn(ldap_inst-zone_register, ptr_name, ptr_dn));
+
+ /*
+ * @example
+ * owner_dn_ptr = idnsName=100.0.168, idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+ * owner_zone_dn_ptr = idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+ */
+ char *owner_zone_dn_ptr = strstr(str_buf(ptr_dn),, ) + 1;
+
+ /* Get attribute idnsAllowDynUpdate for reverse zone or use default. */
+ CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, zone_name, NULL));
+
+cleanup:
+ return result;
+}
+
+/**
+ * Check if PTR record's value in LDAP == name of the modified A/ record.
+ * Update will be refused if the PTR name contains multiple PTR records or
+ * if the value in LDAP != expected name.
+ *
+ * @param[in] a_name Name of modified A/ record.
+ * @param[in] a_name_str Name of modified A/ record as NUL terminated string.
+ * @param[in] ptr_name Name of PTR record generated from IP address in A/.
+ * @param[in] mod_op LDAP_MOD_DELETE if A/ record is being deleted
+ * or LDAP_MOD_ADD if A/ record is being added.
+ *
+ * @retval ISC_R_IGNORE A and PTR records match, no change is required.
+ * @retval ISC_R_SUCCESS Prerequisites fulfilled, update is allowed.
+ * @retval other Errors
+ *
+ * @code
+ * ** A record deletion **
+ * ; nsupdate command:
+ * update delete www.example.com. IN A 192.0.2.1
+ *
+ * ; PTR update will be allowed if the zone contains following data:
+ * www.example.com. A 192.0.2.1
+ * 1.2.0.192.in-addr.arpa. PTR www.example.com.
+
+ * ; PTR update will not be allowed if the zone contains following data:
+ * www.example.com. A 192.0.2.1
+ * 1.2.0.192.in-addr.arpa. PTR mail.example.com.
+ * @endcode
+ *
+ * @code
+ * ** A record addition **
+ * ; nsupdate command:
+ * update add www.example.com. 3600 IN A 192.0.2.1
+ *
+ * ; PTR update will be allowed if the zone does not contain A and PTR record.
+ *
+ * ; PTR update will not be allowed if the zone contains following data:
+ * 1.2.0.192.in-addr.arpa. PTR mail.example.com.
+ * @endcode
+ */
+static isc_result_t
Re: [Freeipa-devel] [PATCH 0150] Do not delete whole node during PTR record synchronization.
On 18.4.2013 16:58, Petr Spacek wrote:
Hello,
Do not delete whole node during PTR record synchronization.
https://fedorahosted.org/bind-dyndb-ldap/ticket/115
Tbabej's GCC cries about uninitialized variable 'delete_node', but we weren't
able to find any real error.
This version of the patch contains a workaround for the GCC oddities.
--
Petr^2 Spacek
From 532e454b46720b098328701edcfcd632055dcf0a Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Thu, 18 Apr 2013 16:57:00 +0200
Subject: [PATCH] Do not delete whole node during PTR record synchronization.
https://fedorahosted.org/bind-dyndb-ldap/ticket/115
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 28 +---
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 6c5cf2e79d762251954e3bb099dbef98a0b2d805..198568016303b8aa5c330c69eaf94b1b0ab8fd89 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2922,6 +2922,8 @@ cleanup:
* @param[in] ptr_name Name of PTR record generated from IP address in A/.
* @param[in] mod_op LDAP_MOD_DELETE if A/ record is being deleted
* or LDAP_MOD_ADD if A/ record is being added.
+ * @param[out] delete_node Will be set to ISC_TRUE if the database node
+ * is empty after PTR record deletion.
*
* @retval ISC_R_IGNORE A and PTR records match, no change is required.
* @retval ISC_R_SUCCESS Prerequisites fulfilled, update is allowed.
@@ -2934,11 +2936,11 @@ cleanup:
*
* ; PTR update will be allowed if the zone contains following data:
* www.example.com. A 192.0.2.1
- * 1.2.0.192.in-addr.arpa. PTR www.example.com.
-
+ * 1.2.0.192.in-addr.arpa. PTR www.example.com.
+ *
* ; PTR update will not be allowed if the zone contains following data:
* www.example.com. A 192.0.2.1
- * 1.2.0.192.in-addr.arpa. PTR mail.example.com.
+ * 1.2.0.192.in-addr.arpa. PTR mail.example.com.
* @endcode
*
* @code
@@ -2955,7 +2957,7 @@ cleanup:
static isc_result_t
ldap_sync_ptr_validate(ldap_instance_t *ldap_inst, dns_name_t *a_name,
const char *a_name_str, dns_name_t *ptr_name,
- int mod_op) {
+ int mod_op, isc_boolean_t *delete_node) {
isc_result_t result;
isc_mem_t *mctx = ldap_inst-mctx;
@@ -2977,6 +2979,8 @@ ldap_sync_ptr_validate(ldap_instance_t *ldap_inst, dns_name_t *a_name,
ptr_found = ISC_FALSE;
result = ldapdb_rdatalist_get(mctx, ldap_inst, ptr_name,
NULL, ldap_rdlist);
+
+ *delete_node = ISC_FALSE;
if (result != ISC_R_SUCCESS result != ISC_R_NOTFOUND) {
log_error_r(SYNCPTR_FMTPRE failed in ldapdb_rdatalist_get(),
SYNCPTR_FMTPOST);
@@ -3037,6 +3041,13 @@ ldap_sync_ptr_validate(ldap_instance_t *ldap_inst, dns_name_t *a_name,
SYNCPTR_FMTPOST, ptr_name_str, ptr_rdata_str,
a_name_str);
CLEANUP_WITH(ISC_R_UNEXPECTEDTOKEN);
+
+ } else if (HEAD(ldap_rdlist) == TAIL(ldap_rdlist)) {
+ /* Exactly one PTR record was found and rdlist contains
+ * exactly one RRset, so the deleted PTR record
+ * is the only RR in the node. */
+ REQUIRE(HEAD(ldap_rdlist)-type == dns_rdatatype_ptr);
+ *delete_node = ISC_TRUE;
}
} else if (mod_op == LDAP_MOD_ADD ptr_found == ISC_TRUE) {
@@ -3066,7 +3077,7 @@ cleanup:
static isc_result_t
ldap_sync_ptr(ldap_instance_t *ldap_inst, dns_name_t *a_name,
- const char *ip_str, int mod_op, isc_boolean_t delete_node) {
+ const char *ip_str, int mod_op) {
isc_result_t result;
isc_mem_t *mctx = ldap_inst-mctx;
@@ -3083,6 +3094,8 @@ ldap_sync_ptr(ldap_instance_t *ldap_inst, dns_name_t *a_name,
settings_set_t *zone_settings = NULL;
isc_boolean_t zone_dyn_update;
+ isc_boolean_t delete_node;
+
dns_name_init(zone_name, NULL);
dns_fixedname_init(ptr_name);
CHECK(str_new(mctx, ptr_dn));
@@ -3119,7 +3132,8 @@ ldap_sync_ptr(ldap_instance_t *ldap_inst, dns_name_t *a_name,
}
result = ldap_sync_ptr_validate(ldap_inst, a_name, a_name_str,
- dns_fixedname_name(ptr_name), mod_op);
+ dns_fixedname_name(ptr_name), mod_op,
+ delete_node);
if (result == ISC_R_IGNORE)
CLEANUP_WITH(ISC_R_SUCCESS);
else if (result != ISC_R_SUCCESS)
@@ -3242,7 +3256,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
log_debug(3, sync PTR is enabled for zone '%s', zone_dn);
result = ldap_sync_ptr(ldap_inst, owner, change[0]-mod_values[0],
- mod_op, delete_node);
+ mod_op);
}
cleanup:
--
1.8.2.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0147] Improve error logging for zones with idnsAllowDynUpdate == FALSE.
On 04/16/2013 12:44 PM, Petr Spacek wrote: Hello, Improve error logging for zones with idnsAllowDynUpdate == FALSE. Zones with dynamic updates disabled are re-configured with empty update policy string, so the update is refused by BIND and an error is logged. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, works fine. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
On 05/06/2013 02:03 PM, Petr Spacek wrote: On 18.4.2013 11:04, Petr Spacek wrote: Hello, Clean up PTR record synchronization code and make it more robust. PTR record synchronization was split to smaller functions. Input validation, error handling and logging was improved significantly. Tbabej's GCC cries about uninitialized variable 'ptr_a_equal', but we weren't able to find any real error. This version of the patch contains a workaround for the GCC oddities. This fixes the problem. It was caused by turning on the gcc's optimalization flag -O2. ACK from me. Since this was a rather robust code change, a look second pair of eyes would be helpful. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name-rdata_type conversions.
On 04/08/2013 07:45 PM, Petr Spacek wrote: Hello, Generalize attribute_name-rdata_type conversions. Attribute names are generated on-the-fly: String Record is appended to textual representation of DNS RDATA type. String Record is cut down from the attribute name during attribute name to rdata type conversion. From now, the plugin doesn't add artificial limitation to supported record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate mod_type in LDAPMod in every case and include the mod_type memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
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
From 22c8518387e83b843e7dcf14d1d61b2ef9342c3c Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Mon, 6 May 2013 17:10:56 +0200
Subject: [PATCH] Resolve SIDs in Web UI
Introduce new command, 'trust-resolve', to aid resolving SIDs to names
in the Web UI.
The
Re: [Freeipa-devel] Web UI refactoring effort ready for review
On 05/03/2013 07:35 PM, Endi Sukma Dewata wrote: Hi, Sorry for the delay, I have some questions comments. Registry: In the simpleuser.js the new 'user' entity is registered first then the old 'user' entity is removed, which could be confusing because they are both identified using 'user'. Should register() automatically remove the old object? I've mixed feelings about it. I would rather keep the methods simple - one task oriented. The issue was caused by other problem: The plugin had to wait for metadata and profile information (to check self-service). So the callback is called after menu is created and menu requires the entity to be resolved. Should we postpone the menu creation to a different phase? Maybe a new one? Ideally a class should have complete methods to manage the objects it stores (e.g. unregister(), remove()). This can be added. How is reg.entity created? Are there others beside 'entity'? Registries are created in related modules. So reg.entity get instantiated at the end of entity.js. There is one for each object type, which are, IIRC: action, facet, entity, field, validator and some others. How is Registries_registry in reg.js used? It doesn't seem to be used anywhere else. Wow, I forgot about this peace of code. It isn't and should be removed. Plugins: In plugins.py the list of plugins is generated using os.listdir(). Then each plugin also has a list of dependencies which I suppose can include other plugins. Then when registering the plugin task, it will have a priority as well. So there seem to be several factors that determine the execution order of the plugins. There should be a document explaining how this will work, so plugin writers can be sure that the code will be executed at the right time. In general I'd avoid using task priority because it doesn't guarantee the correct execution order unless the priorities of all tasks are well coordinated (which might be challenging if there are multiple plugins owned by different people). You are right and it gets more complicated with asynchronous tasks. If one wants to react to an asynchronous task, he has to do it next phase. We might set some fixed priorities for certain operations. Anyway the documentation is essential. Could you add more examples of simple plugins for various scenarios including custom entity, custom facet, custom field, custom menu? They can be included in the RPM for reference. Yes, I that's my plan. Another reason to do it to find limitations. Writing a plugin seems to still require programming skills, reliance on good docs, and probably even some source code familiarity. What do you think about simplifying this a little further? So we'll have 2 ways to define a plugin: one is programatically using the current framework already implemented (e.g. simpleuser.js), and the other is completely declaratively using a plain json data (e.g. simpleuser.json). The declarative plugin will obviously be more limited, but much simpler to use. I agree with the idea. But before creating declarative JSON format we should come up with a plugin API, which would be more-or-less stable and therefore plugins might be resilient to Web UI internals changes. When we have this API we might map it to a JSON representation. The hard part will be to find all the use-cases to cover. Builder: b) Second big issue was build of objects. Entities and facets have complex build logic. It can be simplified into three steps: 1) modifications of spec 2) creation of object and class inheritance 3) init logic Yes, creating an object has become very complicated now with the builders, factories, constructors, preops, postops, inits, overrides, diff, etc. I think the problem is that we're trying to create/modify the spec before creating the object and we need a whole set of mechanisms to do that. Maybe we can simplify it into two basic steps: 1. Create an empty/simple object. 2. Initialize the object. The initialization process could be split further into smaller operations such as: * Load the spec and modify it if necessary * Creating dependent objects and initializing them * Other initialization steps The builder, factory, preops, and postops can be included as part of the initialization step. They can be normal class methods rather than loosely defined functions and can be overridden by subclasses. There's probably a lot more details that need to be discussed. +1 Rewriting all the factories into classes will be a huge task though. At the moment, the biggest problem are spec modification which are not defaults (so they can't be overriden). Like the ones described bellow - #2. 1. Move ./_base/metadata_provider to ./metadata? Might simplify stuff. This seems to be IPA-specific, so yes. https://fedorahosted.org/freeipa/ticket/3604 2. Move actions/buttons spec from factories to pre_ops associated with the factories. Example of stuff to be moved (search.js):
Re: [Freeipa-devel] [PATCH] Resolve SIDs in Web UI
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
[Freeipa-devel] mod_auth_kerb and DIR ccache
The latest krb5 in F-19 updates-testing enables DIR ccache by default.
This is breaking mod_auth_kerb.
Attached is a WIP patch that works in permissive mode.
At this point I'm not sure if we're better off trying to force a FILE
ccache onto httpd in /tmp or go with the DIR method in which case we
need to involve the SELinux folks. I think the DIR method is probably
better long-term.
rob
diff --git a/src/mod_auth_kerb.c b/src/mod_auth_kerb.c
index 455e6dd..828fd07 100644
--- a/src/mod_auth_kerb.c
+++ b/src/mod_auth_kerb.c
@@ -75,6 +75,8 @@
#include stdio.h
#include stdarg.h
#include unixd.h
+#include sys/stat.h
+#include sys/types.h
#define MODAUTHKERB_VERSION 5.4
@@ -163,6 +165,8 @@ static const char *lockname;
static apr_global_mutex_t *s4u2proxy_lock = NULL;
#endif
+char *apache_user = NULL;
+
/***
Macros To Ease Compatibility
***/
@@ -232,6 +236,9 @@ krb5_save_realms(cmd_parms *cmd, void *sec, const char *arg);
static const char *
cmd_delegationlock(cmd_parms *cmd, void *dconf, const char *a1);
+const char *
+set_apache_user(cmd_parms *cmd, void *dummy, const char *arg);
+
static int
obtain_server_credentials(request_rec *r, const char *service_name);
@@ -292,6 +299,9 @@ static const command_rec kerb_auth_cmds[] = {
AP_INIT_TAKE1(KrbConstrainedDelegationLock, cmd_delegationlock, NULL,
RSRC_CONF, the filename of a lockfile used for inter-process synchronization),
+
+AP_INIT_TAKE1(User, set_apache_user, NULL, RSRC_CONF,
+ Apache user. Comes from httpd.conf.),
#endif
#ifdef KRB4
@@ -393,6 +403,14 @@ cmd_delegationlock(cmd_parms *cmd, void *dconf, const char *a1)
return NULL;
}
+const char *
+set_apache_user(cmd_parms *cmd, void *dummy, const char *arg)
+{
+apache_user = arg;
+
+return NULL;
+}
+
static void
log_rerror(const char *file, int line,
#ifdef WITH_HTTPD24
@@ -2172,6 +2190,10 @@ kerb_init_handler(apr_pool_t *p, apr_pool_t *plog,
apr_pool_t *ptemp, server_rec *s)
{
apr_status_t rv;
+ int old_umask, ret;
+ char *dirname = NULL;
+ apr_uid_t userid;
+ apr_gid_t groupid;
ap_add_version_component(p, mod_auth_kerb/ MODAUTHKERB_VERSION);
#ifndef HEIMDAL
/* Suppress the MIT replay cache. Requires MIT Kerberos 1.4.0 or later.
@@ -2179,6 +2201,24 @@ kerb_init_handler(apr_pool_t *p, apr_pool_t *plog,
if (getenv(KRB5RCACHETYPE) == NULL have_rcache_type(none))
putenv(strdup(KRB5RCACHETYPE=none));
#endif
+apr_uid_get(userid, groupid, apache_user, ptemp);
+dirname = apr_psprintf(ptemp, /run/user/%d, userid);
+old_umask = umask();
+ret = mkdir(dirname, 0700);
+umask(old_umask);
+if (ret == -1) {
+if (errno != EEXIST) {
+ap_log_error(APLOG_MARK, APLOG_CRIT, 0, s,
+Unable to create ccache directory: %d, errno);
+return HTTP_INTERNAL_SERVER_ERROR;
+}
+}
+ret = chown(dirname, userid, groupid);
+if (ret == -1) {
+ap_log_error(APLOG_MARK, APLOG_CRIT, 0, s,
+Unable to chown ccache directory: %d, errno);
+return HTTP_INTERNAL_SERVER_ERROR;
+}
#ifdef STANDARD20_MODULE_STUFF
rv = s4u2proxylock_create(s, p);
if (rv != APR_SUCCESS) {
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
