Re: [Nagios-devel] Reduce some code duplication
Posted: Thu Jan 13, 2011 4:26 pm
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 [email protected]
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.
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]
> 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 [email protected]
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.
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]