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 /oauth endpoints #55

Closed
wants to merge 1 commit into from

Conversation

Anupya
Copy link
Contributor

@Anupya Anupya commented Oct 22, 2023

Description

Implement the following endpoints:

  • get_code = /oauth
  • get_token = POST /api/token
  • revoke_token = DEL /api/token

Fix some typos.

Pyright issue

Pyright is complaining because requests.Session.cookies.get and requests.Session.cookies.set have unknown Type. Moreover, I don't think using requests.Session.cookies.get/requests.Session.cookies.set is the right approach anyway due to the following issues listed here in a requests Github issue: psf/requests#3669.

I am looking for help/guidance on how to store code_verifier in session storage 🙏 Since self._r is a requests.Session object already, I went down the requests.Session.cookies rabbit hole till I hit a dead end. I don't know enough about storing cookies in Python to confidently put forth another solution so I am opening up my PR for ideas.

Why store it in the first place?

So I can pass code_verifier as a query param in get_token call.

@Anupya Anupya marked this pull request as ready for review October 22, 2023 08:53
@benediktwerner
Copy link
Member

benediktwerner commented Oct 22, 2023

Did you test this? At first glance, I don't think this should work? Afaik the /oauth URL needs to be opened in a browser to let the user log in on Lichess (or access an already logged-in session of the browser).

Also not sure whether it's a good idea to store stuff in session storage like this.

Tbh, in general, it's not super clear to me how an ideal Oauth API would look like but it probably shouldn't match the Lichess endpoints exactly.

I think chariot (the Java Lichess API client) has an option to do a complete oauth flow which will start a local server, open the Oauth URL (could be done using Python's webbrowser module) with the local server as the redirection target, and then does the final call to get the token.

Besides that, it could also make sense to just have a method to generate an URL and code verifier which users of the lib can then give to the user/store in any way they like and then another method to get a token after the user finished the log in. For example, if berserk is used server-side, it couldn't open a browser for the user and the server instead has to pass the URL generated by berserk back to the user.

But it also seems fine to just skip these endpoints until somebody has a concrete use case for them that could better inform how a good API would look. I think in most cases, it makes more sense to do this stuff in JS. Although something like a Discord bot could be a potential use case. But that still would require some form of server as the redirect target which is outside the scope of berserk.

@Anupya
Copy link
Contributor Author

Anupya commented Oct 24, 2023

Ah, thank you for the explanation. I did not test this locally because I wanted to figure out the storage bit first. Agreed that we do not need to build this at the moment 🫡

@Anupya Anupya closed this Oct 24, 2023
@icp1994
Copy link
Contributor

icp1994 commented Oct 24, 2023

IMO keep /oauth endpoint out of berserk

edit: didn't see this was closed, thought this was a draft PR

@Anupya
Copy link
Contributor Author

Anupya commented Oct 25, 2023

We should remove /oauth from the Issue description of #6.

@kraktus kraktus mentioned this pull request Oct 28, 2023
78 tasks
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

Successfully merging this pull request may close these issues.

3 participants