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

Feat/#462 닉네임 변경 API 구현 #470

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9ef006e
feat: 닉네임 변경 API 구현
Cyma-s Sep 26, 2023
95bd4c7
docs: API 명세 추가
Cyma-s Sep 26, 2023
ee8bc8a
style: 개행 추가 및 불필요한 공백 삭제, static import 삭제
Cyma-s Sep 27, 2023
3d14050
refactor: 에러 코드 이름 수정
Cyma-s Sep 27, 2023
ded41a5
refactor: 변수 이름 리팩터링
Cyma-s Sep 27, 2023
d52d13c
style: 헤더 위치 변경
Cyma-s Sep 27, 2023
9bfb040
test: 엣지 케이스를 테스트하도록 변경
Cyma-s Sep 27, 2023
621a625
refactor: member 에게 닉네임이 같은지 물어 보도록 메서드 변경
Cyma-s Sep 27, 2023
4685592
fix: @Valid 어노테이션 추가
Cyma-s Sep 27, 2023
5a64990
feat: 리프레시 토큰이 member id 로만 만들어지도록 수정
Cyma-s Sep 29, 2023
2847c8d
refactor: 닉네임 최소 길이 추가
Cyma-s Sep 29, 2023
05fdcf1
fix: cookie setPath 설정 변경
Cyma-s Sep 29, 2023
514c475
fix: cookie 테스트 수정
Cyma-s Sep 29, 2023
f0e54f9
refactor: 에러코드 문구 수정
Cyma-s Sep 30, 2023
04af872
fix: 닉네임 변경 시 응답을 내려주지 않도록 수정
Cyma-s Oct 3, 2023
3d46182
refactor: 토큰 검증 책임 ReissueController 로 이동
Cyma-s Oct 5, 2023
72ea86a
refactor: 검증 순서 변경
Cyma-s Oct 5, 2023
d9115bf
refactor: delete 메서드도 memberId 를 입력 받도록 변경
Cyma-s Oct 5, 2023
21358a7
refactor: 사용하지 않는 클래스 삭제
Cyma-s Oct 5, 2023
8ca6f07
refactor: 검증 로직 간소화
Cyma-s Oct 5, 2023
9c6fc11
refactor: save 문 삭제
Cyma-s Oct 5, 2023
21f4d60
refactor: TokenProvider, InMemoryTokenPairRepository 의존성 삭제
Cyma-s Oct 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public TokenPair oAuthLogin(final String oauthType, final String authorizationCo
}

