gnuboard / gnuboard5

그누보드5 (영카트 포함) 공개형 Git
Other
311 stars 250 forks source link

새로운 인증 수단을 위한 패스워드 확인 없는 로그인 처리 지원 #205

Closed kkigomi closed 1 year ago

kkigomi commented 1 year ago

https://github.com/gnuboard/gnuboard5/blob/dbc8e1676a4cd3561a74f1d756320a7d82b45bb4/bbs/login_check.php#L30-L39

소셜로그인 기능만을 위해 login_check.php 파일에서 패스워드를 검증하지않고 로그인 처리를 위한 코드입니다.

내장된 소셜로그인 외에도 별도의 기능으로 인증처리 후 그누보드 로그인 세션을 생성(로그인 상태로) 할수 있는 기능을 제공해주시기 바랍니다.

물론 세션만 생성하면 되기 때문에 별도로 구현하면 되지만 login_check.php 파일 내의 이벤트 처리나 포인트, 차단 계정, 이메일 인증 등의 과정이 처리되고 있으므로 이를 동일하게 구현해야하는 문제가 있습니다.

이는 login_check.php가 변경되어 보안체크 등이 추가되면 추가 기능 개발자가 이를 대응해야하는 유지보수 부담 및 업데이트되지 않은 기능으로인해 사용자가 보안상 위험에 노출될수 있습니다.

패스워드 검증 과정을 건너뛸수있도록 방법 제공과 로그인 전후의 보안체크 및 차단 계정 확인 등의 일련의 과정을 이벤트를 수신하여 처리하도록 변경하는 등 개선이되면 좋겠습니다.

패스워드 검증 및 세션 생성 과정을 별도의 함수로 분리하고 패스워드 검증을 패스할수있는 옵션을 처리하면 간단할 것같습니다. 해당 함수 내에서 이벤트 처리를 추가하여 차단계정, 이메일 인증 등을 확인하는 로직이 이벤트를 수신하여 동작시키면 될 것같네요.

많은 변경이 부담된다면 member_login_check_before 이벤트에서 패스워드 검증 과정을 건너뛸수있는 변수 등을 설정할수있도록 처리해도 괜찮을것같습니다. 근데 이벤트 등록 시점이 맞을지가 문제겠군요.

플러그인 개발중이며 다수의 일반 사용자가 안전하게 사용할수 있도록 로직 개선 부탁드립니다.

kkigomi commented 1 year ago

/bbs/register_form.php에서도 소셜로그인 기능만을 위한 패스워드 검증을 우회하는 로직을 확인했습니다. 추가 인증방법 등을 지원하는 범용으로 활용할수 있도록 개선 부탁드립니다.

thisgun commented 1 year ago

안녕하세요. SIR 입니다. 의견 주셔서 감사합니다.

브랜치 next_v 에 해당 내용을 적용할수 있게 코드를 수정해 놓았습니다. https://github.com/gnuboard/gnuboard5/tree/next_v

해당 코드 변경 내용 https://github.com/gnuboard/gnuboard5/commit/4c6b833f73805d9bb34f4a3b1505d2ad0623be91

브랜치 next_v 를 적용해 주신 상태에서

extend/아무파일.php 에 아래 코드를 넣으면, 패스워드를 묻지 않고 회원가입 로그인 할수 있습니다.

// bbs/login_check.php 에서 비밀번호 공백 체크를 무시 합니다.
add_replace('check_empty_member_login_password', 'pass_login_check_password', 1, 2);
function pass_login_check_password($is_input_password, $mb_id){

    // 여기서 조건이 맞으면 false 을 리턴하여 비밀번호 공백 체크를 무시 합니다.
    if (1 == 1) {

        return false;
    }

    return $is_input_password;
}

// bbs/login_check.php 에서 비밀번호가 맞는지 틀리는지 비교과정을 무시 합니다.
add_replace('login_check_need_not_password', 'pass_login_check_need_not_password', 1, 5);
function pass_login_check_need_not_password($is_social_password_check, $mb_id, $mb_password, $mb, $is_social_login){

    // 여기서 조건이 맞으면 true 를 리턴하여 비밀번호 비교를 무시합니다.
    if (1 == 1) {

        // 회원정보 수정시 패스워드를 묻지 않게 session 값을 저장합니다.
        set_session('is_member_no_password', 1);

        return true;
    }

    return $is_social_password_check;
}

// bbs/register_form_update.php 에서 자동등록방지 체크값을 무시 합니다.
add_replace('register_member_chk_captcha', 'pass_register_chk_captcha', 1, 2);
function pass_register_chk_captcha($boolean_captcha_result, $w){

    // 여기서 조건이 맞으면 false 을 리턴하여 자동등록방지 체크값을 무시 합니다.
    if (1 == 1) {

        // 회원가입시에만 적용
        if ($w=='') {
            // 패스워드를 검증하지 않더라고 패스워드는 난수로 저장해야 합니다. POST 입력값이 없으면 난수로 저장합니다.
            if (! isset($_POST['mb_password']))
                $_POST['mb_password'] = md5(pack('V*', rand(), rand(), rand(), rand()));

            return false;
        }
    }

    return $boolean_captcha_result;
}

// bbs/register_form_update.php 에서 비밀번호 체크와 비밀번호 비교과정을 무시합니다.
add_replace('register_member_password_check', 'pass_register_password', 1, 5);
function pass_register_password($is_boolean, $mb_id, $mb_nick, $mb_email, $w){

    // 여기서 조건이 맞으면 false 을 리턴하여 비밀번호 체크 비교를 무시 합니다.
    if (1 == 1) {

        // 회원가입시에만 적용
        if ($w=='') {
            if (! get_session('ss_check_mb_id'))
                set_session('ss_check_mb_id', $mb_id);

            if (! get_session('ss_check_mb_nick'))
                set_session('ss_check_mb_nick', $mb_nick);

            if (! get_session('ss_check_mb_email'))
                set_session('ss_check_mb_email', $mb_email);

            // 회원정보 수정시 패스워드를 묻지 않게 session 값을 저장합니다.
            set_session('is_member_no_password', 1);

            return false;
        }
    }

    return $is_boolean;
}

// bbs/member_confirm.php 에서 비밀번호 체크와 비밀번호 비교과정을 무시합니다.
add_replace('member_confirm_next_url', 'edit_member_confirm_next_url', 1, 1);
function edit_member_confirm_next_url($url=''){

    // 회원정보 수정시 해당 세션값이 있으면, 기존의 social_nonce 을 이용하여 nonce 값을 세션에 저장 후에 register_form.php 으로 리다이렉트 합니다.
    if ($url === 'register_form.php' && get_session('is_member_no_password')) {

         // 여기서 조건을 체크하여 맞으면 nonce 값을 부여하여 회원정보수정으로 리다이렉트 합니다.
        if (1 == 1) {
            $provider_name = 'nopassword';
            $social_token = social_nonce_create($provider_name);
            set_session('social_link_token', $social_token);

            $params = array('provider'=>$provider_name);

            goto_url(get_params_merge_url($params, $url));
        }
    }

    return $url;
}

이렇게 해도 괜찮으시면, 다음 패치에 포함하여 배포하겠습니다. 감사합니다.

kkigomi commented 1 year ago

login_check.php 파일을 include하여 사용했으며, 추가해주신 Hook으로 패스워드 검증 과정을 정상적으로 우회할 수 있었습니다. 이 과정에서 $_POST['mb_id'] 값만 적절한 값으로 설정했습니다. goto_url() 함수 호출로인해 이 또한 우회해야하는 문제가 있긴하네요.

/**
 * 회원 로그인 처리
 * @param string $mb_id 회원 ID
 * @param string|null $mb_password 회원 Password
 * @param bool $auto_login 자동로그인
 * @param bool $bypass_password_check true로 전달 시 패스워드 검증을 처리하지 않음
 * @return bool
 */
function member_login($mb_id, $mb_password = null, $auto_login = false, $bypass_password_check = false)
{
    // global $g5, $config;
    // $mb = get_member($mb_id);

    // 사전 검사. 패스워드 검증 및 차단 아이디 등등
        // 검증을 통과하지 못하면
        // return false

    // 회원아이디 세션 생성

    // 포인트, 영카트 장바구니, data 폴더 권한 검사 등등

    // return true;
}

이런식으로 함수로 분리되면 사용하기 더 편리할 것 같습니다.

감사합니다.


회원정보 수정폼은 register_form_update.php 파일 외에도 register_form.php 파일에서도 패스워드 검증을 사용하고 있습니다.