Merge pull request #11711 from cslzchen/feature/fix-auth-views

[ENG-10872] Fix auth views and session changes introduced by #11531 and #11624

Author: cslzchen

framework/auth/utils.py

@@ -11,6 +11,7 @@
 
 from framework import sentry
 from website import settings
+from website.util import web_url_for
 
 logger = logging.getLogger(__name__)
 
@@ -169,3 +170,16 @@ def generate_csl_given_name(given_name, middle_names='', suffix=''):
     if suffix:
         given = f'{given}, {suffix}'
     return given
+
+def get_default_osf_login_url():
+    """Return the default OSF login URL.
+    """
+    next_url = web_url_for(view_name='index', _absolute=True, _angular_route=True)
+    return web_url_for(view_name='auth_login', _absolute=True, next=next_url)
+
+
+def get_default_osf_logout_url():
+    """Return the default OSF logout URL.
+    """
+    next_url = web_url_for(view_name='index', _absolute=True, _angular_route=True)
+    return web_url_for(view_name='auth_logout', _absolute=True, next=next_url)

framework/auth/views.py

@@ -84,7 +84,7 @@ def _reset_password_get(auth, uid=None, token=None, institutional=False):
         raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data=error_data)
 
     # override routes.py login_url to redirect to my-projects
-    service_url = web_url_for('my_projects', _absolute=True)
+    service_url = web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     return {
         'uid': user_obj._id,
@@ -142,7 +142,7 @@ def reset_password_post(uid=None, token=None):
             status.push_status_message('Password reset', kind='success', trust=False)
             # redirect to CAS and authenticate the user automatically with one-time verification key.
             return redirect(cas.get_login_url(
-                web_url_for('user_account', _absolute=True),
+                web_url_for('user_account', _absolute=True, _angular_route=True),
                 username=user_obj.username,
                 verification_key=user_obj.verification_key
             ))
@@ -176,7 +176,7 @@ def forgot_password_get(auth):
 
     #overriding the routes.py sign in url to redirect to the my-projects after login
     context = {}
-    context['login_url'] = web_url_for('my_projects', _absolute=True)
+    context['login_url'] = web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     return context
 
@@ -324,7 +324,7 @@ def login_and_register_handler(auth, login=True, campaign=None, next_url=None, l
             # unlike other campaigns, institution login serves as an alternative for authentication
             if campaign == 'institution':
                 if next_url is None:
-                    next_url = web_url_for('my_projects', _absolute=True)
+                    next_url = web_url_for('dashboard', _absolute=True, _angular_route=True)
                 data['status_code'] = http_status.HTTP_302_FOUND
                 if auth.logged_in:
                     data['next_url'] = next_url
@@ -391,7 +391,7 @@ def login_and_register_handler(auth, login=True, campaign=None, next_url=None, l
         # `/login/` or `/register/` without any parameter
         if auth.logged_in:
             data['status_code'] = http_status.HTTP_302_FOUND
-        data['next_url'] = web_url_for('my_projects', _absolute=True)
+        data['next_url'] = web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     return data
 
@@ -614,7 +614,7 @@ def external_login_confirm_email_get(auth, uid, token):
             return redirect(campaign_url)
         if new:
             status.push_status_message(language.WELCOME_MESSAGE, kind='default', jumbotron=True, trust=True, id='welcome_message')
-        return redirect(web_url_for('my_projects'))
+        return redirect(web_url_for('dashboard', _absolute=True, _angular_route=True))
 
     # token is invalid
     if token not in user.email_verifications:
@@ -712,10 +712,10 @@ def confirm_email_get(token, auth=None, **kwargs):
                 status.push_status_message(language.WELCOME_MESSAGE, kind='default', jumbotron=True, trust=True, id='welcome_message')
             if token in auth.user.email_verifications:
                 status.push_status_message(language.CONFIRM_ALTERNATE_EMAIL_ERROR, kind='danger', trust=True, id='alternate_email_error')
-            return redirect(web_url_for('my_projects'))
+            return redirect(web_url_for('dashboard', _absolute=True, _angular_route=True))
 
         status.push_status_message(language.MERGE_COMPLETE, kind='success', trust=False)
-        return redirect(web_url_for('user_account'))
+        return redirect(web_url_for('user_account', _absolute=True, _angular_route=True))
 
     try:
         user.confirm_email(token, merge=is_merge)
@@ -1053,8 +1053,7 @@ def external_login_email_post():
     fullname = session.get('auth_user_fullname', None) or form.name.data
     service_url = session.get('service_url', None)
 
-    # TODO: @cslzchen use user tags instead of destination
-    destination = 'my_projects'
+    destination = 'dashboard'
     for campaign in campaigns.get_campaigns():
         if campaign != 'institution':
             # Handle different url encoding schemes between `furl` and `urlparse/urllib`.
@@ -1203,7 +1202,7 @@ def validate_next_url(next_url):
     """
 
     # allow redirection to angular locally
-    if settings.LOCAL_ANGULAR_URL in next_url and settings.DEBUG_MODE:
+    if settings.LOCAL_MODE and next_url.startswith(settings.LOCAL_ANGULAR_DOMAIN):
         return True
 
     # disable external domain using `//`: the browser allows `//` as a shortcut for non-protocol specific requests

framework/sessions/__init__.py

@@ -15,7 +15,6 @@
 from osf.utils.fields import ensure_str
 from osf.exceptions import InvalidCookieOrSessionError
 from website import settings
-from website.util import web_url_for
 
 SessionStore = import_module(django_conf_settings.SESSION_ENGINE).SessionStore
 
@@ -165,7 +164,7 @@ def create_session(response, data=None):
 def before_request():
     # TODO: Fix circular import
     from framework.auth.core import get_user
-    from framework.auth import cas
+    from framework.auth import cas, utils
     UserSessionMap = apps.get_model('osf.UserSessionMap')
 
     # Request Type 1: Service ticket validation during CAS login.
@@ -216,7 +215,8 @@ def before_request():
         try:
             user_session = flask_get_session_from_cookie(cookie)
         except InvalidCookieOrSessionError:
-            response = redirect(web_url_for('auth_login'))
+            # If invalid session/cookie happens, perform a full logout to clear both CAS and OSF Sessions
+            response = redirect(utils.get_default_osf_logout_url())
             response.delete_cookie(settings.COOKIE_NAME, domain=settings.OSF_COOKIE_DOMAIN)
             return response
         # Case 1: anonymous session that is used for first time external (e.g. ORCiD) login only

osf/models/institution.py

@@ -216,12 +216,16 @@ def banner_path(self):
     def cas_login_url(self):
         if self.delegation_protocol == IntegrationType.NONE.value:
             return None
-        if 'localhost' in website_settings.DOMAIN:
-            next_param = quote(website_settings.PROTOCOL + website_settings.LOCAL_ANGULAR_URL, safe='')
-        else:
-            next_param = quote(website_settings.DOMAIN, safe='')
-        service_url = quote(f'{website_settings.DOMAIN}login?next={next_param}', safe='')
-        return f'{website_settings.CAS_SERVER_URL}/login?campaign=institution&institutionId={self._id}&service={service_url}'
+        # Note: admin app can't use `web_url_for()` due to out of context
+        next_url_param = quote(website_settings.DOMAIN, safe='')
+        service_url_param = quote(f'{website_settings.DOMAIN}login?next={next_url_param}', safe='')
+        institution_id_param = quote(self._id, safe='')
+        return (
+            f'{website_settings.CAS_SERVER_URL}/login'
+            f'?campaign=institution'
+            f'&institutionId={institution_id_param}'
+            f'&service={service_url_param}'
+        )
 
     def update_search(self):
         from website.search.search import update_institution

tests/test_auth.py

@@ -96,7 +96,7 @@ def test_confirm_email(self):
             res = self.app.resolve_redirect(res)
 
             assert res.status_code == 302
-            assert '/my-projects/' == urlparse(res.location).path
+            assert '/dashboard/' == urlparse(res.location).path
         # assert len(get_session()['status']) == 1
 
     def test_get_user_by_id(self):

tests/test_auth_basic_auth.py

@@ -99,4 +99,4 @@ def test_expired_cookie(self):
         self.app.set_cookie(settings.COOKIE_NAME, str(cookie))
         res = self.app.get(self.reachable_url)
         assert res.status_code == 302
-        assert '/login/' == res.location
+        assert 'http://localhost:5000/logout/?next=http://localhost:4200/' == res.location

tests/test_auth_views.py

@@ -551,32 +551,32 @@ def setUp(self):
         self.no_auth = Auth()
         self.user_auth = AuthUserFactory()
         self.auth = Auth(user=self.user_auth)
-        self.next_url = web_url_for('my_projects', _absolute=True)
+        self.next_url = web_url_for('dashboard', _absolute=True, _angular_route=True)
         self.invalid_campaign = 'invalid_campaign'
 
     def test_osf_login_with_auth(self):
         # login: user with auth
         data = login_and_register_handler(self.auth)
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_osf_login_without_auth(self):
         # login: user without auth
         data = login_and_register_handler(self.no_auth)
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_osf_register_with_auth(self):
         # register: user with auth
         data = login_and_register_handler(self.auth, login=False)
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_osf_register_without_auth(self):
         # register: user without auth
         data = login_and_register_handler(self.no_auth, login=False)
         assert data.get('status_code') == http_status.HTTP_200_OK
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_next_url_login_with_auth(self):
         # next_url login: user with auth
@@ -585,13 +585,13 @@ def test_next_url_login_with_auth(self):
         assert data.get('next_url') == self.next_url
 
     def test_next_url_angular_login_with_auth(self):
-        data = login_and_register_handler(self.auth, next_url=settings.LOCAL_ANGULAR_URL)
+        data = login_and_register_handler(self.auth, next_url=settings.LOCAL_ANGULAR_DOMAIN)
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == settings.LOCAL_ANGULAR_URL
+        assert data.get('next_url') == settings.LOCAL_ANGULAR_DOMAIN
 
     def test_next_url_angular_login_without_auth(self):
-        request.url = web_url_for('auth_login', next=settings.LOCAL_ANGULAR_URL, _absolute=True)
-        data = login_and_register_handler(self.no_auth, next_url=settings.LOCAL_ANGULAR_URL)
+        request.url = web_url_for('auth_login', next=settings.LOCAL_ANGULAR_DOMAIN, _absolute=True)
+        data = login_and_register_handler(self.no_auth, next_url=settings.LOCAL_ANGULAR_DOMAIN)
         assert data.get('status_code') == http_status.HTTP_302_FOUND
         assert data.get('next_url') == cas.get_login_url(request.url)
 
@@ -621,14 +621,16 @@ def test_institution_login_with_auth(self):
         # institution login: user with auth
         data = login_and_register_handler(self.auth, campaign='institution')
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_institution_login_without_auth(self):
         # institution login: user without auth
         data = login_and_register_handler(self.no_auth, campaign='institution')
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == cas.get_login_url(web_url_for('my_projects', _absolute=True),
-                                                         campaign='institution')
+        assert data.get('next_url') == cas.get_login_url(
+            web_url_for('dashboard', _absolute=True, _angular_route=True),
+            campaign='institution'
+        )
 
     def test_institution_login_next_url_with_auth(self):
         # institution login: user with auth and next url
@@ -646,13 +648,16 @@ def test_institution_register_with_auth(self):
         # institution register: user with auth
         data = login_and_register_handler(self.auth, login=False, campaign='institution')
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == web_url_for('my_projects', _absolute=True)
+        assert data.get('next_url') == web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     def test_institution_register_without_auth(self):
         # institution register: user without auth
         data = login_and_register_handler(self.no_auth, login=False, campaign='institution')
         assert data.get('status_code') == http_status.HTTP_302_FOUND
-        assert data.get('next_url') == cas.get_login_url(web_url_for('my_projects', _absolute=True), campaign='institution')
+        assert data.get('next_url') == cas.get_login_url(
+            web_url_for('dashboard', _absolute=True, _angular_route=True),
+            campaign='institution'
+        )
 
     def test_campaign_login_with_auth(self):
         for campaign in get_campaigns():
@@ -838,15 +843,13 @@ def test_logout_with_no_parameter(self):
         assert resp.status_code == http_status.HTTP_302_FOUND
         assert cas.get_logout_url(self.goodbye_url) == resp.headers['Location']
 
-    @mock.patch('framework.auth.views.settings.LOCAL_ANGULAR_URL', 'http://localhost:4200')
     def test_logout_with_angular_next_url_logged_in(self):
         angular_url = 'http://localhost:4200/'
         logout_url = web_url_for('auth_logout', _absolute=True, next=angular_url)
         resp = self.app.get(logout_url, auth=self.auth_user.auth)
         assert resp.status_code == http_status.HTTP_302_FOUND
         assert cas.get_logout_url(logout_url) == resp.headers['Location']
 
-    @mock.patch('framework.auth.views.settings.LOCAL_ANGULAR_URL', 'http://localhost:4200')
     def test_logout_with_angular_next_url_logged_out(self):
         angular_url = 'http://localhost:4200/'
         logout_url = web_url_for('auth_logout', _absolute=True, next=angular_url)

tests/test_campaigns.py

@@ -238,7 +238,7 @@ def setUp(self):
         super().setUp()
         self.url_login = web_url_for('auth_login', campaign='institution')
         self.url_register = web_url_for('auth_register', campaign='institution')
-        self.service_url = web_url_for('my_projects', _absolute=True)
+        self.service_url = web_url_for('dashboard', _absolute=True, _angular_route=True)
 
     # go to CAS institution login page if not logged in
     def test_institution_not_logged_in(self):

tests/test_webtests.py

@@ -79,35 +79,30 @@ def test_can_see_profile_url(self):
         res = self.app.get(self.user.url, follow_redirects=True)
         assert self.user.url in res.text
 
-    # `GET /login/` without parameters is redirected to `/my-projects/` page which has `@must_be_logged_in` decorator
-    # if user is not logged in, she/he is further redirected to CAS login page
+    # `GET /login/` (legacy BE endpoint) without parameters is redirected to `/dashboard/` (angular FE endpoint).
+    # It's impossible to test external redirects in tests, and it is angular's job to redirect correctly to CAS login.
     def test_is_redirected_to_cas_if_not_logged_in_at_login_page(self):
-        res = self.app.resolve_redirect(self.app.get('/login/'))
+        res = self.app.get('/login/')
         assert res.status_code == 302
-        location = res.headers.get('Location')
-        assert 'login?service=' in location
+        assert 'dashboard' in res.headers.get('Location')
 
-    def test_is_redirected_to_myprojects_if_already_logged_in_at_login_page(self):
+    def test_is_redirected_to_dashboard_if_already_logged_in_at_login_page(self):
         res = self.app.get('/login/', auth=self.user.auth)
         assert res.status_code == 302
-        assert 'my-projects' in res.headers.get('Location')
+        assert 'dashboard' in res.headers.get('Location')
 
     def test_register_page(self):
         res = self.app.get('/register/')
         assert res.status_code == 200
 
-    def test_is_redirected_to_myprojects_if_already_logged_in_at_register_page(self):
+    def test_is_redirected_to_dashboard_if_already_logged_in_at_register_page(self):
         res = self.app.get('/register/', auth=self.user.auth)
         assert res.status_code == 302
-        assert 'my-projects' in res.headers.get('Location')
+        assert 'dashboard' in res.headers.get('Location')
 
     def test_sees_projects_in_her_dashboard(self):
-        # the user already has a project
-        project = ProjectFactory(creator=self.user)
-        project.add_contributor(self.user)
-        project.save()
-        res = self.app.get('/my-projects/', auth=self.user.auth)
-        assert 'Projects' in res.text  # Projects heading
+        # Deprecated test, dashboard and my-projects are angular pages
+        pass
 
     def test_does_not_see_osffiles_in_user_addon_settings(self):
         res = self.app.get('/settings/addons/', auth=self.auth, follow_redirects=True)
@@ -123,10 +118,8 @@ def test_sees_osffiles_in_project_addon_settings(self):
         assert 'OSF Storage' in res.text
 
     def test_sees_correct_title_on_dashboard(self):
-        # User goes to dashboard
-        res = self.app.get('/my-projects/', auth=self.auth, follow_redirects=True)
-        title = res.html.title.string
-        assert 'OSF | My Projects' == title
+        # Deprecated test, dashboard and my-projects are angular pages
+        pass
 
     def test_can_see_make_public_button_if_admin(self):
         # User is a contributor on a project

website/routes.py

@@ -230,11 +230,10 @@ def sitemap_file(path):
 
 def goodbye():
     # Redirect to dashboard if logged in
-    redirect_url = util.web_url_for('auth_login')
     if _get_current_user():
-        return redirect(redirect_url)
+        return redirect(util.web_url_for('dashboard', _absolute=True, _angular_route=True))
     else:
-        return redirect(redirect_url + '?goodbye=true')
+        return redirect(util.web_url_for('index', _absolute=True, _angular_route=True))
 
 def make_url_map(app):
     """Set up all the routes for the OSF app.
@@ -294,25 +293,15 @@ def make_url_map(app):
 
     process_rules(app, [
         Rule('/', 'get', website_views.index, notemplate),
-        Rule(
-            '/dashboard/',
-            'get',
-            website_views.dashboard,
-            notemplate
-        ),
+        Rule('/dashboard/', 'get', website_views.dashboard, notemplate),
+        Rule('/my-projects/', 'get', website_views.my_projects, notemplate),
 
         Rule(
             '/metadata/<guid>/',
             'get',
             website_views.metadata_download,
             notemplate
         ),
-        Rule(
-            '/my-projects/',
-            'get',
-            website_views.my_projects,
-            OsfWebRenderer('my_projects.mako', trust=False)
-        ),
 
         Rule(
             '/reproducibility/',