-
-
Notifications
You must be signed in to change notification settings - Fork 263
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
""" responds to various requests to oauth """ | ||
from django.contrib.auth import login | ||
from django.core.exceptions import ObjectDoesNotExist | ||
from django.dispatch import receiver | ||
from django.urls import reverse | ||
from django.shortcuts import render, redirect | ||
from django.template.response import TemplateResponse | ||
from authlib.integrations.django_client import OAuth, OAuthError | ||
|
||
from bookwyrm import models | ||
from bookwyrm.settings import DOMAIN | ||
|
||
oauth = OAuth() | ||
oauth.register('oauth') | ||
oauth = oauth.oauth | ||
|
||
def auth(request): | ||
try: | ||
token = oauth.authorize_access_token(request) | ||
except OAuthError: | ||
data = {} | ||
return TemplateResponse(request, "landing/login.html", data) | ||
acct = oauth.get("https://raphus.social/api/v1/accounts/verify_credentials",token=token) | ||
if (acct.status_code==200): | ||
localname = dict(acct.json())['acct'] | ||
username = "{}@{}".format(localname,DOMAIN) | ||
try: | ||
user = models.User.objects.get(username=username) | ||
except ObjectDoesNotExist: | ||
request.session['oauth-newuser'] = localname | ||
request.session["oauth-newuser-pfp"] = dict(acct.json())['avatar'] | ||
return redirect('oauth-register') | ||
login(request,user) | ||
return redirect('/') | ||
|
||
def request_login(request): | ||
redirect_uri = request.build_absolute_uri(reverse('oauth')) | ||
return oauth.authorize_redirect(request, redirect_uri, force_login=True ) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
{% extends 'layout.html' %} | ||
{% load i18n %} | ||
|
||
{% block title %}{% trans "Create an Account" %}{% endblock %} | ||
|
||
{% block content %} | ||
|
||
<h1 class="title">{% trans "Create an Account" %}</h1> | ||
<div class="columns"> | ||
<div class="column"> | ||
<div class="block"> | ||
{% if valid %} | ||
<div> | ||
<form name="register" method="post" action="/register"> | ||
{% csrf_token %} | ||
<div class="field"> | ||
<label class="label" for="id_localname_register">{% trans "Username:" %}</label> | ||
<div class="control"> | ||
<input | ||
type="hidden" | ||
name="localname" | ||
value="{{ username }}" | ||
><em>{{ username }}</em> | ||
<div id="desc_localname_register_panel"> | ||
<p class="help"> | ||
{% trans "Your username cannot be changed." %} | ||
</p> | ||
</div> | ||
</div> | ||
</div> | ||
<div class="field"> | ||
<label class="label" for="id_email_register">{% trans "Email address:" %}</label> | ||
<div class="control"> | ||
<input | ||
type="email" | ||
name="email" | ||
maxlength="254" | ||
class="input" | ||
id="id_email_register" | ||
value="{% if register_form.email.value %}{{ register_form.email.value }}{% endif %}" | ||
required | ||
aria-describedby="desc_email_register" | ||
> | ||
|
||
{% include 'snippets/form_errors.html' with errors_list=register_form.email.errors id="desc_email_register" %} | ||
</div> | ||
</div> | ||
|
||
<input type="hidden" name="preferred_timezone" /> | ||
|
||
<div class="field"> | ||
<div class="control"> | ||
<button class="button is-primary" type="submit"> | ||
{% trans "Sign Up" %} | ||
</button> | ||
</div> | ||
</div> | ||
</form> | ||
</div> | ||
{% else %} | ||
<div class="content"> | ||
<h1 class="title">{% trans "Permission Denied" %}</h1> | ||
<p>{% trans "Sorry!" %}</p> | ||
</div> | ||
{% endif %} | ||
</div> | ||
</div> | ||
<div class="column"> | ||
<div class="box"> | ||
{% include 'snippets/about.html' %} | ||
</div> | ||
</div> | ||
</div> | ||
|
||
{% endblock %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,3 +164,4 @@ | |
summary_add_key, | ||
summary_revoke_key, | ||
) | ||
from .oauth import OAuthRegister |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ def post(self, request): | |
if settings.install_mode: | ||
raise PermissionDenied() | ||
|
||
if not settings.allow_registration: | ||
if not settings.allow_registration and 'oauth-newuser' not in request.session: | ||
invite_code = request.POST.get("invite_code") | ||
|
||
if not invite_code: | ||
|
@@ -40,8 +40,14 @@ def post(self, request): | |
raise PermissionDenied() | ||
else: | ||
invite = None | ||
|
||
form = forms.RegisterForm(request.POST) | ||
|
||
if 'oauth-newuser' in request.session: | ||
newuser = request.POST.get("localname") | ||
if newuser != request.session["oauth-newuser"]: | ||
raise PremissionDenied() | ||
form = forms.OAuthRegisterForm(request.POST) | ||
else: | ||
form = forms.RegisterForm(request.POST) | ||
if not form.is_valid(): | ||
data = { | ||
"login_form": forms.LoginForm(), | ||
|
@@ -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"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
try: | ||
preferred_timezone = pytz.timezone(form.data.get("preferred_timezone")) | ||
except pytz.exceptions.UnknownTimeZoneError: | ||
|
@@ -86,7 +92,7 @@ def post(self, request): | |
if settings.require_confirm_email: | ||
emailing.email_confirmation_email(user) | ||
return redirect("confirm-email") | ||
|
||
login(request, user) | ||
return redirect("get-started-profile") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
""" invites when registration is closed """ | ||
from functools import reduce | ||
import operator | ||
from urllib.parse import urlencode | ||
|
||
from django.contrib.auth.decorators import login_required, permission_required | ||
from django.core.paginator import Paginator | ||
from django.db.models import Q | ||
from django.http import HttpResponseBadRequest | ||
from django.shortcuts import get_object_or_404, redirect | ||
from django.template.response import TemplateResponse | ||
from django.urls import reverse | ||
from django.utils.decorators import method_decorator | ||
from django.views import View | ||
from django.views.decorators.http import require_POST | ||
|
||
from bookwyrm import emailing, forms, models | ||
from bookwyrm.settings import PAGE_LENGTH | ||
|
||
|
||
# pylint: disable= no-self-use | ||
class OAuthRegister(View): | ||
"""use an invite to register""" | ||
|
||
def get(self, request): | ||
if request.user.is_authenticated or 'oauth-newuser' not in request.session: | ||
return redirect("/") | ||
data = { | ||
"register_form": forms.RegisterForm(), | ||
"username": request.session['oauth-newuser'], | ||
"valid": True, | ||
} | ||
return TemplateResponse(request, "landing/oauth_register.html", data) | ||
|
||
# post handling is in views.register.Register |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
aiohttp==3.8.3 | ||
oauthlib==3.2.2 | ||
bleach==5.0.1 | ||
celery==5.2.7 | ||
colorthief==0.2.1 | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?