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

feat: add OAuth authentication #3114

Merged
merged 6 commits into from
Apr 24, 2024

Conversation

SMillerDev
Copy link
Contributor

This should allow API clients to authenticate using OAuth

Relates to #2292

@mouse-reeve
Copy link
Member

I thought this was going to be a way longer diff when I went to look at it. I'm going to have to take a look at the library and get a little more familiar, but thank you so much for starting on this!

@mouse-reeve
Copy link
Member

It looks like this doesn't run yet, so I'm going to mark it as a draft. Feel free to mark it as ready for review once you're ready :)

@mouse-reeve mouse-reeve marked this pull request as draft November 16, 2023 01:32
@SMillerDev
Copy link
Contributor Author

It was just a missing include. Should work now.

@SMillerDev SMillerDev marked this pull request as ready for review November 16, 2023 11:06
bookwyrm/urls.py Outdated Show resolved Hide resolved
@SMillerDev
Copy link
Contributor Author

Should be good for another CI run @mouse-reeve

@rabc
Copy link

rabc commented Feb 19, 2024

Any idea when it can be merged? Then we can move forward with the API implementation :)

@SMillerDev
Copy link
Contributor Author

CI seems okay with it, and I'm still interested in working on this when it's merged. Anyone up for a review ?

@mouse-reeve
Copy link
Member

Ah I apologize for leaving you hanging on this! I would like to get feedback from other folks on @bookwyrm-social/code-review if yall feel like this is the right first step.

@Minnozz
Copy link
Contributor

Minnozz commented Apr 1, 2024

I think it is currently not very clear what the scope of this PR is. Is it a first step in developing a client API? Can end users already use it after this is merged? If so, should we not integrate OAuth settings into the user preferences, or at least add a link to the management view of the OAuth application? Should there be a test that at least checks if OAuth is working?

@SMillerDev
Copy link
Contributor Author

SMillerDev commented Apr 1, 2024

The goal is to add the Oauth library so we can build on that. After this I plan to:

  • add Oauth for users so it can be configured for login (that does nothing)
  • add an API for user information with that login
  • add other APIs

But I'm waiting with the actual configuration because if people don't like this library addition it'll be a lot of reworking

@Minnozz
Copy link
Contributor

Minnozz commented Apr 1, 2024

Ah yes, I understand. In that case, I think the library you chose seems like a good choice for adding OAuth authentication to BookWyrm.

@SMillerDev
Copy link
Contributor Author

So, what can I do to make this mergable?

@SMillerDev
Copy link
Contributor Author

Small prefix to my plans, because of the federated nature I guess I'll need to add something like https://docs.joinmastodon.org/methods/apps/ first so apps can register with the server.

@mouse-reeve
Copy link
Member

Thank you for your patience with this @SMillerDev -- I've been a little unsure on how to best move forward on this incrementally but I think this is the right first step. Much appreciated!

@mouse-reeve mouse-reeve merged commit c4b21ee into bookwyrm-social:main Apr 24, 2024
10 checks passed
@SMillerDev
Copy link
Contributor Author

Thanks @mouse-reeve ! Hoping to work on app registration soon

@SMillerDev
Copy link
Contributor Author

#3361 is a first draft of that.

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.

None yet

4 participants