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

Added support for oauth 2 login and registration #2659

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rrgeorge
Copy link

I've added support for OAuth 2 login and registration to my instance, so I can login with my mastodon credentials.
Enabling this disables password login.
If an account does not exist in bookwyrm, it will automatically register a new account with the same local part, and requires the user to enter their email address. This is not provided from Mastodon, and I am unable to set it to NULL via Django, so this step is unfortunately unavoidable.
I'm not sure if this is the best way to go about things, but it does work. I am submitting this as a draft pr to see if anyone has any other input or wants to test it out for themselves.

OAUTH_ACTIVE=false
#OAUTH_NAME="OAuth Provider" # Displayed on Login Button as "Login with OAUTH_NAME"
#OAUTH_CLIENT_ID=""
#OAUTH_CLIENT_SECRET=""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would only work with a single external source though, would it?
Since a user could use any Mastodon instance bookwyrm would need to dynamically register an OAuth client with every Mastodon server once it was used the first time + store that securely in the database.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.
The idea is that the bookwyrm server authenticates against a specific mastodon (or other oauth 2.0 provider).
For mastodon specifically, you’d need to use the app registration api to generate these values for your bookwyrm server: https://docs.joinmastodon.org/methods/apps/#create

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I mean, exactly.
I think without the dynamic possibility of authentication users would get frustrated easily, since they wouldn't understand why only a very specific server could be used for authentication—so you'd potentially lock out people who'd like to use their Mastodon login. I'd argue that's against the spirit of the Fediverse.
(But to be clear: I think it's great you want to add external OAuth login!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s just like any other closed registration server. The idea is to unify login for the users of one service that wants to provide multiple fediverse platforms for their users. The way I am using it is to provide a bookwyrm server for my mastodon users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your use case but I am worried that this is such a specific use-case vs. how OAuth login (especially within the Fediverse) works, where one would generally not expect to be restricted to a single server—since at least that is how I’d interpret the idea of the Fediverse.
So maybe it’s possible to allow any Mastodon server with dynamic registration but add a server config to restrict it to a default Mastodon server as well to cover your use case?

@@ -55,7 +61,7 @@ def post(self, request):

localname = form.data["localname"].strip()
email = form.data["email"]
password = form.data["password"]
password = None if 'oauth-newuser' in request.session else form.data["password"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't think that password authentication should be disabled if OAuth is enabled, because that prevents users such as myself using my password manager to automatically authenticate. If it were the case, I would never enable OAuth, whereas I without doubt would otherwise.

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