Conversation
|
Warning Rate limit exceeded@wlgns12370 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces a comprehensive refactor and feature addition centered on authentication, token management, and KMC (Korea Mobile Certification) integration. It removes legacy token services, introduces provider-based token management, adds KMC verification flows with supporting controllers, services, and DTOs, and updates user role handling and profile endpoints. Several new configuration files and JSP views are added to support KMC workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant AuthTokenManager
participant AccessTokenProvider
participant RefreshTokenProvider
Client->>AuthController: POST /auth/reissue (Refresh-Token header)
AuthController->>AuthService: reissueAccessToken(refreshToken)
AuthService->>AuthTokenManager: reissueAccessToken(refreshToken)
AuthTokenManager->>RefreshTokenProvider: getAuthentication(refreshToken)
RefreshTokenProvider-->>AuthTokenManager: Authentication (User)
AuthTokenManager->>RefreshTokenProvider: invalidateToken(userId)
AuthTokenManager->>AccessTokenProvider: generateJwtToken(user)
AuthTokenManager->>RefreshTokenProvider: generateJwtToken(user)
AuthTokenManager-->>AuthService: Token DTO
AuthService-->>AuthController: Token DTO
AuthController-->>Client: Response with new token
sequenceDiagram
participant Client
participant KmcAuthController
participant KmcService
participant KmcCryptoManager
participant KmcEventTxService
Client->>KmcAuthController: POST /kmc/confirm (KmcCertRequest)
KmcAuthController->>KmcService: createCertRequest(request)
KmcService->>KmcCryptoManager: encryptRequestData(request, certNum)
KmcCryptoManager-->>KmcService: encryptedData
KmcService->>KmcEventTxService: publishIssueEvent(certNum, expiration)
KmcEventTxService-->>KmcService: (void)
KmcService-->>KmcAuthController: KmcCertResponse
KmcAuthController-->>Client: KmcCertResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @wlgns12370, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on integrating KMC (Korea Mobile Certification) for identity verification, which is a significant new feature. This integration involves substantial changes across the application, including refactoring of existing authentication and token management services, introducing new domain and infrastructure components for KMC, and updating the user signup flow to incorporate this verification step. Additionally, there are general infrastructure updates and minor refactorings to improve code organization and maintainability.
Highlights
- KMC Identity Verification Integration: Implemented a comprehensive system for KMC (Korea Mobile Certification) identity verification, including new controllers, services, DTOs, and a crypto manager to handle the entire verification flow.
- Authentication and Token Management Refactoring: Revamped the JWT token management by introducing new
Providerclasses (e.g.,AccessTokenProvider,RefreshTokenProvider,JwtTokenProvider) and a centralizedAuthTokenManager, replacing olderServiceclasses for better modularity and clarity. - Multi-step Signup Flow Enhancement: Modified the user signup process to support a multi-step approach, separating account creation from profile registration, and integrated a new
OneTimeTokenProviderandSignUpTokenServicefor managing one-time verification tokens. - Infrastructure and Dependency Updates: Updated
build.gradleto include WAR deployment, KMC SDK dependencies, JSP/Servlet support, and Testcontainers for improved testing. Also,application.ymlwas updated for JSP view resolution andddl-autosetting. - Security Configuration Adjustments: Updated the Spring Security configuration to enable proper CORS handling and introduced a custom
RefreshTokenHeaderargument resolver for cleaner API design.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature for identity verification with KMC. The refactoring of authentication components into providers is a great step towards a cleaner architecture. I've reviewed the changes and found several areas that need attention before this can be merged. My main concerns are around production safety, such as the use of ddl-auto: update, hardcoded URLs, and logging of sensitive data. I've also identified a couple of critical logic errors and several areas for improvement in terms of code quality and maintainability.
| private void logDecryptedData(String[] tokens) { | ||
| if (!log.isInfoEnabled()) return; | ||
|
|
||
| String ci = decryptCiDiValue(tokens[CI_INDEX]); | ||
| String di = decryptCiDiValue(tokens[DI_INDEX]); | ||
|
|
||
| log.info("[KMCIS] Parsed User Information:"); | ||
| log.info(" - CertNum: {}", tokens[CERT_NUM_INDEX]); | ||
| log.info(" - Name: {}", tokens[NAME_INDEX]); | ||
| log.info(" - PhoneNo: {}", tokens[PHONE_NO_INDEX]); | ||
| log.info(" - Decrypted CI: {}", ci); | ||
| log.info(" - Decrypted DI: {}", di); | ||
| log.info(" - PlusInfo: {}", tokens[PLUS_INFO_INDEX]); | ||
|
|
||
| logPlusInfoDetail(tokens[PLUS_INFO_INDEX]); | ||
| } | ||
|
|
||
| // production 시 삭제 | ||
| private void logPlusInfoDetail(String plusInfo) { | ||
| if (!StringUtils.hasText(plusInfo) || !plusInfo.contains("@")) return; | ||
|
|
||
| String[] parts = plusInfo.split("@", -1); | ||
| if (parts.length == 3) { | ||
| byte[] decodedBytes = new Base32().decode(parts[1]); | ||
| String decodedPassword = new String(decodedBytes); | ||
| log.info(" - Parsed PlusInfo -> loginId: {}, password(decoded): {}, term: {}", parts[0], decodedPassword, parts[2]); | ||
| } | ||
| } |
There was a problem hiding this comment.
The methods logDecryptedData and logPlusInfoDetail are marked with comments // production 시 삭제 (delete in production). This is a very risky practice, as this code could easily be deployed to production by mistake, leading to sensitive user information (like decoded passwords) being logged. This kind of debug logging should be handled through proper mechanisms, such as using a DEBUG log level and configuring log levels per environment, or using Spring profiles to conditionally enable/disable certain beans or logic. Please remove this code before merging.
| // Redis에 토큰 정보가 없으면 유효하지 않은 것으로 간주 | ||
| if (signUpToken == null) { | ||
| throw new AuthenticationException("유효하지 않은 회원가입 토큰입니다."); |
There was a problem hiding this comment.
The if (signUpToken == null) check is unreachable code. The oneTimeTokenProvider.getSignUpToken() method calls signUpTokenRepository.findByCertNum(), which in turn throws a CustomRuntimeException if the token is not found, so it will never return null. This null check should be removed as it's dead code and can be misleading.
| public void e(String a) { | ||
| System.out.println("Encoding: " + a); | ||
| } |
There was a problem hiding this comment.
The UserEncoderService class appears to be experimental or placeholder code. The method e has a non-descriptive name and uses System.out.println, which should be avoided in favor of a proper logger. This class should be either completed with proper implementation or removed from the codebase to avoid confusion and dead code.
| String cert = kmcService.createCertRequest(request).tr_cert(); | ||
|
|
||
| model.addAttribute("tr_cert", cert); | ||
| model.addAttribute("tr_url", "http://localhost:8080/api/v1/kmc/tr-url"); |
There was a problem hiding this comment.
The callback URL for KMC is hardcoded. This makes the application difficult to configure for different environments (development, staging, production). This URL should be externalized into a configuration file (e.g., application.yml) and injected via @Value or a configuration properties class.
| model.addAttribute("tr_url", "http://localhost:8080/api/v1/kmc/tr-url"); | |
| model.addAttribute("tr_url", kmcProperties.getTrUrl()); // Assuming you add trUrl to KmcProperties |
| private final WebClient webClient = WebClient.builder() | ||
| .baseUrl("https://www.kmcert.com/kmcis/api") | ||
| .build(); |
There was a problem hiding this comment.
The base URL for the KMC API is hardcoded. This will cause issues when deploying to different environments or if the API endpoint changes. This URL should be moved to a configuration file (like application.yml) and injected as a property.
private final WebClient webClient;
public LegacyService(IcertSecuManager seed, KmcService kmcService, @Value("${kmc.api.base-url}") String kmcApiBaseUrl) {
this.seed = seed;
this.kmcService = kmcService;
this.webClient = WebClient.builder()
.baseUrl(kmcApiBaseUrl)
.build();
}| show-sql: true | ||
| hibernate: | ||
| ddl-auto: create-drop | ||
| ddl-auto: update |
There was a problem hiding this comment.
Using spring.jpa.hibernate.ddl-auto: update is not recommended for production environments. It can lead to an inconsistent database schema and potential data loss. The official Spring documentation warns against its use in production. Since you are using Flyway for database migrations, you should rely on it to manage the schema. I recommend changing this property to validate to ensure the Hibernate entity model is in sync with the database schema managed by Flyway, or none to let Flyway handle everything.
ddl-auto: validate
| private Claims parseClaims(String token) { | ||
| try { | ||
| return Jwts.parserBuilder() | ||
| .setSigningKey(key) | ||
| .build() | ||
| .parseClaimsJws(token) | ||
| .getBody(); | ||
| } catch (ExpiredJwtException exception) { | ||
| return exception.getClaims(); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
The parseClaims method catches ExpiredJwtException and returns the claims, effectively hiding that the token has expired. This can lead to unexpected behavior in calling methods, as they might operate on data from an expired token. It's generally better to either let the exception propagate to be handled by the caller or to wrap it in a custom application exception. This makes the token validation logic more explicit and robust.
private Claims parseClaims(String token) {
return Jwts.parserBuilder()
.setSigningKey(key)
.build()
.parseClaimsJws(token)
.getBody();
}|
|
||
| public enum UserRole { | ||
| ROLE_USER, ROLE_ADMIN, ROLE_PENDING; | ||
| ROLE_USER, ROLE_ADMIN, PENDING |
There was a problem hiding this comment.
The enum constants for UserRole are named inconsistently. ROLE_USER and ROLE_ADMIN have the ROLE_ prefix, while PENDING does not. For consistency and to align with Spring Security's convention (where the ROLE_ prefix is often added automatically), it's better to apply the same naming convention to all enum constants.
| ROLE_USER, ROLE_ADMIN, PENDING | |
| ROLE_USER, ROLE_ADMIN, ROLE_PENDING |
|
|
||
| PrincipalDetails principal = (PrincipalDetails) authentication.getPrincipal(); | ||
|
|
||
| log.info("principalID: {}", principal.getId()); |
There was a problem hiding this comment.
Logging a user's principal ID at the INFO level can lead to excessive and potentially sensitive data in production logs. It's better to use the DEBUG level for this kind of detailed diagnostic information.
| log.info("principalID: {}", principal.getId()); | |
| log.debug("principalID: {}", principal.getId()); |
| * KMC 본인인증 요청 데이터(tr_cert)를 암호화 | ||
| */ | ||
| public String encryptRequestData(KmcCertRequest request, String certNum) { | ||
| String currentDate = new SimpleDateFormat("yyyyMMddHHmmss").format(new Date()); |
There was a problem hiding this comment.
SimpleDateFormat is a legacy, mutable, and not thread-safe class. It's recommended to use the modern java.time.format.DateTimeFormatter from Java 8 onwards, which is immutable and thread-safe.
| String currentDate = new SimpleDateFormat("yyyyMMddHHmmss").format(new Date()); | |
| String currentDate = java.time.format.DateTimeFormatter.ofPattern("yyyyMMddHHmmss").format(java.time.LocalDateTime.now()); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 66
🔭 Outside diff range comments (1)
build.gradle (1)
105-109: Remove duplicate Testcontainers dependencies.The Testcontainers dependencies are duplicated at the end of the file, which could lead to confusion and potential version conflicts.
- testImplementation 'org.springframework.boot:spring-boot-testcontainers:3.3.5' - testImplementation 'org.testcontainers:testcontainers:1.19.3' - testImplementation 'org.testcontainers:junit-jupiter:1.19.3' - testImplementation 'org.testcontainers:mysql:1.20.0'
🧹 Nitpick comments (34)
src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRedisEntity.java (1)
1-1: Package relocation is sensible; double-check bean scanning & Redis key namingMoving refresh-token artifacts into
auth.refreshclarifies the bounded context.
Two quick follow-ups:
- Ensure Spring’s component scan (if restricted with
@ComponentScan(basePackages = …)) includes the new package.@RedisHash(value = "token_verification")is now slightly misleading for a refresh-token entity; consider renaming to something like"refresh_token"for clarity.src/main/java/life/mosu/mosuserver/presentation/admin/annotation/RefreshTokenHeader.java (1)
1-13: Consider relocating to a more appropriate package.The annotation implementation is correct, but the package location
presentation.admin.annotationseems inappropriate since this annotation is used for general authentication token handling, not admin-specific functionality. Consider moving it topresentation.auth.annotationor similar.src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisEntity.java (1)
18-20: Remove redundant @indexed annotation on @id field.The
certNumfield is annotated with both@Idand@Indexed. The@Idannotation already makes the field unique and searchable in Redis, making the@Indexedannotation redundant and potentially confusing.@Id -@Indexed private String certNum;src/main/resources/security-config.yml (1)
43-45: Fix formatting issue: add newline at end of file.The KMC configuration addition looks good and follows the established pattern of using environment variables. However, the static analysis tool correctly identified a missing newline at the end of the file.
Apply this diff to fix the formatting:
kmc: cpid: ${KMC_CPID} - url-code: ${KMC_URLCODE} + url-code: ${KMC_URLCODE} +src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcApiRequest.java (1)
1-6: Consider adding validation and documentation for this DTO.The record implementation is clean and follows good practices. Consider these optional enhancements:
+import jakarta.validation.constraints.NotBlank; + +/** + * DTO for KMC API request containing authentication token and date. + * + * @param apiToken the API authentication token + * @param apiDate the API request date + */ public record KmcApiRequest( + @NotBlank String apiToken, + @NotBlank String apiDate ) {}src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcCertResponse.java (1)
3-10: Consider field naming convention consistency.The field name
tr_certuses snake_case while the factory method parametertrCertuses camelCase. If the snake_case name isn't required by an external API specification, consider using consistent camelCase naming (trCert) for better Java convention alignment.If the snake_case is required by external API contracts, consider adding a comment explaining this requirement:
public record KmcCertResponse( + // Field name matches external KMC API specification String tr_cert ) {src/main/java/life/mosu/mosuserver/infra/kmc/KmcProperties.java (1)
7-14: Consider validation & richer typing for properties
expireTimeis stored asLong, which loses time-unit semantics. Spring Boot supportsjava.time.Duration, making configuration like5m,30s, etc., self-documenting.- Adding Bean Validation (
@Validated,@NotBlank,@Positive) guards against misconfiguration at startup.-import lombok.Data; +import lombok.Data; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Positive; +import org.springframework.validation.annotation.Validated; @@ -@Data -@Component -@ConfigurationProperties(prefix = "kmc") +@Data +@Validated +@Component +@ConfigurationProperties(prefix = "kmc") public class KmcProperties { - private String cpId; - private String urlCode; - private Long expireTime; + @NotBlank + private String cpId; + + @NotBlank + private String urlCode; + + @Positive + private Duration expireTime; }src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRepository.java (1)
3-8: ReturnOptionalto avoid nullsRepository callers currently have to handle potential
nullreturns. ReturningOptional<SignUpToken>makes the contract explicit and eliminates ambiguity.- SignUpToken findByCertNum(String certNum); + Optional<SignUpToken> findByCertNum(String certNum);Corresponding implementation adjustments will be trivial.
src/main/webapp/WEB-INF/views/request.jsp (1)
7-7: Consider security implications of auto-form submission.Auto-submitting forms on page load can be problematic from both UX and security perspectives. Consider adding user confirmation or a brief delay before submission.
-<body onload="javascript:document.reqKMCISForm.submit();"> +<body onload="setTimeout(function(){ document.reqKMCISForm.submit(); }, 2000);">src/main/java/life/mosu/mosuserver/presentation/oauth/SignUpTokenService.java (1)
25-32: Consider transaction safety for token deletion.The token is deleted immediately after validation, which could lead to issues if the downstream processing fails. Consider whether the deletion should happen within a transaction or after successful completion of the signup process.
This depends on your business requirements - if you want strict one-time use regardless of downstream success, the current approach is fine. If you want to allow retry on processing failures, consider delaying the deletion.
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventFactory.java (1)
8-14: Consider improving type safety with a more specific return type.The implementation is correct but the return type
TxEvent<?>could be more specific for better type safety.Consider updating the return type to be more specific:
@Override -public TxEvent<?> create(KmcContext context) { +public TxEvent<KmcContext> create(KmcContext context) { return new KmcIssueTxEvent(context.isSuccess(), context); }This would provide better compile-time type checking and make the intent clearer.
src/main/java/life/mosu/mosuserver/presentation/auth/AuthController.java (1)
6-6: Consider relocating the RefreshTokenHeader annotation.The
@RefreshTokenHeaderannotation is imported from the admin package but used for general authentication purposes. This seems architecturally inconsistent.Consider moving the annotation to a more appropriate package like:
life.mosu.mosuserver.presentation.auth.annotationlife.mosu.mosuserver.global.annotationThis would better reflect its general-purpose nature rather than admin-specific usage.
src/main/java/life/mosu/mosuserver/application/auth/provider/OneTimeTokenProvider.java (1)
19-20: Consider making constants more maintainableThe token type and key are defined as static final fields, but they could be externalized to configuration for better maintainability.
Consider moving these to application properties:
-private static final String tokenType = "ONE_TIME"; -private static final String TOKEN_TYPE_KEY = "type"; +private final String tokenType; +private final String tokenTypeKey; public OneTimeTokenProvider( @Value("${jwt.access-token.expire-time}") final Long expireTime, @Value("${jwt.secret}") final String secretKey, + @Value("${jwt.one-time-token.type:ONE_TIME}") final String tokenType, + @Value("${jwt.one-time-token.type-key:type}") final String tokenTypeKey, SignUpTokenRepository signUpTokenRepository ) { this.expireTime = expireTime; this.key = Keys.hmacShaKeyFor(Decoders.BASE64.decode(secretKey)); + this.tokenType = tokenType; + this.tokenTypeKey = tokenTypeKey; this.signUpTokenRepository = signUpTokenRepository; }src/test/resources/security-config.yml (3)
43-46: KMC configuration looks good, but fix the file formatting.The KMC configuration properly uses environment variables. However, please add a newline at the end of the file as indicated by the static analysis tool.
kmc: cpid: ${KMC_CPID} url-code: ${KMC_URLCODE} - expire-time: ${KMC_EXPIRE_TIME} + expire-time: ${KMC_EXPIRE_TIME} +
43-46: KMC configuration looks good, but fix the formatting issue.The KMC configuration properly uses environment variables for sensitive data. However, please add a newline character at the end of the file as indicated by the static analysis tool.
kmc: cpid: ${KMC_CPID} url-code: ${KMC_URLCODE} - expire-time: ${KMC_EXPIRE_TIME} + expire-time: ${KMC_EXPIRE_TIME} +
25-28: Consider local development implications of secure cookie settings.The
secure: truesetting requires HTTPS, which might cause issues in local development environments. Consider using profiles or environment-specific configurations.server: servlet: session: cookie: http-only: true - secure: true + secure: ${COOKIE_SECURE:false} same-site: Nonesrc/main/webapp/WEB-INF/views/step1.jsp (4)
1-17: Good cache control implementation, but improve code comments.The cache control headers properly prevent caching of this sensitive form. However, consider using English comments in source code to improve accessibility and prevent potential encoding issues.
18-34: Secure random number generation, but update character encoding.The SecureRandom implementation is correct and properly seeded. However, consider updating from EUC-KR to UTF-8 encoding for better international compatibility.
-<%@ page contentType="text/html;charset=ksc5601" %> +<%@ page contentType="text/html;charset=UTF-8" %>And update the meta tag:
-<meta http-equiv="Content-Type" content="text/html; charset=euc-kr"> +<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
20-34: Secure random number generation with minor optimization opportunity.The use of SecureRandom is appropriate for generating certification request numbers. However, explicitly setting the seed with
System.currentTimeMillis()is unnecessary as SecureRandom self-seeds securely.java.security.SecureRandom ran = new java.security.SecureRandom(); - ran.setSeed(System.currentTimeMillis());
130-140: Consider modernizing the form layout.The table-based layout is functional but outdated. Consider using CSS Grid or Flexbox for better responsive design and accessibility.
src/main/webapp/WEB-INF/views/start.jsp (1)
45-51: Consider using a more robust mobile detection approachUser agent string parsing can be unreliable and requires maintenance as new devices are released. Consider using feature detection or CSS media queries instead.
- var UserAgent = navigator.userAgent; - var isMobile = false; - if (UserAgent.match( - /iPhone|iPad|Android|Windows CE|BlackBerry|Symbian|Windows Phone|webOS|Opera Mini|Opera Mobi|POLARIS|IEMobile|lgtelecom|nokia|SonyEricsson/i) - != null || UserAgent.match(/LG|SAMSUNG|Samsung/) != null) { - isMobile = true; - } + // Use modern approach for mobile detection + var isMobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) + || window.matchMedia("(max-width: 768px)").matches;src/main/java/life/mosu/mosuserver/infra/kmc/legacy/LegacyService.java (1)
66-66: Avoid blocking operations in reactive codeUsing
.block()on a reactive stream defeats the purpose of using WebClient. Consider making this method return a reactive type or use RestTemplate if synchronous behavior is required.- .bodyToMono(KmcVerificationRequest.class) - .block(); + .bodyToMono(KmcVerificationRequest.class) + .block(Duration.ofSeconds(30)); // Add timeout at minimumsrc/main/java/life/mosu/mosuserver/presentation/oauth/AccessTokenFilter.java (2)
61-61: Fix typo in log messageThere's a typo in the log message: "oath2" should be "oauth2".
- log.info("oath2 관련 요청: {}", request.getRequestURI()); + log.info("oauth2 관련 요청: {}", request.getRequestURI());
51-56: Inconsistent token extraction methodsThe filter uses different token extraction methods for different endpoints: bearer tokens from headers for signup endpoints and cookies for other API endpoints. This inconsistency might cause confusion. Consider documenting this behavior or standardizing the approach.
Consider implementing a unified token resolution strategy that checks both sources (header and cookie) with a clear precedence order, or clearly document why different endpoints require different token sources.
Also applies to: 66-72
src/main/java/life/mosu/mosuserver/infra/kmc/legacy/KmcMvcController.java (1)
55-63: Improve error handling with specific exceptionsThe generic exception handling makes it difficult to diagnose issues. Consider catching specific exceptions and providing more detailed error information.
try { // 토큰 검증 및 사용자 정보 조회 로직 KmcUserInfo userInfo = legacyService.validateTokenAndGetResult(apiToken, apiCertNum); model.addAttribute("userInfo", userInfo); return "success"; // 성공 시 보여줄 JSP 페이지 + } catch (CustomRuntimeException e) { + log.error("KMC verification failed: {}", e.getMessage()); + model.addAttribute("errorCode", e.getErrorCode()); + model.addAttribute("errorMessage", e.getMessage()); + return "error"; } catch (Exception e) { + log.error("Unexpected error during KMC verification", e); model.addAttribute("errorMessage", e.getMessage()); return "error"; // 실패 시 보여줄 JSP 페이지 }src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java (1)
17-17: Consider removing class-level @transactional annotation.The class-level
@Transactional(propagation = Propagation.NOT_SUPPORTED)annotation may be unnecessary since transactional event listeners already execute outside the original transaction context by design.-@Transactional(propagation = Propagation.NOT_SUPPORTED) public class KmcTxEventListener {src/main/java/life/mosu/mosuserver/application/auth/provider/RefreshTokenProvider.java (2)
37-42: Consider adding input validation for user ID.The
invalidateTokenmethod doesn't validate the input parameter, which could lead to issues with null or invalid user IDs.public void invalidateToken(final Long id) { + if (id == null || id <= 0) { + throw new CustomRuntimeException(ErrorCode.INVALID_USER_ID); + } if (!refreshTokenRepository.existsByUserId(id)) { throw new CustomRuntimeException(ErrorCode.NOT_EXIST_REFRESH_TOKEN); } refreshTokenRepository.deleteByUserId(id); }
44-52: Add validation for token caching parameters.The
cacheTokenmethod should validate its parameters to prevent invalid data from being stored.public void cacheToken(final Long userId, final String refreshToken) { + if (userId == null || userId <= 0) { + throw new CustomRuntimeException(ErrorCode.INVALID_USER_ID); + } + if (refreshToken == null || refreshToken.trim().isEmpty()) { + throw new CustomRuntimeException(ErrorCode.INVALID_REFRESH_TOKEN); + } refreshTokenRepository.save( RefreshToken.of( userId, refreshToken, expireTime ) ); }src/main/java/life/mosu/mosuserver/infra/kmc/KmcService.java (1)
28-28: Consider using a more robust certification number generation strategy.Using UUID for certification numbers is generally good, but consider implementing collision detection or using a more deterministic approach if uniqueness is critical for your use case.
You might want to consider:
- Adding a retry mechanism for UUID collision (though rare)
- Using a cryptographically secure random generator
- Including timestamp-based components for better traceability
build.gradle (1)
111-115: Document the reason for forcing Apache Commons Lang version.The resolution strategy forcing Apache Commons Lang to version 3.18.0 suggests there might be dependency conflicts. This should be documented to help future maintainers understand the reasoning.
configurations.configureEach { resolutionStrategy { + // Force Apache Commons Lang version to resolve conflicts with [specific dependency] force 'org.apache.commons:commons-lang3:3.18.0' } }src/main/java/life/mosu/mosuserver/infra/kmc/KmcAuthController.java (1)
19-26: Consider adding rate limiting for authentication endpoints.KMC authentication endpoints should have rate limiting to prevent abuse and potential security attacks.
Consider implementing:
- Rate limiting annotations (e.g.,
@RateLimiter)- Request throttling based on IP address
- Additional security headers for authentication endpoints
- CORS configuration if needed for cross-origin requests
src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java (1)
6-6: Remove unused import for@UserIdannotationThe
@UserIdannotation import is no longer used in this controller after refactoring to use@AuthenticationPrincipal. Consider removing this unused import.src/main/java/life/mosu/mosuserver/infra/kmc/KmcCryptoManager.java (1)
63-65: Improve exception handling specificityConsider creating a custom exception type for KMC-related errors instead of using generic
RuntimeException. This would help with error handling at higher levels.- } catch (Exception e) { - throw new RuntimeException("KMC 인증 결과를 처리하는 중 오류가 발생했습니다.", e); + } catch (SecurityException e) { + throw e; // Re-throw security exceptions as-is + } catch (Exception e) { + throw new KmcProcessingException("KMC 인증 결과를 처리하는 중 오류가 발생했습니다.", e); }You'll need to create a
KmcProcessingExceptionclass extendingRuntimeException.src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisRepository.java (1)
15-18: Consider adding logging for better observability.Adding debug/trace logging for save operations would help with debugging and monitoring token lifecycle in production environments.
@Override public void save(SignUpToken signUpToken) { + log.debug("Saving sign-up token with certNum: {}", signUpToken.getCertNum()); SignUpTokenRedisEntity entity = SignUpTokenRedisEntity.from(signUpToken); repository.save(entity); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
.DS_Storeis excluded by!**/.DS_Storegradle/.DS_Storeis excluded by!**/.DS_Storesrc/.DS_Storeis excluded by!**/.DS_Storesrc/main/.DS_Storeis excluded by!**/.DS_Storesrc/main/resources/.DS_Storeis excluded by!**/.DS_Storesrc/main/resources/db/.DS_Storeis excluded by!**/.DS_Storesrc/main/webapp/.DS_Storeis excluded by!**/.DS_Storesrc/main/webapp/WEB-INF/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (82)
.gitignore(1 hunks)build.gradle(4 hunks)src/main/java/life/mosu/mosuserver/application/auth/AccessTokenService.java(0 hunks)src/main/java/life/mosu/mosuserver/application/auth/AuthService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/auth/AuthTokenManager.java(0 hunks)src/main/java/life/mosu/mosuserver/application/auth/DanalVerificationService.java(0 hunks)src/main/java/life/mosu/mosuserver/application/auth/PrincipalDetails.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/PrincipalDetailsService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/SignUpService.java(2 hunks)src/main/java/life/mosu/mosuserver/application/auth/VerificationService.java(0 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/KmcEventTxService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcContext.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcIssueTxEvent.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventFactory.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/provider/AccessTokenProvider.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/provider/AuthTokenManager.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/provider/JwtTokenProvider.java(4 hunks)src/main/java/life/mosu/mosuserver/application/auth/provider/OneTimeTokenProvider.java(1 hunks)src/main/java/life/mosu/mosuserver/application/auth/provider/RefreshTokenProvider.java(2 hunks)src/main/java/life/mosu/mosuserver/application/oauth/OAuthUserService.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java(1 hunks)src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxFailureHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshToken.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenKeyValueRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRedisEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRedisRepository.java(2 hunks)src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpToken.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenKeyValueRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisEntity.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRepository.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/user/UserRole.java(1 hunks)src/main/java/life/mosu/mosuserver/domain/user/service/UserEncoderService.java(1 hunks)src/main/java/life/mosu/mosuserver/global/annotation/DomainService.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/KmcConfig.java(1 hunks)src/main/java/life/mosu/mosuserver/global/config/SecurityConfig.java(2 hunks)src/main/java/life/mosu/mosuserver/global/exception/AuthenticationException.java(1 hunks)src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java(1 hunks)src/main/java/life/mosu/mosuserver/global/handler/OAuth2LoginSuccessHandler.java(1 hunks)src/main/java/life/mosu/mosuserver/global/resolver/UserIdArgumentResolver.java(3 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/KmcAuthController.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/KmcCryptoManager.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/KmcDataMapper.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/KmcProperties.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/KmcService.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcApiRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcCertRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcCertResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcResultCallbackRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcResultCallbackResponse.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcUserInfo.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/dto/KmcVerificationRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/legacy/KmcMvcController.java(1 hunks)src/main/java/life/mosu/mosuserver/infra/kmc/legacy/LegacyService.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/admin/annotation/RefreshTokenHeader.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/AuthController.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/AuthControllerDocs.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/SignUpController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/SignUpControllerDocs.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/dto/SignUpAccountRequest.java(2 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/dto/SignUpRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/auth/resolver/RefreshTokenHeaderArgumentResolver.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/oauth/AccessTokenFilter.java(4 hunks)src/main/java/life/mosu/mosuserver/presentation/oauth/SignUpTokenService.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/profile/ProfileControllerDocs.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/user/MyUserController.java(3 hunks)src/main/java/life/mosu/mosuserver/presentation/user/dto/FindIdRequest.java(1 hunks)src/main/java/life/mosu/mosuserver/presentation/user/dto/FindIdResponse.java(1 hunks)src/main/resources/application.yml(2 hunks)src/main/resources/security-config.yml(1 hunks)src/main/webapp/WEB-INF/views/request.jsp(1 hunks)src/main/webapp/WEB-INF/views/result.jsp(1 hunks)src/main/webapp/WEB-INF/views/start.jsp(1 hunks)src/main/webapp/WEB-INF/views/step1.jsp(1 hunks)src/main/webapp/WEB-INF/views/step2.jsp(1 hunks)src/test/java/life/mosu/mosuserver/domain/user/service/UserEncoderServiceTest.java(1 hunks)src/test/java/life/mosu/mosuserver/infra/kmc/KmcServiceTest.java(1 hunks)src/test/resources/security-config.yml(1 hunks)
💤 Files with no reviewable changes (4)
- src/main/java/life/mosu/mosuserver/application/auth/AccessTokenService.java
- src/main/java/life/mosu/mosuserver/application/auth/DanalVerificationService.java
- src/main/java/life/mosu/mosuserver/application/auth/VerificationService.java
- src/main/java/life/mosu/mosuserver/application/auth/AuthTokenManager.java
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxFailureHandler.java (1)
src/main/java/life/mosu/mosuserver/global/runner/ApplicationSchoolPreWarmRunner.java (1)
RequiredArgsConstructor(9-19)
src/main/java/life/mosu/mosuserver/global/config/KmcConfig.java (1)
src/main/java/life/mosu/mosuserver/global/config/SecurityConfig.java (1)
Configuration(32-129)
src/main/java/life/mosu/mosuserver/application/auth/provider/AuthTokenManager.java (2)
src/main/java/life/mosu/mosuserver/presentation/auth/resolver/RefreshTokenHeaderArgumentResolver.java (1)
Component(12-32)src/main/java/life/mosu/mosuserver/application/auth/AuthTokenManager.java (1)
AuthTokenManager(12-44)
src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisEntity.java (1)
src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRedisEntity.java (1)
Getter(13-31)
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxFailureHandler.java (1)
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java (1)
Slf4j(14-39)
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java (2)
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxFailureHandler.java (1)
Slf4j(9-21)src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventFactory.java (1)
Component(7-14)
🪛 YAMLlint (1.37.1)
src/main/resources/security-config.yml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
src/test/resources/security-config.yml
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
🪛 GitHub Actions: Docker CI/CD
src/main/java/life/mosu/mosuserver/global/config/KmcConfig.java
[error] 2-10: Package 'com.icert.comm.secu' does not exist. Import and method return type 'IcertSecuManager' cannot be resolved.
src/main/java/life/mosu/mosuserver/infra/kmc/KmcCryptoManager.java
[error] 3-3: Package 'com.icert.comm.secu' does not exist. Import of 'IcertSecuManager' failed.
[error] 13-17: Cannot find symbol 'IcertSecuManager'. Class reference and @requiredargsconstructor annotation usage failed.
src/main/java/life/mosu/mosuserver/infra/kmc/legacy/LegacyService.java
[error] 3-3: Package 'com.icert.comm.secu' does not exist. Import of 'IcertSecuManager' failed.
[error] 18-21: Cannot find symbol 'IcertSecuManager'. Class reference and @requiredargsconstructor annotation usage failed.
🔇 Additional comments (93)
src/main/java/life/mosu/mosuserver/global/annotation/DomainService.java (1)
11-22: Well-designed custom annotation following Spring conventions.The
@DomainServiceannotation is properly implemented with appropriate meta-annotations and correctly delegates thevalueattribute to Spring's@Componentusing@AliasFor. This provides a semantic marker for domain services while maintaining Spring's component scanning functionality.src/main/java/life/mosu/mosuserver/presentation/user/MyUserController.java (2)
21-21: Request mapping change looks good.The change from
/userto/user/mebetter reflects the user-centric nature of these endpoints and follows RESTful conventions.
45-53: Good restoration of password change functionality.The password change method now properly returns the service response data instead of just a success message, which improves the API's usefulness.
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxFailureHandler.java (1)
13-16: Annotation order change looks good and keeps it consistent with other components
@Componentfirst and Lombok second is commonly adopted in the codebase, and there are no functional side-effects.
No further action required.src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenKeyValueRepository.java (1)
1-1: Package update acknowledgedInterface signature unchanged; only namespace adjusted. Confirm that any
@EnableRedisRepositories/@EnableKeyValueRepositoriesannotations reference the new base package.src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshToken.java (1)
1-1: Domain segregation approvedThe record now lives in a dedicated
refreshpackage, improving cohesion. No code issues spotted.src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRepository.java (1)
1-1: Namespace shift acknowledgedOnly location changed. Make sure any explicit component-scanning config or constructor injection reflects the updated package path.
.gitignore (1)
42-44: LGTM! Standard .gitignore improvements.The changes broaden the logs ignore pattern from a specific file to the entire directory and add exclusion for the libs directory, which aligns well with the build configuration updates mentioned in the summary.
src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpToken.java (1)
3-17: Well-designed domain record.The record implementation follows best practices with immutable data fields, clear factory methods, and appropriate conversion from the Redis entity. The design is clean and serves its purpose well as a domain model for signup tokens.
src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisEntity.java (1)
12-28: Well-structured Redis entity following established patterns.The entity follows the same design pattern as
RefreshTokenRedisEntitywith appropriate annotations, constructor access levels, and factory method. The implementation correctly uses@TimeToLivefor automatic expiration.src/main/java/life/mosu/mosuserver/application/auth/provider/JwtTokenProvider.java (3)
1-1: Excellent refactoring to provider pattern.The move to the provider package and making the class abstract demonstrates good separation of concerns, positioning this as a base class for specific token providers.
33-45: Improved constructor consistency.The parameter rename from
userJpaRepositorytouserRepositoryimproves naming consistency and makes the constructor more readable.
53-116: Solid JWT implementation with proper security practices.The core JWT functionality correctly implements:
- HMAC SHA-256 signing algorithm
- Token type validation to prevent token confusion attacks
- Proper expiration checking
- Appropriate exception handling for various JWT parsing scenarios
The removal of HTTP request handling methods properly separates token operations from request processing concerns.
src/main/java/life/mosu/mosuserver/global/handler/OAuth2LoginSuccessHandler.java (1)
7-7: LGTM: Import update follows package restructuring.The import change correctly reflects the relocation of
AuthTokenManagerto theproviderpackage as part of the authentication refactoring.src/main/java/life/mosu/mosuserver/application/oauth/OAuthUserService.java (1)
63-63: No active role enforcement—default ROLE_USER change has no current impactThe codebase isn’t enforcing any role checks at the moment:
- SecurityConfig.authorizeHttpRequests().anyRequest().permitAll()
- All
@PreAuthorize("…hasRole('…')")and.hasRole()calls are commented out- No controllers or service methods actively restrict access by role
As a result, switching the OAuth default from
PENDINGtoROLE_USERdoes not alter access control today. If you plan to enforce role-based access in the future, be sure to:
- Un-comment or add
requestMatchers(...).hasRole(...)rules inSecurityConfig.java- Re-enable
@PreAuthorize(or@Secured) on protected endpoints- Review default assignments for all user-creation flows (e.g., OAuth vs. standard signup)
src/main/java/life/mosu/mosuserver/application/auth/PrincipalDetailsService.java (1)
21-22: LGTM: Formatting improvement.The multi-line formatting improves readability without changing the logic.
src/main/resources/application.yml (1)
52-55: LGTM: JSP view resolver configuration is correct.The MVC view configuration properly enables JSP rendering with the standard
/WEB-INF/views/prefix and.jspsuffix, supporting the KMC identity verification UI implementation.src/main/java/life/mosu/mosuserver/global/config/SecurityConfig.java (1)
76-76: Verify CORS configuration for KMC integrationI couldn’t locate any KMC-related endpoints or header requirements in the codebase, so please manually confirm that:
- The
corsConfigurationSource()bean’sallowedOriginslist includes every KMC callback/origin URL.allowedHeadersis restricted to only the headers KMC actually sends (e.g.,Authorization,Content-Type) rather than*.allowCredentials(true)is acceptable for your security posture when paired with those specific origins and headers.src/main/java/life/mosu/mosuserver/application/auth/PrincipalDetails.java (1)
12-14: LGTM: Clean up redundant annotation and improve formatting.The removal of the
@Getterannotation is correct since record fields automatically generate getter methods, making the annotation redundant. The multi-line formatting also improves readability.src/main/java/life/mosu/mosuserver/domain/auth/refresh/RefreshTokenRedisRepository.java (2)
1-1: LGTM: Package relocation improves domain organization.Moving refresh token classes to a dedicated
refreshpackage consolidates related functionality and improves code organization.
19-19: LGTM: Minor style improvement.Removing the
finalmodifier from the local variable improves code consistency, asfinalis not necessary for local variables that are only assigned once.src/main/java/life/mosu/mosuserver/global/resolver/UserIdArgumentResolver.java (1)
8-8: LGTM: Logging capability added.Adding the
@Slf4jannotation enables logging functionality for the resolver.src/main/java/life/mosu/mosuserver/presentation/auth/dto/SignUpAccountRequest.java (1)
23-23: LGTM: Password example update aligns with branding.The updated password example maintains complexity requirements while using domain-specific branding.
src/main/java/life/mosu/mosuserver/application/payment/tx/PaymentTxEventListener.java (1)
13-15: LGTM: Annotation ordering improvement.The repositioning of annotations improves consistency with other transactional event handling classes without affecting functionality.
src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenKeyValueRepository.java (1)
5-7: Repository definition looks goodThe interface correctly extends
KeyValueRepositoryand will be detected automatically by Spring Data. No further action required.src/main/java/life/mosu/mosuserver/global/exception/ErrorCode.java (1)
27-28: Error-code additions are consistentThe newly added
VERIFICATION_FAILED(401) andNOT_FOUND_SIGN_UP_TOKEN(404) codes follow the existing naming and status conventions. 👍src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcIssueTxEvent.java (1)
5-9: Event wrapper is minimal and correctExtending
TxEvent<KmcContext>with a passthrough constructor suffices for the current use-case; no issues spotted.src/main/java/life/mosu/mosuserver/global/exception/AuthenticationException.java (1)
1-19: LGTM! Well-designed authentication exception class.The implementation follows Java exception best practices with appropriate constructors and a useful
loginIdfield for debugging authentication failures. The design is clean and focused.src/main/webapp/WEB-INF/views/request.jsp (1)
11-15: tr_cert and tr_url are internal values – no XSS risk
Both${tr_cert}and${tr_url}inrequest.jspare populated by your controller (seeKmcMvcController.java:model.addAttribute(...)) and by the KMC crypto service (encrypted, Base64‐safe output), not from user input. They cannot be manipulated by an attacker and therefore do not introduce an XSS vector.• tr_cert is generated and encrypted in
KmcCryptoManager
• tr_url is hard-coded inKmcMvcControllerIf you’d like defense-in-depth, you can render them with JSTL’s
<c:out escapeXml="true">, but no further validation is required here.src/main/java/life/mosu/mosuserver/presentation/auth/SignUpControllerDocs.java (2)
14-14: Clarify the operation description.The description mentions "step 1, step 2를 모두 받는" (receives both step 1 and step 2) but the method now uses
SignUpAccountRequest, which suggests it might only handle the account creation step. Please verify this description accurately reflects the API's functionality.
7-16: LGTM! Consistent refactoring of signup flow.The changes appropriately reflect the refactoring from
SignUpRequesttoSignUpAccountRequest, maintaining consistency with the broader signup flow restructuring.src/main/java/life/mosu/mosuserver/presentation/auth/SignUpController.java (2)
23-23: LGTM! Fixed the incorrect constant name.Good catch fixing the
ACCESS_TOKEN_COOKIE_NAMEfrom"refreshToken"to"accessToken". This constant name now correctly reflects the token type it represents.
7-34: LGTM! Consistent refactoring with signup flow changes.The change from
SignUpRequesttoSignUpAccountRequestis consistent with the broader signup flow refactoring and maintains the same functionality.src/main/java/life/mosu/mosuserver/presentation/auth/AuthControllerDocs.java (1)
22-23: LGTM: Cleaner interface design with custom annotation.The change from
HttpServletRequestto@RefreshTokenHeader String refreshTokensimplifies the interface and removes unnecessary coupling to servlet API. This aligns well with the new token provider architecture.src/main/java/life/mosu/mosuserver/presentation/auth/resolver/RefreshTokenHeaderArgumentResolver.java (1)
18-21: LGTM: Proper parameter validation.The
supportsParametermethod correctly validates both the annotation presence and parameter type. This ensures the resolver only handles appropriate parameters.src/main/java/life/mosu/mosuserver/application/auth/provider/AccessTokenProvider.java (2)
8-18: LGTM: Well-structured token provider implementation.The class follows good design principles:
- Clean inheritance from
JwtTokenProvider- Proper use of constants for token type and header
- Constructor injection with appropriate
@Valueannotations- Clear separation of concerns
13-14: Verify JWT configuration properties in your application configsPlease confirm that the following properties are declared in your configuration (e.g.
src/main/resources/application.ymlorapplication.properties) and use exactly the same names across all token providers:
jwt.access-token.expire-time(used by AccessTokenProvider & OneTimeTokenProvider)jwt.refresh-token.expire-time(used by RefreshTokenProvider)jwt.secret(used by all providers)Ensure there are no typos or mismatches in your config files so that each
@Value("${…}")reference resolves correctly at runtime.src/main/java/life/mosu/mosuserver/presentation/auth/dto/SignUpRequest.java (1)
3-13: LGTM: Good API documentation with Swagger annotations.The addition of
@Schemaannotations improves API documentation clarity. The Korean descriptions properly describe the two-step signup process:
- Step 1: Account information (계정 정보)
- Step 2: Profile information (프로필 정보)
src/main/webapp/WEB-INF/views/result.jsp (1)
56-56: LGTM: Functional test link for development.The link to restart the KMC verification process is useful for testing and development purposes.
src/main/java/life/mosu/mosuserver/application/auth/AuthService.java (1)
3-3: LGTM! Import updated to use provider-based token management.The import change aligns with the architectural refactoring from legacy token services to the new provider-based approach.
src/main/java/life/mosu/mosuserver/application/auth/SignUpService.java (3)
26-30: LGTM! Clean separation of account creation stepThe refactored
signUpmethod properly focuses on account creation and token generation, with clear transaction boundaries.
32-37: LGTM! Well-structured profile registration stepThe new
registerProfilemethod provides a clean separation of the profile creation step, with appropriate transaction management and clear delegation to the processor.
5-5: Verify new AuthTokenManager integration across servicesPlease confirm that all usages of the provider-based AuthTokenManager match its new API. Currently, references exist in:
SignUpService
src/main/java/life/mosu/mosuserver/application/auth/SignUpService.java
• import at line 5
• field injection at line 22AuthService
src/main/java/life/mosu/mosuserver/application/auth/AuthService.java
• import at line 3
• field injection at line 23OAuth2LoginSuccessHandler
src/main/java/life/mosu/mosuserver/global/handler/OAuth2LoginSuccessHandler.java
• import at line 7
• field injection at line 24Ensure each call site aligns with the methods and constructor signature defined in application/auth/provider/AuthTokenManager.java. If the interface changed (method names, parameters, return types), update these classes accordingly.
src/main/java/life/mosu/mosuserver/domain/auth/signup/SignUpTokenRedisRepository.java (8)
14-18: LGTM! Clean repository implementation.The save method correctly converts the domain object to the Redis entity and delegates to the underlying repository.
20-26: Excellent error handling with domain-specific exception.The method properly handles the not-found case by throwing a custom exception with a specific error code, which provides better error context than a generic exception.
28-31: Simple and appropriate delete implementation.The delete operation correctly delegates to the underlying repository. No error handling needed as Redis delete operations are idempotent.
8-12: Clean repository implementation setup.The class structure follows Spring Data repository patterns correctly with proper annotations and dependency injection.
14-18: Proper domain-to-entity conversion in save method.The save method correctly converts the domain model to Redis entity before persistence, following clean architecture principles.
20-26: Appropriate exception handling and entity-to-domain conversion.The method properly handles the case when token is not found with a descriptive custom exception and correctly converts the Redis entity back to domain model.
28-31: Straightforward delete operation.The delete method implementation is clean and follows the expected pattern for repository delete operations.
8-32: LGTM! Clean repository implementation with proper error handling.The implementation follows repository pattern best practices with appropriate exception handling for missing tokens. The conversion between domain objects and Redis entities is properly handled.
src/test/resources/security-config.yml (10)
1-21: Well-structured OAuth2 configuration with proper security practices.The Kakao OAuth2 configuration correctly uses environment variables for sensitive data and follows standard OAuth2 patterns.
22-29: Good security configuration for session cookies.The cookie settings include appropriate security flags:
http-onlyprevents XSS attacks,secureensures HTTPS transmission, andsame-site: Noneallows cross-origin requests as needed.
30-36: Proper JWT configuration with environment variable usage.JWT secrets and expiration times are correctly configured via environment variables, maintaining security best practices.
37-38: Clear endpoint configuration.The reissue endpoint is clearly defined for token management.
40-42: Appropriate use of environment variable for target URL.The target URL configuration follows the pattern of using environment variables for deployment-specific values.
1-20: Well-configured OAuth2 setup with proper security practices.The Kakao OAuth2 configuration correctly uses environment variables for sensitive data and follows standard OAuth2 patterns with appropriate scopes and endpoints.
22-28: Secure session cookie configuration.The session cookie settings follow security best practices with http-only, secure, and appropriate same-site settings for cross-origin OAuth flows.
30-35: Proper JWT configuration with environment variable usage.The JWT configuration correctly uses environment variables for the secret and allows separate expiration times for access and refresh tokens, following JWT best practices.
37-41: Clean endpoint and target URL configuration.The endpoint configuration is straightforward and properly uses environment variables for the target URL.
1-46: LGTM! Well-structured security configuration with proper environment variable usage.The configuration properly externalizes sensitive values and follows security best practices for session management and OAuth2 setup.
src/main/java/life/mosu/mosuserver/presentation/profile/ProfileControllerDocs.java (8)
12-12: Appropriate imports for authentication principal injection.The new imports support the cleaner authentication approach using Spring Security's principal injection.
Also applies to: 18-18
29-31: Excellent security improvement with authentication principal injection.Replacing explicit
userIdparameters with@AuthenticationPrincipal PrincipalDetailsis a significant security improvement. This ensures the user ID is derived from the authenticated session rather than potentially manipulated request parameters, and provides a cleaner API design.Also applies to: 38-40
12-18: Improved security with authentication principal injection.The addition of imports for
PrincipalDetailsand@AuthenticationPrincipalsupports the security improvement of using injected authenticated user context instead of explicit user ID parameters.
29-32: Excellent security improvement in create method.Replacing the explicit
userIdparameter with@AuthenticationPrincipal PrincipalDetailsprevents potential parameter tampering and ensures users can only create profiles for their authenticated identity.
38-43: No action needed:getProfileis intentionally parameterized for public accessThe
getProfile(@RequestParam Long userId)endpoint inProfileController.javais designed to retrieve any user’s profile by ID, whereas thecreateandupdateoperations rightly restrict access to the authenticated user via@AuthenticationPrincipal. This distinction is intentional and aligns with the API’s requirements for public profile viewing versus secured profile modification.
29-32: LGTM! Improved security by using authenticated principal.Changing from explicit
userIdparameter to@AuthenticationPrincipalprevents potential authorization bypasses and follows Spring Security best practices.
38-43: LGTM! Consistent with create method security improvements.The update method correctly uses authenticated principal instead of explicit user ID parameter.
50-53: Dismiss suggestion:getProfileintentionally uses auserIdquery for public profile retrievalThe
getProfileendpoint and its service layer both take an explicituserIdand don’t reference the authenticated principal, indicating it’s designed to fetch any user’s profile by ID (not just the caller’s). No change needed.• Controller signature (src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java:56)
• Service implementation (src/main/java/life/mosu/mosuserver/application/profile/ProfileService.java:49)Likely an incorrect or invalid review comment.
src/main/java/life/mosu/mosuserver/application/auth/provider/AuthTokenManager.java (9)
12-17: Excellent architectural improvement with provider pattern.The new provider-based approach provides better separation of concerns compared to the previous service-based implementation. Clean dependency injection follows Spring best practices.
19-27: Well-structured token generation with proper caching.The method correctly generates both tokens, caches the refresh token for later validation, and constructs the response with appropriate expiration times. The provider-based token generation is clean and maintainable.
29-42: Significant improvement in API design and testability.The new implementation accepting a
Stringparameter instead ofHttpServletRequestis much cleaner and more testable. The token reissuance flow is correct: validates input, extracts user context, invalidates old token, and generates new token pair. TheinvalidateTokenmethod name is more descriptive than the previousdeleteRefreshToken.
12-17: Clean provider pattern implementation.The refactoring to use
AccessTokenProviderandRefreshTokenProviderimproves separation of concerns and code organization. The move to theproviderpackage is also architecturally sound.
19-27: Token generation logic correctly updated for provider pattern.The method properly uses the new providers and maintains the same token generation workflow. The reference to
JwtTokenProvider.BEARER_TYPEand thecacheTokenmethod align with the provider pattern refactoring.
29-42: Excellent API improvement with cleaner method signature.The change from
HttpServletRequesttoString refreshTokenStringparameter is a significant improvement:
- Better separation of concerns - HTTP request handling is moved to the argument resolver layer
- More testable - No need to mock HttpServletRequest in unit tests
- Cleaner API - Method focus is purely on token logic
The updated method names (
invalidateTokenvsdeleteRefreshToken) and the upfront null check also improve code clarity.
12-27: LGTM! Excellent refactoring with improved separation of concerns.The new provider-based approach is much cleaner than the previous service-based implementation. Moving from
HttpServletRequestparameter to direct token string improves testability and follows single responsibility principle.
29-42: LGTM! Improved API design with cleaner token reissuance.The method signature change from
HttpServletRequesttoString refreshTokenStringis a significant improvement - it separates HTTP concerns from business logic and makes the method more testable and reusable.
25-26: VerifiedBEARER_TYPEin JwtTokenProvider
Confirmed thatJwtTokenProviderdefinesprotected static final String BEARER_TYPE = "Bearer";at line 25 in
src/main/java/life/mosu/mosuserver/application/auth/provider/JwtTokenProvider.java. No further action required.src/main/webapp/WEB-INF/views/start.jsp (1)
79-81: Incorrect review context—no XSS risk here
The controller’s GET handler for/reqreturnsstep2.jsp, notstart.jsp, so the original file reference is invalid. Moreover,tr_urlandtr_verare hard-coded, andtr_certis generated by a trusted service (not user input), so there’s no untrusted data being rendered. You can ignore the XSS sanitization warning in this case.Likely an incorrect or invalid review comment.
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java (1)
26-29: Consider security implications of logging certification numbers.The certification number (
certNum) appears to be sensitive data that is being logged at DEBUG level. Evaluate whether this could pose a security risk in production environments.Please review your logging policy and security requirements to determine if certification numbers should be logged, masked, or excluded from logs entirely.
src/main/java/life/mosu/mosuserver/application/auth/provider/RefreshTokenProvider.java (1)
1-27: Excellent refactoring of token provider architecture.The refactoring from service-based to provider-based architecture with clear separation of concerns is well-implemented. The use of static final constants and improved method naming (
invalidateTokenvsdeleteRefreshToken) enhances readability and maintainability.build.gradle (2)
5-5: Good addition of WAR plugin for JSP support.The addition of the
warplugin properly supports the new JSP views for KMC authentication flows.
28-28: No local JARs detected inlibs/– build reproducibility unaffectedI ran
fd -e jar libs/and found no JARs in thelibsdirectory, so thefileTreeinclusion has no effect today. There’s nothing to document or version. Feel free to leave this configuration as-is.src/main/java/life/mosu/mosuserver/infra/kmc/KmcAuthController.java (1)
21-22: Good use of consistent REST mapping and component structure.The controller follows good REST conventions with clear endpoint mapping and proper dependency injection.
src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java (3)
34-42: Good security improvement using authenticated principalThe change from explicit user ID parameter to authenticated principal is a security improvement, ensuring users can only create profiles for themselves. The logging addition is helpful for debugging.
44-53: Consistent security improvement for profile updatesThe implementation is consistent with the
createmethod, ensuring users can only update their own profiles through authenticated principal.
55-61: Verify the intended access pattern for profile viewingUnlike
createandupdatemethods that use authenticated principal,getProfilestill accepts any user ID as a parameter. This allows viewing any user's profile. Please verify if this is the intended behavior - should profile viewing be public or restricted?Consider adding
@PreAuthorizeannotation if access control is needed.src/main/java/life/mosu/mosuserver/infra/kmc/KmcCryptoManager.java (1)
74-74: Verify the empty string parameter in decryptionThe
getDecmethod is called with an empty string as the second parameter. Please verify if this is correct according to the KMC SDK documentation. If this parameter represents an encryption key or IV, it should not be empty.src/main/java/life/mosu/mosuserver/infra/kmc/KmcDataMapper.java (3)
13-19: LGTM! Clean dependency injection setup.The class structure follows Spring conventions with proper dependency injection. The dependencies on
KmcCryptoManagerandOneTimeTokenProviderare appropriate for the KMC data mapping functionality.
21-35: Good use of constants to eliminate magic numbers.The field index constants improve code readability and maintainability. However, I notice there's no constant defined for index 7 (jumps from
GENDER_INDEX = 6toNAME_INDEX = 8).Could you verify if index 7 is intentionally skipped in the KMC response format? If it contains data that should be processed, consider adding a constant for it.
93-104: Well-implemented utility method with proper error handling.This decryption utility method follows good practices:
- Proper exception handling with fallback values
- Appropriate logging level for failures
- Null/empty string validation
Currently only used by the logging methods that should be removed. Consider if CI/DI values are needed for the actual business logic (user identification, deduplication, etc.) - if so, this method should remain and be integrated into the main mapping flow.
Are the decrypted CI/DI values needed for business logic like user deduplication or identification? If so, they should be included in the
KmcUserInfoDTO.src/main/webapp/WEB-INF/views/step1.jsp (2)
11-16: LGTM! Proper cache control headers for security.The cache control headers properly prevent caching of sensitive authentication pages, which is important for security.
26-33: LGTM! Secure random number generation implementation.Using
SecureRandomwith proper seeding is the correct approach for generating cryptographically secure random numbers for authentication purposes.
| public Token reissueAccessToken(final String refreshTokenHeader) { | ||
| return authTokenManager.reissueAccessToken(refreshTokenHeader); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Method signature improvement enhances testability and separation of concerns.
The change from HttpServletRequest to String refreshTokenHeader decouples the service layer from HTTP servlet dependencies, making the code more testable and following better architectural principles.
Verify that proper validation is implemented in the controller layer or argument resolver to ensure the refresh token string is valid before reaching this service method:
🏁 Script executed:
#!/bin/bash
# Description: Verify that RefreshTokenHeaderArgumentResolver properly validates the refresh token
# Expected: Find validation logic in the argument resolver or controller
# Search for the RefreshTokenHeaderArgumentResolver implementation
ast-grep --pattern 'class RefreshTokenHeaderArgumentResolver {
$$$
}'
# Search for RefreshTokenHeader annotation usage and validation
rg -A 10 "@RefreshTokenHeader"Length of output: 1347
Missing RefreshTokenHeaderArgumentResolver & validation for refreshTokenHeader
I couldn’t find any implementation of RefreshTokenHeaderArgumentResolver or validation logic for the @RefreshTokenHeader parameter. Without it, malformed or missing refresh-token values will reach the service layer unchecked.
Please add and wire up a HandlerMethodArgumentResolver (e.g. RefreshTokenHeaderArgumentResolver) that:
- Extracts the header value
- Verifies it’s non-null/non-blank
- Validates its format (e.g. proper JWT structure)
- Fails fast with a clear 4xx response on invalid input
Likely locations to update:
- Create
src/main/java/life/mosu/mosuserver/presentation/auth/RefreshTokenHeaderArgumentResolver.javaimplementingHandlerMethodArgumentResolver - Register it in your MVC configuration
- Ensure controller method in
AuthController.javarelies on this resolver
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/application/auth/AuthService.java around
lines 47-48, the refreshTokenHeader parameter lacks validation and a custom
argument resolver. Create a new class RefreshTokenHeaderArgumentResolver in
src/main/java/life/mosu/mosuserver/presentation/auth/ that implements
HandlerMethodArgumentResolver to extract the refresh token header, check it is
non-null and non-blank, validate its JWT format, and throw a 4xx error on
invalid input. Register this resolver in the MVC configuration so it is used
automatically in controller methods, and update AuthController.java to use the
@RefreshTokenHeader annotation on the parameter to leverage this validation
before reaching the service layer.
| @Transactional | ||
| public void publishIssueEvent(String certNum, Long expiration) { | ||
| TxEvent<?> event = eventFactory.create( | ||
| KmcContext.ofSuccess(certNum, expiration) | ||
| ); | ||
| txEventPublisher.publish(event); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation for certification number
The method doesn't validate input parameters, which could lead to runtime issues if null or empty values are passed.
Add parameter validation:
@Transactional
public void publishIssueEvent(String certNum, Long expiration) {
+ if (certNum == null || certNum.trim().isEmpty()) {
+ throw new IllegalArgumentException("Certificate number cannot be null or empty");
+ }
+ if (expiration == null || expiration <= 0) {
+ throw new IllegalArgumentException("Expiration must be positive");
+ }
+
TxEvent<?> event = eventFactory.create(
KmcContext.ofSuccess(certNum, expiration)
);
txEventPublisher.publish(event);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Transactional | |
| public void publishIssueEvent(String certNum, Long expiration) { | |
| TxEvent<?> event = eventFactory.create( | |
| KmcContext.ofSuccess(certNum, expiration) | |
| ); | |
| txEventPublisher.publish(event); | |
| } | |
| @Transactional | |
| public void publishIssueEvent(String certNum, Long expiration) { | |
| if (certNum == null || certNum.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Certificate number cannot be null or empty"); | |
| } | |
| if (expiration == null || expiration <= 0) { | |
| throw new IllegalArgumentException("Expiration must be positive"); | |
| } | |
| TxEvent<?> event = eventFactory.create( | |
| KmcContext.ofSuccess(certNum, expiration) | |
| ); | |
| txEventPublisher.publish(event); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/auth/kmc/KmcEventTxService.java
around lines 17 to 23, the publishIssueEvent method lacks validation for the
certNum parameter. Add checks to ensure certNum is neither null nor empty before
proceeding. If validation fails, throw an appropriate exception such as
IllegalArgumentException with a clear message. This prevents runtime errors
caused by invalid input.
| @Transactional | ||
| public void publishFailureEvent(String certNum) { | ||
| TxEvent<?> event = eventFactory.create( | ||
| KmcContext.ofFailure(certNum) | ||
| ); | ||
| txEventPublisher.publish(event); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation for failure events
Similar to the success case, the failure event method should validate the certification number parameter.
@Transactional
public void publishFailureEvent(String certNum) {
+ if (certNum == null || certNum.trim().isEmpty()) {
+ throw new IllegalArgumentException("Certificate number cannot be null or empty");
+ }
+
TxEvent<?> event = eventFactory.create(
KmcContext.ofFailure(certNum)
);
txEventPublisher.publish(event);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Transactional | |
| public void publishFailureEvent(String certNum) { | |
| TxEvent<?> event = eventFactory.create( | |
| KmcContext.ofFailure(certNum) | |
| ); | |
| txEventPublisher.publish(event); | |
| } | |
| } | |
| @Transactional | |
| public void publishFailureEvent(String certNum) { | |
| if (certNum == null || certNum.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Certificate number cannot be null or empty"); | |
| } | |
| TxEvent<?> event = eventFactory.create( | |
| KmcContext.ofFailure(certNum) | |
| ); | |
| txEventPublisher.publish(event); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/auth/kmc/KmcEventTxService.java
around lines 25 to 32, the publishFailureEvent method lacks validation for the
certNum parameter. Add a check at the start of the method to validate certNum
(e.g., ensure it is not null or empty) and handle invalid input appropriately,
such as throwing an IllegalArgumentException, to match the validation approach
used in the success event method.
| public static KmcContext ofSuccess(String certNum, Long expiration) { | ||
| return new KmcContext(certNum, expiration, true); | ||
| } | ||
|
|
||
| public static KmcContext ofFailure(String certNum) { | ||
| return new KmcContext(certNum, 0L, false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation to factory methods
The factory methods don't validate input parameters, which could lead to invalid contexts being created.
Add validation to factory methods:
public static KmcContext ofSuccess(String certNum, Long expiration) {
+ if (certNum == null || certNum.trim().isEmpty()) {
+ throw new IllegalArgumentException("Certificate number cannot be null or empty");
+ }
+ if (expiration == null || expiration <= 0) {
+ throw new IllegalArgumentException("Expiration must be positive");
+ }
return new KmcContext(certNum, expiration, true);
}
public static KmcContext ofFailure(String certNum) {
+ if (certNum == null || certNum.trim().isEmpty()) {
+ throw new IllegalArgumentException("Certificate number cannot be null or empty");
+ }
return new KmcContext(certNum, 0L, false);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static KmcContext ofSuccess(String certNum, Long expiration) { | |
| return new KmcContext(certNum, expiration, true); | |
| } | |
| public static KmcContext ofFailure(String certNum) { | |
| return new KmcContext(certNum, 0L, false); | |
| } | |
| public static KmcContext ofSuccess(String certNum, Long expiration) { | |
| if (certNum == null || certNum.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Certificate number cannot be null or empty"); | |
| } | |
| if (expiration == null || expiration <= 0) { | |
| throw new IllegalArgumentException("Expiration must be positive"); | |
| } | |
| return new KmcContext(certNum, expiration, true); | |
| } | |
| public static KmcContext ofFailure(String certNum) { | |
| if (certNum == null || certNum.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Certificate number cannot be null or empty"); | |
| } | |
| return new KmcContext(certNum, 0L, false); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcContext.java
around lines 10 to 16, the factory methods ofSuccess and ofFailure lack input
parameter validation. Add checks to ensure certNum is not null or empty, and for
ofSuccess, verify expiration is a positive value. Throw appropriate exceptions
if validations fail to prevent creation of invalid KmcContext instances.
| public SignUpToken toSignUpToken() { | ||
| return SignUpToken.of( | ||
| this.certNum, | ||
| this.expiration | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider validating success state before token conversion
The toSignUpToken method converts the context to a SignUpToken without checking if the context represents a successful operation. This could create tokens for failed KMC attempts.
Add success state validation:
public SignUpToken toSignUpToken() {
+ if (!isSuccess) {
+ throw new IllegalStateException("Cannot create SignUpToken from failed KMC context");
+ }
return SignUpToken.of(
this.certNum,
this.expiration
);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcContext.java
around lines 18 to 24, the toSignUpToken method converts the context to a
SignUpToken without verifying if the context indicates a successful operation.
To fix this, add a check for the success state before creating the SignUpToken,
and only proceed with token creation if the context represents success;
otherwise, handle the failure case appropriately, such as throwing an exception
or returning null.
| <td align=center>�������URL</td> | ||
| <td align=left><input type="text" name="tr_url" size="41" | ||
| value="http://�����纰 ���/kmcis_web_sample_step03.jsp"></td> | ||
| </tr> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update hardcoded return URL placeholder.
The return URL field contains a hardcoded placeholder that should be updated to reflect the actual deployment environment.
-value="http://서비스별 도메인/kmcis_web_sample_step03.jsp"
+value="${pageContext.request.scheme}://${pageContext.request.serverName}${pageContext.request.contextPath}/api/v1/step3"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step1.jsp around lines 126 to 129, the input
field for the return URL currently has a hardcoded placeholder URL. Replace this
hardcoded URL with a dynamic value or a configuration-based URL that reflects
the actual deployment environment to ensure it works correctly across different
setups.
| function submitKmcForm() { | ||
| var KMCIS_window; | ||
| var UserAgent = navigator.userAgent; | ||
|
|
||
| // 모바일 기기인지 확인합니다. | ||
| if (UserAgent.match( | ||
| /iPhone|iPad|Android|Windows CE|BlackBerry|Symbian|Windows Phone|webOS|Opera Mini|Opera Mobi|POLARIS|IEMobile|lgtelecom|nokia|SonyEricsson/i) | ||
| != null || UserAgent.match(/LG|SAMSUNG|Samsung/) != null) { | ||
| document.reqKMCISForm.target = ''; // 모바일은 현재 창에서 진행합니다. | ||
| } else { | ||
| // PC 환경에서는 팝업 창을 엽니다. | ||
| KMCIS_window = window.open('', 'KMCISWindow', | ||
| 'width=425, height=550, resizable=0, scrollbars=no, status=0, titlebar=0, left=435, top=250'); | ||
|
|
||
| if (KMCIS_window == null) { | ||
| alert("팝업이 차단되었습니다. 팝업 차단을 해제하고 다시 시도해주세요."); | ||
| return; // 팝업이 차단되면 더 이상 진행하지 않습니다. | ||
| } | ||
| document.reqKMCISForm.target = 'KMCISWindow'; // form의 타겟을 팝업창으로 설정합니다. | ||
| } | ||
|
|
||
| // form을 전송합니다. | ||
| document.reqKMCISForm.submit(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve mobile detection and popup handling for better maintainability.
The current implementation has several areas for improvement:
- Simplify mobile detection: The current regex is overly complex and brittle:
- if (UserAgent.match(
- /iPhone|iPad|Android|Windows CE|BlackBerry|Symbian|Windows Phone|webOS|Opera Mini|Opera Mobi|POLARIS|IEMobile|lgtelecom|nokia|SonyEricsson/i)
- != null || UserAgent.match(/LG|SAMSUNG|Samsung/) != null) {
+ var isMobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(UserAgent);
+ if (isMobile) {- Make popup dimensions responsive:
- KMCIS_window = window.open('', 'KMCISWindow',
- 'width=425, height=550, resizable=0, scrollbars=no, status=0, titlebar=0, left=435, top=250');
+ var width = 425;
+ var height = 550;
+ var left = (screen.width - width) / 2;
+ var top = (screen.height - height) / 2;
+ KMCIS_window = window.open('', 'KMCISWindow',
+ `width=${width}, height=${height}, resizable=0, scrollbars=no, status=0, titlebar=0, left=${left}, top=${top}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function submitKmcForm() { | |
| var KMCIS_window; | |
| var UserAgent = navigator.userAgent; | |
| // 모바일 기기인지 확인합니다. | |
| if (UserAgent.match( | |
| /iPhone|iPad|Android|Windows CE|BlackBerry|Symbian|Windows Phone|webOS|Opera Mini|Opera Mobi|POLARIS|IEMobile|lgtelecom|nokia|SonyEricsson/i) | |
| != null || UserAgent.match(/LG|SAMSUNG|Samsung/) != null) { | |
| document.reqKMCISForm.target = ''; // 모바일은 현재 창에서 진행합니다. | |
| } else { | |
| // PC 환경에서는 팝업 창을 엽니다. | |
| KMCIS_window = window.open('', 'KMCISWindow', | |
| 'width=425, height=550, resizable=0, scrollbars=no, status=0, titlebar=0, left=435, top=250'); | |
| if (KMCIS_window == null) { | |
| alert("팝업이 차단되었습니다. 팝업 차단을 해제하고 다시 시도해주세요."); | |
| return; // 팝업이 차단되면 더 이상 진행하지 않습니다. | |
| } | |
| document.reqKMCISForm.target = 'KMCISWindow'; // form의 타겟을 팝업창으로 설정합니다. | |
| } | |
| // form을 전송합니다. | |
| document.reqKMCISForm.submit(); | |
| } | |
| function submitKmcForm() { | |
| var KMCIS_window; | |
| var UserAgent = navigator.userAgent; | |
| // 모바일 기기인지 확인합니다. | |
| var isMobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(UserAgent); | |
| if (isMobile) { | |
| document.reqKMCISForm.target = ''; // 모바일은 현재 창에서 진행합니다. | |
| } else { | |
| // PC 환경에서는 팝업 창을 엽니다. | |
| var width = 425; | |
| var height = 550; | |
| var left = (screen.width - width) / 2; | |
| var top = (screen.height - height) / 2; | |
| KMCIS_window = window.open('', 'KMCISWindow', | |
| `width=${width}, height=${height}, resizable=0, scrollbars=no, status=0, titlebar=0, left=${left}, top=${top}`); | |
| if (KMCIS_window == null) { | |
| alert("팝업이 차단되었습니다. 팝업 차단을 해제하고 다시 시도해주세요."); | |
| return; // 팝업이 차단되면 더 이상 진행하지 않습니다. | |
| } | |
| document.reqKMCISForm.target = 'KMCISWindow'; // form의 타겟을 팝업창으로 설정합니다. | |
| } | |
| // form을 전송합니다. | |
| document.reqKMCISForm.submit(); | |
| } |
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step2.jsp around lines 8 to 31, the mobile
device detection uses a complex and brittle regex, and the popup window
dimensions are fixed. Simplify mobile detection by using a more maintainable and
widely accepted method, such as checking for 'Mobi' in the user agent string.
Adjust the popup window dimensions to be responsive based on the user's screen
size instead of fixed values, calculating width, height, and position
dynamically to improve usability across different screen resolutions.
| <form name="reqKMCISForm" method="post" | ||
| action="https://www.kmcert.com/kmcis/web/kmcisReq.jsp"> | ||
| <input type="hidden" name="tr_cert" value="${tr_cert}"> | ||
| <input type="hidden" name="tr_url" value="${tr_url}"> | ||
| <input type="hidden" name="tr_ver" value="V2"> | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Externalize the KMC URL and add security considerations.
The hardcoded external URL poses maintenance and security concerns.
- Externalize the KMC URL:
<form name="reqKMCISForm" method="post"
- action="https://www.kmcert.com/kmcis/web/kmcisReq.jsp">
+ action="${kmcCertificationUrl}">-
Add CSRF protection if not already handled at the framework level.
-
Validate server-side: Ensure all hidden input values are validated on the backend before being rendered to prevent injection attacks.
Verify that proper security measures are in place:
🏁 Script executed:
#!/bin/bash
# Description: Check for KMC URL configuration and CSRF protection
# Expected: Find externalized configuration and security measures
# Search for KMC URL configuration
rg -r "kmcCertificationUrl|kmc.*url" --type java
# Check for CSRF protection configuration
rg -r "csrf|CSRF" --type java -A 5Length of output: 881
🏁 Script executed:
#!/bin/bash
set -e
# Search for an externalized KMC URL in properties, YAML, or Java configuration
rg -n -e "kmcCertificationUrl" -t properties -t yml -t java
# Search for CSRF protection setup in Java or XML (e.g., Spring Security)
rg -n -e "csrf" -t java -t xmlLength of output: 103
Externalize the KMC URL & Add CSRF Protection
The JSP still contains a hard-coded external endpoint and no visible CSRF token. Please:
- Move the KMC endpoint into a config (e.g.
application.propertiesorweb.xml), and reference it in the JSP:<form name="reqKMCISForm" method="post" - action="https://www.kmcert.com/kmcis/web/kmcisReq.jsp"> + action="${kmcCertificationUrl}">
- Inject a CSRF token into every form (assuming Spring Security, for example):
<input type="hidden" name="tr_ver" value="V2"> +<input type="hidden" name="${_csrf.parameterName}" value="${_csrf.token}"> - On the backend, validate all incoming parameters (
tr_cert,tr_url,tr_ver) to guard against tampering.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <form name="reqKMCISForm" method="post" | |
| action="https://www.kmcert.com/kmcis/web/kmcisReq.jsp"> | |
| <input type="hidden" name="tr_cert" value="${tr_cert}"> | |
| <input type="hidden" name="tr_url" value="${tr_url}"> | |
| <input type="hidden" name="tr_ver" value="V2"> | |
| <form name="reqKMCISForm" method="post" | |
| action="${kmcCertificationUrl}"> | |
| <input type="hidden" name="tr_cert" value="${tr_cert}"> | |
| <input type="hidden" name="tr_url" value="${tr_url}"> | |
| <input type="hidden" name="tr_ver" value="V2"> | |
| <input type="hidden" name="${_csrf.parameterName}" value="${_csrf.token}"> |
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step2.jsp around lines 37 to 42, the form
action URL is hard-coded and lacks CSRF protection. Move the KMC endpoint URL
into a configuration file like application.properties or web.xml, then reference
this config property in the JSP to avoid hardcoding. Add a CSRF token input
field to the form, using the framework's standard method (e.g., Spring
Security's CSRF token tag or expression). Additionally, ensure the backend
controller validates all incoming parameters (tr_cert, tr_url, tr_ver) to
prevent tampering.
| //package life.mosu.mosuserver.domain.user.service; | ||
| // | ||
| //import static org.junit.jupiter.api.Assertions.*; | ||
| // | ||
| //import lombok.RequiredArgsConstructor; | ||
| //import org.junit.jupiter.api.Test; | ||
| //import org.springframework.beans.factory.annotation.Autowired; | ||
| // | ||
| //@RequiredArgsConstructor(onConstructor_ = @Autowired) | ||
| //class UserEncoderServiceTest { | ||
| // | ||
| // | ||
| // private UserEncoderService userEncoderService; | ||
| // | ||
| // @Test | ||
| // void e() { | ||
| // userEncoderService.e("awef"); | ||
| // } | ||
| //} No newline at end of file |
There was a problem hiding this comment.
Remove commented-out placeholder test code.
This entire test class is commented out and contains no meaningful test logic. Commented-out code should not be committed to the repository as it adds noise and maintenance overhead.
Consider either:
- Remove this file entirely if the service doesn't need testing yet
- Implement a proper test with assertions and Spring Boot test annotations
If you choose to implement the test, use this structure:
-//package life.mosu.mosuserver.domain.user.service;
-//
-//import static org.junit.jupiter.api.Assertions.*;
-//
-//import lombok.RequiredArgsConstructor;
-//import org.junit.jupiter.api.Test;
-//import org.springframework.beans.factory.annotation.Autowired;
-//
-//@RequiredArgsConstructor(onConstructor_ = @Autowired)
-//class UserEncoderServiceTest {
-//
-//
-// private UserEncoderService userEncoderService;
-//
-// @Test
-// void e() {
-// userEncoderService.e("awef");
-// }
-//}
+package life.mosu.mosuserver.domain.user.service;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+
+@SpringBootTest
+class UserEncoderServiceTest {
+
+ @Autowired
+ private UserEncoderService userEncoderService;
+
+ @Test
+ void shouldEncodeString() {
+ // Add meaningful test assertions here
+ userEncoderService.e("test");
+ // Assert expected behavior
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //package life.mosu.mosuserver.domain.user.service; | |
| // | |
| //import static org.junit.jupiter.api.Assertions.*; | |
| // | |
| //import lombok.RequiredArgsConstructor; | |
| //import org.junit.jupiter.api.Test; | |
| //import org.springframework.beans.factory.annotation.Autowired; | |
| // | |
| //@RequiredArgsConstructor(onConstructor_ = @Autowired) | |
| //class UserEncoderServiceTest { | |
| // | |
| // | |
| // private UserEncoderService userEncoderService; | |
| // | |
| // @Test | |
| // void e() { | |
| // userEncoderService.e("awef"); | |
| // } | |
| //} | |
| package life.mosu.mosuserver.domain.user.service; | |
| import org.junit.jupiter.api.Test; | |
| import org.springframework.beans.factory.annotation.Autowired; | |
| import org.springframework.boot.test.context.SpringBootTest; | |
| @SpringBootTest | |
| class UserEncoderServiceTest { | |
| @Autowired | |
| private UserEncoderService userEncoderService; | |
| @Test | |
| void shouldEncodeString() { | |
| // Add meaningful test assertions here | |
| userEncoderService.e("test"); | |
| // e.g. assertThat(...).isEqualTo(...) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/life/mosu/mosuserver/domain/user/service/UserEncoderServiceTest.java
lines 1 to 19, the entire test class is commented out and contains no meaningful
test logic. Remove this commented-out placeholder test code entirely to avoid
clutter in the repository. If testing is needed, either delete this file or
create a proper test class with valid test methods, assertions, and necessary
Spring Boot test annotations.
| class KmcServiceTest { | ||
|
|
||
| @Test | ||
| @DisplayName("KMC 본인인증 요청 데이터 생성 테스트") | ||
| void encryptTrCert() { | ||
| Assertions.assertEquals(1, 1); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement meaningful test logic for KMC identity verification.
This placeholder test with a trivial assertion (Assertions.assertEquals(1, 1)) needs proper implementation to test KMC identity verification request data generation as indicated by the method name and display name.
Would you like me to help generate proper test implementation based on the KMC service functionality, or should this be tracked as a separate task?
🤖 Prompt for AI Agents
In src/test/java/life/mosu/mosuserver/infra/kmc/KmcServiceTest.java around lines
7 to 14, the test method encryptTrCert currently contains only a trivial
assertion and lacks meaningful test logic. Replace the placeholder assertion
with actual test code that verifies the correct generation of KMC identity
verification request data, using relevant inputs and asserting expected outputs
based on the KMC service's functionality.
| implementation 'javax.servlet:jstl:1.2' | ||
| implementation "org.apache.tomcat.embed:tomcat-embed-jasper" | ||
|
|
||
| // swagger | ||
| implementation "org.springdoc:springdoc-openapi-starter-webmvc-ui:2.7.0" | ||
| implementation "org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.9" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify JSP dependency versions for security.
The JSP and servlet dependencies should be checked for known security vulnerabilities, especially since they're handling user authentication flows.
🌐 Web query:
Are there any known security vulnerabilities in javax.servlet:jstl:1.2 and org.apache.tomcat.embed:tomcat-embed-jasper for Spring Boot 3.4.5?
💡 Result:
There are known security vulnerabilities affecting both javax.servlet:jstl:1.2 and, indirectly, some versions of org.apache.tomcat.embed:tomcat-embed-jasper, which you should consider when using them with Spring Boot 3.4.5.
javax.servlet:jstl:1.2
- This version is affected by a security vulnerability (notably, XXE and Remote Code Execution via XSLT extension) that allows processing of external entities in untrusted XML documents, posing a risk of information disclosure or arbitrary code execution[4][1][7].
- The vulnerability applies to Standard Taglib up to version 1.2.1 (javax.servlet:jstl:1.2 is affected)[4].
- Remediation: Use a newer, compatible taglib implementation, such as org.apache.taglibs:taglibs-standard-impl:1.2.5, or jakarta equivalents, as suggested in multiple security advisories[4]. JSTL 1.2 is outdated and not considered secure for modern deployments.
org.apache.tomcat.embed:tomcat-embed-jasper
- This artifact is a part of the embedded Tomcat used in Spring Boot for JSP/Servlet processing.
- Recent security disclosures, such as CVE-2025-24813, affect Apache Tomcat itself, specifically in handling HTTP PUT requests, which could allow unauthenticated remote code execution under non-default, uncommon configurations[2]. The vulnerability can affect applications using embedded Tomcat if exploitable configurations are present.
- Remediation: Use a version of Tomcat (and thus tomcat-embed-jasper) that includes patches for all relevant CVEs (especially CVE-2025-24813). Monitor the official Tomcat security pages for the exact fixed versions[9][6]. Spring Boot does not always include the latest Tomcat by default, so explicit version management may be needed.
- No direct vulnerabilities in tomcat-embed-jasper itself were listed in Snyk, but the underlying Tomcat engine must be kept up to date[5].
Key Recommendations
- Avoid javax.servlet:jstl:1.2. If you require JSTL, use a maintained implementation (such as org.apache.taglibs:taglibs-standard-impl:1.2.5) to address known vulnerabilities[4].
- Ensure Tomcat (embedded via tomcat-embed-jasper) is up to date and at a version patched for known CVEs, especially CVE-2025-24813[2].
- Regularly review your dependency versions for security advisories.
Caveats & Limitations
- The identified vulnerabilities depend on application use and configuration. Not all environments may be directly exploitable, but risk remains significant if defaults are used or if untrusted XML/XSLT inputs are processed.
- No comprehensive vulnerability list for org.apache.tomcat.embed:tomcat-embed-jasper as a standalone artifact was provided[5]; always refer to Tomcat’s official security notifications[9].
In summary: javax.servlet:jstl:1.2 is insecure and should be replaced; keep Tomcat and related embed packages updated to a patched version matching Spring Boot 3.4.5, watching for any new disclosures.
Citations:
- 1: https://security.snyk.io/package/maven/javax.servlet.jsp.jstl:jstl/1.2
- 2: https://www.rapid7.com/blog/post/2025/03/19/etr-apache-tomcat-cve-2025-24813-what-you-need-to-know/
- 3: https://www.javaspringhibernate.com/spring-framework-training/forum/6678/jstl-and-tomcat-embed-jasper-dependency-is-not-recognized
- 4: https://www.redhat.com/en/blog/avoiding-cve-2015-0254-java-applications-openshift-online
- 5: https://security.snyk.io/package/maven/org.apache.tomcat.embed:tomcat-embed-jasper
- 6: https://tomcat.apache.org/security-10.html
- 7: https://security.snyk.io/package/maven/javax.servlet:jstl/1.2
- 8: https://community.jaspersoft.com/forums/topic/49224-upgrade-just-apache-tomcat/
- 9: https://tomcat.apache.org/security-9.html
- 10: https://www.cisa.gov/news-events/bulletins/sb25-049
Update JSP/Servlet dependencies to secure versions
Please replace the outdated JSTL artifact and pin your embedded Tomcat to a patched release:
• File: build.gradle (lines 43–47)
• Replace javax.servlet:jstl:1.2 (XXE/RCE risk) with a maintained implementation
• Pin tomcat-embed-jasper to a Tomcat version patched for CVE-2025-24813 (and other recent CVEs)
Suggested diff:
- implementation 'javax.servlet:jstl:1.2'
- implementation "org.apache.tomcat.embed:tomcat-embed-jasper"
+ implementation 'org.apache.taglibs:taglibs-standard-impl:1.2.5'
+ implementation "org.apache.tomcat.embed:tomcat-embed-jasper:10.1.17"(Optional) If you’re migrating fully to Jakarta EE 9+, consider:
implementation 'jakarta.servlet.jsp.jstl-api:3.0.0'
implementation 'org.apache.taglibs:taglibs-standard-impl:3.0.0'🤖 Prompt for AI Agents
In build.gradle around lines 43 to 47, update the JSP/Servlet dependencies to
secure versions by replacing 'javax.servlet:jstl:1.2' with a maintained JSTL
implementation such as 'jakarta.servlet.jsp.jstl-api:3.0.0' and
'org.apache.taglibs:taglibs-standard-impl:3.0.0'. Also, pin
'org.apache.tomcat.embed:tomcat-embed-jasper' to a specific Tomcat version that
includes patches for CVE-2025-24813 and other recent vulnerabilities to ensure
security.
| public void afterCommitHandler(KmcIssueTxEvent event) { | ||
| KmcContext ctx = event.getContext(); | ||
| log.debug("[AFTER_COMMIT] 커밋 후 처리 시작: certNum={}", ctx.certNum()); | ||
| SignUpToken token = ctx.toSignUpToken(); | ||
| repository.save(token); | ||
| log.debug("[AFTER_COMMIT] 커밋 후 처리 완료: certNum={}", ctx.certNum()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add exception handling to prevent event processing failures.
The afterCommitHandler method lacks exception handling, which could cause the event processing to fail silently or propagate exceptions that might affect other parts of the system.
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void afterCommitHandler(KmcIssueTxEvent event) {
- KmcContext ctx = event.getContext();
- log.debug("[AFTER_COMMIT] 커밋 후 처리 시작: certNum={}", ctx.certNum());
- SignUpToken token = ctx.toSignUpToken();
- repository.save(token);
- log.debug("[AFTER_COMMIT] 커밋 후 처리 완료: certNum={}", ctx.certNum());
+ try {
+ KmcContext ctx = event.getContext();
+ log.debug("[AFTER_COMMIT] 커밋 후 처리 시작: certNum={}", ctx.certNum());
+ SignUpToken token = ctx.toSignUpToken();
+ repository.save(token);
+ log.debug("[AFTER_COMMIT] 커밋 후 처리 완료: certNum={}", ctx.certNum());
+ } catch (Exception ex) {
+ log.error("[AFTER_COMMIT] 토큰 저장 실패: certNum={}", event.getContext().certNum(), ex);
+ // Consider whether to rethrow or handle gracefully based on business requirements
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void afterCommitHandler(KmcIssueTxEvent event) { | |
| KmcContext ctx = event.getContext(); | |
| log.debug("[AFTER_COMMIT] 커밋 후 처리 시작: certNum={}", ctx.certNum()); | |
| SignUpToken token = ctx.toSignUpToken(); | |
| repository.save(token); | |
| log.debug("[AFTER_COMMIT] 커밋 후 처리 완료: certNum={}", ctx.certNum()); | |
| } | |
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) | |
| public void afterCommitHandler(KmcIssueTxEvent event) { | |
| try { | |
| KmcContext ctx = event.getContext(); | |
| log.debug("[AFTER_COMMIT] 커밋 후 처리 시작: certNum={}", ctx.certNum()); | |
| SignUpToken token = ctx.toSignUpToken(); | |
| repository.save(token); | |
| log.debug("[AFTER_COMMIT] 커밋 후 처리 완료: certNum={}", ctx.certNum()); | |
| } catch (Exception ex) { | |
| log.error("[AFTER_COMMIT] 토큰 저장 실패: certNum={}", event.getContext().certNum(), ex); | |
| // Consider whether to rethrow or handle gracefully based on business requirements | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java
around lines 24 to 30, the afterCommitHandler method does not have exception
handling, risking silent failures or unhandled exceptions during event
processing. Add a try-catch block around the method's logic to catch any
exceptions, log the error details for troubleshooting, and prevent exceptions
from propagating and affecting other system components.
| @TransactionalEventListener(phase = TransactionPhase.AFTER_ROLLBACK) | ||
| public void afterRollbackHandler(KmcIssueTxEvent event) { | ||
| KmcContext ctx = event.getContext(); | ||
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 시작: certNum={}", ctx.certNum()); | ||
| kmcFailureHandler.handle(event); | ||
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 완료: certNum={}", ctx.certNum()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add exception handling for rollback event processing.
Similar to the commit handler, the rollback handler should have proper exception handling to prevent cascading failures.
@TransactionalEventListener(phase = TransactionPhase.AFTER_ROLLBACK)
public void afterRollbackHandler(KmcIssueTxEvent event) {
- KmcContext ctx = event.getContext();
- log.debug("[AFTER_ROLLBACK] 롤백 후 처리 시작: certNum={}", ctx.certNum());
- kmcFailureHandler.handle(event);
- log.debug("[AFTER_ROLLBACK] 롤백 후 처리 완료: certNum={}", ctx.certNum());
+ try {
+ KmcContext ctx = event.getContext();
+ log.debug("[AFTER_ROLLBACK] 롤백 후 처리 시작: certNum={}", ctx.certNum());
+ kmcFailureHandler.handle(event);
+ log.debug("[AFTER_ROLLBACK] 롤백 후 처리 완료: certNum={}", ctx.certNum());
+ } catch (Exception ex) {
+ log.error("[AFTER_ROLLBACK] 롤백 처리 실패: certNum={}", event.getContext().certNum(), ex);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @TransactionalEventListener(phase = TransactionPhase.AFTER_ROLLBACK) | |
| public void afterRollbackHandler(KmcIssueTxEvent event) { | |
| KmcContext ctx = event.getContext(); | |
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 시작: certNum={}", ctx.certNum()); | |
| kmcFailureHandler.handle(event); | |
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 완료: certNum={}", ctx.certNum()); | |
| } | |
| @TransactionalEventListener(phase = TransactionPhase.AFTER_ROLLBACK) | |
| public void afterRollbackHandler(KmcIssueTxEvent event) { | |
| try { | |
| KmcContext ctx = event.getContext(); | |
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 시작: certNum={}", ctx.certNum()); | |
| kmcFailureHandler.handle(event); | |
| log.debug("[AFTER_ROLLBACK] 롤백 후 처리 완료: certNum={}", ctx.certNum()); | |
| } catch (Exception ex) { | |
| log.error("[AFTER_ROLLBACK] 롤백 처리 실패: certNum={}", event.getContext().certNum(), ex); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/auth/kmc/tx/KmcTxEventListener.java
around lines 32 to 38, the afterRollbackHandler method lacks exception handling,
which may cause cascading failures during rollback event processing. Wrap the
method body in a try-catch block to catch any exceptions thrown by
kmcFailureHandler.handle(event) or other code, and log the exception details
appropriately to prevent the exception from propagating further.
| @PostMapping("/confirm") | ||
| public ResponseEntity<ApiResponseWrapper<KmcCertResponse>> handleKmcResult( | ||
| @RequestBody KmcCertRequest request | ||
| ) { | ||
| log.info("STEP1 - KMC 본인인증 요청: {}", request.serviceTerm()); | ||
| KmcCertResponse response = kmcService.createCertRequest(request); | ||
| return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add request validation and consider security implications.
The endpoint lacks input validation and handles sensitive authentication data. Consider adding validation annotations and security measures.
@PostMapping("/confirm")
public ResponseEntity<ApiResponseWrapper<KmcCertResponse>> handleKmcResult(
- @RequestBody KmcCertRequest request
+ @Valid @RequestBody KmcCertRequest request
) {
log.info("STEP1 - KMC 본인인증 요청: {}", request.serviceTerm());
KmcCertResponse response = kmcService.createCertRequest(request);
return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response));
}🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/infra/kmc/KmcAuthController.java around
lines 27 to 34, the handleKmcResult method lacks input validation and security
considerations for sensitive authentication data. Add validation annotations
such as @Valid to the KmcCertRequest parameter and ensure the KmcCertRequest
class has appropriate validation constraints on its fields. Additionally,
consider implementing security measures like authentication, authorization
checks, and input sanitization to protect the endpoint from unauthorized access
and malicious input.
| @PostMapping("/decrypt") | ||
| public ResponseEntity<ApiResponseWrapper<KmcResultCallbackResponse>> tokenDecrypt( | ||
| @RequestBody KmcResultCallbackRequest request | ||
| ) { | ||
| log.info("STEP2 - KMC 본인인증 요청: {}", request.apiToken()); | ||
| KmcResultCallbackResponse response = kmcService.tokenDecrypt(request.apiToken()); | ||
| return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider security implications of token logging.
Logging the API token in step 2 could be a security risk as tokens are sensitive authentication data.
@PostMapping("/decrypt")
public ResponseEntity<ApiResponseWrapper<KmcResultCallbackResponse>> tokenDecrypt(
- @RequestBody KmcResultCallbackRequest request
+ @Valid @RequestBody KmcResultCallbackRequest request
) {
- log.info("STEP2 - KMC 본인인증 요청: {}", request.apiToken());
+ log.info("STEP2 - KMC 본인인증 요청 수신 (토큰 길이: {})",
+ request.apiToken() != null ? request.apiToken().length() : 0);
KmcResultCallbackResponse response = kmcService.tokenDecrypt(request.apiToken());
return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @PostMapping("/decrypt") | |
| public ResponseEntity<ApiResponseWrapper<KmcResultCallbackResponse>> tokenDecrypt( | |
| @RequestBody KmcResultCallbackRequest request | |
| ) { | |
| log.info("STEP2 - KMC 본인인증 요청: {}", request.apiToken()); | |
| KmcResultCallbackResponse response = kmcService.tokenDecrypt(request.apiToken()); | |
| return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response)); | |
| } | |
| @PostMapping("/decrypt") | |
| public ResponseEntity<ApiResponseWrapper<KmcResultCallbackResponse>> tokenDecrypt( | |
| @Valid @RequestBody KmcResultCallbackRequest request | |
| ) { | |
| log.info("STEP2 - KMC 본인인증 요청 수신 (토큰 길이: {})", | |
| request.apiToken() != null ? request.apiToken().length() : 0); | |
| KmcResultCallbackResponse response = kmcService.tokenDecrypt(request.apiToken()); | |
| return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "성공", response)); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/life/mosu/mosuserver/infra/kmc/KmcAuthController.java around
lines 36 to 43, the API token is logged directly which poses a security risk.
Remove or mask the token value in the log statement to avoid exposing sensitive
authentication data. Instead of logging the full token, log a non-sensitive
identifier or a masked version to maintain traceability without compromising
security.
| response.setHeader("Cache-Control", "no-cache"); // HTTP1.1 ij�� ���� | ||
| } | ||
| %> | ||
| <%@ page contentType="text/html;charset=ksc5601" %> |
There was a problem hiding this comment.
Fix encoding inconsistency to prevent character corruption.
The page declares charset=ksc5601 but should use UTF-8 for consistency with modern applications and to properly handle Korean characters.
-<%@ page contentType="text/html;charset=ksc5601" %>
+<%@ page contentType="text/html;charset=UTF-8" %>Also update the meta tag:
-<meta http-equiv="Content-Type" content="text/html; charset=euc-kr">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <%@ page contentType="text/html;charset=ksc5601" %> | |
| <%@ page contentType="text/html;charset=UTF-8" %> | |
| <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> |
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step1.jsp at line 18, the page directive sets
the contentType charset to ksc5601, which can cause character encoding issues.
Change the charset to UTF-8 in the page directive to ensure consistent encoding
and proper handling of Korean characters. Also update the HTML meta tag to
specify UTF-8 as the charset to match the page directive.
| <html> | ||
| <head> | ||
| <title>������������ ��Ʈ</title> | ||
| <meta http-equiv="Content-Type" content="text/html; charset=euc-kr"> | ||
| <meta name="robots" content="noindex"> | ||
| <style> | ||
| <!-- | ||
| body, p, ol, ul, td { | ||
| font-family: ����; | ||
| font-size: 12px; | ||
| } | ||
|
|
||
| a:link { | ||
| size: 9px; | ||
| color: #000000; | ||
| text-decoration: none; | ||
| line-height: 12px | ||
| } | ||
|
|
||
| a:visited { | ||
| size: 9px; | ||
| color: #555555; | ||
| text-decoration: none; | ||
| line-height: 12px | ||
| } | ||
|
|
||
| a:hover { | ||
| color: #ff9900; | ||
| text-decoration: none; | ||
| line-height: 12px | ||
| } | ||
|
|
||
| .style1 { | ||
| color: #6b902a; | ||
| font-weight: bold; | ||
| } | ||
|
|
||
| .style2 { | ||
| color: #666666 | ||
| } | ||
|
|
||
| .style3 { | ||
| color: #3b5d00; | ||
| font-weight: bold; | ||
| } | ||
|
|
||
| --> | ||
| </style> | ||
| </head> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Modernize HTML structure and fix encoding inconsistencies.
The HTML structure uses legacy attributes and has encoding conflicts between the JSP page directive (ksc5601) and the meta tag (euc-kr). Consider modernizing the HTML and using web-safe fonts.
<head>
<title>본인인증서비스 테스트</title>
- <meta http-equiv="Content-Type" content="text/html; charset=euc-kr">
+ <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="robots" content="noindex">Consider replacing Korean font names with web-safe alternatives:
- font-family: 굴림;
+ font-family: "Malgun Gothic", "Apple Gothic", sans-serif;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <html> | |
| <head> | |
| <title>본인인증서비스 테스트</title> | |
| <meta http-equiv="Content-Type" content="text/html; charset=euc-kr"> | |
| <meta name="robots" content="noindex"> | |
| <style> | |
| <!-- | |
| body, p, ol, ul, td { | |
| font-family: 굴림; | |
| font-size: 12px; | |
| } | |
| a:link { | |
| size: 9px; | |
| color: #000000; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| a:visited { | |
| size: 9px; | |
| color: #555555; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| a:hover { | |
| color: #ff9900; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| .style1 { | |
| color: #6b902a; | |
| font-weight: bold; | |
| } | |
| .style2 { | |
| color: #666666 | |
| } | |
| .style3 { | |
| color: #3b5d00; | |
| font-weight: bold; | |
| } | |
| --> | |
| </style> | |
| </head> | |
| <html> | |
| <head> | |
| <title>본인인증서비스 테스트</title> | |
| <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> | |
| <meta name="robots" content="noindex"> | |
| <style> | |
| <!-- | |
| body, p, ol, ul, td { | |
| font-family: "Malgun Gothic", "Apple Gothic", sans-serif; | |
| font-size: 12px; | |
| } | |
| a:link { | |
| size: 9px; | |
| color: #000000; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| a:visited { | |
| size: 9px; | |
| color: #555555; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| a:hover { | |
| color: #ff9900; | |
| text-decoration: none; | |
| line-height: 12px | |
| } | |
| .style1 { | |
| color: #6b902a; | |
| font-weight: bold; | |
| } | |
| .style2 { | |
| color: #666666 | |
| } | |
| .style3 { | |
| color: #3b5d00; | |
| font-weight: bold; | |
| } | |
| --> | |
| </style> | |
| </head> |
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step1.jsp between lines 35 and 83, the HTML
uses outdated attributes and has conflicting character encodings between the JSP
directive (ksc5601) and the meta tag (euc-kr). Update the meta charset tag to
use UTF-8 for consistency and modern standards. Replace the Korean font '굴림'
with web-safe font families like 'Arial, sans-serif' or 'Malgun Gothic, Arial,
sans-serif' to ensure better cross-platform compatibility. Also, remove legacy
HTML comments inside the style tag and clean up CSS properties like 'size' which
are invalid for links.
| <td align=left><input type="text" name="tr_url" size="41" | ||
| value="http://�����纰 ���/kmcis_web_sample_step03.jsp"></td> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address additional hardcoded URL in return URL field.
The return URL field contains a hardcoded placeholder URL that needs to be properly configured.
- value="http://도메인명 입력/kmcis_web_sample_step03.jsp">
+ value="${pageContext.request.contextPath}/api/v1/kmc/result">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step1.jsp around lines 127 to 128, the input
field for the return URL contains a hardcoded placeholder URL. Replace this
hardcoded URL with a dynamic value or configuration reference that correctly
sets the return URL based on the application's routing or environment settings
to avoid using fixed, static URLs.
| <tr> | ||
| <td align=center>�������URL</td> | ||
| <td align=left><input type="text" name="tr_url" size="41" | ||
| value="http://�����纰 ���/kmcis_web_sample_step03.jsp"></td> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace placeholder URL with actual configuration.
The placeholder URL should be replaced with actual configuration or removed if not needed.
-value="http://사용자서버 주소/kmcis_web_sample_step03.jsp">
+value="${returnUrl}">Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/step1.jsp at line 128, the value attribute
contains a placeholder URL "http://고객사별 경로/kmcis_web_sample_step03.jsp". Replace
this placeholder with the actual configured URL needed for the application or
remove the attribute entirely if it is not required.
| kmc: | ||
| cpid: ${KMC_CPID} | ||
| url-code: ${KMC_URLCODE} | ||
| expire-time: ${KMC_EXPIRE_TIME} No newline at end of file |
There was a problem hiding this comment.
Fix missing newline at end of file.
Static analysis correctly identified a missing newline character at the end of the file, which should be added for POSIX compliance.
kmc:
cpid: ${KMC_CPID}
url-code: ${KMC_URLCODE}
- expire-time: ${KMC_EXPIRE_TIME}
+ expire-time: ${KMC_EXPIRE_TIME}
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expire-time: ${KMC_EXPIRE_TIME} | |
| kmc: | |
| cpid: ${KMC_CPID} | |
| url-code: ${KMC_URLCODE} | |
| expire-time: ${KMC_EXPIRE_TIME} | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 46-46: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In src/test/resources/security-config.yml at line 46, the file is missing a
newline character at the end. Add a newline character after the last line to
ensure POSIX compliance and satisfy static analysis requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/docker-ci.yaml (1)
26-30: Minor optimisation: clone only the required artefactIf the jar is the only asset you need, consider enabling sparse-checkout (supported by
actions/checkout) or, better yet, publish the jar to a package registry (GitHub Packages / Maven) and declare it as a Gradle dependency. This keeps the CI pipeline stateless, reproducible, and avoids bloating the Docker image with an embedded VCS directory.
.github/workflows/docker-ci.yaml
Outdated
| - name: Clone external repo with jar | ||
| run: | | ||
| mkdir -p ./libs | ||
| cd ./libs | ||
| git clone https://x-access-token:${{ secrets.GH_PAT }}@github.com/mosu-dev/mosu-kmc-jar.git . |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace custom PAT with the built-in GITHUB_TOKEN and use actions/checkout for safer, cleaner access
Passing a personal access token on the CLI exposes it to accidental logging and requires manual rotation.
GitHub Actions already provides a short-lived, least-privilege token (GITHUB_TOKEN) that is masked in logs and automatically scoped to the current repository.
actions/checkout supports checking out another private repo with that token, avoiding the need for git clone scripting entirely.
- - name: Clone external repo with jar
- run: |
- mkdir -p ./libs
- cd ./libs
- git clone https://x-access-token:${{ secrets.GH_PAT }}@github.com/mosu-dev/mosu-kmc-jar.git .
+ - name: Checkout KMC jar repo
+ uses: actions/checkout@v3
+ with:
+ repository: mosu-dev/mosu-kmc-jar
+ token: ${{ secrets.GITHUB_TOKEN }}
+ path: libs # checks out directly into ./libs
+ fetch-depth: 1 # shallow clone for speedBenefits
• No long-lived PAT in secrets
• Token automatically rotated by GitHub
• Simpler, more declarative workflow step
• Shallow clone cuts runner time/bandwidth
🤖 Prompt for AI Agents
In .github/workflows/docker-ci.yaml around lines 26 to 30, replace the manual
git clone command using a personal access token with the built-in GITHUB_TOKEN
and the actions/checkout action. Remove the mkdir, cd, and git clone steps, and
instead add a step using actions/checkout configured to checkout the external
private repository with the GITHUB_TOKEN for authentication. This will improve
security by avoiding exposing a long-lived PAT, simplify the workflow, and
enable a shallow clone.
# Conflicts: # .github/workflows/docker-ci.yaml
wlgns12370
changed the title
1 participant
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores
.gitignoreto exclude additional directories.Tests