Skip to content

[Hyunu] Week10 미션 - #114

Open
cha-hyunwoo wants to merge 36 commits into
Hyeonufrom
Hyeonu-Week10
Open

[Hyunu] Week10 미션#114
cha-hyunwoo wants to merge 36 commits into
Hyeonufrom
Hyeonu-Week10

Conversation

@cha-hyunwoo

Copy link
Copy Markdown

🔗 연관 이슈

🛠 작업 내용

🖼 스크린샷 (선택)

👀 리뷰 요구사항 (선택)

🤖 AI 활용

  • AI 사용 안 함
  • 코드 작성 아이디어 참고
  • 테스트/리팩토링 보조
  • 문서/주석 작성 보조
  • 기타 (아래에 간단히 작성)

💬 나의 프롬프트

🧠 AI 응답

✅ 내가 최종 선택한 방법 (이유)

💡 나만의 Tip (선택)

@cha-hyunwoo cha-hyunwoo self-assigned this Jun 17, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b67340cdcf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +49 to +53
return Member.builder()
.email(dto.getSocialEmail())
.name(dto.getName())
.socialType(dto.getSocialType())
.socialUid(dto.getSocialUid())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge OAuth 회원 저장 전에 필수 필드를 채우세요

OAuth 최초 로그인에서 이 빌더로 만든 Member를 바로 memberRepository.save(newMember)에 넘기지만, Member 엔티티의 birth, address, detailAddress, phoneNumber는 아직 nullable=false입니다. 그래서 카카오에서 처음 들어온 사용자는 DB insert 시 null 제약에 걸려 토큰 발급까지 가지 못합니다. 소셜 가입용 온보딩을 분리하거나, 엔티티의 필수 값 기준을 다시 정리하거나, 저장 전에 필요한 값을 채우는 방식으로 고치고 JPA의 엔티티 불변식과 컬럼 null 제약을 같이 학습해 보세요.

Useful? React with 👍 / 👎.

Comment on lines +26 to +31
public JwtUtil(
@Value("${jwt.token.secretKey}") String secret,
@Value("${jwt.token.expiration.access}") Long accessExpiration
) {
this.secretKey = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8));
this.accessExpiration = Duration.ofMillis(accessExpiration);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge JWT 설정값이 없을 때 앱이 시작되도록 처리하세요

이번 커밋에서 JwtUtil이 필수 빈으로 주입되는데, 기본 application.properties에는 jwt.token.secretKeyjwt.token.expiration.access가 없고 여기에도 기본값이나 설정 검증 메시지가 없습니다. 새로 클론한 학습 환경에서 해당 환경변수를 넣지 않으면 Spring 컨텍스트 생성 시 placeholder 해석 단계에서 바로 실패합니다. 민감한 secret은 환경변수로 두더라도 예시 설정 파일이나 @ConfigurationProperties/검증을 추가해 어떤 값을 넣어야 하는지 드러내고, Spring의 외부 설정 바인딩을 함께 공부해 보세요.

Useful? React with 👍 / 👎.

@kjhh2605 kjhh2605 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[키워드 조사]
클라우드 컴퓨팅, 환경변수, yml 환경 분리, jar와 Docker 이미지의 차이를 전반적으로 정리한 점이 좋습니다. 다만 운영 관점에서 spring.profiles.active를 설정 파일에 고정하는 방식과 실행 시 환경변수로 주입하는 방식의 차이, Docker 이미지가 실제로 Dockerfile 레이어와 실행 명령을 통해 만들어지는 흐름을 함께 보완하면 배포 구조 이해가 더 명확해집니다. 환경변수 예시는 민감 정보 보호 목적과 잘 연결되어 있어 적절합니다.

[코드 리뷰]
회원가입, 로그인, JWT 필터, OAuth 로그인까지 인증 흐름을 MVC 계층으로 나누어 구현한 점이 좋습니다. Controller-Service-Repository 흐름과 Converter 사용도 유지되고 있습니다. 다만 인증 도메인은 작은 불일치가 런타임 오류나 보안 정책 혼선으로 이어질 수 있으므로, JWT subject의 의미, 세션 정책, 회원가입 중복 검증을 명확히 맞추는 것을 권장합니다.

)
// 로그아웃 설정
.formLogin(AbstractHttpConfigurer::disable)
.sessionManagement(AbstractHttpConfigurer::disable)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

JWT 인증을 중심으로 구성한다면 세션 정책도 명시적으로 SessionCreationPolicy.STATELESS로 맞추는 것을 권장합니다. 현재처럼 세션 설정을 비활성화하면 의도는 전달되지만, Spring Security에서 상태를 만들지 않는다는 정책이 코드에 직접 드러나지 않아 학습 관점에서 혼선이 생길 수 있습니다.

.collect(Collectors.joining(","));

return Jwts.builder()
.subject(member.getUsername()) // UID를 Subject로

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getEmail()getUid()가 모두 subject를 반환하지만, 토큰 생성 시 subject는 member.getUsername()으로 설정됩니다. 이메일 기반 로그인과 소셜 로그인 UID 기반 인증을 함께 다룬다면 subject에 어떤 식별자를 넣을지 하나로 정하고 메서드 이름도 그 의미에 맞추는 것을 권장합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants