Skip to content

[✨ Feature] oauth 인증 구현 - 밸리데이션 로직 추가 - #17

Merged
juice119 merged 6 commits into
mainfrom
feat/16-validate-callback
Jul 14, 2026
Merged

[✨ Feature] oauth 인증 구현 - 밸리데이션 로직 추가#17
juice119 merged 6 commits into
mainfrom
feat/16-validate-callback

Conversation

@juice119

@juice119 juice119 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

배경 및 목적

oauth 인증시 callback으로 받은 데이터를 검증하지 않고 회원가입하도록 설계되어 잘못된 데이터가 들어와도 회원가입이 발생

연관 이슈

Closes #16

작업 내용

  • github oauth response DTO 추가
  • staright 에서 검증하는 로직 추가

테스트 결과

  • github.strategy.spec.ts 검증 로직 - 단위테스트, 의존성 모킹
  • GithubOauthCallbackResponseDto.ts validate 단위 테스트 추가

리뷰어 참고사항

  • callback 검증 실패(username/email 없음 등) 시 이제 500(InternalServerErrorException)으로 막힘

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@juice119 juice119 self-assigned this Jul 13, 2026
@juice119 juice119 added the ✨ feature 기능 추가 label Jul 13, 2026
juice119 and others added 3 commits July 13, 2026 16:53
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@juice119 juice119 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

GitHub OAuth callback 검증 로직 PR 올렸습니다. 리뷰 부탁드립니다 🙏

  • callback으로 받은 accessToken/profile 검증 안 하고 바로 가입 처리되던 부분 → DTO 검증 추가

Comment thread backend/apps/api/src/auth/strategies/github.strategy.ts
@juice119
juice119 requested a review from f-lab-namu July 13, 2026 08:14
Comment on lines +45 to +49
if (validationErrors.length > 0) {
throw new InternalServerErrorException(
'oauth 로그인 중 에러가 발생하였습니다. 고객센터에 문의 주세요.',
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

에러 처리 원칙 중 매우 중요한 부분이 있는데요, Do not ignore error 입니다.

현재 코드는 validation 시에 error 가 생기면 InternalServerErrorException 으로 대체 (replace) 하는 코드 인데, 이 경우 어떤 에러가 최초에 발생되었는지 ignore 됩니다. Error.captureStackTrace 를 활용해 error 를 wrapping 하거나, error log 를 출력하거나 에러를 남겨야 합니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

에러 로그 추가했습니다.

Comment thread backend/apps/api/src/auth/strategies/github.strategy.spec.ts
import { describe, expect, it } from 'vitest';
import { GithubOauthCallbackResponseDto } from './GithubOauthCallbackResponseDto';

describe('GithubOauthCallbackResponseDto Unit Test', () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

이 테스트 케이스는 (제기준에서) 불필요해 보입니다. class-validator 오픈소스의 @isnotempty() decorator 는 문제가 있을때 error 를 발생시키는데, 오픈소스에서 충분히 검증되었을텐데, 한번더 검증을 추가해야할 만큼 이유가 보이진 않습니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

맞는 것 같습니다 이쪽 테스트 정리할게요

juice119 and others added 2 commits July 14, 2026 13:38
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
InternalServerErrorException으로 대체되며 최초 validationErrors가
누락되던 문제 수정, logger.error로 남김

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@juice119 juice119 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

리뷰하신 내용 반영했습니다 @f-lab-namu

import { describe, expect, it } from 'vitest';
import { GithubOauthCallbackResponseDto } from './GithubOauthCallbackResponseDto';

describe('GithubOauthCallbackResponseDto Unit Test', () => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

맞는 것 같습니다 이쪽 테스트 정리할게요

Comment on lines +45 to +49
if (validationErrors.length > 0) {
throw new InternalServerErrorException(
'oauth 로그인 중 에러가 발생하였습니다. 고객센터에 문의 주세요.',
);
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

에러 로그 추가했습니다.

@juice119
juice119 merged commit 67df5c4 into main Jul 14, 2026
2 checks passed
@juice119
juice119 deleted the feat/16-validate-callback branch July 31, 2026 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feature 기능 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[✨ Feature] oauth 인증 구현 - 밸리데이션 로직 추가

2 participants