Breeding-Insight / sgn

The code behind the Sol Genomics Network, Cassavabase and BreeDBase websites
https://solgenomics.net
MIT License
2 stars 0 forks source link

[BI-1609] Add configuration to enable/disable PUT/POST /variables endpoints in Breedbase #95

Open timparsons opened 1 year ago

timparsons commented 1 year ago

Update BreedBase to allow for the POST /variables and PUT /variables endpoints to be enabled or disabled based on a configuration property.

Jira story: https://breedinginsight.atlassian.net/browse/BI-1609

Techspecs

  1. Update sgn.conf to have two new configurations:
    1. brapi_post_variables with a default value of 0
    2. brapi_put_variables with a default value of 0
  2. In lib/SGN/Controller/AJAX/BrAPI.pm update the POST variables method to check the value of the brapi_post_variables
    1. if true, continue with the rest of the method
    2. if false, return a 404
  3. In lib/SGN/Controller/AJAX/BrAPI.pm update the PUT variables method to check the value of the brapi_put_variables
    1. if true, continue with the rest of the method
    2. if false, return a 404
  4. In lib/SGN/Controller/AJAX/BrAPI.pm update the serverInfo method:
    1. include POST /variables in the response if brapi_post_variables is true
    2. include PUT /variables in the response if brapi_put_variables is true
MFlores2021 commented 1 year ago

This is a feature that I was trying to add to the BrAPI spec to all endpoints long ago. @BrapiCoordinatorSelby hope this can go to the serverInfo this time.

BrapiCoordinatorSelby commented 1 year ago

@MFlores2021 I think we have a misunderstanding here? It sounds like @timparsons wants to turn whole endpoint on and off. That is the responsibility of the server implementation, not the spec. This would show as the presence or absence of the endpoint in the /serverinfo response. The feature I think you are referring to is this one https://github.com/plantbreeding/BrAPI/issues/445 ? This is different than what Tim is looking for

MFlores2021 commented 1 year ago

It is related I believe. I meant I was enabling and disabling PUT / POST endpoints in the server side. And reflecting that availability by permissions in the serverInfo endpoint. I think availability shouldn't be binary it should be granted by user and it should be shown in serverInfo. Using his example, it can be: brapi_post_variables: all/admin/curator brapi_put_variables: none Let me know if it was not clear :(

BrapiCoordinatorSelby commented 1 year ago

Thanks for the clarification, I see what you mean now! And yes it is related, but I would argue it is not dependent on BrAPI. This functionality could be added to Breedbase before there is a place in the BrAPI spec to represent it, correct?

I just don't want the BrAPI spec to be a blocker for new functionality. I haven't even started looking at the next version of the spec yet, so it will be a long time more before permissions can be officially represented in /serverinfo

MFlores2021 commented 1 year ago

@BrapiCoordinatorSelby BrAPI spec is not a blocker for sure!. I just made the point to keep in mind, no worries.