[mb-commits] branch, mbs-2411, created. MBS-2411, handle redirects to/from https in Controller::Root. Redirect to SSL...
MusicBrainz Git Server <[email protected]> Mon, 24 Dec 2012 15:47:08 +0000
| Newsgroups | gmane.comp.audio.musicbrainz.cvs |
|---|---|
| Message-ID | <E1TnAF9-0000H0-RQ@wiley> |
The branch, mbs-2411 has been created
at 1b50a296295ebe2a1e1b60a755b485ee5d3cf499 (commit)
- Log -----------------------------------------------------------------
commit 1b50a296295ebe2a1e1b60a755b485ee5d3cf499
Author: warp <[email protected]>
Date: Mon Dec 24 16:46:08 2012 +0100
MBS-2411, handle redirects to/from https in Controller::Root. Redirect to SSL on /register.
diff --git a/lib/MusicBrainz/Server/Controller/Account.pm b/lib/MusicBrainz/Server/Controller/Account.pm
index 4f0e5f6..6dd37f7 100644
--- a/lib/MusicBrainz/Server/Controller/Account.pm
+++ b/lib/MusicBrainz/Server/Controller/Account.pm
@@ -393,7 +393,7 @@ new user.
=cut
-sub register : Path('/register') ForbiddenOnSlaves
+sub register : Path('/register') ForbiddenOnSlaves RequireSSL
{
my ($self, $c) = @_;
diff --git a/lib/MusicBrainz/Server/Controller/Root.pm b/lib/MusicBrainz/Server/Controller/Root.pm
index 4dce5d4..c531ee5 100644
--- a/lib/MusicBrainz/Server/Controller/Root.pm
+++ b/lib/MusicBrainz/Server/Controller/Root.pm
@@ -150,20 +150,41 @@ sub error_mirror_404 : Private
$c->detach;
}
-sub begin : Private
+sub _ssl_redirect
{
my ($self, $c) = @_;
- return if exists $c->action->attributes->{Minimal};
+ return unless DBDefs->REQUIRE_SSL_ENABLED;
- if (DBDefs->REQUIRE_SSL_ENABLED &&
- exists $c->action->attributes->{RequireSSL} && !$c->req->secure)
+ if (exists $c->action->attributes->{RequireSSL} && !$c->request->secure)
{
+ $c->response->cookies->{return_to_http} = { value => 1 };
$c->response->redirect(
"https://".DBDefs->WEB_SERVER_SSL.$c->request->env->{REQUEST_URI});
- return;
+ return 1;
+ }
+
+ if (!exists $c->action->attributes->{RequireSSL}
+ && $c->request->secure
+ && $c->request->cookie ('return_to_http'))
+ {
+ # expire in the past == delete cookie
+ $c->response->cookies->{return_to_http} = { value => 1, expires => '-1m' };
+ $c->response->redirect(
+ "http://".DBDefs->WEB_SERVER.$c->request->env->{REQUEST_URI});
}
+ return 0;
+}
+
+sub begin : Private
+{
+ my ($self, $c) = @_;
+
+ return if exists $c->action->attributes->{Minimal};
+
+ return if $self->_ssl_redirect ($c);
+
$c->stats->enable(1) if DBDefs->DEVELOPMENT_SERVER;
# if no javascript cookie is set we don't know if javascript is enabled or not.
diff --git a/lib/MusicBrainz/Server/Controller/User.pm b/lib/MusicBrainz/Server/Controller/User.pm
index 15745e5..5a5d238 100644
--- a/lib/MusicBrainz/Server/Controller/User.pm
+++ b/lib/MusicBrainz/Server/Controller/User.pm
@@ -88,9 +88,6 @@ sub do_login : Private
$self->_set_login_cookie($c);
}
- # Redirect back to non-ssl site unless the user started on https://.
- $redirect = 'http://'.DBDefs->WEB_SERVER.$redirect unless $secure;
-
# Logged in OK
$c->response->redirect($redirect);
$c->detach;
diff --git a/root/components/common-macros.tt b/root/components/common-macros.tt
index 240013c..b0ae95b 100644
--- a/root/components/common-macros.tt
+++ b/root/components/common-macros.tt
@@ -435,7 +435,7 @@ END -%]
IF c.req.secure;
args.secure = 1;
END;
- c.uri_for_action('/user/login', args);
+ html_escape (c.uri_for_action('/user/login', args));
END-%]
[%- MACRO simple_link(url, text) BLOCK %]
diff --git a/root/user/login.tt b/root/user/login.tt
index e93d74e..1fdd95a 100644
--- a/root/user/login.tt
+++ b/root/user/login.tt
@@ -14,7 +14,7 @@
</p>
[%- USE r = FormRenderer(login_form) -%]
- <form action="[% c.req.uri_with({ uri => redirect }) %]" method="post">
+ <form action="[% html_escape(c.req.uri_with({ uri => redirect })) %]" method="post">
[% IF login_form.has_errors || bad_login -%]
<div class="row no-label">
<span class="error"><strong>[% l('Incorrect username or password') %]</strong></span>
diff --git a/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm b/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm
index d02ee20..1e5e36c 100644
--- a/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm
+++ b/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm
@@ -2,28 +2,68 @@ package t::MusicBrainz::Server::Controller::User::Login;
use Test::Routine;
use Test::More;
use MusicBrainz::Server::Test qw( html_ok );
+use DBDefs;
+
+BEGIN {
+ no warnings 'redefine';
+ use DBDefs;
+ *DBDefs::REQUIRE_SSL_ENABLED = sub { 1 };
+}
with 't::Mechanize', 't::Context';
test all => sub {
-my $test = shift;
-my $mech = $test->mech;
-my $c = $test->c;
+ my $test = shift;
+ my $mech = $test->mech;
+ my $c = $test->c;
+
+ MusicBrainz::Server::Test->prepare_test_database($c, '+editor');
+
+ $mech->get_ok('https://localhost/login');
+ html_ok($mech->content);
+ $mech->submit_form( with_fields => { username => '', password => '' } );
+ $mech->content_contains('Incorrect username or password');
+ $mech->submit_form( with_fields => { username => 'new_editor', password => '' } );
+ $mech->content_contains('Incorrect username or password');
+ $mech->submit_form( with_fields => { username => '', password => 'password' } );
+ $mech->content_contains('Incorrect username or password');
+ $mech->submit_form( with_fields => { username => 'new_editor', password => 'password' } );
+ is($mech->uri->path, '/user/new_editor');
+
+};
+
+test 'https login' => sub {
+
+ my $test = shift;
+ my $mech = $test->mech;
+ my $c = $test->c;
+
+ MusicBrainz::Server::Test->prepare_test_database($c, '+editor');
+
+ $mech->get_ok('https://localhost/login');
+ html_ok($mech->content);
+ $mech->submit_form( with_fields => { username => 'new_editor', password => 'password' } );
+ is($mech->uri->path, '/user/new_editor');
+ is($mech->uri->scheme, 'https', 'We started secure, should still be secure');
+};
-MusicBrainz::Server::Test->prepare_test_database($c, '+editor');
+test 'http login with redirects to ssl' => sub {
-$mech->get_ok('/login');
-html_ok($mech->content);
-$mech->submit_form( with_fields => { username => '', password => '' } );
-$mech->content_contains('Incorrect username or password');
-$mech->submit_form( with_fields => { username => 'new_editor', password => '' } );
-$mech->content_contains('Incorrect username or password');
-$mech->submit_form( with_fields => { username => '', password => 'password' } );
-$mech->content_contains('Incorrect username or password');
-$mech->submit_form( with_fields => { username => 'new_editor', password => 'password' } );
-is($mech->uri->path, '/user/new_editor');
+ my $test = shift;
+ my $mech = $test->mech;
+ my $c = $test->c;
+ MusicBrainz::Server::Test->prepare_test_database($c, '+editor');
+
+ $mech->get_ok('http://localhost/login');
+ html_ok($mech->content);
+ is($mech->uri->scheme, 'https', 'Redirected to secure login form');
+ use Data::Dumper;
+ warn $mech->cookie_jar->as_string."\n";
+ $mech->submit_form( with_fields => { username => 'new_editor', password => 'password' } );
+ is($mech->uri->path, '/user/new_editor');
+ is($mech->uri->scheme, 'http', 'We started insecure, so correctly redirected back to http');
};
test 'Can login with usernames that contain the "/" character"' => sub {
commit d9dedc5d90e2db56eba1102951861b6804f0e0cc
Author: warp <[email protected]>
Date: Wed Dec 19 19:12:57 2012 +0100
MBS-2411, use c->req->secure instead of "manually" looking at HTTP_X_FORWARDED_PROTO.
diff --git a/lib/MusicBrainz/Server/Controller/Root.pm b/lib/MusicBrainz/Server/Controller/Root.pm
index 00c6689..4dce5d4 100644
--- a/lib/MusicBrainz/Server/Controller/Root.pm
+++ b/lib/MusicBrainz/Server/Controller/Root.pm
@@ -156,10 +156,8 @@ sub begin : Private
return if exists $c->action->attributes->{Minimal};
- my $secure = "https" eq $c->request->env->{HTTP_X_FORWARDED_PROTO} // "";
-
if (DBDefs->REQUIRE_SSL_ENABLED &&
- exists $c->action->attributes->{RequireSSL} && !$secure)
+ exists $c->action->attributes->{RequireSSL} && !$c->req->secure)
{
$c->response->redirect(
"https://".DBDefs->WEB_SERVER_SSL.$c->request->env->{REQUEST_URI});
@@ -174,7 +172,6 @@ sub begin : Private
$c->response->cookies->{javascript} = { value => ($js eq "unknown" ? "false" : $js) };
$c->stash(
- secure => $secure,
javascript => $js,
no_javascript => $js eq "false",
wiki_server => DBDefs->WIKITRANS_SERVER,
diff --git a/root/components/common-macros.tt b/root/components/common-macros.tt
index 655f7dc..240013c 100644
--- a/root/components/common-macros.tt
+++ b/root/components/common-macros.tt
@@ -432,7 +432,7 @@ END -%]
[%- MACRO login_url(redirect) BLOCK;
SET args = { uri => c.req.query_params.uri || redirect || c.relative_uri };
- IF secure;
+ IF c.req.secure;
args.secure = 1;
END;
c.uri_for_action('/user/login', args);
commit 9c10ffc11a5f05ae40bc0b3bdeeb31df7aa3e4f3
Author: warp <[email protected]>
Date: Wed Nov 28 13:50:46 2012 +0100
MBS-2411, force https:// on the login screen.
diff --git a/lib/DBDefs/Default.pm b/lib/DBDefs/Default.pm
index e160034..e8d4649 100644
--- a/lib/DBDefs/Default.pm
+++ b/lib/DBDefs/Default.pm
@@ -69,7 +69,8 @@ sub REPLICATION_TYPE { RT_STANDALONE }
# The host names used by the server.
# To use a port number other than 80, add it like so: "myhost:8000"
-sub WEB_SERVER { "www.musicbrainz.example.com" }
+sub WEB_SERVER { "localhost:5000" }
+sub WEB_SERVER_SSL { "localhost" }
sub LUCENE_SERVER { "search.musicbrainz.org" }
sub WEB_SERVER_USED_IN_EMAIL { my $self = shift; $self->WEB_SERVER }
@@ -112,6 +113,13 @@ sub DB_STAGING_SERVER_SANITIZED { 1 }
# users to edit user permissions.
sub DB_STAGING_TESTING_FEATURES { my $self = shift; $self->DB_STAGING_SERVER }
+# This should be enabled on production. It enables the "RequireSSL"
+# attribute on Catalyst actions. Currently only the login screen uses
+# this. When you enable this option the login screen will redirect to
+# the SSL version (and redirect back to http:// after login is
+# complete, though only if the user started there)
+sub REQUIRE_SSL_ENABLED { 0 }
+
################################################################################
# Documentation Server Settings
################################################################################
diff --git a/lib/MusicBrainz/Server/Controller/Root.pm b/lib/MusicBrainz/Server/Controller/Root.pm
index 5bb4005..00c6689 100644
--- a/lib/MusicBrainz/Server/Controller/Root.pm
+++ b/lib/MusicBrainz/Server/Controller/Root.pm
@@ -156,6 +156,16 @@ sub begin : Private
return if exists $c->action->attributes->{Minimal};
+ my $secure = "https" eq $c->request->env->{HTTP_X_FORWARDED_PROTO} // "";
+
+ if (DBDefs->REQUIRE_SSL_ENABLED &&
+ exists $c->action->attributes->{RequireSSL} && !$secure)
+ {
+ $c->response->redirect(
+ "https://".DBDefs->WEB_SERVER_SSL.$c->request->env->{REQUEST_URI});
+ return;
+ }
+
$c->stats->enable(1) if DBDefs->DEVELOPMENT_SERVER;
# if no javascript cookie is set we don't know if javascript is enabled or not.
@@ -164,6 +174,7 @@ sub begin : Private
$c->response->cookies->{javascript} = { value => ($js eq "unknown" ? "false" : $js) };
$c->stash(
+ secure => $secure,
javascript => $js,
no_javascript => $js eq "false",
wiki_server => DBDefs->WIKITRANS_SERVER,
diff --git a/lib/MusicBrainz/Server/Controller/User.pm b/lib/MusicBrainz/Server/Controller/User.pm
index 9a4fdf6..15745e5 100644
--- a/lib/MusicBrainz/Server/Controller/User.pm
+++ b/lib/MusicBrainz/Server/Controller/User.pm
@@ -70,9 +70,8 @@ sub do_login : Private
return 1 if $c->user_exists;
my $form = $c->form(form => 'User::Login');
- my $redirect = defined $c->req->query_params->{uri}
- ? $c->req->query_params->{uri}
- : $c->relative_uri;
+ my $redirect = $c->req->query_params->{uri} // $c->relative_uri;
+ my $secure = $c->req->query_params->{secure} // 0;
if ($c->form_posted && $form->process(params => $c->req->params))
{
@@ -89,6 +88,9 @@ sub do_login : Private
$self->_set_login_cookie($c);
}
+ # Redirect back to non-ssl site unless the user started on https://.
+ $redirect = 'http://'.DBDefs->WEB_SERVER.$redirect unless $secure;
+
# Logged in OK
$c->response->redirect($redirect);
$c->detach;
@@ -108,7 +110,7 @@ sub do_login : Private
$c->detach;
}
-sub login : Path('/login') ForbiddenOnSlaves
+sub login : Path('/login') ForbiddenOnSlaves RequireSSL
{
my ($self, $c) = @_;
diff --git a/root/components/common-macros.tt b/root/components/common-macros.tt
index e9d15fb..655f7dc 100644
--- a/root/components/common-macros.tt
+++ b/root/components/common-macros.tt
@@ -431,7 +431,11 @@ END -%]
[%- END -%]
[%- MACRO login_url(redirect) BLOCK;
- c.uri_for_action('/user/login', { uri => c.req.query_params.uri || redirect || c.relative_uri });
+ SET args = { uri => c.req.query_params.uri || redirect || c.relative_uri };
+ IF secure;
+ args.secure = 1;
+ END;
+ c.uri_for_action('/user/login', args);
END-%]
[%- MACRO simple_link(url, text) BLOCK %]
-----------------------------------------------------------------------
hooks/post-receive
--
mb_server