[mb-commits] branch, mbs-2411, updated. MBS-2411, changes based on feedback from code review.

The branch, mbs-2411 has been updated
       via  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=d1a356b533d8b374533979af0873eb2043bbdab3 (commit)
      from  http://git.musicbrainz.org/gitweb/?p=musicbrainz-server/core.git;a=commit;h=5615dacbe20496b386270c27fde24fe928271845 (commit)

Summary of changes:
 lib/DBDefs/Default.pm                              |   13 +++++++------
 lib/MusicBrainz/Server/Controller/Root.pm          |    2 +-
 root/components/common-macros.tt                   |    6 +-----
 root/user/login.tt                                 |    2 +-
 .../t/MusicBrainz/Server/Controller/User/Login.pm  |    9 +++------
 5 files changed, 13 insertions(+), 19 deletions(-)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit d1a356b533d8b374533979af0873eb2043bbdab3
Author: warp <[email protected]>
Date:   Wed Jan 9 10:57:51 2013 +0100

    MBS-2411, changes based on feedback from code review.
    
    - Rename REQUIRE_SSL_ENABLED to SSL_REDIRECTS_ENABLED
    - Remove remnants of old ?secure=1 implementation of returning to http://
    - Use Hook::LexWrap in tests to set SSL_REDIRECTS_ENABLED.

diff --git a/lib/DBDefs/Default.pm b/lib/DBDefs/Default.pm
index e8d4649..ab62bfe 100644
--- a/lib/DBDefs/Default.pm
+++ b/lib/DBDefs/Default.pm
@@ -113,12 +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 }
+# SSL_REDIRECTS_ENABLED should be set to 1 on production.  It enables
+# the "RequireSSL" attribute on Catalyst actions, which will redirect
+# users to the SSL version of a Catalyst action (and redirect back to
+# http:// after the action is complete, though only if the user
+# started there).  If set to 0 no SSL redirects will be done, which is
+# suitable for local or development deployments.
+sub SSL_REDIRECTS_ENABLED { 0 }
 
 ################################################################################
 # Documentation Server Settings
diff --git a/lib/MusicBrainz/Server/Controller/Root.pm b/lib/MusicBrainz/Server/Controller/Root.pm
index 5e334bd..6af82fe 100644
--- a/lib/MusicBrainz/Server/Controller/Root.pm
+++ b/lib/MusicBrainz/Server/Controller/Root.pm
@@ -154,7 +154,7 @@ sub _ssl_redirect
 {
     my ($self, $c) = @_;
 
-    return unless DBDefs->REQUIRE_SSL_ENABLED;
+    return unless DBDefs->SSL_REDIRECTS_ENABLED;
 
     if (exists $c->action->attributes->{RequireSSL} && !$c->request->secure)
     {
diff --git a/root/components/common-macros.tt b/root/components/common-macros.tt
index b0ae95b..e9d15fb 100644
--- a/root/components/common-macros.tt
+++ b/root/components/common-macros.tt
@@ -431,11 +431,7 @@ END -%]
 [%- END -%]
 
 [%- MACRO login_url(redirect) BLOCK;
-    SET args = { uri => c.req.query_params.uri || redirect || c.relative_uri };
-    IF c.req.secure;
-       args.secure = 1;
-    END;
-    html_escape (c.uri_for_action('/user/login', args));
+    c.uri_for_action('/user/login', { uri => c.req.query_params.uri || redirect || c.relative_uri });
 END-%]
 
 [%- MACRO simple_link(url, text) BLOCK %]
diff --git a/root/user/login.tt b/root/user/login.tt
index 1fdd95a..e93d74e 100644
--- a/root/user/login.tt
+++ b/root/user/login.tt
@@ -14,7 +14,7 @@
     </p>
 
     [%- USE r = FormRenderer(login_form) -%]
-    <form action="[% html_escape(c.req.uri_with({ uri => redirect })) %]" method="post">
+    <form action="[% 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 1e5e36c..aa88026 100644
--- a/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm
+++ b/t/lib/t/MusicBrainz/Server/Controller/User/Login.pm
@@ -1,17 +1,14 @@
 package t::MusicBrainz::Server::Controller::User::Login;
 use Test::Routine;
 use Test::More;
+use Hook::LexWrap;
 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';
 
+wrap test, pre => sub { *DBDefs::SSL_REDIRECTS_ENABLED = sub { 1 }; };
+
 test all => sub {
 
     my $test = shift;

-----------------------------------------------------------------------


hooks/post-receive
-- 
mb_server