[PATCH] Bad check for id_provider=local and access_provider=permit

[Ariel Barria]

apologize for the patch file name :D

https://fedorahosted.org/sssd/ticket/1172

[Jakub Hrozek]

On Thu, May 10, 2012 at 01:01:44AM -0500, Ariel Barria wrote:
>     apologize for the patch file name :D
>  
>     https://fedorahosted.org/sssd/ticket/1172 
Hi Ariel,

thank you very much for the patch! The naming and formatting of the patch
is fine.

This approach would remove the startup error as described in the ticket
but maybe it would be better if the "permit" access provider because
that's what the local provider does internally anyway. It would also warn
administrators who tries to use some other access

So I think the proper fix would be to swap "local" for "permit" in the
check.
We should probably also amend the documentation (src/man/sssd.conf.5.xml)
where the permit access provider is described to say that it's the only
allowed access provider for a local domain.

[Ariel Barria]

Thanks for explanation :).
Something like that?

tmp = ldb_msg_find_attr_as_string(res->msgs[0],CONFDB_DOMAIN_ACCESS_PROVIDER,NULL);
        if (tmp && strcasecmp(tmp, "permit") != 0) {
            .......
            
And documentatión

 <term>access_provider (string)</term>
         <listitem>
                <para>
                           The access control provider used for the domain.
                            There are two built-in access providers (in
                            addition to any included in installed backends)
                            Internal special providers are:
                        </para>
                        <para>
                            <quote>permit</quote> always allow access.
It's the only permitted access provider for a local domain. 

>  Date: Thu, 10 May 2012 11:31:58 +0200
>  From: jhrozek(a)redhat.com
>  To: sssd-devel(a)lists.fedorahosted.org
>  Subject: Re: [SSSD] [PATCH] Bad check for id_provider=local and access_provider=permit
>  
>  On Thu, May 10, 2012 at 01:01:44AM -0500, Ariel Barria wrote:
>  >    apologize for the patch file name :D
>  > 
>  >    https://fedorahosted.org/sssd/ticket/1172
>  
>  Hi Ariel,
>  
>  thank you very much for the patch! The naming and formatting of the patch
>  is fine.
>  
>  This approach would remove the startup error as described in the ticket
>  but maybe it would be better if the "permit" access provider because
>  that's what the local provider does internally anyway. It would also warn
>  administrators who tries to use some other access
>  
>  So I think the proper fix would be to swap "local" for "permit" in
> the check.
>  We should probably also amend the documentation (src/man/sssd.conf.5.xml)
>  where the permit access provider is described to say that it's the only
>  allowed access provider for a local domain.
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel

[Stephen Gallagher]

On Thu, 2012-05-10 at 11:56 -0500, Ariel Barria wrote:
>  Thanks for explanation :).
>  Something like that?
>  
>  tmp =
>  ldb_msg_find_attr_as_string(res->msgs[0],CONFDB_DOMAIN_ACCESS_PROVIDER,NULL);
>          if (tmp && strcasecmp(tmp, "permit") != 0) {
>              .......
>              
>  And documentatión
>  
>   <term>access_provider (string)</term>
>           <listitem>
>                  <para>
>                             The access control provider used for the
>  domain.
>                              There are two built-in access providers
>  (in
>                              addition to any included in installed
>  backends)
>                              Internal special providers are:
>                          </para>
>                          <para>
>                              <quote>permit</quote> always allow access.
>  It's the only permitted access provider for a local domain.  
Yes, this looks more like what we need here.

[Ariel Barria]

https://fedorahosted.org/sssd/ticket/1172

From: sgallagh(a)redhat.com
To: sssd-devel(a)lists.fedorahosted.org
Date: Thu, 10 May 2012 13:05:42 -0400
Subject: Re: [SSSD] [PATCH] Bad check for id_provider=local and access_provider=permit

On Thu, 2012-05-10 at 11:56 -0500, Ariel Barria wrote:
>  Thanks for explanation :).
>  Something like that?
>  
>  tmp =
>  ldb_msg_find_attr_as_string(res->msgs[0],CONFDB_DOMAIN_ACCESS_PROVIDER,NULL);
>          if (tmp && strcasecmp(tmp, "permit") != 0) {
>              .......
>              
>  And documentatión
>  
>   <term>access_provider (string)</term>
>           <listitem>
>                  <para>
>                             The access control provider used for the
>  domain.
>                              There are two built-in access providers
>  (in
>                              addition to any included in installed
>  backends)
>                              Internal special providers are:
>                          </para>
>                          <para>
>                              <quote>permit</quote> always allow access.
>  It's the only permitted access provider for a local domain.  
 
Yes, this looks more like what we need here.
 

_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Thu, May 10, 2012 at 10:24:21PM -0500, Ariel Barria wrote:
>     https://fedorahosted.org/sssd/ticket/1172 
This works for me, ack (although I would prefer a single patch for the
change and its documentation, doesn't matter that much though).

Thank your for the contribution, Ariel! There's more easyfix bugs in the
SSSD Trac if you'd like to continue, like tickets #1332 or #1333, that
were reported yesterday.

[Stephen Gallagher]

On Fri, 2012-05-11 at 10:13 +0200, Jakub Hrozek wrote:
>  On Thu, May 10, 2012 at 10:24:21PM -0500, Ariel Barria wrote:
>  >    https://fedorahosted.org/sssd/ticket/1172
>  
>  This works for me, ack (although I would prefer a single patch for the
>  change and its documentation, doesn't matter that much though).
>  
>  Thank your for the contribution, Ariel! There's more easyfix bugs in the
>  SSSD Trac if you'd like to continue, like tickets #1332 or #1333, that
>  were reported yesterday. 
I squashed those two patches together and pushed them to master.

Congratulations on your first accepted patch, Ariel! This will be
included in the 1.9.0 beta 1 release that I'm going to put out later
today.

[Ariel Barria]

Perfect, thanks for good the notice!!
I'm glad being accepted on the team :)

From: sgallagh(a)redhat.com
To: sssd-devel(a)lists.fedorahosted.org
Date: Fri, 11 May 2012 10:24:30 -0400
Subject: Re: [SSSD] [PATCH] Bad check for id_provider=local and access_provider=permit

On Fri, 2012-05-11 at 10:13 +0200, Jakub Hrozek wrote:
>  On Thu, May 10, 2012 at 10:24:21PM -0500, Ariel Barria wrote:
>  >    https://fedorahosted.org/sssd/ticket/1172
>  
>  This works for me, ack (although I would prefer a single patch for the
>  change and its documentation, doesn't matter that much though).
>  
>  Thank your for the contribution, Ariel! There's more easyfix bugs in the
>  SSSD Trac if you'd like to continue, like tickets #1332 or #1333, that
>  were reported yesterday. 
 
I squashed those two patches together and pushed them to master.
 
Congratulations on your first accepted patch, Ariel! This will be
included in the 1.9.0 beta 1 release that I'm going to put out later
today.

_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel