Re: [Nagios-devel] Reduce some code duplication

[Guest]

--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 13, 2011 at 05:26:13PM +0100, Andreas Ericsson said:
> On 01/13/2011 04:52 PM, Stephen Gran wrote:
> > Hi,
> >=20
> > 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 re=
use
> >>> 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 a=
ble
> >> to log in to Nagios.
> >=20
> > I think the code fails gracefully for that case - it just doesn't add
> > any permissions.
> >=20
>=20
> But it should add permissions, since cgi.cfg doesn't require the users
> to have contacts configured.
>=20
> >> 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.
> >=20
> > 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.
> >=20
>=20
> So long as old-school authentication keeps working the same way it
> always has I'm prepared to accept it.
>=20
> > My rough idea for the cgiauth.c patch would be something like:
> >=20
> > if(strstr(input,"authorized_for_all_hosts=3D")=3D=3Dinput){
> > temp_ptr=3Dstrtok(input,"=3D");
> > while((temp_ptr=3Dstrtok(NULL,","))){
> > if(!strcmp(temp_ptr,authinfo->username) || !strcmp(tem=
p_ptr,"*"))
> > authinfo->authorized_for_all_hosts=3DTRUE;
> > }
> > + if(!strcmp(temp_ptr,"@")){
> > + if(is_contact_member_of_contactgroup(temp_ptr +=
1,authinfo->username)){
> > + authinfo->authorized_for_all_hosts=3DTR=
UE;
> > + }
> > + }
> > }
> >=20
> > 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.
> >=20
> > That being said, are you happy enough for the existing patch to go in as
> > is?
>=20
> 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,
--=20
--------------------------------------------------------------------------
| Stephen Gran | F.S. Fitzgerald to Hemingway: "Ernest, |
| [email protected] | the rich are different from us." |
| http://www.lobefin.net/~steve | Hemingway: "Yes. They have more |
| | money." |
--------------------------------------------------------------------------

--R

...[email truncated]...


This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]