discord-net / Discord.Net

An unofficial .Net wrapper for the Discord API (https://discord.com/)
https://discordnet.dev
MIT License
3.28k stars 743 forks source link

Should we warn users when making a request to a prohibited endpoint? #823

Closed foxbot closed 6 years ago

foxbot commented 6 years ago

Discord has previously rolled out anti-spam measures on certain endpoints. When these endpoints are accessed by a userbot, the account will be flagged as spam, and in certain cases, will have its verification level cleared immediately.

More recently, accounts that ping some of these endpoints (notably POST /<private channel>/messages) have been detected as spam, and their accounts have been disabled outright, with support staff refusing to reactivate the account.

As a library, we have a couple of options to help protect our users, and Discord:

We can log a warning to the console if a user accesses one of these endpoints, notifying them that their account may be flagged as spam and unverified. This has the benefit of helping reduce frustration for them when they find their account no longer working, and helps reduce the chances of them opening a support ticket with us, or with Discord.

Additionally, we can add a configuration flag (AllowDangerousRequests e.g.) that is set to false by default. In this case, when the user tries to make a request to an endpoint that would flag their account, the request will never be sent, and an exception will be raised. If the user decides to opt-in to dangerous requests, a warning would be logged, as outlined above.

The only gripes I have with either of these options, is that we are now responsible for keeping a list of flagged endpoints in the library, prompting an update any time Discord adds a new route to the blacklist. If we choose to throw an exception when accessing a dangerous route, we would then have to release a new major version for every route blacklist - which would be less than ideal. We could drop the request and log an error rather than raising an exception - though I'm not sure that changing the behavior of a method to a no-op is allowed under SemVer's patch rules.

Still34 commented 6 years ago

While keeping track of the blacklist and updating with major release is somewhat concerning, I do think it's a good change. If not, perhaps at least an XMLDOCS note would be nice.

FiniteReality commented 6 years ago

Personally I dislike the prospect of maintaining a list of "bad endpoints". Frankly, since userbots and by extension selfbots aren't supported, I don't think we should really be adding explicit support for user accounts at all.

foxbot commented 6 years ago

So after reviewing some options in the Discord API chat, I've come up with the following solution:

Starting in 2.0, TokenType.User will be flagged as Obsolete. We will include a message in the obsolete pointing at a documentation page or a GitHub issue here, stating that user logins are unsupported by Discord and can lead to account unverification, etc etc.

This will also flag user logins as deprecated.

Starting in 3.0, we will no longer support user logins as an element of the main library. Depending on how the 3.0 restructure goes, this issue may be revisited in the user extension.

Auralytical commented 6 years ago

Sounds good to me.

To clarify, in 3.0, we will split the unofficial endpoints off to their own client class in a separate library that in its description explains the risk of its usage. The standard client will not support user logins or expose anything that isn't documented and will always be safe to use.