bancho.py icon indicating copy to clipboard operation
bancho.py copied to clipboard

feature: Implement authentication for API v2 based on OAuth2 potocol.

Open alowave223 opened this issue 2 years ago • 5 comments

Describe your changes

Created authorization for third-party clients based on the OAuth2 protocol standard.

Related Issues / Projects

https://github.com/orgs/osuAkatsuki/projects/2

Checklist

  • [x] The changes pass pre-commit checks (make lint)
  • [x] The changes follow coding style

alowave223 avatar Jan 15 '23 22:01 alowave223

hey, there are quite a few typing errors still, i suspect your vscode is using type_checking_mode: off in your preferences

you can open preferences with ctrl+, and set type checking mode to basic to have vs code redline any of these errors

image

e.g. image

image

cmyui avatar Apr 08 '23 23:04 cmyui

there are also quite a few cases like this state: str = Query(default=None), where the variable is typed as T but at runtime can actually be of type Optional[T] due to the default case

cmyui avatar Apr 09 '23 00:04 cmyui

need to remember to bump version @ release

cmyui avatar Jun 27 '23 19:06 cmyui

started work, a couple of things stand out:

  1. since this is meant to be an api for custom web apps (other than the osu! client), i think we should use jwt for it's symmetric key verification mechanism to avoid storing raw access tokens in our application. we can include verifiable claims in the token such as a token_id, which can be stored server-side to enable token revocation by server staff without the storage of a signed access token.
  2. i'm not certain whether it's best to persist access tokens vs. refresh tokens vs. both -- i'll have to look a bit deeper into the oauth 2.0 standard to see if there's a reason to prefer one over the other.
  3. there hasn't yet been much thought yet put towards the mechanisms which should be made available to server admins w.r.t. access controls -- i think two main ones that come to mind are:
    • ability for server staff to revoke all sessions for a given user
    • ability server staff to temporarily disable a user's account
  4. there isn't much client-identifying information being collecting alongside the authorization grants -- i think it would be wise to store simple identification information such as the client ip address, and user agent along with grants or tokens.
  5. there are some implementation-specifics that need adjustments -- shouldn't be very major; the previously mentioned issues are more important/fundamental than these implementation specifics.

overall i think i'll continue to work on top of this pr, as it's quite decent at matching the spec so far -- nothing particularly wrong.

cmyui avatar Feb 13 '24 05:02 cmyui

  1. i'm not certain whether it's best to persist access tokens vs. refresh tokens vs. both -- i'll have to look a bit deeper into the oauth 2.0 standard to see if there's a reason to prefer one over the other.

It may also be worth storing other things, such as authorization grants and failed authorization attempts.

cmyui avatar Feb 13 '24 05:02 cmyui