Skip to content
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

fix: Dev 환경 쿠키 이슈 해결 및 관련 로직 개선 #276

Merged
merged 18 commits into from
Feb 7, 2024

Conversation

uwoobeat
Copy link
Member

@uwoobeat uwoobeat commented Feb 5, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • EnviromentConstants 를 enum에서 일반 상수 문자열의 묶음 클래스로 변경했습니다.
    • 예전에 논의했던 바와 같아enum.values() 등을 사용할 일이 없기 때문에 제거 후 대체했습니다.
    • 기존 코드와의 간결성 차이를 느껴보세요.
  • 위와 관련하여 SpringEnvironmentUtil 구현을 변경했습니다.
    • isXXXProfile() 메서드의 경우 원시형 boolean을 리턴하도록 개선했습니다.
  • 쿠키 유틸리티가 작동하지 않던 부분을 수정(하려고노력)했습니다.
    • 프론트에서 Chrome 80 SameSite 정책으로 인한 이슈를 해결했습니다.
    • securetrue 이면 SameSite: None을 사용할 수 있습니다. 해당 사항을 반영했습니다.
    • 프론트에서는 react local 환경에서 https 요청을 보내도록 하도록 협의했습니다.
  • CORS 문제 해결
    • dev 환경에서 Allow-Origin 이 와일드카드인 경우 setAllowCredentials(true) 가 작동하지 않습니다 (중요).
    • 이게 작동하지 않으면 1) 쿠키가 존재하거나 2) Authorization 헤더가 존재하는 요청을 처리할 수 없습니다.
    • 따라서 기존의 origin 로직을 복구했습니다. 이때 switch 문을 제거하도록 개선했습니다.

📝 참고사항

📚 기타

@uwoobeat uwoobeat changed the title fix: Dev 환경에서 쿠키가 설정되지 않는 문제 fix: Dev 환경 쿠키 이슈 해결 및 관련 로직 개선 Feb 6, 2024
@uwoobeat uwoobeat marked this pull request as ready for review February 6, 2024 10:08
@uwoobeat uwoobeat self-assigned this Feb 6, 2024
Copy link
Member

@char-yb char-yb left a comment

Choose a reason for hiding this comment

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

리뷰 달았습니당

Copy link
Member

Choose a reason for hiding this comment

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

NoArgsConstructor를 사용했을 때는 문제가 발생될까용?

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 어노테이션 사용해도 상관없습니다. 수정할게요

Comment on lines 22 to 37

ResponseCookie accessTokenCookie =
ResponseCookie.from(ACCESS_TOKEN_COOKIE_NAME, accessToken)
.path("/")
.secure(true)
.secure(isSecured)
.sameSite(sameSite)
.httpOnly(false)
.httpOnly(true)
.build();

ResponseCookie refreshTokenCookie =
ResponseCookie.from(REFRESH_TOKEN_COOKIE_NAME, refreshToken)
.path("/")
.secure(true)
.secure(isSecured)
.sameSite(sameSite)
.httpOnly(false)
.httpOnly(true)
.build();
Copy link
Member

Choose a reason for hiding this comment

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

httpOnly를 true로 했을 때 발생되는 이벤트 한번 설명부탁드려도 될까용??

Copy link
Member Author

Choose a reason for hiding this comment

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

PR 설명에 업데이트해두었습니다~

Copy link

sonarqubecloud bot commented Feb 6, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

71 New issues
0 Security Hotspots
47.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@char-yb char-yb left a comment

Choose a reason for hiding this comment

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

LGTM
설명확인했습니다!

@uwoobeat uwoobeat merged commit eb65361 into develop Feb 7, 2024
2 checks passed
@github-actions github-actions bot added the merged 머지된 PR label Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged 머지된 PR
Projects
Status: 완료
Development

Successfully merging this pull request may close these issues.

🐛 Dev 환경에서 쿠키가 설정되지 않는 문제
2 participants