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

Search API: Support search without an API Token #3900

Closed
michbarsinai opened this issue Jun 14, 2017 · 13 comments
Closed

Search API: Support search without an API Token #3900

michbarsinai opened this issue Jun 14, 2017 · 13 comments

Comments

@michbarsinai
Copy link
Member

Search should be as if used by :guest.

@michbarsinai michbarsinai self-assigned this Jun 14, 2017
@michbarsinai
Copy link
Member Author

related: #1299

@michbarsinai
Copy link
Member Author

Also related: #1838

@michbarsinai
Copy link
Member Author

Implemented, ready for QA.

@pdurbin
Copy link
Member

pdurbin commented Jun 15, 2017

Implemented, ready for QA.

@michbarsinai what about code review?

@pdurbin
Copy link
Member

pdurbin commented Jun 19, 2017

To me this goes all the way back to #1809 (comment) and reverses a policy decision made by @mercecrosas for the out-of-the-box behavior of Dataverse.

Also, it seems like all the logic surrounding :SearchApiNonPublicAllowed has been circumvented. In #1299 is was decided that we're only supporting public search.

In short, there are a lot of policy changes in pull request #3908, a lot of implications to unpack. I am very supportive of work toward "Consider options for opening APIs without tokens" in #1838 but I feel that we need to make sure whatever changes happen are what we want as a project.

@pdurbin
Copy link
Member

pdurbin commented Jun 19, 2017

In standup I suggested that @djbrooke could ping @mercecrosas about the policy considerations. Also, I forgot to mention something I said to @michbarsinai which is that if we open up a lot of APIs (#1838) we should consider rate limiting the API (#1339) like GitHub and others. Otherwise, you're down at a low level blocking individual abusive IP addresses with iptables or whatever.

@djbrooke
Copy link
Contributor

Talked with @mercecrosas and @scolapasta about this yesterday. We should go ahead with this change. If so many people are using our Search API that we have load issues or need to IP ban a few bad apples, we'll revisit or determine some other strategy to handle this.

@pdurbin
Copy link
Member

pdurbin commented Jun 20, 2017

@djbrooke that's fantastic news! In IQSS/dataverse-android#1 I've been talking about the need to add authentication to my Dataverse Android app but once the Search API is open, the Android app should "just work" again!

@pdurbin
Copy link
Member

pdurbin commented Jun 20, 2017

I'm planning on doing some code review of pull request #3908.

@pdurbin
Copy link
Member

pdurbin commented Jun 21, 2017

I closed pull request #3908 in favor of pull request #3940 that I just made. Mine preserves the documented behavior of the Search API that only public data is exposed, which we don't intend to work on until #1299.

@djbrooke djbrooke assigned sekmiller and unassigned pdurbin Jun 21, 2017
@pdurbin
Copy link
Member

pdurbin commented Jun 21, 2017

I gave @sekmiller a brain dump of what's changing and what's staying the same.

Staying the same: the Search API still only returns published data. #1299 is about also returning unpublished data.

Changing: Dataverse 4.6.2 and lower will bark at you if you try to use the Search API without a token saying "Please provide a key query parameter (?key=XXX) or via the HTTP header X-Dataverse-key". As of pull request #3940 the new out of the box behavior is that you don't have to supply a token. If you want the old 4.6.2 behavior, you can set :SearchApiRequiresToken to true.

@pdurbin
Copy link
Member

pdurbin commented Jun 26, 2017

Changing: Dataverse 4.6.2 and lower will bark at you if you try to use the Search API without a token

@kcondon as we discussed, since my pull request missed the 4.7 train, I updated the docs in 0308476 to explain that Dataverse 4.7 and lower will bark at you about not supplying a token.

@pdurbin
Copy link
Member

pdurbin commented Jun 26, 2017

@kcondon found that :SearchApiRequiresToken=true wasn't restoring the 4.7 behavior. Fixed in 6fcd1ce.

pdurbin added a commit that referenced this issue Jun 26, 2017
@kcondon kcondon closed this as completed Jun 26, 2017
@djbrooke djbrooke added this to the 4.7.1 - Dashboard milestone Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants