Skip to content

Commit 6af2852

Browse files
dpgasparashb
dpgaspar
and
ashb
authored
fix: improve next URL on OAuth (#1668)
* fix: improve next URL on OAuth * add tests and extra dependency for OAuth * lint * fix test * add test for unknown provider * lint * Update flask_appbuilder/security/views.py Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
1 parent 0bc5f1e commit 6af2852

5 files changed

Lines changed: 166 additions & 58 deletions

File tree

‎flask_appbuilder/security/views.py‎

Lines changed: 30 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
11
import datetime
22
import logging
33
import re
4-
5-
from flask import abort, current_app, flash, g, redirect, request, session, url_for
4+
from typing import Optional
5+
from urllib.parse import urlparse
6+
7+
from flask import (
8+
abort,
9+
current_app,
10+
flash,
11+
g,
12+
redirect,
13+
request,
14+
Response,
15+
session,
16+
url_for,
17+
)
618
from flask_babel import lazy_gettext
719
from flask_login import login_user, logout_user
820
import jwt
@@ -537,53 +549,6 @@ def login(self):
537549
self.login_template, title=self.title, form=form, appbuilder=self.appbuilder
538550
)
539551

540-
"""
541-
For Future Use, API Auth, must check howto keep REST stateless
542-
"""
543-
544-
"""
545-
@expose_api(name='auth',url='/api/auth')
546-
def auth(self):
547-
if g.user is not None and g.user.is_authenticated:
548-
http_return_code = 401
549-
response = make_response(
550-
jsonify(
551-
{
552-
'message': 'Login Failed already authenticated',
553-
'severity': 'critical'
554-
}
555-
),
556-
http_return_code
557-
)
558-
username = str(request.args.get('username'))
559-
password = str(request.args.get('password'))
560-
user = self.appbuilder.sm.auth_user_ldap(username, password)
561-
if not user:
562-
http_return_code = 401
563-
response = make_response(
564-
jsonify(
565-
{
566-
'message': 'Login Failed',
567-
'severity': 'critical'
568-
}
569-
),
570-
http_return_code
571-
)
572-
else:
573-
login_user(user, remember=False)
574-
http_return_code = 201
575-
response = make_response(
576-
jsonify(
577-
{
578-
'message': 'Login Success',
579-
'severity': 'info'
580-
}
581-
),
582-
http_return_code
583-
)
584-
return response
585-
"""
586-
587552

588553
class AuthOIDView(AuthView):
589554
login_template = "appbuilder/general/security/login_oid.html"
@@ -641,7 +606,9 @@ class AuthOAuthView(AuthView):
641606
@expose("/login/")
642607
@expose("/login/<provider>")
643608
@expose("/login/<provider>/<register>")
644-
def login(self, provider=None, register=None):
609+
def login(
610+
self, provider: Optional[str] = None, register: Optional[str] = None
611+
) -> Response:
645612
log.debug("Provider: {0}".format(provider))
646613
if g.user is not None and g.user.is_authenticated:
647614
log.debug("Already authenticated {0}".format(g.user))
@@ -690,8 +657,12 @@ def login(self, provider=None, register=None):
690657
return redirect(self.appbuilder.get_url_for_index)
691658

692659
@expose("/oauth-authorized/<provider>")
693-
def oauth_authorized(self, provider):
660+
def oauth_authorized(self, provider: str) -> Response:
694661
log.debug("Authorized init")
662+
if provider not in self.appbuilder.sm.oauth_remotes:
663+
flash(u"Provider not supported.", "warning")
664+
log.warning("OAuth authorized got an unknown provider %s", provider)
665+
return redirect(self.appbuilder.get_url_for_login)
695666
resp = self.appbuilder.sm.oauth_remotes[provider].authorize_access_token()
696667
if resp is None:
697668
flash(u"You denied the request to sign in.", "warning")
@@ -735,11 +706,14 @@ def oauth_authorized(self, provider):
735706
except jwt.InvalidTokenError:
736707
raise Exception("State signature is not valid!")
737708

738-
try:
739-
next_url = state["next"][0] or self.appbuilder.get_url_for_index
740-
except (KeyError, IndexError):
741-
next_url = self.appbuilder.get_url_for_index
742-
709+
next_url = self.appbuilder.get_url_for_index
710+
# Check if there is a next url on state
711+
if "next" in state and len(state["next"]) > 0:
712+
parsed_uri = urlparse(state["next"][0])
713+
if parsed_uri.netloc != request.host:
714+
log.warning("Got an invalid next URL: %s", parsed_uri.netloc)
715+
else:
716+
next_url = state["next"][0]
743717
return redirect(next_url)
744718

745719

