changeset 732:897bc2b63bfe feature_318

Overhaul SoundSoftware authentication manager to handle HTTPS redirects itself and also to avoid info leakage for nonexistent projects

--- a/extra/soundsoftware/SoundSoftware.pm	Mon Oct 17 17:03:46 2011 +0100
+++ b/extra/soundsoftware/SoundSoftware.pm	Fri Nov 04 10:28:40 2011 +0000
@@ -110,6 +110,11 @@
     req_override => OR_AUTHCFG,
     args_how => TAKE1,
   },
+  {
+    name => 'SoundSoftwareSslRequired',
+    req_override => OR_AUTHCFG,
+    args_how => TAKE1,
+  },
 );
 
 sub SoundSoftwareDSN { 
@@ -143,6 +148,8 @@
     }
 }
 
+sub SoundSoftwareSslRequired { set_val('SoundSoftwareSslRequired', @_); }
+
 sub trim {
     my $string = shift;
     $string =~ s/\s{2,}/ /g;
@@ -184,33 +191,75 @@
 
     my $project_id = get_project_identifier($dbh, $r);
 
-    if (!defined $read_only_methods{$method}) {
-        print STDERR "SoundSoftware.pm:$$: Method is not read-only\n";
-        if (project_repo_is_readonly($dbh, $project_id, $r)) {
-            print STDERR "SoundSoftware.pm:$$: Project repo is read-only, refusing access\n";
-	    return FORBIDDEN;
-        } else {
-	    print STDERR "SoundSoftware.pm:$$: Project repo is read-write, authentication handler required\n";
-            return OK;
-        }
-    }
+    # We want to delegate most of the work to the authentication
+    # handler (to ensure that user is asked to login even for 
+    # nonexistent projects -- so they can't tell whether a private
+    # project exists or not without authenticating). So 
+    # 
+    # * if the project is public
+    #   - if the method is read-only
+    #     + set handler to OK, no auth needed
+    #   - if the method is not read-only
+    #     + if the repo is read-only, return forbidden
+    #     + else require auth
+    # * if the project is not public or does not exist
+    #     + require auth
+    #
+    # If we are requiring auth and are not currently https, and
+    # https is required, then we must return a redirect to https
+    # instead of an OK.
 
     my $status = get_project_status($dbh, $project_id, $r);
+    my $readonly = project_repo_is_readonly($dbh, $project_id, $r);
 
     $dbh->disconnect();
     undef $dbh;
 
-    if ($status == 0) { # nonexistent
-	print STDERR "SoundSoftware.pm:$$: Project does not exist, refusing access\n";
-	return FORBIDDEN;
-    } elsif ($status == 1) { # public
-	print STDERR "SoundSoftware.pm:$$: Project is public, no restriction here\n";
-	$r->set_handlers(PerlAuthenHandler => [\&OK])
-    } else { # private
-	print STDERR "SoundSoftware.pm:$$: Project is private, authentication handler required\n";
+    if ($status == 1) { # public
+
+	print STDERR "SoundSoftware.pm:$$: Project is public\n";
+
+	if (!defined $read_only_methods{$method}) {
+
+	    print STDERR "SoundSoftware.pm:$$: Method is not read-only\n";
+
+	    if ($readonly) {
+		print STDERR "SoundSoftware.pm:$$: Project repo is read-only, refusing access\n";
+		return FORBIDDEN;
+	    } else {
+		print STDERR "SoundSoftware.pm:$$: Project repo is read-write, auth required\n";
+		# fall through, this is the normal case
+	    }
+
+	} else {
+	    # Public project, read-only method -- this is the only
+	    # case we can decide for certain to accept in this function
+	    print STDERR "SoundSoftware.pm:$$: Method is read-only, no restriction here\n";
+	    $r->set_handlers(PerlAuthenHandler => [\&OK]);
+	    return OK;
+	}
+
+    } else { # status != 1, i.e. nonexistent or private -- equivalent here
+
+	print STDERR "SoundSoftware.pm:$$: Project is private or nonexistent, auth required\n";
+	# fall through
     }
 
-    return OK
+    if ($cfg->{SoundSoftwareSslRequired} eq "on") {
+	if ($r->dir_config('HTTPS') eq "on") {
+	    return OK;
+	} else {
+	    my $redir_to = "https://" . $r->hostname() . $r->unparsed_uri();
+	    print STDERR "SoundSoftware.pm:$$: Need to switch to HTTPS, redirecting to $redir_to\n";
+	    $r->header_out(Location => $redir_to);
+	    return REDIRECT;
+	}
+    } else if ($cfg->{SoundSoftwareSslRequired} eq "off") {
+	return OK;
+    } else {
+	print STDERR "WARNING: SoundSoftware.pm:$$: SoundSoftwareSslRequired should be either 'on' or 'off'\n";
+	return OK;
+    }
 }
 
 sub authen_handler {
@@ -237,6 +286,16 @@
     
     print STDERR "SoundSoftware.pm:$$: User is " . $r->user . ", got password\n";
 
+    my $status = get_project_status($dbh, $project_id, $r);
+    if ($status == 0) {
+	# nonexistent, behave like private project you aren't a member of
+	print STDERR "SoundSoftware.pm:$$: Project doesn't exist, not permitted\n";
+	$dbh->disconnect();
+	undef $dbh;
+	$r->note_auth_failure();
+	return AUTH_REQUIRED;
+    }
+
     my $permitted = is_permitted($dbh, $project_id, $r->user, $redmine_pass, $r);
     
     $dbh->disconnect();