-
Notifications
You must be signed in to change notification settings - Fork 0
[Authentication] 로그인 필터 및 핸들러 개발 #49
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
Changes from all commits
09ca7a1
d24894c
eab5ba2
9c7e3d0
ee74fbe
9f8f8d6
022bba4
2a9c8f8
1902a3e
6e064fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package app.handler; | ||
|
|
||
| import app.db.Database; | ||
| import app.model.User; | ||
| import config.VariableConfig; | ||
| import exception.ErrorCode; | ||
| import exception.ServiceException; | ||
| import http.HttpMethod; | ||
| import http.response.CookieBuilder; | ||
| import web.dispatch.argument.QueryParameters; | ||
| import web.handler.SingleArgHandler; | ||
| import web.response.HandlerResponse; | ||
| import web.response.RedirectResponse; | ||
| import web.session.SessionEntity; | ||
| import web.session.SessionStorage; | ||
|
|
||
| public class LoginWithPost extends SingleArgHandler<QueryParameters> { | ||
| private final SessionStorage sessionManager; | ||
|
|
||
| public LoginWithPost(SessionStorage sessionManager) { | ||
| super(HttpMethod.POST, "/user/login"); | ||
| this.sessionManager = sessionManager; | ||
| } | ||
|
|
||
| @Override | ||
| public HandlerResponse handle(QueryParameters params) { | ||
| String email = params.getQueryValue("email") | ||
| .orElseThrow(() -> new ServiceException(ErrorCode.LOGIN_FAILED, "email required")); | ||
|
|
||
| String password = params.getQueryValue("password") | ||
| .orElseThrow(() -> new ServiceException(ErrorCode.LOGIN_FAILED, "password required")); | ||
|
|
||
| User user = Database.findUserByEmail(email) | ||
| .orElseThrow(() -> new ServiceException(ErrorCode.LOGIN_FAILED)); | ||
|
|
||
| if (!user.getPassword().equals(password)) { | ||
| throw new ServiceException(ErrorCode.LOGIN_FAILED); | ||
|
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 보안 위험: 평문 비밀번호를 데이터베이스에 저장하고 직접 비교하면 안 됩니다. 해킹으로 데이터베이스가 유출되면 모든 사용자의 비밀번호가 노출됩니다.\n\n필수 개선: 비밀번호를 해싱(예: bcrypt, PBKDF2)으로 저장하고, 로그인 시에는 입력 비밀번호를 동일한 해시 알고리즘으로 검증하세요." |
||
| } | ||
|
|
||
| SessionEntity session = sessionManager.create( | ||
| user.getUserId(), | ||
| user.getUserRole()); | ||
|
|
||
| RedirectResponse res = RedirectResponse.to("/"); | ||
| res.setCookie( | ||
| CookieBuilder.of("SID", session.getId()) | ||
| .path("/") | ||
| .httpOnly() | ||
| .sameSite(CookieBuilder.SameSite.LAX) | ||
| .maxAge(VariableConfig.ABSOLUTE_MS) | ||
| ); | ||
| return res; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,36 +1,45 @@ | ||
| package app.model; | ||
|
|
||
| public class User { | ||
| private String userId; | ||
| private Long userId; | ||
| private String password; | ||
| private String name; | ||
| private String nickname; | ||
| private String email; | ||
| private String userRole; | ||
|
|
||
| public User(String userId, String password, String name, String email) { | ||
| this.userId = userId; | ||
| public User(String password, String nickname, String email, String userRole) { | ||
| this.password = password; | ||
| this.name = name; | ||
| this.nickname = nickname; | ||
| this.email = email; | ||
| this.userRole = userRole; | ||
|
Comment on lines
+10
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 설계 문제: User 엔티티의 userId가 null로 초기화되고 나중에 setUserId()로 설정됩니다. 생성 후 즉시 사용하면 런타임 오류가 발생할 수 있습니다.\n\n개선안: 생성자에서 userId를 받거나, 필수 필드를 필드 선언 단계에서 초기화하세요." |
||
| } | ||
|
|
||
| public String getUserId() { | ||
| public Long getUserId() { | ||
| return userId; | ||
| } | ||
|
|
||
| public void setUserId(Long userId){ | ||
| this.userId = userId; | ||
| } | ||
|
|
||
| public String getPassword() { | ||
| return password; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| public String getNickname() { | ||
| return nickname; | ||
| } | ||
|
|
||
| public String getEmail() { | ||
| return email; | ||
| } | ||
|
|
||
| public String getUserRole() { | ||
| return userRole; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "User [userId=" + userId + ", password=" + password + ", name=" + name + ", email=" + email + "]"; | ||
| return "User [userId=" + userId + ", password=" + password + ", name=" + nickname + ", email=" + email + "]"; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,8 @@ private void setFilterChains(){ | |
| .addFilterList(FilterType.ALL, getFilterListByAuthorityType(FilterType.ALL)) | ||
| .addFilterList(FilterType.PUBLIC, getFilterListByAuthorityType(FilterType.PUBLIC)) | ||
| .addFilterList(FilterType.AUTHENTICATED, getFilterListByAuthorityType(FilterType.AUTHENTICATED)) | ||
| .addFilterList(FilterType.RESTRICT, getFilterListByAuthorityType(FilterType.RESTRICT)); | ||
| .addFilterList(FilterType.RESTRICT, getFilterListByAuthorityType(FilterType.RESTRICT)) | ||
| .addFilterList(FilterType.LOG_IN, getFilterListByAuthorityType(FilterType.LOG_IN)); | ||
| } | ||
|
|
||
| private List<ServletFilter> commonFrontFilter(){ | ||
|
|
@@ -47,10 +48,16 @@ private List<ServletFilter> getFilterListByAuthorityType(FilterType type) { | |
| private List<ServletFilter> authorizedFilterList(FilterType type) { | ||
| return switch (type) { | ||
| case ALL -> List.of(); | ||
| case PUBLIC -> List.of(); | ||
| case AUTHENTICATED -> List.of(); | ||
| case RESTRICT -> List.of(appConfig.restrictedFilter()); | ||
| case LOG_IN -> List.of(); | ||
| case PUBLIC -> List.of( | ||
| appConfig.authenticationFilter()); | ||
| case AUTHENTICATED -> List.of( | ||
| appConfig.authenticationFilter(), | ||
| appConfig.memberAuthorizationFilter()); | ||
| case RESTRICT -> List.of( | ||
| appConfig.restrictedFilter()); | ||
| case LOG_IN -> List.of( | ||
| appConfig.authenticationFilter(), | ||
| appConfig.unanimousAuthorizationFilter()); | ||
| }; | ||
| } | ||
|
Comment on lines
+51
to
62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 구조 설계 피드백 (PR 요청사항): 로그인 로직을 핸들러 계층에 배치한 것은 좋은 선택입니다. 이유:\n\n✅ 핸들러 계층 배치의 장점:\n- 로그인은 비즈니스 로직 (사용자 검증, 세션 생성)이므로 handlers에 속함\n- 필터는 요청/응답의 관심사 분리(authentication/authorization 처리)에만 집중\n- 필터-핸들러 분리가 명확하고 유지보수 용이\n\n✅ 구현 모델이 명확함: \n- AuthenticationFilter: 세션 정보 로드 및 사용자 정보 설정\n- MemberAuthorizationFilter/UnanimousAuthorizationFilter: 권한 확인\n- LoginWithPost: 실제 로그인 비즈니스 로직 처리\n\n다만 위의 보안 및 안정성 이슈(비밀번호 해싱, null 체크, 성능 등)를 개선하면 더 견고한 구조가 될 것입니다." |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,4 +6,7 @@ public class VariableConfig { | |
| public static final List<String> STATIC_RESOURCE_ROOTS = List.of( | ||
| "./src/main/resources", | ||
| "./src/main/resources/static"); | ||
|
|
||
| public static final long IDLE_MS = 30*60*100; | ||
| public static final long ABSOLUTE_MS = 180*60*100; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
성능 경고:
stream().filter().findAny()는 매번 전체 컬렉션을 순회합니다. 사용자 수가 증가하면 로그인 성능이 저하됩니다.\n\n개선안: 사용자를 이메일로 검색하려면 별도의 Map을 유지하거나 데이터베이스 인덱싱을 사용하세요.\njava\nprivate static Map<String, User> usersByEmail = new ConcurrentHashMap<>();\n\npublic static Optional<User> findUserByEmail(String email) {\n return Optional.ofNullable(usersByEmail.get(email));\n}\n"