Skip to content

Commit

Permalink
oauth: fix workflow when user requires confirmation and oauth provide…
Browse files Browse the repository at this point in the history
…r does not autoconfirm
  • Loading branch information
zzacharo committed Aug 30, 2023
1 parent 08a7f57 commit c316717
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 32 deletions.
12 changes: 12 additions & 0 deletions invenio_oauthclient/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,15 @@ class OAuthClientMustRedirectLogin(Exception):

class OAuthRemoteNotFound(Exception):
"""Define exception for remote app not found."""


class OAuthClientUserRequiresConfirmation(Exception):
"""User requires to confirm their email."""

def __init__(self, user, *args, **kwargs):
"""Initialize exception.
:param user: User object whose email requires confirmation.
"""
self.user = user
super().__init__(*args, **kwargs)
47 changes: 37 additions & 10 deletions invenio_oauthclient/handlers/authorized.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@

"""Authorize handlers."""

from flask import current_app, g, session
from flask import current_app, session
from flask_login import current_user
from flask_security.confirmable import requires_confirmation
from flask_security.utils import get_message
from invenio_db import db

from ..errors import (
Expand All @@ -19,6 +21,7 @@
OAuthClientTokenNotSet,
OAuthClientUnAuthorized,
OAuthClientUserNotRegistered,
OAuthClientUserRequiresConfirmation,
)
from ..oauth import oauth_authenticate, oauth_get_user, oauth_register
from ..proxies import current_oauthclient
Expand Down Expand Up @@ -108,14 +111,29 @@ def authorized_handler(resp, remote, *args, **kwargs):
# to fill in the registration form with the missing information
raise OAuthClientMustRedirectSignup()

# check if user requires confirmation
# that happens when user was previously logged in but email was not yet
# confirmed
if requires_confirmation(user):
raise OAuthClientUserRequiresConfirmation(user=user)

if not oauth_authenticate(
remote.consumer_key, user, require_existing_link=False
remote.consumer_key,
user,
require_existing_link=False,
require_user_confirmation=False,
):
raise OAuthClientUnAuthorized()

# Store token in the database instead of only the session
token = response_token_setter(remote, resp)

return _complete_authorize(resp, remote, handlers, token)
_complete_authorize(resp, remote, handlers, token)

# Return the URL where to go next
next_url = get_session_next_url(remote.name)
if next_url:
return next_url


def _register_user(resp, remote, account_info, form):
Expand Down Expand Up @@ -163,11 +181,6 @@ def _complete_authorize(resp, remote, handlers, token):
else:
db.session.commit()

# Return the URL where to go next
next_url = get_session_next_url(remote.name)
if next_url:
return next_url


def extra_signup_handler(remote, form, *args, **kwargs):
"""Handle extra signup information.
Expand Down Expand Up @@ -202,11 +215,25 @@ def extra_signup_handler(remote, form, *args, **kwargs):
if token is None:
raise OAuthClientTokenNotSet()

# check if user requires confirmation
# that happens when user was previously logged in but email was not yet
# confirmed
if requires_confirmation(user):
_complete_authorize(response, remote, handlers, token)
raise OAuthClientUserRequiresConfirmation(user=user)

# Authenticate user, without requiring the existence of the RemoteAccount,
# which is created later in the setup handler.
if not oauth_authenticate(
remote.consumer_key, user, require_existing_link=False
remote.consumer_key,
user,
require_existing_link=False,
require_user_confirmation=False,
):
raise OAuthClientUnAuthorized()

return _complete_authorize(response, remote, handlers, token)
_complete_authorize(response, remote, handlers, token)
# Return the URL where to go next
next_url = get_session_next_url(remote.name)
if next_url:
return next_url
3 changes: 2 additions & 1 deletion invenio_oauthclient/handlers/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
OAuthClientTokenNotSet,
OAuthClientUnAuthorized,
OAuthClientUserNotRegistered,
OAuthClientUserRequiresConfirmation,
OAuthError,
OAuthRejectedRequestError,
)
Expand Down Expand Up @@ -100,7 +101,7 @@ def _oauth_error_handler(remote, f, *args, **kwargs):
remote_app_config["error_redirect_url"],
payload=dict(message=msg, code=400),
)
except OAuthClientUnAuthorized:
except (OAuthClientUnAuthorized, OAuthClientUserRequiresConfirmation):
return response_handler(
remote,
remote_app_config["error_redirect_url"],
Expand Down
28 changes: 21 additions & 7 deletions invenio_oauthclient/handlers/ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
session,
url_for,
)
from flask_login import current_user
from flask_security.utils import do_flash, get_message
from invenio_db import db
from invenio_i18n import gettext as _

Expand All @@ -33,6 +33,7 @@
OAuthClientTokenNotSet,
OAuthClientUnAuthorized,
OAuthClientUserNotRegistered,
OAuthClientUserRequiresConfirmation,
OAuthError,
OAuthRejectedRequestError,
)
Expand Down Expand Up @@ -141,11 +142,20 @@ def authorized_signup_handler(resp, remote, *args, **kwargs):
:param resp: The response.
:returns: Redirect response.
"""
next_url = authorized_handler(resp, remote, *args, **kwargs)
# Redirect to next
if next_url:
return redirect(next_url)
return redirect(url_for("invenio_oauthclient_settings.index"))
try:
next_url = authorized_handler(resp, remote, *args, **kwargs)
# Redirect to next
if next_url:
return redirect(next_url)
return redirect(url_for("invenio_oauthclient_settings.index"))
except OAuthClientUserRequiresConfirmation as exc:
do_flash(
_(
f"A confirmation email has already been sent to {exc.user.email}. Please confirm your email to be able to log in."
),
category="success",
)
return redirect("/")


