-
Notifications
You must be signed in to change notification settings - Fork 1
test: add google login test #264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
feat(guestbook, luckydraw): λ°©λͺ λ‘ λ° λ°©λͺ λ‘ μ°κ³ μΆμ²¨ κΈ°λ₯ μΆκ°
κ°μμλ‘μ΄ ν μ€νΈ μλν¬μΈνΈλ₯Ό μΆκ°νμ¬ λ¦¬νλ μ ν ν° μΏ ν€μ Authorization ν€λμ μ‘΄μ¬ μ¬λΆλ₯Ό νμΈνλ©°, 보μ μ€μ μ ν΅ν΄ ν΄λΉ μλν¬μΈνΈλ₯Ό μΈμ¦ μμ΄ μ κ·Ό κ°λ₯νλλ‘ κ΅¬μ±νμ΅λλ€. λ³κ²½ μ¬ν
μμ μ½λ 리뷰 λμ΄λπ― 2 (λ¨μ) | β±οΈ ~10λΆ μΆμ μ
Pre-merge checks and finishing touchesβ Failed checks (2 warnings)
β Passed checks (1 passed)
β¨ Finishing touches
π§ͺ Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
π Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (2)
src/main/java/inha/gdgoc/domain/test/controller/TestController.javasrc/main/java/inha/gdgoc/global/security/SecurityConfig.java
π§° Additional context used
π§ Learnings (2)
π Learning: 2025-08-30T10:43:25.889Z
Learnt from: kaswhy
Repo: GDGoCINHA/24-2_GDGoC_Server PR: 207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.889Z
Learning: The GDGoCINHA project has a consistent security architecture where public endpoints are handled in two layers: 1) SecurityConfig uses .permitAll() for paths like /api/v1/auth/**, /api/v1/game/**, /api/v1/apply/**, /api/v1/check/** and 2) TokenAuthenticationFilter excludes the same paths in shouldNotFilter(). This dual-layer approach is correct and intentional.
Applied to files:
src/main/java/inha/gdgoc/global/security/SecurityConfig.java
π Learning: 2025-08-30T10:43:25.889Z
Learnt from: kaswhy
Repo: GDGoCINHA/24-2_GDGoC_Server PR: 207
File: src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java:31-39
Timestamp: 2025-08-30T10:43:25.889Z
Learning: In the GDGoCINHA project, the following API paths are intentionally designed as public endpoints that should not require authentication: /api/v1/auth/, /api/v1/test/, /api/v1/game/, /api/v1/apply/, /api/v1/check/. These paths are correctly excluded from the TokenAuthenticationFilter in shouldNotFilter() method.
Applied to files:
src/main/java/inha/gdgoc/global/security/SecurityConfig.java
𧬠Code graph analysis (1)
src/main/java/inha/gdgoc/global/security/SecurityConfig.java (1)
src/main/java/inha/gdgoc/global/config/openapi/OpenApiConfig.java (1)
Configuration(15-82)
π Additional comments (1)
src/main/java/inha/gdgoc/global/security/SecurityConfig.java (1)
48-48: μ΄μ€ λ μ΄μ΄ 보μ μν€ν μ²μ μΌκ΄μ±μ΄ μ΄λ―Έ μ μ§λκ³ μμ΅λλ€.
/api/v1/test/**κ²½λ‘λ TokenAuthenticationFilterμshouldNotFilter()λ©μλμ μ΄λ―Έ ν¬ν¨λμ΄ μμ΅λλ€ (line 43). SecurityConfigμ TokenAuthenticationFilter κ°μ μΌκ΄μ±μ΄ μ¬λ°λ₯΄κ² μ μ§λκ³ μμΌλ―λ‘ μΆκ° μ‘°μΉλ νμνμ§ μμ΅λλ€.
| package inha.gdgoc.domain.test.controller; | ||
|
|
||
| import inha.gdgoc.global.dto.response.ApiResponse; | ||
| import java.util.Map; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.CookieValue; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.RequestHeader; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/v1/test") | ||
| public class TestController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR μ λͺ©κ³Ό ꡬν λ΄μ©μ΄ λΆμΌμΉν©λλ€.
PR μ λͺ©μ "test: add google login test"μ΄μ§λ§, μ΄ μ»¨νΈλ‘€λ¬λ Google λ‘κ·ΈμΈμ ν μ€νΈνμ§ μμ΅λλ€. λ¨μν ν ν°κ³Ό ν€λμ μ‘΄μ¬ μ¬λΆλ§ νμΈν©λλ€.
- Google OAuth λ‘κ·ΈμΈ νλ‘μ°λ₯Ό μ€μ λ‘ ν μ€νΈνλ €λ©΄, Google μΈμ¦ μλΉμ€μμ ν΅ν©μ κ²μ¦ν΄μΌ ν©λλ€
- νμ¬ κ΅¬νμ ν ν° μ‘΄μ¬ μ¬λΆλ§ νμΈνλ μΌλ°μ μΈ μΈμ¦ μν μ²΄ν¬ μλν¬μΈνΈμ λλ€
PR μ λͺ©μ ꡬνμ λ§κ² μμ νκ±°λ, Google λ‘κ·ΈμΈμ μ€μ λ‘ ν μ€νΈνλ λ‘μ§μ μΆκ°νλ κ²μ κΆμ₯ν©λλ€.
| @GetMapping("/login_test") | ||
| public ResponseEntity<ApiResponse<Map<String, Object>, Void>> loginTest( | ||
| @CookieValue(value = "refresh_token", required = false) String refreshToken, | ||
| @RequestHeader(value = "Authorization", required = false) String authorization | ||
| ) { | ||
| boolean hasRefreshToken = refreshToken != null && !refreshToken.isBlank(); | ||
| boolean hasAuthorization = authorization != null && !authorization.isBlank(); | ||
|
|
||
| Map<String, Object> data = Map.of( | ||
| "has_refresh_token", hasRefreshToken, | ||
| "has_authorization", hasAuthorization | ||
| ); | ||
|
|
||
| return ResponseEntity.ok(ApiResponse.ok("LOGIN_TEST_OK", data)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
νλ‘λμ νκ²½μμ ν μ€νΈ μλν¬μΈνΈ λ ΈμΆμ μ νν΄μΌ ν©λλ€.
μ΄ ν μ€νΈ μλν¬μΈνΈκ° λͺ¨λ νκ²½μμ 곡κ°μ μΌλ‘ μ κ·Ό κ°λ₯νλλ‘ μ€μ λμ΄ μμ΅λλ€. νλ‘λμ νκ²½μμ ν μ€νΈ μλν¬μΈνΈλ₯Ό λ ΈμΆνλ©΄ 보μ μνμ΄ λ°μν μ μμ΅λλ€.
λ€μ μ€ νλλ₯Ό μ μ©νλ κ²μ κΆμ₯ν©λλ€:
- Spring Profileμ μ¬μ©νμ¬ κ°λ°/ν μ€νΈ νκ²½μμλ§ νμ±ν
- Feature flagλ‘ μ μ΄
- λ³λμ ν μ€νΈ λͺ¨λλ‘ λΆλ¦¬
π νλ‘ν κΈ°λ° νμ±ν μ μ
+import org.springframework.context.annotation.Profile;
+
+@Profile({"local", "dev", "test"})
@RestController
@RequestMapping("/api/v1/test")
public class TestController {λλ μ‘°κ±΄λΆ λΉ λ±λ‘μ μ¬μ©:
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+
+@ConditionalOnProperty(name = "feature.test-endpoints.enabled", havingValue = "true")
@RestController
@RequestMapping("/api/v1/test")
public class TestController {Committable suggestion skipped: line range outside the PR's diff.
π€ Prompt for AI Agents
In src/main/java/inha/gdgoc/domain/test/controller/TestController.java around
lines 16-30 the login_test endpoint is always exposed; restrict it so it is not
available in production by enabling it only for non-production profiles or via a
feature flag. Fix by annotating the controller (or its @Configuration) with a
profile/condition such as @Profile({"dev","test"}) or @Profile("!prod") or use a
conditional on a property (e.g.,
@ConditionalOnProperty(name="feature.test-endpoints.enabled",
havingValue="true", matchIfMissing=false)) and add the corresponding property to
dev/test configs (false or absent in production), then update docs and tests to
ensure the endpoint is only reachable in intended environments.
π μ°κ΄λ μ΄μ
β¨ μμ λ΄μ©
π¬ 리뷰 μꡬμ¬ν(μ ν)
Summary by CodeRabbit
New Features
βοΈ Tip: You can customize this high-level summary in your review settings.