Skip to content

Conversation

@m-a-king
Copy link
Collaborator

@m-a-king m-a-king commented Jan 7, 2025

resolved :

📌 과제 설명

기존에는 기관만 가능했던 ID, PW 로그인을 봉사자도 사용할 수 있도록 확장했습니다.
이를 위해 인증 및 인가 로직과 관련된 다양한 수정 사항을 반영했습니다.

👩‍💻 요구 사항과 구현 내용

주요 변경 사항:

  1. email ->accountId

    • 이메일 인증에 대한 회의 끝에 이메일을 사용하지 않기로 했습니다. 이에 대한 수정이 있습니다.
  2. 기관과 봉사자 구분 제거

    • ID, PW 로그인이 기관에 한정되지 않고, 봉사자도 사용할 수 있도록 로직 수정.
    • 역할(Role)에 따라 인증된 토큰을 발급.
  3. 컨트롤러 수정

  4. 정적 팩토리 메서드 추가

  5. 필터 개선

    • IdPwAuthFilter 의존성 주입 방식을 변경하여 가독성과 유지보수성 향상.
    • 인증/인가 성공 시 로직을 수정하고 쿠키가 아닌 헤더로 토큰을 반환하도록 변경.
  6. 테스트 코드 개선

    • 프로덕션 코드 변화에 따라 기존 테스트 코드 수정 및 추가.

✅ PR 포인트 & 궁금한 점

  • "해내야만 한다."
    어지러운 정신 상태와 로직 속에서 길을 잃을 뻔했지만, 끝내 완성했습니다.
    리팩토링할 부분이 다소 있을 것으로 예상합니다. 부담 없이 피드백 주세요.
    궁금한 점도 많이 물어봐 주세용

  • 유저에게 단일 권한이 아닌 다중 권한이 필요한 케이스가 있을까요?

@m-a-king m-a-king self-assigned this Jan 7, 2025
@m-a-king m-a-king linked an issue Jan 7, 2025 that may be closed by this pull request
1 task
@m-a-king m-a-king changed the title [Feature]281 add volunteer login by idpw [Feature] 봉사자 ID, PW 로그인 추가 Jan 7, 2025
@m-a-king m-a-king added the 재중 label Jan 8, 2025
import org.springframework.web.bind.annotation.RestController;

@RestController
@Slf4j
Copy link
Collaborator

@7zrv 7zrv Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

필요 없으시다면 이제 제거하셔도 될 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 복붙해서 사용하다가 같이 적은 것 같습니다

import lombok.Builder;

@JsonNaming(SnakeCaseStrategy.class)
@Builder
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순 전송을 위한거라면 빌더 어노테이션이 없어도 될 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다

throw new BadCredentialsException("비밀번호가 일치하지 않습니다.");
}

// TODO 비활성 계정 검증
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 올라오겠지만 비활성 계정 검증은 어떻게 하실 계획이신지 여쭤보고싶습니다!
순수하게 궁금해서 여쭤봤습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

필드를 하나 더 추가해야 할 것 같습니다. 삭제와 비활성이 구분되는 경우가 있어, 이를 고려해 추후에 구현할 계획입니다.
대략 3개월 동안 사용하지 않은 유저를 비활성 상태로 전환할 수 있을 것 같고, 비활성 유저에게는 알림이 DB에 저장되지 않도록 처리할 수 있을 것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

답변 감사합니다

Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~

private final AuthenticationManager authenticationManager;
private final GenerateTokensOnLoginUseCase generateTokensOnLoginUseCase;
private final CookieUseCase cookieUseCase;
// private final CookieUseCase cookieUseCase;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

얘는 왜 주석으로 두셨나용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후에 프론트엔드 개발이 끝나면, 쿠키로 변경할 수도 있다고 하셔서 그런 맥락을 코드에 남겨두고 싶었습니다.

Comment on lines 102 to 103
return JwtAuthenticationToken.from(userId, role, accessToken
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 개행은 자동으로 잘려서 이렇게 된 걸까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오.. 수정하겠습니다 감사합니다~

Copy link
Collaborator

@leebs0521 leebs0521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다.

유저에게 단일 권한이 아닌 다중 권한이 필요한 케이스가 있을까요?

저는 저희 도메인에서는 일단 없다라고 생각합니다

}

public static JwtAuthenticationToken from(User user, EncodedToken accessToken) {
return new JwtAuthenticationToken(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중요한건 아닌데
파라미터 1개면 from 2개 이상이면 of이지 않았나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞네요.. create, from, of, to 섞어가면서 사용하다보니 헷갈리네요 ㅋㅋ ㅠㅠ

@m-a-king m-a-king force-pushed the feature/281-add-volunteer-login-by-idpw branch from 517f4c2 to 70817b6 Compare January 8, 2025 08:04
- from에서 of로 변경
@m-a-king m-a-king force-pushed the feature/281-add-volunteer-login-by-idpw branch from 70817b6 to bbaad1d Compare January 8, 2025 08:08
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2025

@m-a-king m-a-king merged commit ef9c1b2 into main Jan 8, 2025
3 checks passed
@m-a-king m-a-king deleted the feature/281-add-volunteer-login-by-idpw branch January 8, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] 봉사자 ID,PW 로그인 방식 추가

5 participants