Проверка пароля в Java

Я просто написал эту функцию проверки пароля, которая проверяет, будет ли пароль принят в AD или нет. Я не уверен, что это лучший способ сделать это, но пока это работает нормально. Мне бы хотелось, чтобы некоторые советы о том, как сделать это лучше.

public String validateNewPass(String pass1, String pass2){
        StringBuilder retVal = new StringBuilder();

        if(pass1.length() < 1 || pass2.length() < 1 )retVal.append("Empty fields <br>");

        if (pass1 != null && pass2 != null) {

            if (pass1.equals(pass2)) {
                logger.info(pass1 + " = " + pass2);

                pass1 = pass2;
                boolean hasUppercase = !pass1.equals(pass1.toLowerCase());
                boolean hasLowercase = !pass1.equals(pass1.toUpperCase());
                boolean hasNumber = pass1.matches(".*\\d.*");
                boolean noSpecialChar = pass1.matches("[a-zA-Z0-9 ]*");

                if (pass1.length() < 11) {
                    logger.info(pass1 + " is length < 11");
                    retVal.append("Password is too short. Needs to have 11 characters <br>");
                }

                if (!hasUppercase) {
                    logger.info(pass1 + " <-- needs uppercase");
                    retVal.append("Password needs an upper case <br>");
                }

                if (!hasLowercase) {
                    logger.info(pass1 + " <-- needs lowercase");
                    retVal.append("Password needs a lowercase <br>");
                }

                if (!hasNumber) {
                    logger.info(pass1 + "<-- needs a number");
                    retVal.append("Password needs a number <br>");
                }

                if(noSpecialChar){
                    logger.info(pass1 + "<-- needs a specail character");
                    retVal.append("Password needs a special character i.e. !,@,#, etc.  <br>");
                }
            }else{
                logger.info(pass1 + " != " + pass2);
                retVal.append("Passwords don't match<br>");
            }
        }else{
            logger.info("Passwords = null");
            retVal.append("Passwords Null <br>");
        }
        if(retVal.length() == 0){
            logger.info("Password validates");
            retVal.append("Success");
        }

        return retVal.toString();

    }

** Примечание. Эти строки должны гарантировать, что другой случай обрабатывает строку null vs empty

