Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

41 add auth to harvest api #8106

Merged
merged 26 commits into from
Oct 12, 2021
Merged

41 add auth to harvest api #8106

merged 26 commits into from
Oct 12, 2021

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented Sep 23, 2021

What this PR does / why we need it: The Delete Harvesting set api did not require any credentials. To mimic the ui super user permissions have been added. Also the modify harvesting set api end point had not been implemented so that was completed. And finally all of the Manage Harvesting sets apis were documented as they had not been previously.

Which issue(s) this PR closes:

Closes # https://github.com/IQSS/dataverse-security/issues/41
and closes #4514 Document Harvesting set endpoints

Special notes for your reviewer: There was also and endpoint for "/datasets" which returns an empty string. It's not clear what the intended response is, possibly a count of datasets that would be harvested given the definition. Without a clear intent I left it as I found it.

Suggestions on how to test this: verify that the endpoints work as documented and that a non-super user may not create, modify or delete a harvesting set.

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?:

Additional documentation:

StringReader rdr = new StringReader(jsonBody);

try( JsonReader jrdr = Json.createReader(rdr) )
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[reviewdog] reported by reviewdog 🐶
'{' at column 2 should be on the previous line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK bot, point to you.

@coveralls
Copy link

coveralls commented Sep 23, 2021

Coverage Status

Coverage decreased (-0.009%) to 19.017% when pulling dbd68e0 on 41-add-auth-to-harvest-api into 97b7c88 on develop.

@djbrooke djbrooke self-assigned this Sep 27, 2021
@djbrooke
Copy link
Contributor

@sekmiller I'm going to rerun the tests on this issue, just seeing if it was an intermittent failure

@djbrooke
Copy link
Contributor

(updating from develop to rerun the tests)

@djbrooke djbrooke removed their assignment Sep 27, 2021
@djbrooke
Copy link
Contributor

OK! Tests pass. I'll unassign myself so that someone can review this.

@pdurbin
Copy link
Member

pdurbin commented Sep 28, 2021

One quick thing is that the docs don't built, even though it says "all checks have passed" above.

@djbrooke djbrooke self-assigned this Sep 28, 2021
@djbrooke
Copy link
Contributor

I wrote these guides, so I broke them. I will fix.

@djbrooke djbrooke assigned scolapasta and unassigned djbrooke Sep 29, 2021
@sekmiller sekmiller removed their assignment Oct 4, 2021
@kcondon kcondon assigned sekmiller and unassigned kcondon Oct 5, 2021
@kcondon
Copy link
Contributor

kcondon commented Oct 5, 2021

having trouble with create set

curl -H X-Dataverse-key:xxxx-yyyy-zzzz-aaaa -X POST "http://localhost:8080/api/harvest/server/oaisets/" --upload-file harvestset1.json
{"status":"ERROR","code":405,"message":"API endpoint does not support this method. Consult our API guide at http://guides.dataverse.org.","requestUrl":"http://localhost:8080/api/v1/harvest/server/oaisets/harvestset1.json","requestMethod":"POST"}[root@dataverse-internal tmp]# cat harvestset1.json
{
"name":"ffAuthor",
"definition":"authorName:Finch, Fiona",
"description":"Fiona Finch’s Datasets"
}

@sekmiller sekmiller removed their assignment Oct 5, 2021
@djbrooke djbrooke added this to the 5.7 milestone Oct 7, 2021
@kcondon kcondon self-assigned this Oct 7, 2021
@kcondon
Copy link
Contributor

kcondon commented Oct 7, 2021

tested endpoints, work as expected, slight doc issue with curved " chars in examples not working when cut and paste.

@djbrooke
Copy link
Contributor

djbrooke commented Oct 7, 2021

My bad on the docs @kcondon @sekmiller - should be OK now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation: Document Harvesting/OAI set endpoints
6 participants