[Sugar-devel] [PATCH] Simple NetworkManager-0.9 port

[Sascha Silbe]

Excerpts from Daniel Drake's message of 2011-11-11 18:18:31 +0100:
> On Mon, Oct 24, 2011 at 6:41 PM, Sascha Silbe <silbe at activitycentral.com> wrote:

[GSM settings]
> >> -    def undo(self):
> >> -        self._model.undo()
> >
> > What happened to the undo functionality? Since we still apply the
> > changes after a timeout rather than after closing the dialog with the
> > "apply" / "OK" button, we need this for the "cancel" button.
> 
> I couldn't figure out how this is supposed to work. It looks broken -
> where is undo implemented? undo with an instant-apply configuration
> system doesn't make much sense to me.

FTR: There's generic undo code in jarabe.controlpanel.gui.ModelWrapper.


> >> +    @dbus.service.method(NM_SECRET_AGENT_IFACE,
> >>                           async_callbacks=('reply', 'error'),
> >> +                         in_signature='a{sa{sv}}osasb',
> >> +                         out_signature='a{sa{sv}}',
> >> +                         sender_keyword='sender',
> >> +                         byte_arrays=True)
> >> +    def GetSecrets(self, settings, connection_path, setting_name, hints,
> >> +                   request_new, reply, error, sender=None):
> >> +        if setting_name != '802-11-wireless-security':
> >> +            raise ValueError("Unsupported setting type %s" % setting_name)
> >> +        if not sender:
> >> +            raise Exception("Internal error: couldn't get sender")
> >> +        uid = self._bus.get_unix_user(sender)
> >> +        if uid != 0:
> >> +            raise Exception("UID %d not authorized" % uid)
> >> +
> >> +        response = SecretsResponse(reply, error)
> >> +        try:
> >> +            self.secrets_request.send(self, settings=settings, response=response)
> >> +        except Exception:
> >> +            logging.exception('Error requesting the secrets via dialog')
> >
> > Shouldn't we return something?
> 
> Don't think so - why do you think this is needed?

Probably misread the code. However there might still be an issue w.r.t.
error handling: if self.secrets_request.send(...) fails, reply() most
likely won't have been called, so we should raise an exception (assuming
that actually works even when using async_callbacks - otherwise we'd
need to replace all raise's with error() calls).


> >> +    def clear(self):
> >> +        """Remove all connections except Sugar-internal ones."""
[...]
> >
> > Should we really attempt to delete _all_ connections, including those
> > created by other users and / or the administrator? What happens if we
> > are disallowed from removing an individual connection (e.g. because it
> > was set up by the administrator and we have no permission to remove it)?
> > Will we end up with only some of the connections removed?
> 
> Connections which we do not have permission to access are not
> presented to Sugar at all, so this already behaves OK.

Being allowed to use (and thus see) a connection does not mean we're
allowed to remove or even just modify it. I didn't find a good overview
of how it's supposed to work, but the PolicyKit config file [1] in the
NM source is a good start. Since
org.freedesktop.NetworkManager.settings.modify.system is different from
org.freedesktop.NetworkManager.settings.modify.own, it's possible that
we may remove some connections (namely those the current user created),
but not others.

Sascha

[1] http://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/policy/org.freedesktop.NetworkManager.policy.in
-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/sugar-devel/attachments/20111114/bbbf254a/attachment.pgp>