-
Notifications
You must be signed in to change notification settings - Fork 0
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
[fix] 이력서 비로그인 회원 조회 가능 #284
Conversation
Spring Security Config file을 수정하여 여러 이력서 조회를 하는 로직이 가능하도록 함
다중 조회용 API endpoint 추가
WalkthroughThe changes update the Spring Security configuration by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant SC as SecurityConfig
participant API as API Endpoint
C->>SC: GET /api/v1/resumes\nor /api/v1/resumes/view
SC->>SC: Evaluate request matchers
SC-->>C: Permit request (no authentication)
C->>API: Forward GET request
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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.
Code review by ChatGPT
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: 0
🧹 Nitpick comments (3)
backend/src/main/java/com/techeer/backend/global/config/SecurityConfig.java (3)
86-87
: Verify the security implications of permitting all access to the search endpoint.The search endpoint (/api/v1/resumes/search) was previously restricted to ADMIN role (see commented code), but is now completely open. This change might have security implications:
- Consider implementing rate limiting to prevent abuse
- Evaluate if search functionality should expose sensitive data
- Consider implementing pagination to prevent data dumping
Would you like me to suggest a rate limiting implementation or help evaluate the search endpoint's data exposure?
62-66
: Remove or document commented-out security configuration.The old security configuration is commented out but provides valuable context about previous authorization rules. Consider either:
- Removing it if it's no longer relevant
- Moving it to documentation if it provides important context
- Adding a comment explaining why it was changed
54-102
: Document security decisions in the configuration.Consider adding JavaDoc comments to document important security decisions:
- Why CSRF is disabled
- The rationale for opening up specific endpoints
- The authentication flow
Example documentation:
/** * Security configuration for the resume service. * * Key decisions: * - CSRF disabled: Using JWT-based authentication * - Public endpoints: * - GET /api/v1/resumes: Allow resume listing without authentication * - GET /api/v1/resumes/view: Allow individual resume viewing * - /api/v1/resumes/search: Open search functionality (consider rate limiting) */ @Bean public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/com/techeer/backend/global/config/SecurityConfig.java
(3 hunks)
🔇 Additional comments (2)
backend/src/main/java/com/techeer/backend/global/config/SecurityConfig.java (2)
13-13
: LGTM!The HttpMethod import is correctly added to support the new GET request matchers.
70-73
: LGTM! Well-structured endpoint configuration.The GET-specific permission is correctly implemented using HttpMethod.GET matcher, providing granular access control.
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 Security에서 permitall url endpoint에 이력서 개별 조회 or 유저 조회를 제외한 나머지 조회 url 추가
해당 api 호출 시 인증 인가 로직이 발생하지 않아 api 사용 가능
테스트 결과 (테스트를 했으면)
확인은 다 했는데, 테스트 이미지는 첨부하지 않겠습니다
Summary by CodeRabbit