Avoid double redirects to /login and then to /login/.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit is contained in:
Anders Kaseorg 2018-12-03 17:12:08 -08:00 committed by Tim Abbott
parent 02a79b677b
commit 9ba860b4f2
11 changed files with 20 additions and 30 deletions

View File

@ -216,10 +216,10 @@ to debug.
* Since you've configured `/etc/zulip/settings.py` to only define the * Since you've configured `/etc/zulip/settings.py` to only define the
`zproject.backends.ZulipRemoteUserBackend`, `zproject/settings.py` `zproject.backends.ZulipRemoteUserBackend`, `zproject/settings.py`
configures `/accounts/login/sso` as `HOME_NOT_LOGGED_IN`. This configures `/accounts/login/sso/` as `HOME_NOT_LOGGED_IN`. This
makes `https://zulip.example.com/` (a.k.a. the homepage for the main makes `https://zulip.example.com/` (a.k.a. the homepage for the main
Zulip Django app running behind nginx) redirect to Zulip Django app running behind nginx) redirect to
`/accounts/login/sso` for a user that isn't logged in. `/accounts/login/sso/` for a user that isn't logged in.
* nginx proxies requests to `/accounts/login/sso/` to an Apache * nginx proxies requests to `/accounts/login/sso/` to an Apache
instance listening on `localhost:8888`, via the config in instance listening on `localhost:8888`, via the config in

View File

