fix: Sheet API 요청자 접근 권한 검증 추가 #491
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,8 +23,12 @@ public SheetImportResponse analyzeAndImportPreMembers( | |
| clubPermissionValidator.validateManagerAccess(clubId, requesterId); | ||
|
|
||
| String spreadsheetId = SpreadsheetUrlParser.extractId(spreadsheetUrl); | ||
| // OAuth 미연결이면 기존 동작대로 검증을 건너뛴다. | ||
| // 다만 Drive OAuth가 연결된 경우에는 요청자 계정의 실제 시트 접근 권한을 먼저 검증한다. | ||
| googleSheetPermissionService.validateRequesterAccessAndTryGrantServiceAccountWriterAccess( | ||
| requesterId, | ||
| spreadsheetId | ||
| ); | ||
|
Comment on lines
+26
to
+31
There was a problem hiding this comment. ClubSheetIntegratedService의 주석이 현재 동작과 불일치합니다. 이제 analyzeAndImportPreMembers()는 OAuth 미연결 시 검증을 ‘건너뛰는’ 것이 아니라 validateRequesterAccess...에서 NOT_FOUND_GOOGLE_DRIVE_AUTH 예외로 즉시 실패합니다. 주석을 실제 정책(미연결 시 실패/재연결 요구)로 수정하거나, 정말로 우회 허용이 목적이라면 호출 로직을 변경해야 합니다. |
||
|
|
||
| SheetHeaderMapper.SheetAnalysisResult analysis = | ||
| sheetHeaderMapper.analyzeAllSheets(spreadsheetId); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| import org.springframework.util.StringUtils; | ||
|
|
||
| import com.google.api.services.drive.Drive; | ||
| import com.google.api.services.drive.model.File; | ||
| import com.google.auth.oauth2.ServiceAccountCredentials; | ||
|
|
||
| import gg.agit.konect.domain.user.enums.Provider; | ||
|
|
@@ -23,15 +24,25 @@ | |
| public class GoogleSheetPermissionService { | ||
|
|
||
| private final ServiceAccountCredentials serviceAccountCredentials; | ||
| private final Drive googleDriveService; | ||
| private final GoogleSheetsConfig googleSheetsConfig; | ||
| private final UserOAuthAccountRepository userOAuthAccountRepository; | ||
|
|
||
| public void validateRequesterAccessAndTryGrantServiceAccountWriterAccess( | ||
| Integer requesterId, | ||
| String spreadsheetId | ||
| ) { | ||
| String refreshToken = requireRefreshToken(requesterId); | ||
| Drive userDriveService = buildUserDriveService(refreshToken, requesterId); | ||
| validateRequesterSpreadsheetAccess(userDriveService, requesterId, spreadsheetId); | ||
| boolean granted = tryGrantServiceAccountWriterAccess(userDriveService, requesterId, spreadsheetId); | ||
| if (!granted) { | ||
| requireServiceAccountSpreadsheetAccess(spreadsheetId, requesterId); | ||
| } | ||
|
Comment on lines
+31
to
+41
There was a problem hiding this comment. validateRequesterAccessAndTryGrantServiceAccountWriterAccess()가 requireRefreshToken()을 사용해 OAuth 미연결/blank token을 곧바로 NOT_FOUND_GOOGLE_DRIVE_AUTH로 예외 처리하고 있습니다. 그런데 동일 PR에서 “OAuth 미연결이면 기존 동작 유지(스킵)”를 명시했고, ClubSheetIntegratedService 주석도 스킵을 기대하고 있어 실제 런타임 동작이 설명과 불일치합니다. 이 메서드를 (1) refresh token이 없으면 조용히 return 하도록 변경하거나, (2) ‘반드시 OAuth 연결이 필요’한 API에서만 호출되도록 호출부를 조정하는 등 의도에 맞게 계약을 명확히 해주세요. |
||
| } | ||
|
|
||
| public boolean tryGrantServiceAccountWriterAccess(Integer requesterId, String spreadsheetId) { | ||
| String refreshToken = resolveRefreshToken(requesterId); | ||
|
|
||
| if (refreshToken == null) { | ||
| log.warn( | ||
|
|
@@ -41,14 +52,35 @@ public boolean tryGrantServiceAccountWriterAccess(Integer requesterId, String sp | |
| return false; | ||
| } | ||
|
|
||
| Drive userDriveService = buildUserDriveService(refreshToken, requesterId); | ||
| return tryGrantServiceAccountWriterAccess(userDriveService, requesterId, spreadsheetId); | ||
| } | ||
|
|
||
| private String requireRefreshToken(Integer requesterId) { | ||
| return userOAuthAccountRepository.findByUserIdAndProvider(requesterId, Provider.GOOGLE) | ||
| .map(account -> account.getGoogleDriveRefreshToken()) | ||
| .filter(StringUtils::hasText) | ||
| .orElseThrow(() -> { | ||
| log.warn( | ||
| "Rejecting spreadsheet registration because Google Drive OAuth is not connected. requesterId={}", | ||
| requesterId | ||
| ); | ||
| return CustomException.of(ApiResponseCode.NOT_FOUND_GOOGLE_DRIVE_AUTH); | ||
| }); | ||
| } | ||
|
|
||
| private String resolveRefreshToken(Integer requesterId) { | ||
| return userOAuthAccountRepository.findByUserIdAndProvider(requesterId, Provider.GOOGLE) | ||
| .map(account -> account.getGoogleDriveRefreshToken()) | ||
| .filter(StringUtils::hasText) | ||
| .orElse(null); | ||
| } | ||
|
|
||
| private boolean tryGrantServiceAccountWriterAccess( | ||
| Drive userDriveService, | ||
| Integer requesterId, | ||
| String spreadsheetId | ||
| ) { | ||
| try { | ||
| GoogleDrivePermissionHelper.ensureServiceAccountPermission( | ||
| userDriveService, | ||
|
|
@@ -91,6 +123,105 @@ public boolean tryGrantServiceAccountWriterAccess(Integer requesterId, String sp | |
| } | ||
| } | ||
|
|
||
| private Drive buildUserDriveService(String refreshToken, Integer requesterId) { | ||
| try { | ||
| return googleSheetsConfig.buildUserDriveService(refreshToken); | ||
| } catch (IOException | GeneralSecurityException e) { | ||
| log.error("Failed to build user Drive service. requesterId={}", requesterId, e); | ||
| throw CustomException.of(ApiResponseCode.FAILED_INIT_GOOGLE_DRIVE); | ||
| } | ||
| } | ||
|
|
||
| private void validateRequesterSpreadsheetAccess( | ||
| Drive userDriveService, | ||
| Integer requesterId, | ||
| String spreadsheetId | ||
| ) { | ||
| try { | ||
| File file = userDriveService.files().get(spreadsheetId) | ||
| .setFields("id") | ||
| .setSupportsAllDrives(true) | ||
| .execute(); | ||
|
Comment on lines
+141
to
+144
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
service_file="$(fd -i 'GoogleSheetPermissionService.java' src | head -n1)"
helper_file="$(fd -i 'GoogleDrivePermissionHelper.java' src | head -n1)"
# 신규 접근 검증 경로에서 supportsAllDrives 사용 여부 확인
rg -n -C2 'files\(\)\.get|setSupportsAllDrives' "$service_file"
# 권한 조회/부여 helper 경로에서도 동일 플래그 사용 여부 확인
if [[ -n "${helper_file:-}" ]]; then
rg -n -C2 'permissions\(\)\.(list|create|update)|setSupportsAllDrives' "$helper_file"
fiRepository: BCSDLab/KONECT_BACK_END Length of output: 909 공유 드라이브 스프레드시트 접근 검증에 [LEVEL: high] Line 135-137의 관련 코드🤖 Prompt for AI Agents |
||
| if (file == null || !StringUtils.hasText(file.getId())) { | ||
| throw GoogleSheetApiExceptionHelper.accessDenied(); | ||
| } | ||
| } catch (IOException e) { | ||
| if (GoogleSheetApiExceptionHelper.isInvalidGrant(e)) { | ||
| log.warn( | ||
| "Google Drive OAuth token is invalid while validating spreadsheet access. requesterId={}, " | ||
| + "spreadsheetId={}, cause={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| GoogleSheetApiExceptionHelper.extractDetail(e) | ||
| ); | ||
| throw GoogleSheetApiExceptionHelper.invalidGoogleDriveAuth(e); | ||
| } | ||
|
|
||
| if (GoogleSheetApiExceptionHelper.isAuthFailure(e)) { | ||
| log.warn( | ||
| "Google Drive OAuth auth failure while validating spreadsheet access. requesterId={}, " | ||
| + "spreadsheetId={}, cause={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| GoogleSheetApiExceptionHelper.extractDetail(e) | ||
| ); | ||
| throw GoogleSheetApiExceptionHelper.invalidGoogleDriveAuth(e); | ||
| } | ||
|
|
||
| if (GoogleSheetApiExceptionHelper.isAccessDenied(e) | ||
| || GoogleSheetApiExceptionHelper.isNotFound(e)) { | ||
| log.warn( | ||
| "Requester has no spreadsheet access. requesterId={}, spreadsheetId={}, cause={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| e.getMessage() | ||
| ); | ||
| throw GoogleSheetApiExceptionHelper.accessDenied(); | ||
| } | ||
|
|
||
| log.error( | ||
| "Unexpected error while validating requester spreadsheet access. requesterId={}, spreadsheetId={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| e | ||
| ); | ||
| throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); | ||
| } | ||
| } | ||
|
|
||
| private void requireServiceAccountSpreadsheetAccess(String spreadsheetId, Integer requesterId) { | ||
| try { | ||
| File file = googleDriveService.files().get(spreadsheetId) | ||
| .setFields("id") | ||
| .setSupportsAllDrives(true) | ||
| .execute(); | ||
| if (file == null || !StringUtils.hasText(file.getId())) { | ||
| throw GoogleSheetApiExceptionHelper.accessDenied(); | ||
| } | ||
| } catch (IOException e) { | ||
| if (GoogleSheetApiExceptionHelper.isAccessDenied(e) | ||
| || GoogleSheetApiExceptionHelper.isNotFound(e)) { | ||
| log.warn( | ||
| "Service account has no spreadsheet access after auto-share failed. requesterId={}, " | ||
| + "spreadsheetId={}, cause={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| e.getMessage() | ||
| ); | ||
| throw GoogleSheetApiExceptionHelper.accessDenied(); | ||
| } | ||
|
|
||
| log.error( | ||
| "Unexpected error while re-checking service account spreadsheet access. requesterId={}, " | ||
| + "spreadsheetId={}", | ||
| requesterId, | ||
| spreadsheetId, | ||
| e | ||
| ); | ||
| throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); | ||
| } | ||
| } | ||
|
|
||
| private String getServiceAccountEmail() { | ||
| return serviceAccountCredentials.getClientEmail(); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
ClubSheetIntegratedService의 주석(“OAuth 미연결이면 … 건너뛴다”) 및 PR 설명(Drive OAuth 미연결 사용자 흐름 유지)과 달리, 현재는 validateRequesterAccessAndTryGrantServiceAccountWriterAccess()를 무조건 호출해서 OAuth 계정/refresh token이 없으면 NOT_FOUND_GOOGLE_DRIVE_AUTH 예외로 흐름이 중단됩니다(기존에는 tryGrant...가 false 반환 후 계속 진행). OAuth 미연결 시에는 기존처럼 검증/권한부여를 스킵하고 다음 단계로 진행하도록 분기(예: refresh token 존재 시에만 validate 메서드 호출, 또는 validate 메서드 내부에서 token 없으면 no-op) 중 하나로 정합성을 맞춰주세요.