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

[COZY-232] feat : 메일 인증 기능 추가 #100

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Conversation

genius00hwan
Copy link
Contributor

#️⃣ 요약 설명

메일 보내기 기능을 추가했습니다.
아직 메일 패턴적용은 안했구여.. 인증코드 생성해서 메일 보내고, 검증하는 api만 만들었습니다.
일단 지금은 자바 맵으로 구현했습니다.

📝 작업 내용

ex) 코드의 흐름이나 중요한 부분을 작성해주세요.
ex) 기존 calculate 함수의 버그를 수정했습니다.

public void sendUniversityAuthenticationCode(String clientId, String mailAddress) {
        SimpleMailMessage message = new SimpleMailMessage();
        String authCode = UUID.randomUUID().toString().substring(0, 6);
        message.setTo(mailAddress);
        message.setSubject("COZYMATE 대학교 메일인증");
        message.setText("COZYMATE 대학교 메일인증 코드입니다 : " + authCode);
        mailSender.send(message);

        memberAuthenticationCodeMap.put(clientId, authCode);
    }

6자리 코드를 만들어 메일을 보내는 코드입니다.

 public MailResponseDTO.verifyResponseDTO verifyAuthenticationCode(String clientId, String authenticationCode) {
        if (memberAuthenticationCodeMap.get(clientId).equals(authenticationCode)){
            return MailResponseDTO.verifyResponseDTO.builder()
                    .isVerified(true)
                    .message("성공입니다")
                    .build();
        }
        return MailResponseDTO.verifyResponseDTO.builder()
                .isVerified(false)
                .message("인증 코드가 일치하지 않습니다")
                .build();
    }

메일로 보낸 인증코드와 사용자가 보낸 코드의 일치여부를 판단하는 메소드 입니다.

Response는 다음과 같습니다.

    @AllArgsConstructor
    @NoArgsConstructor
    @Getter
    @Builder
    public static class verifyResponseDTO{
        private String message;
        private Boolean isVerified;
    }

동작 확인

image

image

💬 리뷰 요구사항(선택)

도메인의 역할이 단순해서 멤버 도메인에 포함시킬까 고민했습니다.

역할이 많지 않아 Converter를 적용하지 않았는데 적용하라고 하면 하겠습니다!!

추후에 Redis를 사용해볼까 고민중입니다!

Copy link
Member

@eple0329 eple0329 left a comment

Choose a reason for hiding this comment

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

그냥 하나 제안입니다~