‎flask_appbuilder/tests/config_oauth.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import os
2+
3+
from flask_appbuilder.security.manager import AUTH_OAUTH
4+
5+
basedir = os.path.abspath(os.path.dirname(__file__))
6+
7+
SQLALCHEMY_DATABASE_URI = os.environ.get(
8+
"SQLALCHEMY_DATABASE_URI"
9+
) or "sqlite:///" + os.path.join(basedir, "app.db")
10+
11+
SECRET_KEY = "thisismyscretkey"
12+
13+
AUTH_TYPE = AUTH_OAUTH
14+
15+
OAUTH_PROVIDERS = [
16+
{
17+
"name": "google",
18+
"icon": "fa-google",
19+
"token_key": "access_token",
20+
"remote_app": {
21+
"client_id": "CLIENT_ID",
22+
"client_secret": "CLIENT_SECRET",
23+
"api_base_url": "https://www.googleapis.com/oauth2/v2/",
24+
"client_kwargs": {"scope": "email profile"},
25+
"request_token_url": None,
26+
"access_token_url": "https://accounts.google.com/o/oauth2/token",
27+
"authorize_url": "https://accounts.google.com/o/oauth2/auth",
28+
},
29+
}
30+
]
31+
32+
# Will allow user self registration
33+
AUTH_USER_REGISTRATION = True
34+
35+
# The default user self registration role for all users
36+
AUTH_USER_REGISTRATION_ROLE = "Admin"

‎flask_appbuilder/tests/test_mvc_oauth.py‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
from flask_appbuilder import SQLA
2+
from flask_appbuilder.tests.base import FABTestCase
3+
import jwt
4+
5+
6+
class UserInfoReponseMock:
7+
def json(self):
8+
return {
9+
"id": "1",
10+
"given_name": "first-name",
11+
"family_name": "last-name",
12+
"email": "user1@fab.org",
13+
}
14+
15+
16+
class OAuthRemoteMock:
17+
def authorize_access_token(self):
18+
return {"access_token": "some-key"}
19+
20+
def get(self, item):
21+
if item == "userinfo":
22+
return UserInfoReponseMock()
23+
24+
25+
class APICSRFTestCase(FABTestCase):
26+
def setUp(self):
27+
from flask import Flask
28+
from flask_wtf import CSRFProtect
29+
from flask_appbuilder import AppBuilder
30+
31+
self.app = Flask(__name__)
32+
self.app.config.from_object("flask_appbuilder.tests.config_oauth")
33+
self.app.config["WTF_CSRF_ENABLED"] = True
34+
35+
self.csrf = CSRFProtect(self.app)
36+
self.db = SQLA(self.app)
37+
self.appbuilder = AppBuilder(self.app, self.db.session)
38+
39+
def test_oauth_login(self):
40+
"""
41+
OAuth: Test login
42+
"""
43+
client = self.app.test_client()
44+
45+
self.appbuilder.sm.oauth_remotes = {"google": OAuthRemoteMock()}
46+
47+
raw_state = {}
48+
state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
49+
50+
response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
51+
self.assertEqual(response.location, "http://localhost/")
52+
53+
def test_oauth_login_unknown_provider(self):
54+
"""
55+
OAuth: Test login with unknown provider
56+
"""
57+
client = self.app.test_client()
58+
59+
self.appbuilder.sm.oauth_remotes = {"google": OAuthRemoteMock()}
60+
61+
raw_state = {}
62+
state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
63+
64+
response = client.get(
65+
f"/oauth-authorized/unknown_provider?state={state.decode('utf-8')}"
66+
)
67+
self.assertEqual(response.location, "http://localhost/login/")
68+
69+
def test_oauth_login_next(self):
70+
"""
71+
OAuth: Test login next
72+
"""
73+
client = self.app.test_client()
74+
75+
self.appbuilder.sm.oauth_remotes = {"google": OAuthRemoteMock()}
76+
77+
raw_state = {"next": ["http://localhost/users/list/"]}
78+
state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
79+
80+
response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
81+
self.assertEqual(response.location, "http://localhost/users/list/")
82+
83+
def test_oauth_login_next_check(self):
84+
"""
85+
OAuth: Test login next check
86+
"""
87+
client = self.app.test_client()
88+
89+
self.appbuilder.sm.oauth_remotes = {"google": OAuthRemoteMock()}
90+
91+
raw_state = {"next": ["http://www.google.com"]}
92+
state = jwt.encode(raw_state, self.app.config["SECRET_KEY"], algorithm="HS256")
93+
94+
response = client.get(f"/oauth-authorized/google?state={state.decode('utf-8')}")
95+
self.assertEqual(response.location, "http://localhost/")

‎requirements-extra.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ mysqlclient==2.0.1
77
psycopg2-binary==2.8.6
88
pyodbc==4.0.30
99
requests==2.25.0
10-
Authlib==0.15.2
10+
Authlib==0.15.4
1111
python-ldap==3.3.1

‎setup.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ def desc():
6767
"PyJWT>=1.7.1, <2.0.0",
6868
"sqlalchemy-utils>=0.32.21, <1",
6969
],
70-
extras_require={"jmespath": ["jmespath>=0.9.5"]},
70+
extras_require={
71+
"jmespath": ["jmespath>=0.9.5"],
72+
"oauth": ["Authlib>=0.14, <1.0.0"],
73+
},
7174
tests_require=["nose>=1.0", "mockldap>=0.3.0"],
7275
classifiers=[
7376
"Development Status :: 5 - Production/Stable",

0 commit comments

Comments
 (0)