@oauth_remote_error_handler
Expand Down Expand Up @@ -203,9 +213,13 @@ def signup_handler(remote, *args, **kwargs):
try:
next_url = extra_signup_handler(remote, form, *args, **kwargs)
except OAuthClientUnAuthorized:
# Redirect the user to login page
return redirect(url_for("security.login"))
except OAuthClientUserRequiresConfirmation as exc:
# Redirect the user after registration (which doesn't include the
# activation), waiting for user to confirm his email.
return redirect(url_for("security.login"))
do_flash(*get_message("CONFIRM_REGISTRATION", email=exc.user.email))
return redirect("/")

if next_url:
return redirect(next_url)
Expand Down
27 changes: 16 additions & 11 deletions invenio_oauthclient/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ def oauth_get_user(client_id, account_info=None, access_token=None):
return None


def oauth_authenticate(client_id, user, require_existing_link=False):
def oauth_authenticate(
client_id, user, require_existing_link=False, require_user_confirmation=False
):
"""Authenticate an oauth authorized callback.
:param client_id: The client id.
Expand All @@ -81,17 +83,20 @@ def oauth_authenticate(client_id, user, require_existing_link=False):
(Default: ``False``)
:returns: ``True`` if the user is successfully authenticated.
"""
# this is for backwards compatibility and tests
if require_user_confirmation:
if requires_confirmation(user):
return False

# Authenticate via the access token (access token used to get user_id)
if not requires_confirmation(user):
after_this_request(_commit)
if login_user(user):
if require_existing_link:
account = RemoteAccount.get(user.id, client_id)
if account is None:
logout_user()
return False
return True
return False
after_this_request(_commit)
if login_user(user):
if require_existing_link:
account = RemoteAccount.get(user.id, client_id)
if account is None:
logout_user()
return False
return True


def oauth_register(form, user_info=None, precedence_mask=None, signup_options={}):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,18 @@
{% endif %}
{{ super () }}
{% endblock %}

{%- block recoverable %}
{%- if security.recoverable or security.confirmable %}
<div class="ui basic segment padded">
{%- if security.recoverable %}
<a class="ui inverted header tiny"
href="{{ url_for_security('forgot_password') }}">{{ _('Forgot password?') }}</a>
{%- endif %}
{%- if security.confirmable %}
<a class="ui inverted header tiny"
href="{{url_for('security.send_confirmation')}}" class="text-muted">{{_('Resend confirmation email?')}}</a>
{%- endif %}
</div>
{%- endif %}
{%- endblock %}
2 changes: 1 addition & 1 deletion tests/test_handlers_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def test_unauthorized_signup(remote, app, models_fixture):
user.confirmed_at = None
app.config["OAUTHCLIENT_REMOTE_APPS"][remote.name] = {}
resp = authorized_signup_handler(example_response, remote)
check_redirect_location(resp, lambda x: x.startswith("/login"))
check_redirect_location(resp, "/")


def test_signup_handler(remote, app, models_fixture):
Expand Down
6 changes: 4 additions & 2 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ def test_utilities(app, models_fixture):

# Authenticate
assert not _get_external_id({})
assert not oauth_authenticate("dev", user, require_existing_link=True)
assert not oauth_authenticate(
"dev", user, require_existing_link=True, require_user_confirmation=False
)

_security.confirmable = True
_security.login_without_confirmation = False
user.confirmed_at = None
assert not oauth_authenticate("dev", user)
assert not oauth_authenticate("dev", user, require_user_confirmation=True)

# Tokens
t = RemoteToken.create(user.id, "dev", "mytoken", "mysecret")
Expand Down

0 comments on commit c316717

Please sign in to comment.