Re: nm_setting_gsm_get_pin() retuns null

[Dan Williams <dcbw redhat com>]

On Mon, 2011-10-03 at 14:20 +0200, Anders Feder wrote:
> Where would you propose the new code be added?
> 
> In the first iteration, I imagine it would work something like this:
> 
> If (UnlockRequired)
>      Get SimIdentifier
>      If (SimIdentifier is found)
>          Get PIN for SimIdentifier from keyring
>          If (PIN is found for SimIdentifier)
>              Try unlock using saved PIN
>              While (unlock failed)
>                  Prompt user for new PIN
>                  Try unlock using new PIN
>                  If (unlock succeeded)
>                      Save new PIN for SimIdentifier to keyring
> 
> If this works, a similar procedure could be applied for 
> DeviceIdentifier, if SimIdentifier is not found.
> 
> Is this the best approach?

Yeah, that's basically it.  So UnlockRequired and DeviceIdentifier are
both properties of the org.freedesktop.ModemManager.Modem interface, and
thus you can retrieve them both in the same D-Bus properties call using
GetAll().  For that, in src/applet-device-gsm.c's gsm_device_added()
function, where it calls the Get() method with UnlockRequired, what you
want to do is call "GetAll" instead and kill the "UnlockRequired"
argument.  Then in the unlock_reply() for the dbus_g_proxy_end_call()
you'll do something like:

GHashTable *props = NULL;

if (dbus_g_proxy_end_call (proxy, call, &error,
                           DBUS_TYPE_G_MAP_OF_VARIANT, &props,
                           G_TYPE_INVALID)) {
    GHashTableIter iter;
    const char *prop_name;
    GValue *value;

    g_hash_table_iter_init (&iter, props);
    while (g_hash_table_iter_next (&iter, (gpointer) &prop_name, (gpointer) &value)) {
        if ((strcmp (prop_name, "UnlockRequired") == 0) && G_VALUE_HOLDS_STRING (value)) {
	    g_free (info->unlock_required);
            info->unlock_required = parse_unlock_required (value);
        }

        if ((strcmp (prop_name, "DeviceIdentifier") == 0) && G_VALUE_HOLDS_STRING (value)) {
            g_free (info->devid);
            info->devid = g_value_dup_string (value);
        }
    }
}

That takes care of the UnlockRequired and DeviceIdentifier properties.
Now you need to get the SimIdentifier property, which is a GSM-specific
property (unlike UnlockRequired and DeviceIdentifier which are provided
for CDMA devices too).  For that you'll want to do something like this
in gsm_device_added():

	dbus_g_proxy_begin_call (info->props_proxy, "Get",
	                         sim_id_reply, info, NULL,
	                         G_TYPE_STRING, MM_DBUS_INTERFACE_MODEM_GSM_CARD,
	                         G_TYPE_STRING, "SimIdentifier",
	                         G_TYPE_INVALID);

and then basically copy & paste the current unlock_reply() function as
sim_id_reply(), fix up the property stuff there (obviously you're
handling the SimIdentifier property instead of UnlockRequired) and then
at the end of that function, you want something like this:

    if (info->unlock_required && (info->simid || info->devid))
	unlock_dialog_new (info->device, info);

which will actually do the unlock.  You dont' want to call
unlock_dialog_new() in unlock_reply() because you don't have the
SimIdentifier yet.

Then unlock_dialog_new() is where you'd query the keyring for existing
PINs using SimIdentifier as one of the attributes the PIN would be
stored with (using gnome_keyring_find_itemsv_sync()) and if there wasn't
any result try again with DeviceIdentifier.  Check out utils/utils.c and
src/applet-agent.c for more examples of how to write and read items from
the keyring.

Dan

> Anders Feder
> 
> On 03-10-2011 06:00, Dan Williams wrote:
> > On Sat, 2011-10-01 at 02:41 +0200, Anders Feder wrote:
> >> Thanks. I've left you a question in the bug report. I may be
> >> interested in taking a stab at this if it isn't overwhelmingly
> >> difficult.
> > Responded.  It shouldn't be too difficult; there's already code there to
> > track the modem object from ModemManager and do the right thing.  So in
> > addition to the UnlockRequired property, you'd also want to grab and
> > watch the SimIdentifier and DeviceIdentifier properties, and use those
> > to look in the keyring for the right PIN.
> >
> > Dan
> >
> >> Anders Feder
> >>
> >> On 30-09-2011 07:31, Dan Williams wrote:
> >>> On Wed, 2011-09-21 at 08:34 +0200, Anders Feder wrote:
> >>>> Hi,
> >>>>
> >>>> I'm trying to write a fix for this bug. I've been experimenting with
> >>>> testing whether a given connection is configured to autoconnect, using
> >>>> this code (inside applet-device-gsm.c):
> >>>>          NMSettingConnection *setting =
> >>>>          nm_connection_get_setting_connection (connection);
> >>>>          NMSettingGsm *setting_gsm = nm_connection_get_setting_gsm
> >>>>          (connection);
> >>>>          if ((autoconnects = autoconnects ||
> >>>>          (nm_setting_connection_get_autoconnect (setting)&&
> >>>>          nm_setting_gsm_get_pin (setting_gsm))))
> >>>>              break;
> >>>> However, nm_setting_gsm_get_pin (setting_gsm) seems to always return
> >>>> null, even when a PIN is set for the connection. Can someone pelase
> >>>> tell me why this might be? Under what circumstances may this function
> >>>> return null even when a PIN is set?
> >>> PIN codes in the connection data are deprecated because PINs are
> >>> specific to the SIM card, not to the connection.  If you lose your SIM
> >>> and get another from the same provider, the PIN will be different, and
> >>> the PIN in the connection data will be wrong.  I've written up some
> >>> notes on what I think should be done in the GNOME bug report for this
> >>> issue:
> >>>
> >>> https://bugzilla.gnome.org/show_bug.cgi?id=618532
> >>>
> >>> Happy to help if there are more questions.
> >>>
> >>> Dan
> >>>
> >>>
> >>>
> >>>
> >
> >
>