if(pass1.length() < 1 || pass2.length() < 1 )retVal.append("Empty fields <br>");

        if (pass1 != null && pass2 != null) {
30 голосов | спросил pt18cher 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 00:00:34 +0400 2014, 00:00:34

7 ответов


26

Поведение багги

Сначала вы проверяете длину паролей, а затем, если они равны null:

if(pass1.length() < 1 || pass2.length() < 1 )retVal.append("Empty fields <br>");

if (pass1 != null && pass2 != null) {

Это не сработает: если какой-либо из паролей был пустым, при проверке длины вы получите исключение NullPointerException.

Кроме того, лучший способ проверить, является ли пустая строка, используется pass1.isEmpty().

Кроме того, это бессмысленно и потенциально запутывает:

pass1 = pass2;

Упростить логику проверки

Было бы лучше и эффективнее создавать члены private final Pattern, которые скомпилировали регулярные выражения и многократно повторялись:

private final Pattern hasUppercase = Pattern.compile("[A-Z]");
private final Pattern hasLowercase = Pattern.compile("[a-z]");
private final Pattern hasNumber = Pattern.compile("\\d");
private final Pattern hasSpecialChar = Pattern.compile("[^a-zA-Z0-9 ]");

Например, это возвращает true, если pass1 содержит символ верхнего регистра:

hasUppercase.matcher(pass1).find()

Обратите внимание на patter для hasSpecialChar: соответствие неалфавиту, незначению, не пробелу.

Рекомендуемая реализация

Основываясь на приведенных выше советах, вы можете упростить свою реализацию следующим образом:

private final Pattern hasUppercase = Pattern.compile("[A-Z]");
private final Pattern hasLowercase = Pattern.compile("[a-z]");
private final Pattern hasNumber = Pattern.compile("\\d");
private final Pattern hasSpecialChar = Pattern.compile("[^a-zA-Z0-9 ]");

public String validateNewPass(String pass1, String pass2) {
    if (pass1 == null || pass2 == null) {
        logger.info("Passwords = null");
        return "One or both passwords are null";
    }

    StringBuilder retVal = new StringBuilder();

    if (pass1.isEmpty() || pass2.isEmpty()) {
        retVal.append("Empty fields <br>");
    }

    if (pass1.equals(pass2)) {
        logger.info(pass1 + " = " + pass2);

        if (pass1.length() < 11) {
            logger.info(pass1 + " is length < 11");
            retVal.append("Password is too short. Needs to have 11 characters <br>");
        }

        if (!hasUppercase.matcher(pass1).find()) {
            logger.info(pass1 + " <-- needs uppercase");
            retVal.append("Password needs an upper case <br>");
        }

        if (!hasLowercase.matcher(pass1).find()) {
            logger.info(pass1 + " <-- needs lowercase");
            retVal.append("Password needs a lowercase <br>");
        }

        if (!hasNumber.matcher(pass1).find()) {
            logger.info(pass1 + "<-- needs a number");
            retVal.append("Password needs a number <br>");
        }

        if (!hasSpecialChar.matcher(pass1).find()) {
            logger.info(pass1 + "<-- needs a specail character");
            retVal.append("Password needs a special character i.e. !,@,#, etc.  <br>");
        }
    } else {
        logger.info(pass1 + " != " + pass2);
        retVal.append("Passwords don't match<br>");
    }
    if (retVal.length() == 0) {
        logger.info("Password validates");
        retVal.append("Success");
    }

    return retVal.toString();
}

Поддержка Unicode

Если вы хотите разрешить символы Unicode в пароле, определите шаблоны с использованием специальных классов символов java, как описано в javadoc :

private final Pattern hasUppercase = Pattern.compile("\\p{javaUpperCase}");
private final Pattern hasLowercase = Pattern.compile("\\p{javaLowerCase}");
private final Pattern hasNumber = Pattern.compile("\\p{javaDigit}");
private final Pattern hasSpecialChar = Pattern.compile("[^\\p{javaLetterOrDigit} ]");
ответил janos 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 01:17:17 +0400 2014, 01:17:17
23

Ваш код попросит использовать Chain Of Responsibility patern (вернее, вариант его).

Но сначала некоторые наблюдения:

  1. Вы разыскиваете пароли перед тем, как проверить их на null:

    public String validateNewPass(String pass1, String pass2){
            if(pass1.length() < 1 || pass2.length() < 1 )retVal.append("Empty fields <br>");
            if (pass1 != null && pass2 != null) {
    

    Если пароль был пустым, вы получите NullPointerException до , чтобы проверить, являются ли они пустыми.

  2. Используйте String.isEmpty() вместо вызовов length().


Альтернативный алгоритм

То, что у вас есть, - это набор требований, каждый из которых должен быть удовлетворен, чтобы пароль был подходящим. В Java8 это будет сделано с помощью функций, но, давайте использовать интерфейс на данный момент:

interface PasswordRule {
    boolean passRule(String password);
    String failMessage();
}

Для удобства я бы представил абстрактный класс, и он выглядел бы так:

abstract static class BaseRule implements PasswordRule {
    private final String message;
    BaseRule(String message) {
        this.message = message;
    }

    public String failMessage() {
        return message;
    }
}

ОК, так что теперь вам нужно создать экземпляр для каждого правила пароля:

private static final PasswordRule[] RULES = {
        new BaseRule("Password is too short. Needs to have 11 characters") {

            @Override
            public boolean passRule(String password) {
                return password.length() >= 11;
            }
        },

        new BaseRule("Password needs an upper case") {

            private final Pattern ucletter = Pattern.compile(".*[\\p{Lu}].*");
            @Override
            public boolean passRule(String password) {
                return ucletter.matcher(password).matches();
            }
        },

        /// .... more rules.
};

Теперь ваш метод будет выглядеть следующим образом:

public static String validateNewPass(String pass1, String pass2){
    if(pass1 == null || pass2 == null) {
        //logger.info("Passwords = null");
        return "Passwords Null <br>";
    }

    if (pass1.isEmpty() || pass2.isEmpty()) {
        return "Empty fields <br>";
    }

    if (!pass1.equals(pass2)) {
        //logger.info(pass1 + " != " + pass2);
        return "Passwords don't match<br>";        
    }

    StringBuilder retVal = new StringBuilder();

    boolean pass = true;
    for (PasswordRule rule : RULES) {
        if (!rule.passRule(pass1)) {
            // logger.info(pass + "<--- " + rule.failMessage());
            retVal.append(rule.failMessage()).append(" <br>");
            pass = false;
        }
    }

    return pass ? "success" : retVal.toString();
}

По мере добавления или уточнения правил для паролей все, что вам нужно сделать, это изменить массив RULES.

ответил rolfl 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 01:14:07 +0400 2014, 01:14:07
15

Что-то, о чем я не думаю, было упомянуто:

Не возвращайте такие строки. Логика должна быть отделена от представления. Так что сделайте что-нибудь вроде

public static enum PasswordValidationResult {
    SUCCESS, IS_EMPTY, DO_NOT_MATCH, TOO_SHORT, MISSING_UPPERCASE, MISSING_LOWERCASE, MISSING_NUMBER, MISSING_SPECIAL_CHARACTER
}

public PasswordValidationResult validatePassword( String password, String repeatedPassword ) {
    // ...
}

Фактически, я бы лично пошел на еще более сложный подход, который также отделяет типы результатов, чтобы сделать изменения в логике еще проще. Но в этот момент это может быть чрезмерным. Вы получаете идею.

ответил Ingo Bürk 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 09:56:58 +0400 2014, 09:56:58
14

В общем, неплохо.

logger.info(pass1 + " = " + pass2);

Регистрация паролей может быть проблемой безопасности.

boolean hasUppercase = !pass1.equals(pass1.toLowerCase());

Если бы вы хотели узнать, есть ли черный лебедь, вы бы нарисовали все лебеди белые и посмотрели, изменилось ли что-то?

boolean hasNumber = pass1.matches(".*\\d.*");
boolean noSpecialChar = pass1.matches("[a-zA-Z0-9 ]*");

Каждая из этих строк будет хорошо, но вместе ... 3 разных способа сделать что-то одно и то же. Есть регулярные выражения для всего (нормально, не совсем):

boolean hasUppercase = pass1.matches(".*[A-Z].*");

(при условии, что ASCII, в противном случае есть группа юникодов)

boolean hasSpecialChar = pass1.matches(".*[^0-9A-Za-z].*");

Я отклонил условие (так, как и все остальные), а ^ отрицает зарядную группу.

 logger.info(pass1 + " is length < 11");

Собственный регистратор позволяет писать

 logger.info("{} is length < 11", pass1);

Это немного более читаемо и намного быстрее, если регистрация неактивна (и большая часть журналов отключена большую часть времени).

Код довольно повторяющийся. Не можете ли вы вернуть возвращаемое значение вместо очень похожего сообщения?

logger.info("Passwords = null");

В большинстве случаев оптимальная обработка null

Preconditions.checkNotNull(pass1);

который выдает NPE, если он прошел null . Большинство методов НЕ ДОЛЖНЫ разрешать null и метать ASAP, это лучше всего - вы сразу исправляете проблему и не должны ее искать позже. Используйте

Strings.nullToEmpty(s);

в получить избавиться от нулевых строк , где вы не можете бросить.

ответил maaartinus 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 00:26:21 +0400 2014, 00:26:21
11

Я взял ваш код и добавил ему return s. вы выполняли все инструкции if, даже когда отсутствовало целое поле.

Я также добавил return , если я ударил (что, вероятно, проверяет все выражения два раза сейчас, если это неверно), он сбрасывается как успешный и сохраняет некоторую обработку время.

вот что я придумал.

return "success"

Вероятно, вы можете вернуться в каждую проверку. например, если нет прописных букв, но это может стать монотонным, если вы не пройдете проверку по одному за раз.

Итак, вместо этого я возвращался только после проверок на вещи, которые полностью остановили остальные проверки, которые

  • одна из строк пароля: null
  • одна из строк пароля пуста.
  • строки не соответствуют
  • и успех во всех остальных проверках

Это последнее, действительно, зависит от того, является ли он избыточным, потому что все они проверяются дважды, если одна из проверок терпит неудачу. но проверки на самом деле выполняются только один раз, а затем значение хранится в логической переменной, поэтому мы проверяем только значение переменной, что означает, что технически сама проверка происходит только один раз.

Чтобы сделать все это, я отменил некоторые из операторов if и вернулся как можно раньше, не теряя функциональности или создавая монотонность. Я также перемещал нулевую проверку, чтобы это было первое, что было сделано, иначе будет public String validateNewPass(String pass1, String pass2){ StringBuilder retVal = new StringBuilder(); if(pass1.length() < 1 || pass2.length() < 1 ){ return "Empty fields <br>"; } if (pass1 != null && pass2 != null) { if (pass1.equals(pass2)) { logger.info(pass1 + " = " + pass2); pass1 = pass2; boolean hasUppercase = !pass1.equals(pass1.toLowerCase()); boolean hasLowercase = !pass1.equals(pass1.toUpperCase()); boolean hasNumber = pass1.matches(".*\\d.*"); boolean noSpecialChar = pass1.matches("[a-zA-Z0-9 ]*"); if (hasUppercase && hasLowercase && hasNumber && !noSpecialChar && pass1.length) { logger.info("Password validates"); return "success"; } if (pass1.length() < 11) { logger.info(pass1 + " is length < 11"); retVal.append("Password is too short. Needs to have 11 characters <br>"); } if (!hasUppercase) { logger.info(pass1 + " <-- needs uppercase"); retVal.append("Password needs an upper case <br>"); } if (!hasLowercase) { logger.info(pass1 + " <-- needs lowercase"); retVal.append("Password needs a lowercase <br>"); } if (!hasNumber) { logger.info(pass1 + "<-- needs a number"); retVal.append("Password needs a number <br>"); } if(noSpecialChar){ logger.info(pass1 + "<-- needs a specail character"); retVal.append("Password needs a special character i.e. !,@,#, etc. <br>"); } }else{ logger.info(pass1 + " != " + pass2); retVal.append("Passwords don't match<br>"); } }else{ logger.info("Passwords = null"); return "Passwords Null <br>"; } if(retVal.length() == 0){ logger.info("Password validates"); retVal.append("Success"); } return retVal.toString(); }

Сделав это, мы закончим с этим

NullPointerException

Я понял, что самый последний оператор if никогда не должен оцениваться как истинный, потому что все уже проверено, поэтому это избыточная проверка, мы также можем избавиться от этого.

ответил Malachi 19 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowFri, 19 Sep 2014 00:38:56 +0400 2014, 00:38:56
4

Почти все, что было сказано о вашем коде., но есть, по крайней мере, одна последняя вещь, чтобы сказать, что это очень важно, что maaartinus упоминает в своем answer : не записывайте пароли. Когда-либо.

В этом случае пароли должны быть секретными и известны только владельцу учетной записи. Я надеюсь, что когда вы будете хранить свой пароль, вы сделаете все, чтобы защитить его (хэш, соль и т. Д.). Проблема в том, что если вы зарегистрировали его, независимо от того, как вы его защитили, когда вы его сохранили, он будет сохранен навсегда в ясном тексте. Журналы обычно доступны и могут быть прочитаны людьми, которые, вероятно, не должны видеть пароль вашего пользователя. Это проблема безопасности и может быть серьезной проблемой безопасности.

Если вам нужен какой-то вывод, вы можете использовать logger.debug, который больше всего времени не активируется в процессе производства, но имейте в виду, что вы не должны доверять конфигурациям ведения журнала с точки зрения безопасности. Лучший вариант - удалить пароль из журнала.

ответил Marc-Andre 22 ndEurope/Moscowp30Europe/Moscow09bEurope/MoscowMon, 22 Sep 2014 18:00:08 +0400 2014, 18:00:08
0

Помимо всех других полезных ответов, я бы предложил разделить валидацию пароля из поколения HTML. Вы можете определить перечисление со значениями для разных типов ошибок + успех и вернуть один из них. Затем вы можете легко написать некоторые модульные тесты, чтобы проверить правильность поведения этого метода. Наконец, отдельный метод вызовет процедуру проверки, а затем сгенерирует соответствующий HTML.

ответил Kirill Rakhman 20 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowSat, 20 Sep 2014 16:36:56 +0400 2014, 16:36:56

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132