Conversation
…발급, apikey와 비밀번호 hash처리 )
src/models/dashboardClientModel.ts
Outdated
| email: { type: DataTypes.STRING, allowNull: false, primaryKey: true, unique: true }, | ||
| hashedPassword: { type: DataTypes.STRING, allowNull: false }, | ||
| domain: { type: DataTypes.STRING, allowNull: false, unique: true }, | ||
| hashedApiKey: { type: DataTypes.STRING, allowNull: false }, |
There was a problem hiding this comment.
해시한 api 키를 저장하면 사용자한테 api 키를 어떻게 알려주나요? 해시한 값에서 원본 값을 알아낼 수 있나요?
There was a problem hiding this comment.
사용자에게 알려줄수 없는 문제가 발생해서 sha-256으로 refactor 진행하였습니다.
There was a problem hiding this comment.
8af0afa 이 커밋 말씀하시는 건가요? 이렇게 하면 양방향으로 값을 알아낼 수 있나요?
There was a problem hiding this comment.
아니요 sha-256도 단방향이긴한데 기존 api를 암호화할때 같은값으로 암호화할수있어서 선택했습니다.
| where: { email }, | ||
| attributes: ['hashedPassword', 'domain'], | ||
| raw: true, | ||
| })) as ClientType | null; |
| if (!client) { | ||
| throw new Error('등록된 유저가 아닙니다.'); | ||
| } | ||
| const isValidPassword = await bcrypt.compare(password, client.hashedPassword); | ||
| if (!isValidPassword) { | ||
| throw new Error('비밀번호가 올바르지 않습니다.'); | ||
| } |
There was a problem hiding this comment.
이 에러들을 클라이언트까지 그대로 보내나요? 인증 실패했을 땐 보통 이렇게 자세하게 사유를 알려주지 않아요.
There was a problem hiding this comment.
앗 한번 확인해보겠습니다 저는 클라이언트까지 전송하는줄 알았습니다.
| if (!isValidPassword) { | ||
| throw new Error('비밀번호가 올바르지 않습니다.'); | ||
| } | ||
| return client.domain; |
There was a problem hiding this comment.
로그인 성공했을때 domain을 전송해서 고객사 대시보드에서 어떤 domian으로 사용하는지 판별하기 위함이였습니다.
…roller(email, domain을 session에 저장), 미들웨어 session 생성
| secret: process.env.SESSION_SECRET as string, | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| cookie: { secure: false, httpOnly: true }, |
There was a problem hiding this comment.
cookie secure는 https환경에서만 쿠키가 전송되는건데 프로젝트 테스트를 위해서 로컬에있는 프로젝트로 테스트해보려고했습니다.
지금 변경해놓을까요??
…에러 추가), dashboardRoutes (대시보드에서사용하는 api url 수정)
| const domain = req.session.client?.domain; | ||
| if (!domain) { | ||
| next(new Error('로그인 필요')); | ||
| return; | ||
| } |
There was a problem hiding this comment.
이 코드 중복이 엄청 많은데 미들웨어로 처리해보세요.
별개로 domain이 있는지 없는지를 보고 로그인 여부를 판단하는 것도 어색합니다.
There was a problem hiding this comment.
넵 중복된코드 미들웨어 활용해서 처리했습니다.
domain이있는지 없는지로 로그인 여부 판단하는게 어색하다고하셨는데 1차원적으로 "도메인이 없다" 라고 표현하는게 더 자연스러울까요?
src/routes/dashBoardRoutes.ts
Outdated
| dashboardRouter.get('/dashboard/visitorsPageByPeriodCount', dashboardController.getPageViewCount); | ||
| dashboardRouter.get( | ||
| '/dashboard/onlineUsersCount', | ||
| hasDashboardDomain, |
There was a problem hiding this comment.
근데 도메인 유무를 왜 확인해야 하나요? 무조건 있어야 하는 거 아닌가요? 스키마상으로 domain이 nullable한가요?
로그인 여부를 확인해야지 왜 도메인 유무를 확인하는지 알기가 어렵네요.
There was a problem hiding this comment.
스키마상으로 domain은 nullable 하지않습니다 무조건 존재합니다 저렇게 코드를 작성한 이유는 타입스크립트에서 인식할수있도록 작성했습니다.
There was a problem hiding this comment.
- '저렇게'가 뭔가요?
- nullable 하지 않으면 타입스트립트도 그렇게 이해를 할 텐데 왜 따로 처리를 해야 하나요?
- nullable 하지 않은 domain 유무를 왜 미들웨어로 판단해야 하냐는 질문이었어요.
There was a problem hiding this comment.
- '저렇게'란 domain이 nullable하지않은데 if문으로 있는지 없는지 판단하게 한 코드입니다
- 현재 로그인했을때 session에 저장되는 사용자의 도메인으로 호출하는 방법을 사용하고있습니다 따라서 인식을 못하고있는것 같습니다
- 맞습니다 nullable 하지않은 domain을 미들웨어로 판단해야하는것이 지금 타입스크립트가 인지를 못하고있어서였습니다
- 지금 확인해보니 controller단에서 req.session.client?.domain을 가져와서 service단으로 넘겨주면 타입오류가 안나는것같습니다
There was a problem hiding this comment.
이 코드리뷰에서 내부 구현은 관심사가 아니고요, hasDashboardDomain, 그러니까 domain 유무를 왜 확인하냐는 겁니다. 도메인은 무조건 있는 건데.
There was a problem hiding this comment.
그럼 위 댓글에서 말씀하신 '실제 세션 데이터'라는 건 어차피 실체가 없는 거 아닌가요?
There was a problem hiding this comment.
express-session의 타입 정의를 보게되면 session: session.Session & Partial<session.SessionData>;
SessionData가 Partial<>감싸져있는데 SessionData내부에서 client를 넣어도 역시 Partial 때문에 보장이 안되고 있습니다
멘토님이 말씀하신 Request를 확장은 혹시 clinet를 넣어 확장하여 미들웨어에서 req.client = req.session.client 이렇게 우회하라는 말씀이신가요?
There was a problem hiding this comment.
interface AuthenticatedRequest extends Request {
session: Session & {
client: {
email: string;
domain: string;
}
}
}만들고
async (req: AuthenticatedRequest, res: Response) => {}이렇게 써보세요.
There was a problem hiding this comment.
제시해주신 방법도 전에 시도해봤었는데 라이터 쪽 RequestHandler 타입에러가 있어서 실패했었습니다
그러면 RequsetHandler 까지 타입 확장해서 해결해야하는 문제일가요?
There was a problem hiding this comment.
HOF 사용하면 될 것 같은데 힘들면 라우터 쪽에 as RequestHandler 사용하세요~
|
2 participants
feature/ dashboardClient controller, service, model 생성, 회원가입기능(apikey발급, apikey와 비밀번호 hash처리 )