From 6b8ead6c4b0bb3665fa1c47c31eb4c658c3a79a4 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Tue, 14 Nov 2017 19:37:07 -0500 Subject: [PATCH] KEYCLOAK-5459 --- ...lacklistPasswordPolicyProviderFactory.java | 471 +++++++++--------- .../resources/templates/kc-tabs-client.html | 2 +- .../resources/templates/kc-tabs-group.html | 2 +- .../templates/kc-tabs-identity-provider.html | 2 +- .../resources/templates/kc-tabs-role.html | 2 +- .../resources/templates/kc-tabs-users.html | 2 +- 6 files changed, 245 insertions(+), 236 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/policy/BlacklistPasswordPolicyProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/policy/BlacklistPasswordPolicyProviderFactory.java index cb7af6253b..e3936eb95b 100644 --- a/server-spi-private/src/main/java/org/keycloak/policy/BlacklistPasswordPolicyProviderFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/policy/BlacklistPasswordPolicyProviderFactory.java @@ -64,268 +64,277 @@ import java.util.concurrent.ConcurrentMap; */ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyProviderFactory { - private static final Logger LOG = Logger.getLogger(BlacklistPasswordPolicyProviderFactory.class); + private static final Logger LOG = Logger.getLogger(BlacklistPasswordPolicyProviderFactory.class); - public static final String ID = "passwordBlacklist"; + public static final String ID = "passwordBlacklist"; - public static final String SYSTEM_PROPERTY = "keycloak.password.blacklists.path"; + public static final String SYSTEM_PROPERTY = "keycloak.password.blacklists.path"; - public static final String BLACKLISTS_PATH_PROPERTY = "blacklistsPath"; + public static final String BLACKLISTS_PATH_PROPERTY = "blacklistsPath"; - public static final String JBOSS_SERVER_DATA_DIR = "jboss.server.data.dir"; + public static final String JBOSS_SERVER_DATA_DIR = "jboss.server.data.dir"; - public static final String PASSWORD_BLACKLISTS_FOLDER = "password-blacklists/"; + public static final String PASSWORD_BLACKLISTS_FOLDER = "password-blacklists/"; - private ConcurrentMap blacklistRegistry = new ConcurrentHashMap<>(); + private ConcurrentMap blacklistRegistry = new ConcurrentHashMap<>(); - private Path blacklistsBasePath; + private volatile Path blacklistsBasePath; - @Override - public PasswordPolicyProvider create(KeycloakSession session) { - return new BlacklistPasswordPolicyProvider(session.getContext(), this); - } + private Config.Scope config; - @Override - public void init(Config.Scope config) { - this.blacklistsBasePath = FileBasedPasswordBlacklist.detectBlacklistsBasePath(config); - } - - @Override - public void postInit(KeycloakSessionFactory factory) { - } - - @Override - public void close() { - } - - @Override - public String getDisplayName() { - return "Password Blacklist"; - } - - @Override - public String getConfigType() { - return PasswordPolicyProvider.STRING_CONFIG_TYPE; - } - - @Override - public String getDefaultConfigValue() { - return ""; - } - - @Override - public boolean isMultiplSupported() { - return false; - } - - @Override - public String getId() { - return ID; - } - - - /** - * Resolves and potentially registers a {@link PasswordBlacklist} for the given {@code blacklistName}. - * - * @param blacklistName - * @return - */ - public PasswordBlacklist resolvePasswordBlacklist(String blacklistName) { - - Objects.requireNonNull(blacklistName, "blacklistName"); - - String cleanedBlacklistName = blacklistName.trim(); - if (cleanedBlacklistName.isEmpty()) { - throw new IllegalArgumentException("Password blacklist name must not be empty!"); + @Override + public PasswordPolicyProvider create(KeycloakSession session) { + if (this.blacklistsBasePath == null) { + synchronized (this) { + if (this.blacklistsBasePath == null) { + this.blacklistsBasePath = FileBasedPasswordBlacklist.detectBlacklistsBasePath(config); + } + } + } + return new BlacklistPasswordPolicyProvider(session.getContext(), this); } - return blacklistRegistry.computeIfAbsent(cleanedBlacklistName, (name) -> { - FileBasedPasswordBlacklist pbl = new FileBasedPasswordBlacklist(this.blacklistsBasePath, name); - pbl.lazyInit(); - return pbl; - }); - } + @Override + public void init(Config.Scope config) { + this.config = config; + } - /** - * A {@link PasswordBlacklist} describes a list of too easy to guess - * or potentially leaked passwords that users should not be able to use. - */ - public interface PasswordBlacklist { + @Override + public void postInit(KeycloakSessionFactory factory) { + } + + @Override + public void close() { + } + + @Override + public String getDisplayName() { + return "Password Blacklist"; + } + + @Override + public String getConfigType() { + return PasswordPolicyProvider.STRING_CONFIG_TYPE; + } + + @Override + public String getDefaultConfigValue() { + return ""; + } + + @Override + public boolean isMultiplSupported() { + return false; + } + + @Override + public String getId() { + return ID; + } /** - * @return the logical name of the {@link PasswordBlacklist} - */ - String getName(); - - /** - * Checks whether a given {@code password} is contained in this {@link PasswordBlacklist}. + * Resolves and potentially registers a {@link PasswordBlacklist} for the given {@code blacklistName}. * - * @param password + * @param blacklistName * @return */ - boolean contains(String password); - } + public PasswordBlacklist resolvePasswordBlacklist(String blacklistName) { - /** - * A {@link FileBasedPasswordBlacklist} uses password-blacklist files as - * to construct a {@link PasswordBlacklist}. - *

- * This implementation uses a dynamically sized {@link BloomFilter} - * to provide a false positive probability of 1%. - * - * @see BloomFilter - */ - public static class FileBasedPasswordBlacklist implements PasswordBlacklist { + Objects.requireNonNull(blacklistName, "blacklistName"); - private static final double FALSE_POSITIVE_PROBABILITY = 0.01; - - private static final int BUFFER_SIZE_IN_BYTES = 512 * 1024; - - /** - * The name of the blacklist filename. - */ - private final String name; - - /** - * The concrete path to the password-blacklist file. - */ - private final Path path; - - /** - * Initialized lazily via {@link #lazyInit()} - */ - private BloomFilter blacklist; - - public FileBasedPasswordBlacklist(Path blacklistBasePath, String name) { - - this.name = name; - this.path = blacklistBasePath.resolve(name); - - - if (name.contains("/")) { - // disallow '/' to avoid accidental filesystem traversal - throw new IllegalArgumentException("" + name + " must not contain slashes!"); - } - - if (!Files.exists(this.path)) { - throw new IllegalArgumentException("Password blacklist " + name + " not found!"); - } - } - - public String getName() { - return name; - } - - public boolean contains(String password) { - return blacklist != null && blacklist.mightContain(password); - } - - void lazyInit() { - - if (blacklist != null) { - return; - } - - this.blacklist = load(); - } - - /** - * Loads the referenced blacklist into a {@link BloomFilter}. - * - * @return the {@link BloomFilter} backing a password blacklist - */ - private BloomFilter load() { - - try { - LOG.infof("Loading blacklist with name %s from %s - start", name, path); - - long passwordCount = getPasswordCount(); - - BloomFilter filter = BloomFilter.create( - Funnels.stringFunnel(StandardCharsets.UTF_8), - passwordCount, - FALSE_POSITIVE_PROBABILITY); - - try (BufferedReader br = newReader(path)) { - br.lines().forEach(filter::put); + String cleanedBlacklistName = blacklistName.trim(); + if (cleanedBlacklistName.isEmpty()) { + throw new IllegalArgumentException("Password blacklist name must not be empty!"); } - LOG.infof("Loading blacklist with name %s from %s - end", name, path); - - return filter; - } catch (IOException e) { - throw new RuntimeException("Could not load password blacklist from path: " + path, e); - } + return blacklistRegistry.computeIfAbsent(cleanedBlacklistName, (name) -> { + FileBasedPasswordBlacklist pbl = new FileBasedPasswordBlacklist(this.blacklistsBasePath, name); + pbl.lazyInit(); + return pbl; + }); } /** - * Determines password blacklist size to correctly size the {@link BloomFilter} backing this blacklist. - * - * @return - * @throws IOException + * A {@link PasswordBlacklist} describes a list of too easy to guess + * or potentially leaked passwords that users should not be able to use. */ - private long getPasswordCount() throws IOException { + public interface PasswordBlacklist { + + + /** + * @return the logical name of the {@link PasswordBlacklist} + */ + String getName(); + + /** + * Checks whether a given {@code password} is contained in this {@link PasswordBlacklist}. + * + * @param password + * @return + */ + boolean contains(String password); + } + + /** + * A {@link FileBasedPasswordBlacklist} uses password-blacklist files as + * to construct a {@link PasswordBlacklist}. + *

+ * This implementation uses a dynamically sized {@link BloomFilter} + * to provide a false positive probability of 1%. + * + * @see BloomFilter + */ + public static class FileBasedPasswordBlacklist implements PasswordBlacklist { + + private static final double FALSE_POSITIVE_PROBABILITY = 0.01; + + private static final int BUFFER_SIZE_IN_BYTES = 512 * 1024; + + /** + * The name of the blacklist filename. + */ + private final String name; + + /** + * The concrete path to the password-blacklist file. + */ + private final Path path; + + /** + * Initialized lazily via {@link #lazyInit()} + */ + private BloomFilter blacklist; + + public FileBasedPasswordBlacklist(Path blacklistBasePath, String name) { + + this.name = name; + this.path = blacklistBasePath.resolve(name); + + + if (name.contains("/")) { + // disallow '/' to avoid accidental filesystem traversal + throw new IllegalArgumentException("" + name + " must not contain slashes!"); + } + + if (!Files.exists(this.path)) { + throw new IllegalArgumentException("Password blacklist " + name + " not found!"); + } + } + + public String getName() { + return name; + } + + public boolean contains(String password) { + return blacklist != null && blacklist.mightContain(password); + } + + void lazyInit() { + + if (blacklist != null) { + return; + } + + this.blacklist = load(); + } + + /** + * Loads the referenced blacklist into a {@link BloomFilter}. + * + * @return the {@link BloomFilter} backing a password blacklist + */ + private BloomFilter load() { + + try { + LOG.infof("Loading blacklist with name %s from %s - start", name, path); + + long passwordCount = getPasswordCount(); + + BloomFilter filter = BloomFilter.create( + Funnels.stringFunnel(StandardCharsets.UTF_8), + passwordCount, + FALSE_POSITIVE_PROBABILITY); + + try (BufferedReader br = newReader(path)) { + br.lines().forEach(filter::put); + } + + LOG.infof("Loading blacklist with name %s from %s - end", name, path); + + return filter; + } catch (IOException e) { + throw new RuntimeException("Could not load password blacklist from path: " + path, e); + } + } + + /** + * Determines password blacklist size to correctly size the {@link BloomFilter} backing this blacklist. + * + * @return + * @throws IOException + */ + private long getPasswordCount() throws IOException { /* * TODO find a more efficient way to determine the password count, * e.g. require a header-line in the password-blacklist file */ - try (BufferedReader br = newReader(path)) { - return br.lines().count(); - } - } - - private static BufferedReader newReader(Path path) throws IOException { - return new BufferedReader(Files.newBufferedReader(path), BUFFER_SIZE_IN_BYTES); - } - - /** - * Discovers password blacklists location. - *

- *

    - *
  1. - * system property {@code keycloak.password.blacklists.path} if present - *
  2. - *
  3. SPI config property {@code blacklistsPath}
  4. - *
- * and fallback to the {@code /data/password-blacklists} folder of the currently - * running wildfly instance. - * - * @param config - * @return the detected blacklist path - * @throws IllegalStateException if no blacklist folder could be detected - */ - private static Path detectBlacklistsBasePath(Config.Scope config) { - - String pathFromSysProperty = System.getProperty(SYSTEM_PROPERTY); - if (pathFromSysProperty != null) { - return ensureExists(Paths.get(pathFromSysProperty)); - } - - String pathFromSpiConfig = config.get(BLACKLISTS_PATH_PROPERTY); - if (pathFromSpiConfig != null) { - return ensureExists(Paths.get(pathFromSpiConfig)); - } - - String pathFromJbossDataPath = System.getProperty(JBOSS_SERVER_DATA_DIR) + "/" + PASSWORD_BLACKLISTS_FOLDER; - if (!Files.exists(Paths.get(pathFromJbossDataPath))) { - if (!Paths.get(pathFromJbossDataPath).toFile().mkdirs()) { - LOG.errorf("Could not create folder for password blacklists: %s", pathFromJbossDataPath); + try (BufferedReader br = newReader(path)) { + return br.lines().count(); + } + } + + private static BufferedReader newReader(Path path) throws IOException { + return new BufferedReader(Files.newBufferedReader(path), BUFFER_SIZE_IN_BYTES); + } + + /** + * Discovers password blacklists location. + *

+ *

    + *
  1. + * system property {@code keycloak.password.blacklists.path} if present + *
  2. + *
  3. SPI config property {@code blacklistsPath}
  4. + *
+ * and fallback to the {@code /data/password-blacklists} folder of the currently + * running wildfly instance. + * + * @param config + * @return the detected blacklist path + * @throws IllegalStateException if no blacklist folder could be detected + */ + private static Path detectBlacklistsBasePath(Config.Scope config) { + + String pathFromSysProperty = System.getProperty(SYSTEM_PROPERTY); + if (pathFromSysProperty != null) { + return ensureExists(Paths.get(pathFromSysProperty)); + } + + String pathFromSpiConfig = config.get(BLACKLISTS_PATH_PROPERTY); + if (pathFromSpiConfig != null) { + return ensureExists(Paths.get(pathFromSpiConfig)); + } + + String pathFromJbossDataPath = System.getProperty(JBOSS_SERVER_DATA_DIR) + "/" + PASSWORD_BLACKLISTS_FOLDER; + if (!Files.exists(Paths.get(pathFromJbossDataPath))) { + if (!Paths.get(pathFromJbossDataPath).toFile().mkdirs()) { + LOG.errorf("Could not create folder for password blacklists: %s", pathFromJbossDataPath); + } + } + return ensureExists(Paths.get(pathFromJbossDataPath)); + } + + private static Path ensureExists(Path path) { + + Objects.requireNonNull(path, "path"); + + if (Files.exists(path)) { + return path; + } + + throw new IllegalStateException("Password blacklists location does not exist: " + path); } - } - return ensureExists(Paths.get(pathFromJbossDataPath)); } - - private static Path ensureExists(Path path) { - - Objects.requireNonNull(path, "path"); - - if (Files.exists(path)) { - return path; - } - - throw new IllegalStateException("Password blacklists location does not exist: " + path); - } - } } diff --git a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-client.html b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-client.html index aa91fe8789..40aab54633 100755 --- a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-client.html +++ b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-client.html @@ -51,7 +51,7 @@ {{:: 'service-account-roles' | translate}} {{:: 'service-account-roles.tooltip' | translate}} -
  • +
  • {{:: 'authz-permissions' | translate}} {{:: 'manage-permissions-client.tooltip' | translate}}
  • diff --git a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-group.html b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-group.html index e73d998f5b..b0abb953dc 100755 --- a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-group.html +++ b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-group.html @@ -9,7 +9,7 @@
  • {{:: 'attributes' | translate}}
  • {{:: 'role-mappings' | translate}}
  • {{:: 'members' | translate}}
  • -
  • +
  • {{:: 'authz-permissions' | translate}} {{:: 'manage-permissions-group.tooltip' | translate}}
  • diff --git a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-identity-provider.html b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-identity-provider.html index 439bcfda43..c8e69e3a04 100644 --- a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-identity-provider.html +++ b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-identity-provider.html @@ -12,6 +12,6 @@
  • {{:: 'settings' | translate}}
  • {{:: 'mappers' | translate}}
  • {{:: 'export' | translate}}
  • -
  • {{:: 'authz-permissions' | translate}}
  • +
  • {{:: 'authz-permissions' | translate}}
  • diff --git a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-role.html b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-role.html index 9c3c4a7beb..9128dfd2e9 100755 --- a/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-role.html +++ b/themes/src/main/resources/theme/base/admin/resources/templates/kc-tabs-role.html @@ -5,7 +5,7 @@