@ -18,7 +18,7 @@ function log_in(credentials) {
} }
casper.test.info('Logging in'); casper.test.info('Logging in');
casper.fill('form[action^="/accounts/login"]', { casper.fill('form[action^="/accounts/login/"]', {
username: credentials.username, username: credentials.username,
password: credentials.password, password: credentials.password,
}, true /* submit form */); }, true /* submit form */);
@ -106,7 +106,7 @@ exports.then_log_in = function (credentials) {
}; };
exports.start_and_log_in = function (credentials, viewport) { exports.start_and_log_in = function (credentials, viewport) {
var log_in_url = "http://zulip.zulipdev.com:9981/accounts/login"; var log_in_url = "http://zulip.zulipdev.com:9981/accounts/login/";
exports.init_viewport(); exports.init_viewport();
casper.start(log_in_url, function () { casper.start(log_in_url, function () {
exports.initialize_casper(viewport); exports.initialize_casper(viewport);

View File

@ -6,7 +6,7 @@ common.init_viewport();
casper.start(realm_url, common.initialize_casper); casper.start(realm_url, common.initialize_casper);
casper.then(function () { casper.then(function () {
casper.test.assertUrlMatch(/^http:\/\/[^/]+\/login/, 'Redirected to /login'); casper.test.assertUrlMatch(/^http:\/\/[^/]+\/login\/$/, 'Redirected to /login/');
}); });
common.then_log_in(); common.then_log_in();

View File

@ -472,7 +472,7 @@ exports.set_up = function () {
url: '/json/users/me', url: '/json/users/me',
success: function () { success: function () {
$("#deactivate_self_modal").modal("hide"); $("#deactivate_self_modal").modal("hide");
window.location.href = "/login"; window.location.href = "/login/";
}, },
error: function (xhr) { error: function (xhr) {
var error_last_admin = i18n.t("Error: Cannot deactivate the only organization administrator."); var error_last_admin = i18n.t("Error: Cannot deactivate the only organization administrator.");

View File

@ -9,7 +9,7 @@
{% else %} {% else %}
<h4 class="login-page-header">Click on a user to log in!</h4> <h4 class="login-page-header">Click on a user to log in!</h4>
{% endif %} {% endif %}
<p class="devlogin_subheader">{{ _('(Or visit the <a href="/login">normal login page</a>)') }}</p> <p class="devlogin_subheader">(Or visit the <a href="/login/">normal login page</a>.)</p>
<form name="direct_login_form" id="direct_login_form" method="post" class="login-form"> <form name="direct_login_form" id="direct_login_form" method="post" class="login-form">
<div class="control-group"> <div class="control-group">
<div class="controls"> <div class="controls">

View File

@ -6,16 +6,6 @@ WARNING: no certificate subject alternative name matches
Content-Type: text/html; charset=utf-8 Content-Type: text/html; charset=utf-8
Content-Length: 0 Content-Length: 0
Connection: keep-alive Connection: keep-alive
Location: /login
Strict-Transport-Security: max-age=15768000
X-Frame-Options: DENY
Location: /login [following]
Reusing existing connection to localhost:443.
HTTP/1.1 301 Moved Permanently
Server: nginx/1.4.6 (Ubuntu)
Content-Type: text/html; charset=utf-8
Content-Length: 0
Connection: keep-alive
Location: /login/ Location: /login/
Strict-Transport-Security: max-age=15768000 Strict-Transport-Security: max-age=15768000
X-Frame-Options: DENY X-Frame-Options: DENY

View File

@ -113,7 +113,7 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
# browser, send the user to the login page # browser, send the user to the login page
if 'text/html' in request.META.get('HTTP_ACCEPT', ''): if 'text/html' in request.META.get('HTTP_ACCEPT', ''):
# TODO: It seems like the `?next=` part is unlikely to be helpful # TODO: It seems like the `?next=` part is unlikely to be helpful
return HttpResponseRedirect('%s/?next=%s' % (settings.HOME_NOT_LOGGED_IN, request.path)) return HttpResponseRedirect('%s?next=%s' % (settings.HOME_NOT_LOGGED_IN, request.path))
# Ask for basic auth (email:apiKey) # Ask for basic auth (email:apiKey)
elif request.path.startswith("/api"): elif request.path.startswith("/api"):
return json_unauthorized(_("Not logged in: API authentication or user session required")) return json_unauthorized(_("Not logged in: API authentication or user session required"))

View File

@ -336,7 +336,7 @@ class PlansPageTest(ZulipTestCase):
realm.save(update_fields=["plan_type"]) realm.save(update_fields=["plan_type"])
result = self.client_get("/plans/", subdomain="zulip") result = self.client_get("/plans/", subdomain="zulip")
self.assertEqual(result.status_code, 302) self.assertEqual(result.status_code, 302)
self.assertEqual(result["Location"], "/accounts/login?next=plans") self.assertEqual(result["Location"], "/accounts/login/?next=plans")
# Test valid domain, with login # Test valid domain, with login
self.login(self.example_email('hamlet')) self.login(self.example_email('hamlet'))
result = self.client_get("/plans/", subdomain="zulip") result = self.client_get("/plans/", subdomain="zulip")

View File

@ -27,7 +27,7 @@ class TestSessions(ZulipTestCase):
action() action()
if expected_result: if expected_result:
result = self.client_get('/', subdomain=realm.subdomain) result = self.client_get('/', subdomain=realm.subdomain)
self.assertEqual('/login', result.url) self.assertEqual('/login/', result.url)
else: else:
self.assertIn('_auth_user_id', self.client.session) self.assertIn('_auth_user_id', self.client.session)
@ -39,7 +39,7 @@ class TestSessions(ZulipTestCase):
for session in user_sessions(user_profile): for session in user_sessions(user_profile):
delete_session(session) delete_session(session)
result = self.client_get("/") result = self.client_get("/")
self.assertEqual('/login', result.url) self.assertEqual('/login/', result.url)
def test_delete_user_sessions(self) -> None: def test_delete_user_sessions(self) -> None:
user_profile = self.example_user('hamlet') user_profile = self.example_user('hamlet')
@ -75,7 +75,7 @@ class TestSessions(ZulipTestCase):
self.client_post('/accounts/logout/') self.client_post('/accounts/logout/')
delete_all_deactivated_user_sessions() delete_all_deactivated_user_sessions()
result = self.client_get("/") result = self.client_get("/")
self.assertEqual('/login', result.url) self.assertEqual('/login/', result.url)
# Test nothing happens to an active user's session # Test nothing happens to an active user's session
self.login(self.example_email("othello")) self.login(self.example_email("othello"))
@ -92,4 +92,4 @@ class TestSessions(ZulipTestCase):
user_profile_3.save() user_profile_3.save()
delete_all_deactivated_user_sessions() delete_all_deactivated_user_sessions()
result = self.client_get("/") result = self.client_get("/")
self.assertEqual('/login', result.url) self.assertEqual('/login/', result.url)

View File

@ -1304,17 +1304,17 @@ USING_APACHE_SSO = ('zproject.backends.ZulipRemoteUserBackend' in AUTHENTICATION
if len(AUTHENTICATION_BACKENDS) == 1 and (AUTHENTICATION_BACKENDS[0] == if len(AUTHENTICATION_BACKENDS) == 1 and (AUTHENTICATION_BACKENDS[0] ==
"zproject.backends.ZulipRemoteUserBackend"): "zproject.backends.ZulipRemoteUserBackend"):
HOME_NOT_LOGGED_IN = "/accounts/login/sso" HOME_NOT_LOGGED_IN = "/accounts/login/sso/"
ONLY_SSO = True ONLY_SSO = True
else: else:
HOME_NOT_LOGGED_IN = '/login' HOME_NOT_LOGGED_IN = '/login/'
ONLY_SSO = False ONLY_SSO = False
AUTHENTICATION_BACKENDS += ('zproject.backends.ZulipDummyBackend',) AUTHENTICATION_BACKENDS += ('zproject.backends.ZulipDummyBackend',)
# Redirect to /devlogin by default in dev mode # Redirect to /devlogin/ by default in dev mode
if DEVELOPMENT: if DEVELOPMENT:
HOME_NOT_LOGGED_IN = '/devlogin' HOME_NOT_LOGGED_IN = '/devlogin/'
LOGIN_URL = '/devlogin' LOGIN_URL = '/devlogin/'
POPULATE_PROFILE_VIA_LDAP = bool(AUTH_LDAP_SERVER_URI) POPULATE_PROFILE_VIA_LDAP = bool(AUTH_LDAP_SERVER_URI)

View File

@ -136,8 +136,8 @@ TERMS_OF_SERVICE = 'corporate/terms.md'
INLINE_URL_EMBED_PREVIEW = False INLINE_URL_EMBED_PREVIEW = False
HOME_NOT_LOGGED_IN = '/login' HOME_NOT_LOGGED_IN = '/login/'
LOGIN_URL = '/accounts/login' LOGIN_URL = '/accounts/login/'
# By default will not send emails when login occurs. # By default will not send emails when login occurs.
# Explicity set this to True within tests that must have this on. # Explicity set this to True within tests that must have this on.