After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 737656 - Support locked accounts
Support locked accounts
Status: RESOLVED FIXED
Product: gnome-online-accounts
Classification: Core
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: GNOME Online Accounts maintainer(s)
GNOME Online Accounts maintainer(s)
Depends on:
Blocks: 737665
 
 
Reported: 2014-09-30 12:11 UTC by Debarshi Ray
Modified: 2014-10-02 14:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Support locked accounts (3.62 KB, patch)
2014-09-30 12:19 UTC, Debarshi Ray
committed Details | Review
Don't leak the GErrors (4.10 KB, patch)
2014-09-30 14:25 UTC, Debarshi Ray
committed Details | Review
Minor clean up (2.86 KB, patch)
2014-09-30 14:25 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2014-09-30 12:11:11 UTC
The idea is to add support for accounts that were added by the administrator to pre-configure systems in an enterprise deployment. Ideally, the user interface should prevent, or at least discourage, removal of these accounts.

The plan is to have a boolean property on the org.gnome.OnlineAccounts.Account interface, which will be set to TRUE for these accounts. This property will be backed up by an entry in the accounts.conf file. Creation of such accounts are very likely to be in batch mode, so the value of this property can be passed through the 'a{ss}'-typed 'details' parameter in org.gnome.OnlineAccounts.Manager.AddAccount.
Comment 1 Debarshi Ray 2014-09-30 12:19:15 UTC
Created attachment 287441 [details] [review]
Support locked accounts

I chose to use 'IsLocked' as the name of the property because we already had 'IsTemporary'.
Comment 2 Matthew Barnes 2014-09-30 13:51:27 UTC
For the record, I asked Debarshi for this to help keep GOA accounts deployed by Fleet Commander from being deleted by users.

Patch looks good to me.  Your use of g_key_file_get_boolean() for 'IsLocked' relies on the value returned on error (for missing keys), which might warrant a code comment.  But that's a minor nit.

I do also like 'IsLocked' better than 'Locked'.
Comment 3 Ray Strode [halfline] 2014-09-30 14:13:09 UTC
Review of attachment 287441 [details] [review]:

looks okay to me too.  As far as IsLocked bikeshedding goes... i vote for "IsImmutable" or "IsImortal" since a "locked" account is normally one that's been prevented access from login.  But doesn't matter.  Is there a control-center half to this too?
Comment 4 Debarshi Ray 2014-09-30 14:25:17 UTC
Created attachment 287452 [details] [review]
Don't leak the GErrors

Spotted these leaks while reading the code.
Comment 5 Debarshi Ray 2014-09-30 14:25:43 UTC
Created attachment 287453 [details] [review]
Minor clean up
Comment 6 Debarshi Ray 2014-09-30 14:27:25 UTC
Comment on attachment 287441 [details] [review]
Support locked accounts

Thanks for the reviews!
Comment 7 Debarshi Ray 2014-09-30 14:53:49 UTC
(In reply to comment #3)
>  Is there a
> control-center half to this too?

Bug 737665
Comment 8 Matthew Barnes 2014-09-30 15:44:54 UTC
(In reply to comment #3)
> looks okay to me too.  As far as IsLocked bikeshedding goes... i vote for
> "IsImmutable" or "IsImortal" since a "locked" account is normally one that's
> been prevented access from login.

I think the shed should be purple!  And also "Removable" is the term I used for this kind of thing in E-D-S, but whatever Debarshi prefers.
Comment 9 Debarshi Ray 2014-09-30 15:51:30 UTC
(In reply to comment #8)
> (In reply to comment #3)
> > looks okay to me too.  As far as IsLocked bikeshedding goes... i vote for
> > "IsImmutable" or "IsImortal" since a "locked" account is normally one that's
> > been prevented access from login.
> 
> I think the shed should be purple!  And also "Removable" is the term I used for
> this kind of thing in E-D-S, but whatever Debarshi prefers.

Yeah "IsRemovable" is probably better because it matches with the name of the "Remove" method. I went with "IsLocked" because we (mbarnes and I) were using "Lock" in our discussions, but I can change it? What do you think?
Comment 10 Matthew Barnes 2014-09-30 15:59:48 UTC
Note that "IsRemovable" would flip the default, and makes backward-compat a little trickier.  Also I wasn't sure at first if "removable" would be too specific or limiting for possible future uses, but I guess not.

I'm happy with either one.
Comment 11 Debarshi Ray 2014-09-30 16:09:39 UTC
(In reply to comment #10)
> Note that "IsRemovable" would flip the default, and makes backward-compat a
> little trickier.

Yes, that is a good point. Let's stick with "IsLocked", then. :)