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 690628 - <gAcl:withKey/> element Not Handled by libgdata
<gAcl:withKey/> element Not Handled by libgdata
Status: RESOLVED FIXED
Product: libgdata
Classification: Platform
Component: Google Documents service
git master
Other Linux
: Normal normal
: 0.16
Assigned To: libgdata-maint
libgdata-maint
Depends on:
Blocks:
 
 
Reported: 2012-12-21 22:37 UTC by Meg Ford
Modified: 2014-08-24 15:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
core: Add support for ACL entries with keys (13.23 KB, patch)
2014-08-24 15:09 UTC, Philip Withnall
committed Details | Review

Description Meg Ford 2012-12-21 22:37:50 UTC
Because libgdata does not handle the <gAcl:withKey/> element public and "shared with link" permissions cannot be added to Google documents via the API, and existing public and "shared with link" permissions cannot be modified for documents with an existing "shared with link" permission. Other acl list changes are not affected by this.

This is because the only difference between a public permission and a "shared with link" permission is the <gAcl:withKey/> element. An attempt to set a public permission will return the error:
JS LOG: Error inserting new ACL scope for document The entry has been modified since it was downloaded: <errors xmlns='http://schemas.google.com/g/2005'><error><domain>GData</domain><code>ServiceException</code><internalReason>This user already has access to the document.</internalReason></error></errors> 

An attempt to update the "shared with link" permission" will return the error: 
libgdata-DEBUG:   
libgdata-Message: Unhandled XML in GDataAccessRule: <gAcl:withKey key="with link"><gAcl:role value="writer"/></gAcl:withKey>
Comment 1 Meg Ford 2012-12-22 01:32:15 UTC
Ugh, sorry, wrong error message.
An attempt to update the "shared with link" permission will return the error:    
JS LOG: Error updating ACL scope for documentInvalid request URI or header, or unsupported nonstandard parameter: <errors xmlns='http://schemas.google.com/g/2005'><error><domain>GData</domain><code>InvalidEntryException</code><internalReason>The posted entry has an invalid value for the field(s): role withKey</internalReason></error></errors>
Comment 2 Philip Withnall 2012-12-22 10:36:47 UTC
Fixing this should be as simple as adding support for <gAcl:withKey/> to parse_xml() and get_xml() in gdata-access-rule.c:

http://git.gnome.org/browse/libgdata/tree/gdata/gdata-access-rule.c#n326

A new property should be added to GDataAccessRule allowing access to the key, with a getter and setter method. A new set of tests for handling access rules with keys should be added to tests/general.c:

http://git.gnome.org/browse/libgdata/tree/gdata/tests/general.c#n1057

and a test or two for handling Google documents which are ‘shared with a link’ should be added to tests/documents.c.

Meg, do you fancy coming up with a patch for this? I’m not sure when I’ll be able to find time for it otherwise. Thanks for investigating this.
Comment 3 Meg Ford 2012-12-22 18:47:27 UTC
Hi Phillip,
Yeah, the code there doesn't look too bad. I don't actually know C but I can certainly hack up something as long as you don't mind reviewing it.
Comment 4 Philip Withnall 2012-12-22 21:10:28 UTC
(In reply to comment #3)
> Hi Phillip,
> Yeah, the code there doesn't look too bad. I don't actually know C but I can
> certainly hack up something as long as you don't mind reviewing it.

Please do. I’m always happy to review patches!
Comment 5 Meg Ford 2012-12-26 06:25:10 UTC
I have a question. I looked at the Google API and it has a different constructor for the acl withKey, as you can see in the example code (https://developers.google.com/google-apps/documents-list/#sharing_resources_with_authorization_keys):
     DocumentEntry entry = (DocumentEntry)feed.Entries[0];

      // Instantiate a AclEntryWithKey object to share with keys.
      AclEntryWithKey acl = new AclEntryWithKey();

      // Set the ACL scope.
      acl.Scope = new AclScope("", "default");

      // Instantiate a AclWithKey object to specify a key.
      AclWithKey withKey = new AclWithKey("[ACL KEY]");

      // Set the ACL role.
      withKey.Role = new AclRole("reader");

      // Set the WithKey property of the AclEntryWithKey element.
      acl.WithKey = withKey;

      // Insert the new role into the ACL feed.
      service.Insert(new Uri(entry.AccessControlList), acl); 

It looks like the properties are the same as a regular acl entry's except there is the withKey element.

At first I thought that the withKey element was just a property of an acl entry, and should therefore could be created or modified using GDataAccessRule, but I wanted to be sure that this is correct. Is it proper to consider the withKey element to be part of the GDataAccessRule, or does there need to be a GDataAccessRuleWithKey?  

Thanks, and sorry I didn't look at this carefully before I filed the bug!
Comment 6 Philip Withnall 2012-12-26 12:15:17 UTC
(In reply to comment #5)
> At first I thought that the withKey element was just a property of an acl
> entry, and should therefore could be created or modified using GDataAccessRule,
> but I wanted to be sure that this is correct. Is it proper to consider the
> withKey element to be part of the GDataAccessRule, or does there need to be a
> GDataAccessRuleWithKey?  

The Google documentation isn’t clear about the semantics of the withKey element, so the best we can do is guess. The documentation says each ACL entry has a unique key, so I assume there can only be one withKey element per entry. So I think it’s OK to implement this as a property on GDataAccessRule.

e.g. GDataAccessRule::sharing-key. It would normally be NULL (meaning no key is needed). It must not be the empty string — it must either be NULL or a valid key. The Google documentation says the keys are chosen by the server, so the libgdata property must be read-only.

> Thanks, and sorry I didn't look at this carefully before I filed the bug!

No worries. Bugs are for discussing things like this and working out the details!
Comment 7 Meg Ford 2012-12-29 05:26:59 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > At first I thought that the withKey element was just a property of an acl
> > entry, and should therefore could be created or modified using GDataAccessRule,
> > but I wanted to be sure that this is correct. Is it proper to consider the
> > withKey element to be part of the GDataAccessRule, or does there need to be a
> > GDataAccessRuleWithKey?  
> 
> The Google documentation isn’t clear about the semantics of the withKey
> element, so the best we can do is guess. The documentation says each ACL entry
> has a unique key, so I assume there can only be one withKey element per entry.
> So I think it’s OK to implement this as a property on GDataAccessRule.

I guess the reason I was asking is this: the other properties of the GDataAccessRule are parsed using xmlGetProp (is that a libxml function?). I don't need to get/set properties (attributes) of the withKey node, though -- I need to get/set properties of its child node(s). So if I want to retrieve a property from a child node can I write: withKey_role = xmlGetProp (node->firstChild, (xmlChar*) "value"); ?
e.g. when the acl <entry> contains these elements <gAcl:role value='writer'/><gAcl:scope type='default'/><gAcl:withKey key="with link"><gAcl:role value="reader"/></gAcl:withKey>

Sorry to ask so many questions. when I don't know the language I'm using at all I try to get a clear concept of what I need to do so it's a little easier debug the terrible code I write :) 
> 
> e.g. GDataAccessRule::sharing-key. It would normally be NULL (meaning no key is
> needed). It must not be the empty string — it must either be NULL or a valid
> key. The Google documentation says the keys are chosen by the server, so the
> libgdata property must be read-only.
> 
> > Thanks, and sorry I didn't look at this carefully before I filed the bug!
> 
> No worries. Bugs are for discussing things like this and working out the
> details!
Comment 8 Meg Ford 2012-12-30 06:54:04 UTC
Hey,
I also found a workaround for the bug in js that I can use in Documents, so if you don't want to bother answering a million not-very-intelligent questions about xml parsing in C, I can just use the workaround I think.
Thanks!
Comment 9 Philip Withnall 2012-12-30 23:44:41 UTC
(In reply to comment #7)
> I guess the reason I was asking is this: the other properties of the
> GDataAccessRule are parsed using xmlGetProp (is that a libxml function?).

It is a libxml function.

> So if I want to retrieve a
> property from a child node can I write: withKey_role = xmlGetProp
> (node->firstChild, (xmlChar*) "value"); ?

You could, but that won’t be robust to changes in the number/order of children of the withKey element. The only robust way to handle this is to iterate over the children of the withKey element and check each of their names against "role".

See here for an example: http://git.gnome.org/browse/libgdata/tree/gdata/georss/gdata-georss-where.c?id=88e381defb549aa499d7cc482dc7d2dfe5a93577#n93

> Sorry to ask so many questions.

No problem. Might be easier to answer questions like this on IRC though. Catch me in #libgdata!
Comment 10 Meg Ford 2012-12-31 06:17:56 UTC
OK, thanks, Philip! I start learning C at school next week so I'll stop by on irc sometime soon when I have made some progress with the code. I appreciate your help!
Comment 11 Meg Ford 2014-08-14 04:26:24 UTC
Sorry I never got to this! I have no idea if I'll have time to actually look at it as I'm in the middle of finishing my thesis and job hunting. When is your release?
Comment 12 Philip Withnall 2014-08-14 12:41:43 UTC
(In reply to comment #11)
> Sorry I never got to this! I have no idea if I'll have time to actually look at
> it as I'm in the middle of finishing my thesis and job hunting. When is your
> release?

Hey, no worries. I was updating the bug because I was planning to take a look at it before 0.16. However, it would be great if you want to continue working on it, and I would be very happy to answer questions/review patches as needed. :-)  If you don’t get a chance to work on it, I’ll do it.
Comment 13 Meg Ford 2014-08-14 12:57:22 UTC
It's hard to imagine that I will ever have any free time again, but if I do I'll ping you. I hate to pass up  chance to use LibXML, but if I don't get to it I'll see if I can find another bug in the future :)
Comment 14 Philip Withnall 2014-08-24 15:09:02 UTC
Created attachment 284346 [details] [review]
core: Add support for ACL entries with keys

This adds support for <gAcl:withKey/> elements, allowing for
authorisation keys in ACLs, e.g. for Google Documents.

This adds the following API:
 • GDataAccessRule:key
 • gdata_access_rule_get_key()
and appropriate tests.
Comment 15 Philip Withnall 2014-08-24 15:09:34 UTC
Done!

Attachment 284346 [details] pushed as f817fda - core: Add support for ACL entries with keys