move the user_id field to conditions

[hackmod]

user_id는 업데이트되는 field가 아니며, 다른 값으로 변경되어도 안됩니다.
conditions으로 넣습니다.

XE는 grant all privileges를 가정하나, 좀 더 안전하게 만들려면 alter,drop는 revoke시키고,
update는 특정 컬럼에 유효하게 고칠 수도 있을 것입니다.

간단히 xe_member에 대해서 user_id 및 member_srl에 대해서 update를 revoke시키고, 나머지 컬럼들만 grant update(...)한 후에 테스트를 해보니 이 문제때문에 제대로 작동을 하지 않았습니다.

[kijin]

대부분의 사이트에서는 아이디 변경을 허용하지 않지만, 사이트에 따라서는 아이디 변경을 허용하는 모듈을 만들어 쓸 수도 있습니다. 타 CMS와 달리 XE에서는 user_id가 아닌 member_srl로 회원을 식별하며 아이디는 로그인에 사용할 뿐이므로, user_id가 변경된다고 해서 특별히 문제가 될 일도 없고요.

[hackmod]

XE 기본 기능은 user_id 변경을 지원하지 않고 있습니다. 그렇다면 이에 맞게 기본 xml은 user_id변경이 되면 안되는 쿼리문을 가져야 맞겠죠. 모듈이 user_id 변경을 지원한다면 해당 모듈이 자체적으로 query문을 조금 고치고 그걸 쓰면 그만입니다.

[kijin]

기본으로 지원하지 않는 것과 허용하지 않는 것은 다릅니다. 서드파티 자료는 가능하면 코어에서 제공하는 함수와 쿼리를 활용하는 것이 좋습니다. 지금도 누군가는 해당 쿼리에서 user_id 변경을 허용하는 것을 이용하여 어떤 기능을 만들어 쓰고 있을지도 모릅니다. 코어는 확장 기능을 편리하게 만들어쓸 수 있도록 안정적인 기반이 되어 주어야 합니다. 10년 넘게 잘 되던 것이 갑자기 안 되면 @hackmod 님이 책임지고 고쳐주실 건가요?

현실적으로 버그를 일으키는 기능도 아니고 보안취약점도 아닌데, 님이 특수한 설정 하에서 사용하는 데 지장을 준다는 이유만으로 XE를 사용하는 다른 모든 사용자에게서 update 권한을 박탈하려고 하시면 곤란합니다.

게다가 member_srl만 condition으로 주어도 충분히 레코드를 특정할 수 있는데 user_id는 왜 condition으로 옮기셨는지도 의문이군요. user_id를 PK로 사용하는 다른 CMS의 사고방식을 XE에 끌고 오신 것이 아닌가 싶습니다. 심지어 notnull 조건이 붙어 있어서 user_id를 넣어주지 않으면 쿼리가 아예 실행되지 않습니다. 이건 개선이 아니라 개악이네요.

[hackmod]

이유없이 너무 공격적이신것 같네요?

XE는 코어에서 user_id 변경을 지원하지 않으니 그 의도에 맞게 user_id를 변경을 지원하는 쿼리문은 의도에 맞지 않다고 봤습니다.

updateMember.xml은 다음의 파일에서 사용되고 있으며,
executeQuery('member.updateMember')는 여기서만 호출되고 있습니다.
https://github.com/xpressengine/xe-core/blob/develop/modules/member/member.controller.php#L2440

user_id가 지정되지 않은 경우 다음과 같이 user_id가 세팅됩니다.

if(!$args->user_id) $args->user_id = $orgMemberInfo->user_id;

member_srl 를 이용해 유저정보를 다시 가져오기대문에 user_id는 항상 null이 아닙니다.

그리고 user_id를 손쉽게 변경이 가능하게 허락한 커뮤니티가 과연 얼마나 될까요? 현재 운용중인 커뮤니티는 7년째 XE를 사용중입니다.

---

현재 XE는 단순히 form상으로 id를 편집하지 못하게 disable시킨 상태이므로 사용자가 임의로 폼을 고쳐 user_id를 수정하는것이 가능한 것처럼 보입니다.

그러나 잘 살펴보면, 이렇게 사용자가 임의로 user_id를 고치는 것은 이미 막혀있습니다.

https://github.com/xpressengine/xe-core/blob/develop/modules/member/member.controller.php#L2364

(위 줄을 커멘트 처리하면 user_id를 임의로 바꾸는 것이 가능해짐)
따라서 xml 쿼리문상으로 고치더라도 전혀 문제가 되지 않는다고 보고 PR을 올린것입니다.

[bjrambo]

@hackmod 음.. 이미 https://xetown.com/thirdparties/1092 아이템 샵이라는 서드파티에서는 아이디 변경권이라는 포인트 아이템이 존재하고 있고 결국은 아이디는 변경될 수 있는 여지가 있습니다.

만약 지금 PR주신것처럼 바꾸게 된다면 이러한 서드파티를 잘 이용하고 있던 유저들은(심지어 유료이며 사용자들도 꾀있습니다.) 불편함을 겪을 수 있다는 것입니다.

실제로 XE함수 및 쿼리문을 이미 서드파티에서 많이 사용하고 있기도 합니다.

그래서 이런 변경점에 대해서 부정적인 시각을 가질 수 있습니다.

코어가 가지고 있던 원래 기본 기능이 변경되면 지금까지 쭉 유지되어왓던 사이트들에서 모두 문제가 발생되기 마련입니다.

따라서 PR이 코어에 적용되지 않는 것이 좋을 것 같아요.

[hackmod]

XE 게시판에서 서드파티 얘기만 나오면 논의가 급격하게 중단되거나 식어버리더군요.

궁금증이 생겨 더 찾아봤습니다. user_id 변경을 막은 PR입니다. 513077f 이때 왜 막았는지 이유를 생각해본다면 서드파티 프로그램에 임의로 user_id 변경을 하게 하는 것이 어떤 모양으로 지원되길래 유료인지 아닌지 모르겠으나, 그다지 바람직해보이지 않네요.

현재 운용중인 커뮤니티는, 닉네임 등을 사용자가 바꾸는 것도 몇개월에 한번씩만 고칠 수 있도록 제한하고 있는 상태입니다.

---

코어에서는 막혀있는데 서드파티 모듈에서 user_id를 고치는 것이 가능하다면 어떤 방식일까요?
저라면 queries/updateMember.xml를 서드파티 모듈에 복사해서 혹시 모를 core 업데이트시 오작동할 수 있는 여지를 제거했을 것 같습니다.

[kijin]

@hackmod

코어에서는 막혀있는데 서드파티 모듈에서 user_id를 고치는 것이 가능하다면 어떤 방식일까요?

말씀하셨다시피 executeQuery('member.updateMember') 하면 됩니다. 많은 서드파티 자료들이 코어의 쿼리를 직접 호출하고 있습니다.

XE 게시판에서 서드파티 얘기만 나오면 논의가 급격하게 중단되거나 식어버리더군요.

제3자가 멀쩡하게 잘 쓰고 있던 기능을 고장낼 가능성이 있는 패치라면 당연히 조심해야지요. 님 사이트에서 해당 기능을 쓰는지 안 쓰는지는 상관없습니다. XE는 님 혼자 쓰는 물건이 아니니까요.

서드파티 얘기가 나와도 논의가 끊기지 않도록 하는 방법을 알려드릴게요. 기존 유저들에게 영향을 주지 않도록 코드를 짜면 됩니다. 이를 위해 호환 레이어를 제공해야 할 수도 있고, 다양한 환경에서 장기간 테스트해야 할 수도 있고, 특정 옵션을 켰을 때만 새 기능이 작동하도록 해야 할 수도 있습니다. 예를 들어 #889 #894 는 다양한 상황에서의 호환성 문제를 고려하여 보완한 끝에 9개월만에 코어에 반영된 바 있습니다.

이 PR의 경우에는 updateMember.xml에서 user_id 부분의 notnull="notnull"을 제거하고, $args->user_id = $orgMemberInfo->user_id; 이것도 unset($args->user_id);로 변경하면 님이 원하시는 결과를 얻을 수 있을 것 같습니다. 코어의 작동 방식에도 변함이 없고, 쿼리를 직접 호출하는 서드파티 자료에도 영향을 주지 않고, user_id 컬럼에 update 권한을 줄 필요도 없게 되지요. 조금만 생각해 보면 서드파티 자료를 배려하는 것은 어렵지 않습니다.

[hackmod]

게다가 member_srl만 condition으로 주어도 충분히 레코드를 특정할 수 있는데 user_id는 왜 condition으로 옮기셨는지도 의문이군요. user_id를 PK로 사용하는 다른 CMS의 사고방식을 XE에 끌고 오신 것이 아닌가 싶습니다. 심지어 notnull 조건이 붙어 있어서 user_id를 넣어주지 않으면 쿼리가 아예 실행되지 않습니다. 이건 개선이 아니라 개악이네요.

member_srl 컨디션만으로도 유효한 쿼리이므로 user_id 컨디션에서 notnull을 빼봤습니다.

• updateMember()를 호출하는 경우도 기존과 같은 결과. 문제 없음
• 서드파티 모듈이 user_id를 바꾸는 기능이 아닌 이상 정상 작동
• executeQuery('member.updateMember')를 직접호출하는 경우 문제 발생 가능

서드파티 얘기가 나와도 논의가 끊기지 않도록 하는 방법을 알려드릴게요. 기존 유저들에게 영향을 주지 않도록 코드를 짜면 됩니다. 이를 위해 호환 레이어를 제공해야 할 수도 있고, 다양한 환경에서 장기간 테스트해야 할 수도 있고, 특정 옵션을 켰을 때만 새 기능이 작동하도록 해야 할 수도 있습니다. 예를 들어 #889 #894 는 다양한 상황에서의 호환성 문제를 고려하여 보완한 끝에 9개월만에 코어에 반영된 바 있습니다.

이 PR의 경우에는 updateMember.xml에서 user_id 부분의 notnull="notnull"을 제거하고, $args->user_id = $orgMemberInfo->user_id; 이것도 unset($args->user_id);로 변경하면 님이 원하시는 결과를 얻을 수 있을 것 같습니다. 코어의 작동 방식에도 변함이 없고, 쿼리를 직접 호출하는 서드파티 자료에도 영향을 주지 않고, user_id 컬럼에 update 권한을 줄 필요도 없게 되지요. 조금만 생각해 보면 서드파티 자료를 배려하는 것은 어렵지 않습니다.

흠.. 아래처럼 고치면 $user_id를 세팅하지 않았을 경우 update 컬럼에 포함되지는 않겠네요.

diff --git a/modules/member/queries/updateMember.xml b/modules/member/queries/updateMember.xml
index 71cc337..e49e10d 100644
--- a/modules/member/queries/updateMember.xml
+++ b/modules/member/queries/updateMember.xml
@@ -6,7 +6,7 @@
         <column name="password" var="password" notnull="notnull" />
         <column name="user_name" var="user_name" notnull="notnull" minlength="2" maxlength="40" />
         <column name="nick_name" var="nick_name" notnull="notnull" minlength="2" maxlength="40" />
-        <column name="user_id" var="user_id" notnull="notnull" />
+        <column name="user_id" var="user_id" />
                <column name="email_address" var="email_address" notnull="notnull"/>
                <column name="email_id" var="email_id" />
                <column name="email_host" var="email_host" />

[kijin]

흠.. 아래처럼 고치면 $user_id를 세팅하지 않았을 경우 update 컬럼에 포함되지는 않겠네요.

네, 기존의 쿼리에서 notnull만 빼고 member.controller.php에서 unset()해주면 코어와 서드파티의 작동 방식이 모두 기존과 동일하게 유지될 것 같습니다. 그러면 저도 반대할 명분이 없습니다.^^

컨디션 쪽으로 옮기셨던 user_id 조건은... 음... 코어에서 user_id로 회원을 특정하여 업데이트하는 기능을 지원하지 않는데요. 님의 논리에 따르면 코어에서 지원하지 않는 기능이니 해당 컨디션은 없어야 하는데, 왜 추가하셨는지 아직도 모르겠다는;;;

[hackmod]

흠.. 아래처럼 고치면 $user_id를 세팅하지 않았을 경우 update 컬럼에 포함되지는 않겠네요.

네, 기존의 쿼리에서 notnull만 빼고 member.controller.php에서 unset()해주면 코어와 서드파티의 작동 방식이 모두 기존과 동일하게 유지될 것 같습니다. 그러면 저도 반대할 명분이 없습니다.^^

컨디션 쪽으로 옮기셨던 user_id 조건은... 음... 코어에서 user_id로 회원을 특정하여 업데이트하는 기능을 지원하지 않는데요. 님의 논리에 따르면 코어에서 지원하지 않는 기능이니 해당 컨디션은 없어야 하는데, 왜 추가하셨는지 아직도 모르겠다는;;;

음.. user_id를 조건절에 넣는 경우 다음과 같은 효과가 있을 것이고, updateMember()와 xml 쿼리 레벨에서 작동이 일관성있게 작동된다고 본것이죠.

• 코어에서는 user_id 변경을 지원하지 않으니 => user_id가 조건절에 있건 없건 상관 없음. (updateMember()는 현재 user_id를 변경하려 해도 막혀있음. (다만 변경을 시도해도 오류를 내거나 하지는 않음))
• executeQuery("member.updateMember")를 직접 호출하는 경우에 user_id 변경을 시도하면 쿼리 레벨의 오류가 남. (이 경우 서드파티 모듈 호환 안될 수 있음)