-
Notifications
You must be signed in to change notification settings - Fork 2
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/#41 깃허브 로그인 기능 구현 #50
The head ref may contain hidden characters: "Feature/#41-\uAE43\uD5C8\uBE0C_\uB85C\uADF8\uC778_\uAE30\uB2A5_\uAD6C\uD604"
Changes from all commits
ba1c7c3
2f33d70
8c528a6
52eb783
1f9b17c
9d99baf
d1cd05c
85e3454
eff599f
1a101cb
bb1b348
5422e89
38a1afb
4701ddb
6179009
7a7b853
5abbf9b
28f71c0
feeab0e
e7f913d
da1decf
0cd66d0
c79cb57
39c97a4
c003c2a
4d99f76
1cc219d
b52fc15
fac42f4
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,30 @@ | ||
package com.emmsale.login.api; | ||
|
||
import com.emmsale.login.application.LoginService; | ||
import com.emmsale.login.application.dto.TokenResponse; | ||
import com.emmsale.login.exception.LoginException; | ||
import com.emmsale.login.exception.LoginExceptionType; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequestMapping("/login") | ||
public class LoginApi { | ||
|
||
private final LoginService loginService; | ||
|
||
public LoginApi(final LoginService loginService) { | ||
this.loginService = loginService; | ||
} | ||
|
||
@GetMapping("/github/callback") | ||
public ResponseEntity<TokenResponse> login(@RequestParam final String code) { | ||
if (code == null) { | ||
throw new LoginException(LoginExceptionType.NOT_FOUND_GITHUB_CODE); | ||
} | ||
return ResponseEntity.ok().body(loginService.createToken(code)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package com.emmsale.login.application; | ||
|
||
import com.emmsale.login.application.dto.GithubAccessTokenRequest; | ||
import com.emmsale.login.application.dto.GithubAccessTokenResponse; | ||
import com.emmsale.login.application.dto.GithubProfileResponse; | ||
import com.emmsale.login.exception.LoginException; | ||
import com.emmsale.login.exception.LoginExceptionType; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.http.HttpEntity; | ||
import org.springframework.http.HttpHeaders; | ||
import org.springframework.http.HttpMethod; | ||
import org.springframework.http.MediaType; | ||
import org.springframework.stereotype.Component; | ||
import org.springframework.web.client.HttpClientErrorException; | ||
import org.springframework.web.client.RestTemplate; | ||
|
||
@Component | ||
public class GithubClient { | ||
|
||
private static final RestTemplate REST_TEMPLATE = new RestTemplate(); | ||
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. 제 의도는 RestTemplate 도 그냥 주입 받으면 되지 않을까 생각해서 남겨뒀습니다. 굳이 상수화를 시키신 이유가 있으신가요?? |
||
|
||
@Value("${github.client.id}") | ||
private String clientId; | ||
@Value("${github.client.secret}") | ||
private String clientSecret; | ||
@Value("${github.url.access-token}") | ||
private String accessTokenUrl; | ||
@Value("${github.url.profile}") | ||
private String profileUrl; | ||
|
||
public String getAccessTokenFromGithub(final String code) { | ||
final GithubAccessTokenRequest githubAccessTokenRequest = buildGithubAccessTokenRequest(code); | ||
|
||
try { | ||
final String accessToken = getAccessTokenResponse(githubAccessTokenRequest); | ||
throwIfNotFoundAccessToken(accessToken); | ||
return accessToken; | ||
} catch (final HttpClientErrorException httpClientErrorException) { | ||
throw new LoginException(LoginExceptionType.INVALID_GITHUB_CODE); | ||
} | ||
} | ||
|
||
public GithubProfileResponse getGithubProfileFromGithub(final String accessToken) { | ||
try { | ||
return getGithubProfileResponse(accessToken); | ||
} catch (final HttpClientErrorException httpClientErrorException) { | ||
throw new LoginException(LoginExceptionType.INVALID_GITHUB_ACCESS_TOKEN); | ||
} | ||
} | ||
|
||
private GithubProfileResponse getGithubProfileResponse(final String accessToken) { | ||
final HttpHeaders headers = new HttpHeaders(); | ||
headers.add(HttpHeaders.AUTHORIZATION, "token " + accessToken); | ||
|
||
final HttpEntity<String> httpEntity = new HttpEntity<>(headers); | ||
|
||
return REST_TEMPLATE | ||
.exchange(profileUrl, HttpMethod.GET, httpEntity, GithubProfileResponse.class) | ||
.getBody(); | ||
} | ||
|
||
private GithubAccessTokenRequest buildGithubAccessTokenRequest(final String code) { | ||
return new GithubAccessTokenRequest( | ||
code, | ||
clientId, | ||
clientSecret | ||
); | ||
} | ||
|
||
private String getAccessTokenResponse(final GithubAccessTokenRequest githubAccessTokenRequest) { | ||
final HttpHeaders headers = new HttpHeaders(); | ||
headers.add(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE); | ||
|
||
final HttpEntity<GithubAccessTokenRequest> httpEntity = new HttpEntity<>( | ||
githubAccessTokenRequest, | ||
headers | ||
); | ||
|
||
return REST_TEMPLATE | ||
.exchange(accessTokenUrl, HttpMethod.POST, httpEntity, GithubAccessTokenResponse.class) | ||
.getBody() | ||
.getAccessToken(); | ||
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. 제가 잘 몰라서 그러는데 get함과 동시에 accessToken이 존재하지 않는다면 바로 NPE가 발생하나요??
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. final String accessToken = REST_TEMPLATE
.exchange(accessTokenUrl, HttpMethod.POST, httpEntity, GithubAccessTokenResponse.class)
.getBody()
.getAccessToken(); 위 코드에서 RestTemplate과 동일한 라이브러리는 아니지만 유사항 사례가 있어 참고로 남깁니다. |
||
} | ||
|
||
private void throwIfNotFoundAccessToken(final String accessToken) { | ||
if (accessToken == null || accessToken.isBlank()) { | ||
throw new LoginException(LoginExceptionType.NOT_FOUND_GITHUB_ACCESS_TOKEN); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package com.emmsale.login.application; | ||
|
||
import com.emmsale.login.application.dto.GithubProfileResponse; | ||
import com.emmsale.login.application.dto.MemberQueryResponse; | ||
import com.emmsale.login.application.dto.TokenResponse; | ||
import com.emmsale.login.utils.JwtTokenProvider; | ||
import com.emmsale.member.application.MemberQueryService; | ||
import org.springframework.stereotype.Service; | ||
|
||
@Service | ||
public class LoginService { | ||
|
||
private final GithubClient githubClient; | ||
private final JwtTokenProvider tokenProvider; | ||
private final MemberQueryService memberQueryService; | ||
|
||
public LoginService( | ||
final GithubClient githubClient, | ||
final JwtTokenProvider tokenProvider, | ||
final MemberQueryService memberQueryService | ||
) { | ||
this.githubClient = githubClient; | ||
this.tokenProvider = tokenProvider; | ||
this.memberQueryService = memberQueryService; | ||
} | ||
|
||
public TokenResponse createToken(final String code) { | ||
final String githubAccessToken = githubClient.getAccessTokenFromGithub(code); | ||
final GithubProfileResponse githubProfileFromGithub = githubClient.getGithubProfileFromGithub( | ||
githubAccessToken); | ||
|
||
final MemberQueryResponse memberQueryResponse = memberQueryService.findOrCreateMember( | ||
githubProfileFromGithub | ||
); | ||
final String accessToken = tokenProvider.createToken( | ||
String.valueOf(memberQueryResponse.getMemberId()) | ||
); | ||
|
||
return new TokenResponse( | ||
memberQueryResponse.getMemberId(), | ||
memberQueryResponse.isNewMember(), | ||
accessToken | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package com.emmsale.login.application.dto; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
|
||
@AllArgsConstructor | ||
@Getter | ||
public class GithubAccessTokenRequest { | ||
|
||
private final String code; | ||
@JsonProperty("client_id") | ||
private final String clientId; | ||
@JsonProperty("client_secret") | ||
private final String clientSecret; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package com.emmsale.login.application.dto; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import lombok.AccessLevel; | ||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
|
||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
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. 혹시 NoArgsCounstructor를 PROTECTED로 추가하신 이유가 있을까요? |
||
@AllArgsConstructor | ||
@Getter | ||
public class GithubAccessTokenResponse { | ||
|
||
@JsonProperty("access_token") | ||
private String accessToken; | ||
@JsonProperty("token_type") | ||
private String tokenType; | ||
private String scope; | ||
private String bearer; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package com.emmsale.login.application.dto; | ||
|
||
import com.emmsale.member.domain.Member; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import lombok.AccessLevel; | ||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
|
||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
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. 얘도요!! |
||
@AllArgsConstructor | ||
@Getter | ||
public class GithubProfileResponse { | ||
|
||
@JsonProperty("id") | ||
private String githubId; | ||
@JsonProperty("name") | ||
private String name; | ||
@JsonProperty("login") | ||
private String username; | ||
@JsonProperty("avatar_url") | ||
private String imageUrl; | ||
|
||
public Long getGithubId() { | ||
return Long.valueOf(githubId); | ||
} | ||
|
||
public Member toMember() { | ||
return new Member( | ||
getGithubId(), | ||
getUsername() | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package com.emmsale.login.application.dto; | ||
|
||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
|
||
@AllArgsConstructor | ||
@Getter | ||
public class MemberQueryResponse { | ||
|
||
private long memberId; | ||
private boolean isNewMember; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.emmsale.login.application.dto; | ||
|
||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
|
||
@AllArgsConstructor | ||
@Getter | ||
public class TokenResponse { | ||
|
||
private long memberId; | ||
private boolean isNewMember; | ||
private String accessToken; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package com.emmsale.login.exception; | ||
|
||
import com.emmsale.base.BaseException; | ||
import com.emmsale.base.BaseExceptionType; | ||
|
||
public class LoginException extends BaseException { | ||
|
||
private final LoginExceptionType exceptionType; | ||
|
||
public LoginException(final LoginExceptionType exceptionType) { | ||
super(exceptionType.errorMessage()); | ||
this.exceptionType = exceptionType; | ||
} | ||
|
||
@Override | ||
public BaseExceptionType exceptionType() { | ||
return exceptionType; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package com.emmsale.login.exception; | ||
|
||
import com.emmsale.base.BaseExceptionType; | ||
import org.springframework.http.HttpStatus; | ||
|
||
public enum LoginExceptionType implements BaseExceptionType { | ||
|
||
NOT_FOUND_GITHUB_ACCESS_TOKEN( | ||
HttpStatus.BAD_REQUEST, | ||
"GitHub Access Token을 찾을 수 없습니다." | ||
), | ||
INVALID_GITHUB_ACCESS_TOKEN( | ||
HttpStatus.NOT_FOUND, | ||
"GitHub Profile을 찾을 수 없습니다." | ||
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. '~을 찾을 수 없습니다'라는 종류의 에러라면 INVALID_~보다는 NOT_FOUND_~라는 수식어가 더 적절한 것 같다는 생각이 듭니다! 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. 저는 GithubProfile을 찾을 수 없는 이유가 유효하지 않은 github access token 때문이라고 생각하여 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. 저는 프롤로그 에러 목록에서 에러 명과 에러 메시지가 전달하는 내용이 동일한 것을 보고 코멘트를 드렸었는데, 제 생각을 말씀드리자면 원인이 조금 더 디테일한 정보를 줄 수 있으니 오류 메시지를 'Github Access Token이 유효하지 않다'라고 바꾸는 것도 좋아 보여요. 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. 이러한 에러 메시지의 경우에는, 어떻게 하면 이 API를 사용하는 사람(저희한테선 안드로이드겠죠?)에게 더 잘 이해가 되는지를 고려해볼 필요가 있다고 생각합니다. 저는 그런 입장에서 유효하지 않다는 의미보다 "찾을 수 없다"는 의미가 더 중요한 것 같아요. |
||
), | ||
INVALID_ACCESS_TOKEN( | ||
HttpStatus.BAD_REQUEST, | ||
"토큰이 유효하지 않습니다." | ||
), | ||
NOT_FOUND_AUTHORIZATION_TOKEN( | ||
HttpStatus.BAD_REQUEST, | ||
"인증 토큰을 찾을 수 없습니다." | ||
), | ||
NOT_FOUND_GITHUB_CODE( | ||
HttpStatus.BAD_REQUEST, | ||
"깃허브 코드를 찾을 수 없습니다." | ||
), | ||
INVALID_GITHUB_CODE( | ||
HttpStatus.BAD_REQUEST, | ||
"깃허브 코드가 유효하지 않습니다." | ||
); | ||
|
||
private final HttpStatus httpStatus; | ||
private final String errorMessage; | ||
|
||
LoginExceptionType(final HttpStatus httpStatus, final String errorMessage) { | ||
this.httpStatus = httpStatus; | ||
this.errorMessage = errorMessage; | ||
} | ||
|
||
@Override | ||
public HttpStatus httpStatus() { | ||
return httpStatus; | ||
} | ||
|
||
@Override | ||
public String errorMessage() { | ||
return errorMessage; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package com.emmsale.login.utils; | ||
|
||
import javax.servlet.http.HttpServletRequest; | ||
import org.springframework.http.HttpHeaders; | ||
|
||
public class AuthorizationExtractor { | ||
|
||
private static final String ACCESS_TOKEN_TYPE = | ||
AuthorizationExtractor.class.getSimpleName() + ".ACCESS_TOKEN_TYPE"; | ||
private static final String BEARER_TYPE = "Bearer"; | ||
|
||
public static String extract(final HttpServletRequest request) { | ||
String authHeaderValue = request.getHeader(HttpHeaders.AUTHORIZATION) | ||
.substring(BEARER_TYPE.length()).trim(); | ||
|
||
if (authHeaderValue.isBlank()) { | ||
return null; | ||
} | ||
|
||
request.setAttribute(ACCESS_TOKEN_TYPE, BEARER_TYPE); | ||
final int commaIndex = authHeaderValue.indexOf(','); | ||
if (commaIndex > 0) { | ||
authHeaderValue = authHeaderValue.substring(0, commaIndex); | ||
} | ||
return authHeaderValue; | ||
} | ||
} |
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.
패키지 상으로도 그렇고 기능적으로도 Service에 해당하는 것 같은데 클래스명을 GithubClientService가 아니라 GithubClient로 지어주신 이유가 있을까요??
제가 OAuth쪽은 익숙하지 않아서 잘 모르는 것일 수도 있습니다...!
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.
컨트롤러의 요청을 받아 도메인 로직을 실행하고 이것을 반환하는 것이 아니므로 서비스라고 짓지 않았습니다.