feat(be): modify signup logic#3598
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the user sign-up process by adding nickname and jobType fields to the SignUpDto and implementing validation logic based on the user's job type. Specifically, it removes the @skku.edu email restriction during the initial pin generation and instead enforces it during registration for users who identify as Sungkyunkwan University (SKKU) students, alongside requiring their student ID and major. Other college students are required to provide their college name. Feedback on these changes highlights potential issues with the validation logic: the check for SKKU affiliation can be easily bypassed with variations like '성균관대' or 'SKKU', and the email domain check is case-sensitive and should convert the email to lowercase first to prevent false negatives.
| if ( | ||
| signUpDto.jobType === JobType.CollegeStudent && | ||
| signUpDto.college?.includes('성균관대학교') | ||
| ) { |
There was a problem hiding this comment.
성균관대학교 학생인 경우 학번, 학과 및 @skku.edu 이메일 인증을 강제하고 있으나, signUpDto.college?.includes('성균관대학교') 조건은 '성균관대' 또는 'SKKU'와 같은 입력에 대해 우회될 수 있습니다. 대학 이름 입력값을 정규화하거나 대표적인 약칭들을 함께 검사하는 로직을 추가하는 것이 안전합니다.
if (
signUpDto.jobType === JobType.CollegeStudent &&
signUpDto.college &&
/성균관대|skku/i.test(signUpDto.college)
) {
There was a problem hiding this comment.
프론트에서 쓰는 대학교 이름 소스에서 우리 학교 공식 명칭만 잘 확인해보면 문제없을듯~
| if (!email.endsWith('@skku.edu')) { | ||
| throw new UnprocessableDataException('SKKU not using @skku.edu email') | ||
| } |
There was a problem hiding this comment.
email.endsWith('@skku.edu') 검사는 대소문자를 구분합니다. 사용자가 이메일 주소에 대문자를 포함하여 인증받은 경우(예: user@SKKU.EDU), 이 조건문에서 검증이 실패할 수 있습니다. 안전하게 비교하기 위해 이메일을 소문자로 변환한 후 검사하는 것이 좋습니다.
| if (!email.endsWith('@skku.edu')) { | |
| throw new UnprocessableDataException('SKKU not using @skku.edu email') | |
| } | |
| if (!email.toLowerCase().endsWith('@skku.edu')) { | |
| throw new UnprocessableDataException('SKKU not using @skku.edu email') | |
| } |
| data: { | ||
| realName: createUserProfileData.realName, | ||
| user: { | ||
| connect: { id: createUserProfileData.userId } | ||
| } | ||
| } |
There was a problem hiding this comment.
위 PR에서 profileImageUrl이 not nullable 하게 바뀔 예정이라 기본 url 설정이 필요합니다.
CreateUserProfileData의 userId를 사용하거나 migration PR과 동일하게 username을 사용해서 기본 profileImageUrl을 설정해주세요.
There was a problem hiding this comment.
migration과 동일하게 username으로 기본 url 설정 진행했습니다.
commited 5586a45
| @IsOptional() | ||
| @IsString() | ||
| readonly nickname?: string | ||
|
|
There was a problem hiding this comment.
UpdateUserDto가 사용되는 updateUser를 확인해봤더니,
UpdateUserDto에서 profileImageUrl도 같이 받아야 할 거 같네요.
일단 지금은 string에 optional 하게 두고, Url 타입인지 검사하면 될 것 같아요.
| userProfile: { | ||
| update: { realName: updateUserDto.realName } | ||
| } |
There was a problem hiding this comment.
여기에 profileImage도 update Data에 들어가야 할 것 같습니다.
| userProfile: { | ||
| select: { | ||
| realName: true | ||
| } | ||
| } |
There was a problem hiding this comment.
여기서도 return 시에 함께 조회해서 반환해야 할 것 같네요
| if ( | ||
| signUpDto.jobType === JobType.CollegeStudent && | ||
| signUpDto.college?.includes('성균관대학교') | ||
| ) { |
There was a problem hiding this comment.
프론트에서 쓰는 대학교 이름 소스에서 우리 학교 공식 명칭만 잘 확인해보면 문제없을듯~
| if (!email.endsWith('@skku.edu')) { | ||
| throw new UnprocessableDataException('SKKU not using @skku.edu email') | ||
| } |
3 participants
Description
Additional context
화이트리스트 제거에 따른 회원가입 로직 수정
SignUpDto에nickname,jobType필드 추가UpdateUserDto에nickname필드 추가@skku.edu도메인 제한 제거jobType에 따른 회원가입 분기 처리@skku.edu이메일 검증createUser에nickname,jobType,profileImageUrl저장 추가getUserProfile에nickname,jobType,profileImageUrl응답 추가UpdateUserDto에nickname,profileImageUrl필드 추가updateUser에nickname,profileImageUrl수정 기능 추가Before submitting the PR, please make sure you do the following
fixes #123).