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

[Feature] - 로그인 명세 수정 #202

Merged
merged 7 commits into from
Aug 5, 2024
Merged

[Feature] - 로그인 명세 수정 #202

merged 7 commits into from
Aug 5, 2024

Conversation

Libienz
Copy link

@Libienz Libienz commented Aug 3, 2024

✅ 작업 내용

  • 쿼리파라미터로 인코딩된 redirectUri를 받도록 수정

🙈 참고 사항

  • 쿼리파라미터의 디폴트 값을 활용하여 백엔드 로컬에서도 지금까지와 동일한 방식으로 로그인을 테스트 할 수 있습니다.
  • 다만 로그인 같은 경우 현재 저희의 로직이라면 HTTP METHOD post가 맞는 것 같습니다.
  • 백엔드 로컬에서의 로그인 확인을 포기하고 post로 명세를 수정하는 것이 좋을 지 현행처럼 get으로 둘지 고민이에요
  • 의견 주시면 감사링

- 쿼리파라미터로 인코딩된 redirectUri를 받도록 수정
@Libienz Libienz added the BE label Aug 3, 2024
@Libienz Libienz added this to the sprint 3 milestone Aug 3, 2024
@Libienz Libienz self-assigned this Aug 3, 2024
@Libienz Libienz linked an issue Aug 3, 2024 that may be closed by this pull request
1 task
Copy link

github-actions bot commented Aug 3, 2024

Test Results

 22 files   22 suites   4s ⏱️
151 tests 151 ✅ 0 💤 0 ❌
159 runs  159 ✅ 0 💤 0 ❌

Results for commit 98ac8aa.

♻️ This comment has been updated with latest results.

Copy link

@eunjungL eunjungL left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 리비!
저도 다른 해결법 없는지 계속 찾아보겠습니다~

public ResponseEntity<LoginResponse> login(@RequestParam(name = "code") String authorizationCode) {
public ResponseEntity<LoginResponse> login(
@RequestParam(name = "code") String authorizationCode,
@RequestParam(name = "redirectUri", defaultValue = "http%3A%2F%2Flocalhost%3A8080%2Fapi%2Fv1%2Flogin%2Foauth%2Fkakao") String encodedRedirectUri
Copy link

Choose a reason for hiding this comment

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

query string이 안들어오면 백엔드를 가르키게 해뒀군요 좋네요 👍

Copy link

@hangillee hangillee left a comment

Choose a reason for hiding this comment

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

변경 상황 확인했습니다!
고생 많으셨어요 리비! 제 의견 코멘트로 달았으니 확인 부탁드려요!

@@ -37,8 +37,11 @@ public class LoginController {
)
})
@GetMapping("/oauth/kakao")

Choose a reason for hiding this comment

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

확실히 제 생각에도 POST가 더 나을 것 같다는 생각이 드네요.. 회원가입도 같이 진행되다보니 멱등을 지키긴 어려울 것 같습니다. 다만, 로컬에서의 테스트가 가능하도록 자체 회원 가입 및 로그인 기능 구현 전까지는 지금처럼 GET으로 두는 것도 좋을 것 같습니다.

@Libienz
Copy link
Author

Libienz commented Aug 4, 2024

@hangillee
@eunjungL

지금 생각했을 때는 로컬 프로파일로 컨테이너 띄울 때만 예외적으로 한명의 멤버가 자동으로 영속화되는 것이 자연스러워 보이네요.
그리고 이에 대한 만료기한이 긴 토큰을 발급받아 로그로 찍어놓는것이죠.

이렇게 하면 POST 메서드 유지하면서 로컬 테스트도 진행할 수 있을 것 같네요

기술적으로 가능한지 모르겠는데 학습해보고 적용가능하면 해보겠습니다.

해당 PR 머지는 살짝 미룰게요!

@hangillee
Copy link

오! 그런 방법도 있겠네요! 좋은 것 같습니다!

@eunjungL
Copy link

eunjungL commented Aug 4, 2024

로컬에서만 sql init data true 해두면 될 것 같습니다! 좋은 방법 같네용! 👍👍

@Libienz
Copy link
Author

Libienz commented Aug 5, 2024

정보

로그인 명세는 POST로 변경되었고 백엔드 로컬에서의 로그인은 더 이상 고려되지 않음 (자체 로그인 구현)

@Libienz Libienz merged commit 1565018 into develop/be Aug 5, 2024
3 checks passed
@Libienz Libienz deleted the feature/be/#192 branch August 5, 2024 13:37
hangillee pushed a commit to hangillee/2024-touroot that referenced this pull request Aug 20, 2024
* fix: 로그인 API 명세 수정

- 쿼리파라미터로 인코딩된 redirectUri를 받도록 수정

* fix: 로그인 명세 POST 메서드로 수정 및 로컬 redirect default 제거

* test: 로그인 명세 수정에 따른 테스트 코드 수정
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature] - 로그인 명세 수정
3 participants