[Engine-devel] Design review summary for 3.2 - adding support for External Events

[Eli Mesika]

----- Original Message -----
> From: "Itamar Heim" <iheim at redhat.com>
> To: "Eli Mesika" <emesika at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Tuesday, December 4, 2012 2:09:56 AM
> Subject: Re: [Engine-devel] Design review summary for 3.2 - adding support for External Events
> 
> On 11/29/2012 11:11 PM, Eli Mesika wrote:
> > Hi
> >
> > please note that this section
> > http://wiki.ovirt.org/wiki/Features/Design/DetailedExternalEvents#Permissions
> > was changed and approved by PM (Miki)
> >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
> 
> 
> 
> > Invoke ..api/events/add API giving at least AuditLogType, Severity,
> > Origin & CustomEventId
> 
> why do you need the AuditLogType - you already know it is
> ExternalEvent/Alert? you only need to know the severity iiuc.

Sorry for the late respond (too many urgent issue :-))

I need that because we have not just and Alert but also Warning , Error & Normal External Events 

> 
> > When the Event/Alert is on a specific object, the object instance
> > id should be set.
> 
> what if it is on multiple objects, like events are today?

The statement says 'at least' so one/multiple entity IDs should be applied as well

> 
> ...
> 
> > Permissions on Entity Instances
> >
> > There will be no permission check on entity instances on which
> > External Events are injected
> 
> this means this permission is always at system level, breaking the
> current permission scheme where permission can be given on a portion
> of
> the system.

I had talk with that with miki and that was our understanding of the requirement, it can be changed if it does not match the requirements or if we have misunderstood them. 

> 
> > The reason is that in order to invoke an External Events on an
> > entity instance, the invoker should know the entity instance UUID
> > and therefore we had already checked that the invoker has the
> > right permissions on the entity instance when he gets the
> > information.
> 
> UUID are not cryptographically secure. knowing a UUID is not a way to
> decide to give a permission.
> (also, this is an admin action, and admins always see all objects in
> the
> system today)
> 
> > Also, double checking that in the AddExternalEvent command is not
> > simple, since each Entity may have several ActionGroups (Create,
> > Edit etc.) associated with it, so it is not clear which to check
> 
> it is very clear - you check for each entity the user has the new
> ActionGroup that allows to inject events (same one you added to the
> superuser and to the newly created special role).

Yea, as I said above , we didn't look in External Events as an permission per object type and that leads to a general implementation..... 

> 
> > So, in order to keep things simple, we will assume that if the
> > caller to Add External Event has the Entity UUID in hand, all we
> > have to check is that he has permission to inject External Events
> 
> as i stated above, this means you are creating an ActionGroup which
> is
> global, regardless of the entity the role with it is given on.

This can be changed if required and still be ready until the 24 DEC deadline 

> 
> ...
> 
> 
> > User Experience
> >
> > Global External Events will be displayed on the Global Events TAB
> > Entity instance External Events will be displayed on the Events TAB
> > when selecting the Entity instance
> 
> 1. not clear here they will also be displayed in the global tab?
> 2. not clear here that multiple entities can be specified for an
> event.
> 
> ...
> 
> > Installation/Upgrade
> >
> > Add additional fields to audit_log table upon upgrade
> > Add the permission(ActionGroup) to manipulate External Events to
> > other admin roles already defined upon upgrade.
> 
> but in permission it is stated only superuser and a new role will be
> given this actiongroup, and here more admin roles are mentioned (if
> so,
> which is the correct one).
> 
> Thanks,
>     Itamar
>