public ReissueAccessTokenResponse reissueAccessTokenByRefreshToken(final String refreshToken,
final String accessToken) {
final String accessToken) {
final Claims claims = tokenProvider.parseClaims(refreshToken);
final Long memberId = claims.get("memberId", Long.class);
final String nickname = claims.get("nickname", String.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public class AuthConfig implements WebMvcConfigurer {
private final TokenInterceptor tokenInterceptor;

public AuthConfig(final AuthArgumentResolver authArgumentResolver,
final LoginCheckerInterceptor loginCheckerInterceptor,
final TokenInterceptor tokenInterceptor
final LoginCheckerInterceptor loginCheckerInterceptor,
final TokenInterceptor tokenInterceptor
) {
this.authArgumentResolver = authArgumentResolver;
this.loginCheckerInterceptor = loginCheckerInterceptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public enum ErrorCode {
NOT_FOUND_OAUTH(1010, "현재 지원하지 않는 OAuth 요청입니다."),
INVALID_REFRESH_TOKEN(1011, "존재하지 않는 refreshToken 입니다."),
TOKEN_PAIR_NOT_MATCHING_EXCEPTION(1012, "올바르지 않은 TokenPair 입니다."),
ALREADY_EXIST_NICKNAME(1013, "존재하는 닉네임입니다."),
DUPLICATE_NICKNAME(1013, "중복되는 닉네임입니다."),

// 2000: 킬링파트 - 좋아요, 댓글

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public Member register(final String email) {
final Member newMember = new Member(email, BASIC_NICKNAME);
final Member savedMember = memberRepository.save(newMember);
savedMember.updateNickname(savedMember.getNickname() + savedMember.getId());

return savedMember;
}

Expand All @@ -63,10 +64,7 @@ public Member findByIdAndNicknameThrowIfNotExist(final Long id, final Nickname n
public void deleteById(final Long id, final MemberInfo memberInfo) {
final Member member = getMemberIfValidRequest(id, memberInfo);

final List<KillingPartLike> membersExistLikes = likeRepository.findAllByMemberAndIsDeleted(
member,
false
);
final List<KillingPartLike> membersExistLikes = likeRepository.findAllByMemberAndIsDeleted(member, false);

membersExistLikes.forEach(KillingPartLike::updateDeletion);
commentRepository.deleteAllByMember(member);
Expand All @@ -77,16 +75,9 @@ private Member getMemberIfValidRequest(final Long memberId, final MemberInfo mem
final long requestMemberId = memberInfo.getMemberId();
final Member requestMember = findById(requestMemberId);
final Member targetMember = findById(memberId);

validateMemberAuthentication(requestMember, targetMember);
return targetMember;
}

private Member findById(final Long id) {
return memberRepository.findById(id)
.orElseThrow(() -> new MemberException.MemberNotExistException(
Map.of("Id", String.valueOf(id))
));
return targetMember;
}

private void validateMemberAuthentication(final Member requestMember, final Member targetMember) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

메서드 순서 변경이 필요해 보입니다! 이 메서드를 위로 올리는 것은 어떨까요?

Expand All @@ -100,13 +91,20 @@ private void validateMemberAuthentication(final Member requestMember, final Memb
}
}

private Member findById(final Long id) {
return memberRepository.findById(id)
.orElseThrow(() -> new MemberException.MemberNotExistException(
Map.of("Id", String.valueOf(id))
));
}

@Transactional
public TokenPair updateNickname(final Long memberId, final MemberInfo memberInfo,
Copy link
Collaborator

@somsom13 somsom13 Sep 27, 2023

Choose a reason for hiding this comment

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

닉네임 변경으로 인해 토큰 재발급이 일어나고, 응답이 반환되는 보안 문제가 걱정된다면 닉네임 같은 개인정보를 바꾼 후에 재로그인을 요청하는 것은 어떨까요?

닉네임이 변경되면 서버 내부적으로 TOKEN을 삭제 -> 로그인이 만료된 것 처럼 취급이 되어서 프론트엔드에서 자연스럽게 재로그인을 요청하도록 하는 것은 어떤가요?

final NicknameUpdateRequest request) {
final NicknameUpdateRequest request) {
final Member member = getMemberIfValidRequest(memberId, memberInfo);
final Nickname nickname = new Nickname(request.getNickname());

if (isSameNickname(member, nickname)) {
if (member.hasSameNickname(nickname)) {
return null;
}

Expand All @@ -121,15 +119,8 @@ private TokenPair reissueTokenPair(final Long memberId, final String nickname) {
final String reissuedAccessToken = tokenProvider.createAccessToken(memberId, nickname);
final String reissuedRefreshToken = tokenProvider.createRefreshToken(memberId, nickname);
inMemoryTokenPairRepository.addOrUpdateTokenPair(reissuedRefreshToken, reissuedAccessToken);
return new TokenPair(reissuedAccessToken, reissuedRefreshToken);
}


private boolean isSameNickname(final Member member, final Nickname nickname) {
final String originalNickname = member.getNickname();
final String nicknameForUpdate = nickname.getValue();

return originalNickname.equals(nicknameForUpdate);
return new TokenPair(reissuedAccessToken, reissuedRefreshToken);
}

private void validateDuplicateNickname(final Nickname nickname) {
Expand Down
4 changes: 4 additions & 0 deletions backend/src/main/java/shook/shook/member/domain/Member.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ public void updateNickname(final Nickname newNickname) {
this.nickname = newNickname;
}

public boolean hasSameNickname(final Nickname nickname) {
return nickname.equals(this.nickname);
}

public String getEmail() {
return email.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ public MemberNotExistException(final Map<String, String> inputValuesByProperty)
public static class ExistNicknameException extends MemberException {

public ExistNicknameException() {
super(ErrorCode.ALREADY_EXIST_NICKNAME);
super(ErrorCode.DUPLICATE_NICKNAME);
}

public ExistNicknameException(final Map<String, String> inputValuesByProperty) {
super(ErrorCode.ALREADY_EXIST_NICKNAME, inputValuesByProperty);
super(ErrorCode.DUPLICATE_NICKNAME, inputValuesByProperty);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -42,7 +43,7 @@ public ResponseEntity<Void> deleteMember(
public ResponseEntity<ReissueAccessTokenResponse> updateNickname(
@PathVariable(name = "member_id") final Long memberId,
@Authenticated final MemberInfo memberInfo,
@RequestBody final NicknameUpdateRequest request,
@Valid @RequestBody final NicknameUpdateRequest request,
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 매의 눈을 뜨기 위해 노력할게요 🦅 👀

final HttpServletResponse response
) {
final TokenPair tokenPair = memberService.updateNickname(memberId, memberInfo, request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static shook.shook.auth.ui.Authority.MEMBER;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand All @@ -16,6 +15,7 @@
import shook.shook.auth.application.dto.TokenPair;
import shook.shook.auth.exception.AuthorizationException;
import shook.shook.auth.repository.InMemoryTokenPairRepository;
import shook.shook.auth.ui.Authority;
import shook.shook.auth.ui.argumentresolver.MemberInfo;
import shook.shook.member.application.dto.NicknameUpdateRequest;
import shook.shook.member.domain.Member;
Expand Down Expand Up @@ -168,7 +168,7 @@ void success_delete() {

saveAndClearEntityManager();
// when
memberService.deleteById(targetId, new MemberInfo(targetId, MEMBER));
memberService.deleteById(targetId, new MemberInfo(targetId, Authority.MEMBER));

// then
assertThat(likeRepository.findAllByMemberAndIsDeleted(savedMember, false)).isEmpty();
Expand All @@ -185,7 +185,7 @@ void fail_delete() {

// when, then
assertThatThrownBy(() ->
memberService.deleteById(targetId, new MemberInfo(unsavedMemberId, MEMBER))
memberService.deleteById(targetId, new MemberInfo(unsavedMemberId, Authority.MEMBER))
).isInstanceOf(MemberException.MemberNotExistException.class);
}

Expand All @@ -199,7 +199,7 @@ void fail_delete_unauthenticated() {
// when, then
assertThatThrownBy(() ->
memberService.deleteById(targetMember.getId(),
new MemberInfo(requestMember.getId(), MEMBER))
new MemberInfo(requestMember.getId(), Authority.MEMBER))
).isInstanceOf(AuthorizationException.UnauthenticatedException.class);
}

Expand All @@ -216,7 +216,8 @@ void success_update(final String newNickname) {

// when
final TokenPair tokenPair = memberService.updateNickname(savedMember.getId(),
new MemberInfo(savedMember.getId(), MEMBER),
new MemberInfo(savedMember.getId(),
Authority.MEMBER),
request);

// then
Expand All @@ -236,18 +237,20 @@ void success_updateNickname_same_nickname_before() {

// when
// then
assertThat(memberService.updateNickname(savedMember.getId(), new MemberInfo(savedMember.getId(), MEMBER),
request)).isNull();
assertThat(
memberService.updateNickname(savedMember.getId(),
new MemberInfo(savedMember.getId(), Authority.MEMBER),
request)).isNull();
}

@DisplayName("변경할 닉네임이 중복되면 예외를 던진다.")
@Test
void fail_updateNickname_duplicate_nickname() {
// given
final Member newMember = memberRepository.save(new Member("temp@email", "shook2"));
final String newNickname = "shook"; // 중복된 닉네임
final NicknameUpdateRequest request = new NicknameUpdateRequest(newNickname);
final MemberInfo newMemberInfo = new MemberInfo(newMember.getId(), MEMBER);
final String duplicateNickname = "shook";
final NicknameUpdateRequest request = new NicknameUpdateRequest(duplicateNickname);
final MemberInfo newMemberInfo = new MemberInfo(newMember.getId(), Authority.MEMBER);

// when
// then
Expand All @@ -265,7 +268,7 @@ void fail_updateNickname_empty_nickname(final String emptyValue) {
// when
// then
assertThatThrownBy(() -> memberService.updateNickname(savedMember.getId(),
new MemberInfo(savedMember.getId(), MEMBER),
new MemberInfo(savedMember.getId(), Authority.MEMBER),
request))
.isInstanceOf(MemberException.NullOrEmptyNicknameException.class);
}
Expand All @@ -280,7 +283,7 @@ void fail_updateNickname_too_long_nickname(final String tooLongNickname) {
// when
// then
assertThatThrownBy(() -> memberService.updateNickname(savedMember.getId(),
new MemberInfo(savedMember.getId(), MEMBER),
new MemberInfo(savedMember.getId(), Authority.MEMBER),
request))
.isInstanceOf(MemberException.TooLongNicknameException.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void create_fail_lessThanOne(final String nickname) {
@Test
void create_fail_lengthOver20() {
//given
final String nickname = ".".repeat(101);
final String nickname = ".".repeat(21);

//when
//then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ void deleteMember() {

// when, then
RestAssured.given().log().all()
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.when().log().all()
.delete("/members/{member_id}", member.getId())
.then().log().all()
.statusCode(HttpStatus.NO_CONTENT.value());
Expand All @@ -55,8 +55,8 @@ void deleteMember_forbidden() {

// when, then
RestAssured.given().log().all()
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.when().log().all()
.delete("/members/{member_id}", member.getId())
.then().log().all()
.statusCode(HttpStatus.FORBIDDEN.value());
Expand All @@ -73,10 +73,10 @@ void updateNickname_OK() {
// when
// then
final ReissueAccessTokenResponse response = RestAssured.given().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.patch("/members/{member_id}/nickname", member.getId())
.then().log().all()
.statusCode(HttpStatus.OK.value())
Expand All @@ -100,10 +100,10 @@ void updateNickname_noContent() {
// when
// then
RestAssured.given().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.patch("/members/{member_id}/nickname", member.getId())
.then().log().all()
.statusCode(HttpStatus.NO_CONTENT.value());
Expand All @@ -124,10 +124,10 @@ void updateNickname_badRequest(final String invalidNickname) {
// when
// then
RestAssured.given().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.patch("/members/{member_id}/nickname", newMember.getId())
.then().log().all()
.statusCode(HttpStatus.BAD_REQUEST.value());
Expand Down
Loading