]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-3138 Prevent possible security flaws
authorEvgeny Mandrikov <mandrikov@gmail.com>
Tue, 24 Jan 2012 05:21:30 +0000 (09:21 +0400)
committerEvgeny Mandrikov <mandrikov@gmail.com>
Tue, 24 Jan 2012 05:27:15 +0000 (09:27 +0400)
* Save external password only if enabled "sonar.security.savePassword".
* Bypass restriction on password length (4), when external system enabled.
* Improve error handling.

plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
sonar-server/src/main/java/org/sonar/server/ui/AuthenticatorNotFoundException.java [deleted file]
sonar-server/src/main/java/org/sonar/server/ui/SecurityRealmFactory.java
sonar-server/src/main/webapp/WEB-INF/lib/need_authentication.rb
sonar-server/src/test/java/org/sonar/server/ui/SecurityRealmFactoryTest.java

index 6a105b10e98634b1c29e849a6da33180038c3e65..0d33ee0d00c14a1d17a99d5dc871628f6302d27e 100644 (file)
@@ -188,6 +188,18 @@ import java.util.List;
 
   // SERVER-SIDE TECHNICAL PROPERTIES
 
+  @Property(
+    key = "sonar.security.realm",
+    name = "Security Realm",
+    project = false,
+    global = false
+  ),
+  @Property(
+    key = "sonar.security.savePassword",
+    name = "Save external password",
+    project = false,
+    global = false
+  ),
   @Property(
     key = "sonar.authenticator.downcase",
     name = "Downcase login",
diff --git a/sonar-server/src/main/java/org/sonar/server/ui/AuthenticatorNotFoundException.java b/sonar-server/src/main/java/org/sonar/server/ui/AuthenticatorNotFoundException.java
deleted file mode 100644 (file)
index 0ea7621..0000000
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Sonar, open source software quality management tool.
- * Copyright (C) 2008-2012 SonarSource
- * mailto:contact AT sonarsource DOT com
- *
- * Sonar is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * Sonar is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with Sonar; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
- */
-package org.sonar.server.ui;
-
-/**
- * @deprecated in 2.14 and should be removed
- */
-@Deprecated
-public class AuthenticatorNotFoundException extends RuntimeException {
-
-  public AuthenticatorNotFoundException(String classname) {
-    super(classname);
-  }
-}
index 2761f575399a43463e7e90e83ebcce663169e2fc..23c1f653d0f1c209e36a44054118eb71fa3e2a5a 100644 (file)
@@ -27,6 +27,7 @@ import org.sonar.api.ServerComponent;
 import org.sonar.api.config.Settings;
 import org.sonar.api.security.LoginPasswordAuthenticator;
 import org.sonar.api.security.SecurityRealm;
+import org.sonar.api.utils.SonarException;
 
 /**
  * @since 2.14
@@ -34,7 +35,6 @@ import org.sonar.api.security.SecurityRealm;
 public class SecurityRealmFactory implements ServerComponent {
 
   private static final Logger INFO = LoggerFactory.getLogger("org.sonar.INFO");
-  private static final Logger LOG = LoggerFactory.getLogger(SecurityRealmFactory.class);
 
   private final boolean ignoreStartupFailure;
   private final SecurityRealm realm;
@@ -49,16 +49,14 @@ public class SecurityRealmFactory implements ServerComponent {
     if (!StringUtils.isEmpty(realmName)) {
       selectedRealm = selectRealm(realms, realmName);
       if (selectedRealm == null) {
-        LOG.error("Realm not found. Please check the property '" + REALM_PROPERTY + "' in conf/sonar.properties");
-        throw new AuthenticatorNotFoundException(realmName);
+        throw new SonarException("Realm '" + realmName + "' not found. Please check the property '" + REALM_PROPERTY + "' in conf/sonar.properties");
       }
     }
     if (selectedRealm == null && !StringUtils.isEmpty(className)) {
       LoginPasswordAuthenticator authenticator = selectAuthenticator(authenticators, className);
       if (authenticator == null) {
-        LOG.error("Authenticator plugin not found. Please check the property '" + CoreProperties.CORE_AUTHENTICATOR_CLASS
+        throw new SonarException("Authenticator '" + className + "' not found. Please check the property '" + CoreProperties.CORE_AUTHENTICATOR_CLASS
           + "' in conf/sonar.properties");
-        throw new AuthenticatorNotFoundException(className);
       }
       selectedRealm = new CompatibilityRealm(authenticator);
     }
@@ -85,10 +83,9 @@ public class SecurityRealmFactory implements ServerComponent {
         INFO.info("Security realm started");
       } catch (RuntimeException e) {
         if (ignoreStartupFailure) {
-          LOG.error("IGNORED - Realm fails to start: " + e.getMessage());
+          INFO.error("IGNORED - Security realm fails to start: " + e.getMessage());
         } else {
-          LOG.error("Realm fails to start: " + e.getMessage());
-          throw e;
+          throw new SonarException("Security realm fails to start: " + e.getMessage(), e);
         }
       }
     }
index afa3160562bf79dd37943c4023ce812d333bd27e..daa8c4addb7998627d99846780b512431464dfc7 100644 (file)
@@ -45,6 +45,8 @@ class PluginRealm
     @java_authenticator = java_realm.getLoginPasswordAuthenticator()
     @java_users_provider = java_realm.getUsersProvider()
     @java_groups_provider = java_realm.getGroupsProvider()
+
+    @save_password = Java::OrgSonarServerUi::JRubyFacade.new.getSettings().getBoolean('sonar.security.savePassword')
   end
 
   def authenticate?(username, password)
@@ -53,10 +55,11 @@ class PluginRealm
         details = @java_users_provider.doGetUserDetails(username)
       rescue Exception => e
         Rails.logger.error("Error from external users provider: #{e.message}")
+        return false if !@save_password
         return fallback(username, password)
       else
         # User exist in external system
-        auth(username, password, details) if details
+        return auth(username, password, details) if details
         # No such user in external system
         return fallback(username, password)
       end
@@ -109,7 +112,7 @@ class PluginRealm
       java_facade = Java::OrgSonarServerUi::JRubyFacade.new
       return nil if !java_facade.getSettings().getBoolean('sonar.authenticator.createUsers')
       # Automatically create a user in the sonar db if authentication has been successfully done
-      user = User.new(:login => username, :name => username, :email => '', :password => password, :password_confirmation => password)
+      user = User.new(:login => username, :name => username, :email => '')
       default_group_name = java_facade.getSettings().getString('sonar.defaultGroup')
       default_group = Group.find_by_name(default_group_name)
       if default_group
@@ -119,11 +122,16 @@ class PluginRealm
       end
     end
     if details
-      user.update_attributes(:name => details.getName(), :email => details.getEmail())
+      user.name = details.getName()
+      user.email = details.getEmail()
+    end
+    if @save_password
+      user.password = password
+      user.password_confirmation = password
     end
-    user.update_attributes(:password => password, :password_confirmation => password)
     synchronize_groups(user)
-    user.save
+    # Note that validation disabled
+    user.save(false)
     return user
   end
 
index 38ba8cd26620003be7ed6828ed7c50111fd51d5e..2e5904a9661c534cacd9b3b30138c9f049e7d06f 100644 (file)
@@ -25,9 +25,11 @@ import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
 import org.sonar.api.security.LoginPasswordAuthenticator;
 import org.sonar.api.security.SecurityRealm;
+import org.sonar.api.utils.SonarException;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
@@ -61,11 +63,16 @@ public class SecurityRealmFactoryTest {
     assertThat(factory.getRealm(), nullValue());
   }
 
-  @Test(expected = AuthenticatorNotFoundException.class)
+  @Test
   public void realmNotFound() {
     settings.setProperty(SecurityRealmFactory.REALM_PROPERTY, "Fake");
 
-    new SecurityRealmFactory(settings);
+    try {
+      new SecurityRealmFactory(settings);
+      fail();
+    } catch (SonarException e) {
+      assertThat(e.getMessage(), containsString("Realm 'Fake' not found."));
+    }
   }
 
   @Test
@@ -90,11 +97,16 @@ public class SecurityRealmFactoryTest {
     assertThat(factory.getRealm(), is(realm));
   }
 
-  @Test(expected = AuthenticatorNotFoundException.class)
+  @Test
   public void authenticatorNotFound() {
     settings.setProperty(CoreProperties.CORE_AUTHENTICATOR_CLASS, "Fake");
 
-    new SecurityRealmFactory(settings);
+    try {
+      new SecurityRealmFactory(settings);
+      fail();
+    } catch (SonarException e) {
+      assertThat(e.getMessage(), containsString("Authenticator 'Fake' not found."));
+    }
   }
 
   @Test
@@ -107,12 +119,18 @@ public class SecurityRealmFactoryTest {
     verify(realm).init();
   }
 
-  @Test(expected = IllegalStateException.class)
+  @Test
   public void shouldFail() {
     SecurityRealm realm = spy(new AlwaysFailsRealm());
     settings.setProperty(SecurityRealmFactory.REALM_PROPERTY, realm.getName());
 
-    new SecurityRealmFactory(settings, new SecurityRealm[] {realm}).start();
+    try {
+      new SecurityRealmFactory(settings, new SecurityRealm[] {realm}).start();
+      fail();
+    } catch (SonarException e) {
+      assertThat(e.getCause(), instanceOf(IllegalStateException.class));
+      assertThat(e.getMessage(), containsString("Security realm fails to start"));
+    }
   }
 
   private static class AlwaysFailsRealm extends FakeRealm {