Reduce some code duplication

[Stephen Gran]

On Thu, Jan 13, 2011 at 05:26:13PM +0100, Andreas Ericsson said:
> 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.

I think I may not understand, sorry about that.  The patch I've just
submitted doesn't change any behavior in the cgi, as far as I can tell -
it just substitutes some code duplication for a function call.  This is
the patch I'm asking if you're happy enough with.

The pseudo code above is of course not ready to go in as is, and I
didn't mean that it should.  If the pseudo code has the same logic flow
as above (where it only tries to add additional permissions if a group
is matched) I don't think it will break existing permissions setups: am
I missing something?

Cheers,
-- 
 --------------------------------------------------------------------------
|  Stephen Gran                  | F.S. Fitzgerald to Hemingway:  "Ernest, |
|  steve at lobefin.net             | the rich are different from us."        |
|  http://www.lobefin.net/~steve | Hemingway:  "Yes.  They have more       |
|                                | money."                                 |
 --------------------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://www.monitoring-lists.org/archive/developers/attachments/20110113/a2a6e831/attachment.sig>
-------------- next part --------------
------------------------------------------------------------------------------
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
-------------- next part --------------
_______________________________________________
Nagios-devel mailing list
Nagios-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nagios-devel