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

implement api keys #953

Closed
r888888888 opened this issue Mar 18, 2013 · 14 comments
Closed

implement api keys #953

r888888888 opened this issue Mar 18, 2013 · 14 comments

Comments

@r888888888
Copy link
Collaborator

My proposal:

Each user can have one API key. They must provide this API key to make any XML or JSON call. An API key is basically just a random string and is not intended to be secure.

All API calls must include this API key and some way of authenticating a user (session, password hash, oAuth type authentication service). A given user-API key pair can make a maximum of X calls a day. If no user is provided the call goes against the API key owner's count.

A paid service will be offered to increase X, effectively reducing the need for a client to authenticate.

The desired goal is if someone wants to implement a Greasemonkey script for example, the script can use the end user's session to avoid hitting the API limit. But for API users who don't need to authenticate (say they're sucking the posts XML feed), they will be throttled so as to not abuse the system.

@Type-kun
Copy link
Collaborator

Won't this break all unmaintained scripts?
Unless I'm mistaken, you can throttle frequent calls to XML API using nginx or other tools, allowing one xml call per second per IP, for example.

@dobacco
Copy link
Contributor

dobacco commented Mar 18, 2013

This is a good idea, since there's a lot of abusive usage. For example http://bisimplex.com/?app=animeboxes parses api and then loads the images. It also overlays its own ads in the free version.

(You can block it by looking for idanbooru as client id)

That is just one of the few examples.

@RaisingK
Copy link
Collaborator

A given user-API key pair can make a maximum of X calls a day. If no user is provided the call goes against the API key owner's count.

So X for a user/key pair... and that same X for just the key? I'm also confused by how the "script" is avoiding the limit in spite of the X limit, or is that a different limit? What kind of X are we talking about?

I have a Danbooru script using the API, a Pixiv script using the API, and a Java program using the API...

@Type-kun
Copy link
Collaborator

I see you're implementing it after all. Then, how much X would be? Pretty much any simple tweak userscript calls to API at least once per page load. I think, 2 or 3 hours of danbooru browsing may easily result in user opening about 1000 pages - it's only 1 page per 10 seconds, people can browse even faster. Would X exceed 1000?

And then again, what would stop abusive API callers from bulk-registering 100 users and getting all the dumps they want? I just don't see the point.

@Type-kun
Copy link
Collaborator

Umm, and also, would you please use something other than password hash for API key? I still don't want to expose even partial hash to everyone in case I write a script.

UPD: wait, that's more than unpleasant. If user's hashed username or your secret token for signed cookies leaks somehow, it'll leave API developers completely exposed.

@r888888888
Copy link
Collaborator Author

The implementation is different from my initial proposal.

A user script would not use an api key. It would be driven off the user's ip address, which (for basic accounts) is limited to 3,000 requests per hour. The user does need to be logged in for the user script to work.

The api key should mainly be used for private scripts that are not public.

I've considered schemes like encrypting a random api key using AES with your password as your secret, or vice versa, but I'm not sure how encryption savvy people are. It would certainly be hard to do something like that in Javascript.

I'm not sure why you consider Basic Auth to be more (or as) insecure than passing in the hash/key through the request parameters. At least basic auth isn't exposed in log files, but it's just as vulnerable to MITM or replay attacks.

Regarding HTTPS, I was planning on adding support for it. I'll just have to do some research on what the best way is.

@r888888888
Copy link
Collaborator Author

Relevant commits:

7470d18
2ac22d0

@Type-kun
Copy link
Collaborator

I'm not sure why you consider Basic Auth to be more (or as) insecure than passing in the hash/key through the request parameters. At least basic auth isn't exposed in log files, but it's just as vulnerable to MITM or replay attacks.

I probably misunderstood how keys would work. I thought you must use your API key when developing any userscript, i.e. expose your own API key in script. If API key never gets exposed, it's probably fine.

@ghostrigger
Copy link
Contributor

^ the reply was probably meant for me because i requested to disable basic auth because it's insecure (yes that's the word i used if i remember correctly) and asked if https is now possible. but i deleted my reply soon after and moved it instead to #1018 . albert was probably reading from mail notifications. sorry for the confusion.

@Type-kun
Copy link
Collaborator

Well, plaintext auth is certainly vulnerable if you connect from behind some proxy that you don't control and that analyzes your data package contents, but pretty much any non-encrypted connection is "vulnerable" to it, and same listener proxies usually just don't allow https. It would be nice to have as an option, but, as I said in #1018, please don't make it mandatory.

Also, I have a question. Is limit currently implemented as "access per IP" or "access per IP per user"? I wouldn't really want to get blocked because of some asshole who has the same IP.

@r888888888
Copy link
Collaborator Author

Currently it's based per-ip. I've thought of potential issues from NATing and if it's an issue I can add a per-use constraint.

@Type-kun
Copy link
Collaborator

Well, that of course can wait until actual problems happen, but API error should be verbose about "hourly limit per IP" reached, else it might get hard to track.

@r888888888
Copy link
Collaborator Author

fixed

@r888888888
Copy link
Collaborator Author

The API error for that case returns a 421 as documented in the api docs.

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

No branches or pull requests

5 participants