-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor: 로그인 방식 변경 #235
Refactor: 로그인 방식 변경 #235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 !
name: 'email', | ||
type: 'string' | ||
}) | ||
async profile(@Query('email') email: string, @Res() res: Response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마이페이지는 아직 살아있어서 이 부분은 추가해야 되지 않을까요?
backend/src/auth/auth.service.ts
Outdated
throw new HttpException('해당 사용자가 없습니다.', HttpStatus.NOT_FOUND); | ||
} | ||
const userInfo = new UserInfoDto({ username: user.username, email: user.email }); | ||
return await this.generateCookie(userInfo); | ||
} | ||
|
||
async findUser(email: string): Promise<User> { | ||
return await this.userModel.findOne({ email: email }); | ||
} | ||
|
||
async generateCookie(userInfo: UserInfoDto) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드 이름도 수정하면 좋을 것 같습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findUserByEmail
로 수정했습니다!
backend/src/auth/auth.service.ts
Outdated
|
||
async signIn(signInDto: SignInDto): Promise<string> { | ||
const user = await this.findUser(signInDto.email); | ||
const validatedPassword = await bcrypt.compare(signInDto.password, user.password); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 22라인 bcrypt로 해시하는 부분과 29라인 해싱된 값 비교하는 부분은 util로 빼도 좋을 것 같다는 생각이 있는데 승연님은 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util로 뺐는데 util이 클래스여서 매번 new GenerateUtil
선언을 해주고 써야하는게 반복되더라고요. 클래스가 아니고 함수를 정의하도록 변경했는데 이 부분 괜찮을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 좋은 것 같아요 !
backend/src/auth/auth.controller.ts
Outdated
throw new HttpException('해당 사용자가 없습니다.', HttpStatus.NOT_FOUND); | ||
} | ||
const result = await this.authService.signIn(signInDto); | ||
return res.status(HttpStatus.OK).send(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
body로 access token을 넘겨주는 부분 같은데, 바로 넘겨주기보다는 signInResDto같은 객체로 한번 감싸줘서 내려주면 좋을 것 같습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 경우에만 쓰이는 응답형식이다보니 dto를 따로 만들면 dto가 너무 많아질 것 같아서 일단은 { token: result }
이런 식으로 컨트롤러에서 형식을 바꿔 리턴하도록 했습니다. 따로 dto를 선언해주는게 나을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 이건 뭔가 사람 성향차이 같아서 ! 일단 이대로도 괜찮은 것 같습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ~!
backend/src/auth/auth.controller.ts
Outdated
throw new HttpException('해당 사용자가 없습니다.', HttpStatus.NOT_FOUND); | ||
} | ||
const result = await this.authService.signIn(signInDto); | ||
return res.status(HttpStatus.OK).send(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 이건 뭔가 사람 성향차이 같아서 ! 일단 이대로도 괜찮은 것 같습니다 !
작업 개요
close #234
Refactor: 로그인 방식 변경
작업 사항
email과 password를 사용하여 회원가입 및 로그인을 진행하도록 수정했습니다.
user.schema.ts
에서 profile 필드를 제거하고 password 필드를 추가했습니다.