public void sendUniversityAuthenticationCode(String clientId, String mailAddress) {
SimpleMailMessage message = new SimpleMailMessage();
String authCode = UUID.randomUUID().toString().substring(0, 6);
Copy link
Member

Choose a reason for hiding this comment

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

UUID를 randomUUID로 생성해서 6자리를 가져오는건 정말 좋은 방식이라고 생각합니다.
추가로 제안드리고 싶은 부분은 Base64 인코딩입니다.
uuid는 16진수로 authCode의 가지수가 대략 1600만 정도가 존재하는데요. 엄청 큰 수이긴 하지만, 좀 더 가지수를 늘린다면 Base64 인코딩을 하는게 좋겠다는 생각입니다. Base64로 6자리 코드를 만들어 제공한다면 대략 690억 정도의 가지수가 존재하게 됩니다. ㅎㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 감사합니다아 적용해볼게여~~

Copy link
Contributor

@jpark0506 jpark0506 left a comment

Choose a reason for hiding this comment

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

LGTM~한가지 궁금증 있어서 남겼습니다 고생하셨어요!!

message.setText("COZYMATE 대학교 메일인증 코드입니다 : " + authCode);
mailSender.send(message);

memberAuthenticationCodeMap.put(clientId, authCode);
Copy link
Contributor

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.

저도 이 부분 생각하긴 했는데,
일단 단일 서버에서는 효율적이라고 생각합니다.

말해주신 이야기대로

  1. 메모리 접근을 하기 때문에 빠르며
  2. DB에 접근을 하지 않아도 되기 때문에 통신 속도를 고려하지 않아도 되며
  3. 마찬가지로 DB와 연관이 없기 때문에 부하를 주지 않는다.
    가 해당 방식에서 볼 수 잇는 장점이라고 봅니다.

여기서 생길 수 있는 단점은

  1. 서버가 다중 서버가 되었을 때 해당 정보를 통해서 인증을 하기 어렵다. (어떤 서버에는 데이터가 존재하겠지만, 그 서버에만 있을 것이기 때문에)
  2. 서버가 터지거나 꺼지면 사라진다. (사실 이건 크게 상관이 없다고 느끼는데, 인증코드를 거치는 과정이 사용자당 한번이라 데이터가 사라지면 다시 인증과정을 거치도록 해도 문제 없을 것 같습니다.)
  3. 인증코드의 만료기간이 지났을 때 삭제과정을 거치지 않는다면, 사용자가 엄청나게 많아졌을 때(비약이긴 하지만) 메모리가 넘치는 현상도 생길 수 있을 것이다.

정도라고 생각합니다.

그래서 이런 문제를 해결하기 위해 Redis 도입을 고려하셨고, Redis를 도입하게 되면 모든 문제가 해결되기는 합니다...ㅎ
하지만 이 인증과정을 위해 Redis를 도입하는 것은 지나치게 오버엔지니어링이라 말을 전달드렸고...
(한번 경험을 해보고싶은 목적이 있다면 써도 되지만) 이 과정을 효율적으로 처리하기 위한 다른 방법을 찾아보긴 해야할 것 같습니다.

그래서 저는 그냥 DB에 데이터를 저장하고 (1, 2 해결) 이벤트 브릿지나 스케줄러를 통해서 정기적으로 삭제 (3번도 해결)하는 방식이 좋지않을까 생각합니다.
장점 중 빠르고, DB에 부하를 주지 않는다는것은 해당 기능에서는 그렇게 중요한 요소라고 보이지는 않기 때문입니다. (사용자당 1번의 인증 과정만을 거치는게 대부분일테니 DB에 부하는 눈에 띄지도 않는 수준일 뿐더러, 빠른 응답이 필수적인 그런 부분도 아니라고 생각합니다)

주절주절...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

대리답변 감사드립니다..
아마 분산서버 도입에 대비해서 추후에 DB에 저장해두려구요
그리고 굳이 쓸데없는 데이터 저장하고 있는 건 별로라 생각돼서 스케줄러돌리면서 삭제하려고 해요~

@eple0329
Copy link
Member

[REVIEW]

안녕하세요. 코드 리뷰 감사합니다. 아래와 같이 개선할 점을 제안드립니다.

  1. build.gradle

    • 특별한 문제는 없습니다. spring-boot-starter-mail 의존성이 추가되어 이메일 발송 기능 구현을 위한 준비가 되었습니다.
  2. config

    • 서브모듈 업데이트가 되었습니다.
  3. ClientIdMaker.java

    • generateUUID() 메서드에서 Base64 인코딩을 사용하여 클라이언트 ID를 생성하고 있습니다. 이는 성능 및 가독성 측면에서 개선이 필요할 수 있습니다.
    • 대신 UUID.randomUUID().toString() 을 그대로 사용하는 것이 더 간단하고 효율적일 수 있습니다.
  4. MailController.java

    • 전반적으로 잘 구현되었습니다. 메일 발송과 인증 코드 검증 기능이 잘 정의되어 있습니다.
    • sendUniversityAuthenticationCode 메서드에서 mailAddress 파라미터 값의 유효성 검사를 추가하는 것이 좋습니다.
    • verifyAuthenticationCode 메서드에서도 code 파라미터 값의 유효성 검사를 추가하는 것이 좋습니다.
  5. MailRequestDTO.java

    • 이메일 주소와 인증 코드를 담는 DTO가 잘 정의되어 있습니다.
  6. MailResponseDTO.java

    • 인증 코드 검증 결과를 담는 DTO가 잘 정의되어 있습니다.
  7. MailService.java

    • 이메일 발송과 인증 코드 검증 로직이 잘 구현되어 있습니다.
    • 인증 코드 생성 시 Base64 인코딩을 사용하는 것은 성능 및 가독성 측면에서 개선이 필요할 수 있습니다. 대신 UUID.randomUUID().toString().substring(0, 6) 과 같은 방식을 사용하는 것이 더 간단할 수 있습니다.
    • 인증 코드 저장 시 Map을 사용하고 있는데, 실제 서비스에서는 DB 또는 Redis 와 같은 저장소를 사용하는 것이 더 적절할 것 같습니다.

전반적으로 잘 구현되었습니다. 일부 성능 및 가독성 개선, 유효성 검사 추가 등의 피드백을 드렸습니다. 전반적인 코드 구조와 로직은 매우 좋다고 생각합니다.

@genius00hwan genius00hwan added the enhancement New feature or request label Sep 30, 2024
Copy link
Member

@veronees veronees left a comment

Choose a reason for hiding this comment

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

LGTM!! 고생하셨습니다~

Copy link

github-actions bot commented Oct 11, 2024

리뷰해드려요~

  • ClientIdMaker.java

  • UUID 값을 Base64 인코딩하여 반환하는 generateUUID 메서드가 추가되었습니다.

  • 이렇게 수정하면 UUID 값이 더 안전하게 사용될 수 있습니다.

  • UUID 값을 반환하는 메서드는 암호화된 값을 반환하는 것이 아니라 원래 값을 반환하므로 안전성이 떨어집니다.

  • 따라서 UUID 값을 Base64 인코딩하여 반환하는 것이 좋습니다.

  • MailController.java

  • MailController 클래스가 추가되었습니다.

  • 이 클래스는 메일 서비스에 대한 REST API를 제공합니다.

  • 이 클래스는 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

  • MailRequestDTO.java

  • MailRequestDTO 클래스가 추가되었습니다.

  • 이 클래스는 메일 서비스에 대한 요청 데이터를 정의하는 데 사용됩니다.

  • 이 클래스는 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

  • MailResponseDTO.java

  • MailResponseDTO 클래스가 추가되었습니다.

  • 이 클래스는 메일 서비스에 대한 응답 데이터를 정의하는 데 사용됩니다.

  • 이 클래스는 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

  • MailService.java

  • MailService 클래스가 추가되었습니다.

  • 이 클래스는 메일 서비스를 제공하는 데 사용됩니다.

  • 이 클래스는 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

  • 메일 서비스에 대한 코드가 추가되었습니다.

  • 이 코드는 메일 서비스를 제공하는 데 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

  • 이 코드에서는 인증 코드를 메일로 보내고 받은 인증 코드를 검증하는 기능을 제공합니다.

  • 이 기능은 유용한 기능이므로 유지하는 것이 좋습니다.

  • 또한 인증 코드를 저장하는 메커니즘도 제공하고 있습니다.

  • 이 메커니즘은 인증 코드를 저장하고 검증하는 데 유용한 기능을 제공하므로 유지하는 것이 좋습니다.

Copy link

리뷰해드려요~

  • ClientIdMaker.java

  • Review 1: UUID 값을 Base64 인코딩하여 사용하는 것이 좋습니다. 이렇게 하면 UUID 값이 더 작아지고 암호화되어 있기 때문에 더 안전합니다.

  • Review 2: 코드의 가독성을 높이기 위해 주석을 추가하는 것이 좋습니다.

  • MailController.java

  • Review 1: 컨트롤러 파일에 멤버 정보를 받아오는 것이 좋습니다. 이렇게 하면 서비스 파일에 접근하지 않아도 멤버 정보를 얻을 수 있습니다.

  • Review 2: 컨트롤러 파일에 응답 객체를 생성하는 것이 좋습니다. 이렇게 하면 서비스 파일에 응답 객체를 생성하지 않아도 됩니다.

  • MailRequestDTO.java

  • Review 1: 요청 객체를 단순화하는 것이 좋습니다. 이렇게 하면 코드가 더 간결해집니다.

  • MailResponseDTO.java

  • Review 1: 응답 객체를 단순화하는 것이 좋습니다. 이렇게 하면 코드가 더 간결해집니다.

  • MailService.java

  • Review 1: 로깅 정보를 추가하는 것이 좋습니다. 이렇게 하면 서비스 로그를 확인할 수 있습니다.

  • Review 2: 인코딩된 인증 코드를 멤버 정보 맵에 저장하는 것이 좋습니다. 이렇게 하면 인증 코드를 안전하게 저장할 수 있습니다.

  • Review 3: 인코딩된 인증 코드를 비교하는 것이 좋습니다. 이렇게 하면 정확한 인증 코드를 비교할 수 있습니다.

  • Review 4: 응답 객체를 생성하는 것이 좋습니다. 이렇게 하면 컨트롤러 파일에서 응답 객체를 생성하지 않아도 됩니다.

@genius00hwan genius00hwan merged commit 817c6b6 into develop Oct 15, 2024
1 check passed
@genius00hwan genius00hwan deleted the feature/COZY-232 branch October 18, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants