Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-11817

Enhance api _spec with api permissions & fn description

    Details

    • Type: Bug
    • Status: Done/Fixed
    • Priority: Trivial
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.3.0
    • Fix Version/s: 4.4.0
    • Component/s: None
    • Labels:
      None
    • Documentation Required?:
      None
    • Funding Source:
      Core Team Funds

      Description

      I also agree with the first suggestion (using _spec to declare permissions).

      I think that a 'Generic.getinfo' or 'Generic.getspec' (which covers all API metadata, including permissions and fields) would make more sense than 'Generic.getpermissions'. However, I'm a little nervous about shooting from the hip – might be good to step back and relist the use-cases that 'getfields/getinfo/getspec' does or should enable (SOAP-bindings, Drupal Rules bindings, etc).

      On Jan 27, 2013, at 6:38 AM, Xavier Dutoit wrote:

      > No strong opinion either way either, but I think that will be easier
      > to communicate (and promote the other existing features of _spec) if
      > we go with your first suggestion.
      >
      > To naming details for the api:
      > - getspecifications or get spec instead of getinfo to be more clear
      > about where it comes from?
      > - When/if the needs come, if filtering for a specific action param
      > "api_action" instead of "action" (to avoid collision with the
      > action=getspec on rest/ajax),
      > so:
      > civicrm_api('contact', 'getspec', array("api_action"=>"create"))
      >
      >
      > X+
      >
      > On 27 January 2013 11:30, Eileen McNaughton <> wrote:
      >> OK - so preferences for how that would look? I'm largely agnostic - I think the first perhaps overloads the _spec function - but at least it forces developers to take a look & understand it.
      >>
      >> I would envisage functions like civicrm_api('contact', 'getinfo',
      >> array()); (which might or might not incorporate getfields into the
      >> response
      >>
      >> &
      >> I would envisage functions like civicrm_api('contact',
      >> 'getpermissions', array());
      >>
      >> _civicrm_apiv3_quicksearch_spec(&$params)

      { >> $params['api.permission'] = 'Access CiviCRM'; $params['api.info'] = >> "API Function to return QuickSearch results based on search >> preferences & user permissionts"; .... blah blah specify required >> fields , defaults etc }

      >>
      >> OR
      >>
      >> _civicrm_apiv3_quicksearch_info(&$params)

      { >> $params['api.info'] = "API Function to return QuickSearch results based on search preferences & user permissions"; >> $params['api.permission'] = 'Access CiviCRM'; }

      >>
      >> AND / OR Split the permissions into it's own function
      >> _civicrm_apiv3_quicksearch_permissions(&$params)

      { >> $params['api.permission'] = 'Access CiviCRM'; }

      >>
      >> ----Original Message----
      >> From: Xavier Dutoit []
      >> Sent: Sunday, 27 January 2013 9:22 p.m.
      >> To: Eileen McNaughton
      >> Cc: Donald Lobo; Tim Otten; Dave Greenberg;
      >>
      >> Subject: Re: CRM-11730 - quicksearch widget broken
      >>
      >> HI,
      >>
      >> For new custom apis, threre is a hook civicrm_alterAPIPermissions
      >> http://wiki.civicrm.org/confluence/display/CRMDOC41/CiviCRM+hook+spec
      >> ification#CiviCRMhookspecification-hook_civicrm_alterAPIPermissions
      >>
      >> For our own core api (eg. quicksearch) we can set the priority we want in permissions.php, obviously, Kurund/dgg, don't think you are using the right permission for the getquick, should be "access CiviCRM" not 'access AJAX API' :
      >> 'access AJAX API' is not meant to be used for a specific API, but only as a mean to have public ajax APIs:
      >> No matter the acl at the api level, you can't access /ajax/rest/api unless you have *access CiviCRM" OR 'access AJAX API'. The later was to use with the alterAPIPermission to open further so anonymous is granted access to a specfic API, for instance a custom one.
      >> http://issues.civicrm.org/jira/browse/CRM-8840
      >>
      >> Putting the permissions in spec is probably the most logical indeed, so it's more "in your face" when you work on the api who can access it. The only thing is now you are forced to create a _spec per action, but that's something we'd rather have anyway.
      >>
      >> As for renaming the files, I think it was because the aim was to define the permissions in the xml and the .permissions was meant as a temp solution. Don't see any reason not to rename.
      >>
      >> n 27 January 2013 07:57, Eileen McNaughton <> wrote:
      >>> I believe there is a very good reason – no idea what it is though! X
      >>> – can you think of why it was done that way – was it a Polish thing?
      >>> I assume there is no problem renaming.
      >>>
      >>>
      >>>
      >>> Re Tim’s question.
      >>>
      >>>
      >>>
      >>> “Aside: When an downstream developer provides an API in his
      >>> extension/module, there's no way to modify .permissions.php, so
      >>> their permission is necessarily the default. This seems like an
      >>> issue – shouldn't we open this up somehow? “
      >>>
      >>>
      >>>
      >>> Perhaps we should allow the _spec function to define permissions too?
      >>> Or have a separate perm function would make it much more visible
      >>> then even a non-hidden permissions file. I would quite like to
      >>> extend the _spec to also return a description of the api. Given the
      >>> way we are going this is probably more useful than putting
      >>> explanatory notes in a comment block (ie. we haven’t been running the script to update the phpdoc generated doc).
      >>>
      >>>
      >>>
      >>> Eileen
      >>>
      >>>
      >>>
      >>>
      >>>
      >>> From: Donald Lobo []
      >>> Sent: Sunday, 27 January 2013 4:04 p.m.
      >>> To: Tim Otten
      >>> Cc: Eileen McNaughton; 'Dave Greenberg';
      >>>
      >>> Subject: Re: CRM-11730 - quicksearch widget broken
      >>>
      >>>
      >>>
      >>>
      >>> also any specific reason, we have themas:
      >>>
      >>> .permissions.php
      >>> .listAll.php
      >>>
      >>> in general hidden files are not great to used. Can we just rename
      >>> them and let them follow our normal conventions
      >>>
      >>> thanx
      >>>
      >>> lobo
      >>>
      >>> On 01/26/2013 06:42 PM, Tim Otten wrote:
      >>>
      >>> Aside: When an downstream developer provides an API in his
      >>> extension/module, there's no way to modify .permissions.php, so
      >>> their permission is necessarily the default. This seems like an
      >>> issue – shouldn't we open this up somehow?
      >>>
      >>>
      >>>
      >>> On Jan 26, 2013, at 5:14 PM, Eileen McNaughton wrote:
      >>>
      >>>
      >>>
      >>> When an API function is added the relevant permission should be
      >>> added to CRM_Core_DAO/.permissions.php – it used to be that if
      >>> someone adds an api & forgets (or doesn’t know to) add the
      >>> permission defaulted to ‘Access CiviCRM’ – so we were giving people with ‘Access CiviCRM’
      >>> access to a whole bunch of things they shouldn’t get. I changed it
      >>> so that if people didn’t set the permission when adding a new api
      >>> function it would default to a higher level of security.
      >>>
      >>>
      >>>
      >>> So, yes, Kurund’s fix is correct – the appropriate permission for
      >>> each API should be added per
      >>>
      >>>
      >>>
      >>> “I have added permission in CRM/Core/DAO/.permissions.php to
      >>>
      >>> 'getquick' => array('access AJAX API'),”
      >>>
      >>>
      >>>
      >>> Although – I expect appropriate permission for this one function is
      >>> ‘Access CiviCRM’
      >>>
      >>>
      >>>
      >>> Eileen
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>>
      >>> From: Dave Greenberg []
      >>> Sent: Saturday, 26 January 2013 2:03 p.m.
      >>> To:
      >>> Cc: Eileen McNaughton; Tim Otten; Donald A. Lobo
      >>> Subject: Re: CRM-11730 - quicksearch widget broken
      >>>
      >>>
      >>>
      >>> Eileen - What was your thinking in setting permission on getquick to
      >>> 'administer CiviCRM'. Given that the main quick search as well as
      >>> all the other autocomplete form fields use this api - this is not viable.
      >>> All backoffice users would have to be assigned that permission.
      >>>
      >>> Tim / Kurund - given how pervasive the functionality is that uses
      >>> this api, I think we need to figure out a way to make sure things
      >>> are secure AND allow folks who can 'view all contacts' or some
      >>> subset via ACL's have the ability to use this widget w/o special and
      >>> obscure permissions. (I consider 'access AJAX API' to be pretty
      >>> obscure permission to require for our basic search
      >>> widget.)
      >>>
      >>> dgg
      >>>
      >>> On Fri, Jan 25, 2013 at 12:36 PM, Kurund Jalmi
      >>> <> wrote:
      >>>
      >>> Hey Dave,
      >>>
      >>>
      >>>
      >>> I have checked CRM-11730, and problem is due to wrong permissions
      >>> for "getquick" api.
      >>>
      >>> Prior to 4.3 permission was "access CiviCRM" and now Eileen has
      >>> modified it to "administer CiviCRM" hence it was failing.
      >>>
      >>>
      >>>
      >>> I have added permission in CRM/Core/DAO/.permissions.php to
      >>>
      >>> 'getquick' => array('access AJAX API'),
      >>>
      >>>
      >>>
      >>> This will fix the problem with fresh installs, but it might be
      >>> problem in some upgrades. For example, below permission allow users
      >>> to add participants and after upgrade it won't work.
      >>>
      >>> CiviEvent: edit event participants
      >>> CiviCRM: access CiviCRM
      >>> CiviCRM: add contacts
      >>> CiviCRM: view all contacts
      >>> CiviCRM: edit all contacts
      >>>
      >>>
      >>>
      >>> your thoughts on this..
      >>>
      >>>
      >>>
      >>> Kurund
      >>>
      >>>
      >>>
      >>>
      >>> –
      >>> Kurund Jalmi

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              eileen Eileen McNaughton
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved: