Skip to content
14 changes: 14 additions & 0 deletions framework/auth/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from framework import sentry
from website import settings
from website.util import web_url_for

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -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)
21 changes: 10 additions & 11 deletions framework/auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
))
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions framework/sessions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions osf/models/institution.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_auth_basic_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
35 changes: 19 additions & 16 deletions tests/test_auth_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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():
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_campaigns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
31 changes: 12 additions & 19 deletions tests/test_webtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
19 changes: 4 additions & 15 deletions website/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, _local_angular=True))
Comment thread
cslzchen marked this conversation as resolved.
Outdated
else:
return redirect(redirect_url + '?goodbye=true')
return redirect(util.web_url_for('index', _absolute=True, _local_angular=True))
Comment thread
cslzchen marked this conversation as resolved.
Outdated

def make_url_map(app):
"""Set up all the routes for the OSF app.
Expand Down Expand Up @@ -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/',
Expand Down
Loading
Loading