Reduce some code duplication

[Andreas Ericsson]

On 01/13/2011 04:52 PM, Stephen Gran wrote:
> Hi,
> 
> On Thu, Jan 13, 2011 at 02:58:01PM +0100, Andreas Ericsson said:
>> On 01/13/2011 01:43 PM, Stephen Gran wrote:
>>> Hi,
>>>
>>> I'm looking slightly longer term at extending cgi.cfg to support using
>>> contact_group names in the authorized_for* settings, and this is step
>>> one on the road.  If someone thinks the above is a bad idea (or if reuse
>>> of code is a bad idea) let me know and I'll stop.
>>
>> There's one problem with this approach;
>> The users in cgi.cfg don't have to be contacts. They only have to be able
>> to log in to Nagios.
> 
> I think the code fails gracefully for that case - it just doesn't add
> any permissions.
> 

But it should add permissions, since cgi.cfg doesn't require the users
to have contacts configured.

>> With that in light, I wonder what happens when eu-admins is both a user
>> (from the apache view of things) as well as a contactgroup, but not a
>> contact. That's one of the things that absolutely has to keep working,
>> or a lot of people's setups will break.
> 
> I was planning to use a marker to specify that it is a group, whether %
> like sudo or @ like many other things, I don't know (or particularly
> mind).  So with that in mind, eu-admins and @eu-admins would be parsed
> differently.
> 

So long as old-school authentication keeps working the same way it
always has I'm prepared to accept it.

> My rough idea for the cgiauth.c patch would be something like:
> 
> if(strstr(input,"authorized_for_all_hosts=")==input){
>          temp_ptr=strtok(input,"=");
>          while((temp_ptr=strtok(NULL,","))){
>                  if(!strcmp(temp_ptr,authinfo->username) || !strcmp(temp_ptr,"*"))
>                          authinfo->authorized_for_all_hosts=TRUE;
>                  }
> +               if(!strcmp(temp_ptr,"@")){
> +                       if(is_contact_member_of_contactgroup(temp_ptr + 1,authinfo->username)){
> +                               authinfo->authorized_for_all_hosts=TRUE;
> +                       }
> +               }
>          }
> 
> This patch is of course a nonsense patch, as
> is_contact_member_of_contactgroup() takes a pair of structs and not
> strings, and this function doesn't have access to the structs at
> this point.  I hope it gives you a rough sense of how I'm hoping to
> introduce it, though - preserve existing usage and only extend it if the
> name matches a certain marker.
> 
> That being said, are you happy enough for the existing patch to go in as
> is?
> 

No I'm not. It has to maintain existing functionality or I really can't
accept it. Breaking people's setups is considered terribly bad form.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

------------------------------------------------------------------------------
Protect Your Site and Customers from Malware Attacks
Learn about various malware tactics and how to avoid them. Understand 
malware threats, the impact they can have on your business, and how you 
can protect your company and customers by using code signing.
http://p.sf.net/sfu/oracle-sfdevnl