From 79edea7f72a136e8074601afa057219e85d46e30 Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 13:38:01 +0900 Subject: [PATCH 1/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20403=20=EC=9D=91=EB=8B=B5=20=EC=B2=98=EB=A6=AC=20?= =?UTF-8?q?=EB=B0=8F=20=EA=B6=8C=ED=95=9C=20=EB=B6=80=EC=97=AC=20=EB=B3=B4?= =?UTF-8?q?=EA=B0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service/ClubSheetIntegratedService.java | 3 + .../GoogleSheetApiExceptionHelper.java | 32 ++++ .../service/GoogleSheetPermissionService.java | 175 ++++++++++++++++++ .../club/service/SheetHeaderMapper.java | 24 +++ .../club/service/SheetImportService.java | 10 + .../club/service/SheetMigrationService.java | 40 ++++ .../club/service/SheetSyncExecutor.java | 9 + .../konect/global/code/ApiResponseCode.java | 2 + .../global/exception/CustomException.java | 2 +- .../ClubSheetIntegratedServiceTest.java | 8 + .../club/ClubSheetMigrationApiTest.java | 23 +++ 11 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java create mode 100644 src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java diff --git a/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java b/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java index d6b8ac7c1..1816ef2e5 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java @@ -10,6 +10,7 @@ public class ClubSheetIntegratedService { private final ClubPermissionValidator clubPermissionValidator; + private final GoogleSheetPermissionService googleSheetPermissionService; private final SheetHeaderMapper sheetHeaderMapper; private final ClubMemberSheetService clubMemberSheetService; private final SheetImportService sheetImportService; @@ -22,6 +23,8 @@ public SheetImportResponse analyzeAndImportPreMembers( clubPermissionValidator.validateManagerAccess(clubId, requesterId); String spreadsheetId = SpreadsheetUrlParser.extractId(spreadsheetUrl); + googleSheetPermissionService.tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId); + SheetHeaderMapper.SheetAnalysisResult analysis = sheetHeaderMapper.analyzeAllSheets(spreadsheetId); diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java new file mode 100644 index 000000000..5ed5b0c14 --- /dev/null +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java @@ -0,0 +1,32 @@ +package gg.agit.konect.domain.club.service; + +import java.io.IOException; + +import com.google.api.client.googleapis.json.GoogleJsonResponseException; + +import gg.agit.konect.global.code.ApiResponseCode; +import gg.agit.konect.global.exception.CustomException; + +public final class GoogleSheetApiExceptionHelper { + + private GoogleSheetApiExceptionHelper() {} + + public static boolean isAccessDenied(IOException exception) { + return getStatusCode(exception) == 403; + } + + public static boolean isNotFound(IOException exception) { + return getStatusCode(exception) == 404; + } + + public static CustomException accessDenied(String detail) { + return CustomException.of(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS, detail); + } + + private static int getStatusCode(IOException exception) { + if (exception instanceof GoogleJsonResponseException responseException) { + return responseException.getStatusCode(); + } + return -1; + } +} diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java new file mode 100644 index 000000000..6cf36d0ce --- /dev/null +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java @@ -0,0 +1,175 @@ +package gg.agit.konect.domain.club.service; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.List; + +import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; + +import com.google.api.services.drive.Drive; +import com.google.api.services.drive.model.Permission; +import com.google.auth.oauth2.GoogleCredentials; +import com.google.auth.oauth2.ServiceAccountCredentials; + +import gg.agit.konect.domain.user.enums.Provider; +import gg.agit.konect.domain.user.repository.UserOAuthAccountRepository; +import gg.agit.konect.global.code.ApiResponseCode; +import gg.agit.konect.global.exception.CustomException; +import gg.agit.konect.infrastructure.googlesheets.GoogleSheetsConfig; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@Service +@RequiredArgsConstructor +public class GoogleSheetPermissionService { + + private final GoogleCredentials googleCredentials; + private final GoogleSheetsConfig googleSheetsConfig; + private final UserOAuthAccountRepository userOAuthAccountRepository; + + public boolean tryGrantServiceAccountWriterAccess(Integer requesterId, String spreadsheetId) { + String refreshToken = userOAuthAccountRepository + .findByUserIdAndProvider(requesterId, Provider.GOOGLE) + .map(account -> account.getGoogleDriveRefreshToken()) + .filter(StringUtils::hasText) + .orElse(null); + + if (!StringUtils.hasText(refreshToken)) { + log.warn( + "Skipping service account auto-share because Google Drive OAuth is not connected. requesterId={}", + requesterId + ); + return false; + } + + Drive userDriveService; + try { + userDriveService = 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); + } + + try { + ensureServiceAccountPermission(userDriveService, spreadsheetId, "writer"); + return true; + } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e) + || GoogleSheetApiExceptionHelper.isNotFound(e)) { + log.warn( + "Failed to auto-share spreadsheet with service account. requesterId={}, spreadsheetId={}, cause={}", + requesterId, + spreadsheetId, + e.getMessage() + ); + return false; + } + + log.error( + "Unexpected error while auto-sharing spreadsheet. requesterId={}, spreadsheetId={}", + requesterId, + spreadsheetId, + e + ); + throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); + } + } + + private void ensureServiceAccountPermission( + Drive userDriveService, + String fileId, + String targetRole + ) throws IOException { + String serviceAccountEmail = getServiceAccountEmail(); + Permission existingPermission = findServiceAccountPermission( + userDriveService, + fileId, + serviceAccountEmail + ); + + if (existingPermission == null) { + Permission permission = new Permission() + .setType("user") + .setRole(targetRole) + .setEmailAddress(serviceAccountEmail); + + userDriveService.permissions().create(fileId, permission) + .setSendNotificationEmail(false) + .execute(); + log.info( + "Service account access granted. fileId={}, role={}, email={}", + fileId, + targetRole, + serviceAccountEmail + ); + return; + } + + String currentRole = existingPermission.getRole(); + if (roleRank(currentRole) >= roleRank(targetRole)) { + log.info( + "Service account permission already satisfies requested role. fileId={}, role={}, email={}", + fileId, + currentRole, + serviceAccountEmail + ); + return; + } + + Permission updatedPermission = new Permission().setRole(targetRole); + userDriveService.permissions().update(fileId, existingPermission.getId(), updatedPermission) + .execute(); + log.info( + "Service account permission upgraded. fileId={}, fromRole={}, toRole={}, email={}", + fileId, + currentRole, + targetRole, + serviceAccountEmail + ); + } + + private Permission findServiceAccountPermission( + Drive userDriveService, + String fileId, + String serviceAccountEmail + ) throws IOException { + List permissions = userDriveService.permissions().list(fileId) + .setFields("permissions(id,emailAddress,role)") + .execute() + .getPermissions(); + + if (permissions == null) { + return null; + } + + return permissions.stream() + .filter(permission -> serviceAccountEmail.equals(permission.getEmailAddress())) + .findFirst() + .orElse(null); + } + + private String getServiceAccountEmail() { + if (!(googleCredentials instanceof ServiceAccountCredentials serviceAccountCredentials)) { + throw new IllegalStateException( + "Google credentials is not a ServiceAccountCredentials. actual type=" + + googleCredentials.getClass().getName() + ); + } + return serviceAccountCredentials.getClientEmail(); + } + + private int roleRank(String role) { + if (role == null) { + return 0; + } + + return switch (role) { + case "reader" -> 1; + case "commenter" -> 2; + case "writer", "fileOrganizer", "organizer", "owner" -> 3; + default -> 0; + }; + } +} diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java b/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java index 5b10c997b..68190ce9c 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java @@ -19,6 +19,7 @@ import com.google.api.services.sheets.v4.model.ValueRange; import gg.agit.konect.domain.club.model.SheetColumnMapping; +import gg.agit.konect.global.exception.CustomException; import gg.agit.konect.infrastructure.claude.config.ClaudeProperties; import lombok.extern.slf4j.Slf4j; @@ -66,6 +67,8 @@ public SheetAnalysisResult analyzeAllSheets(String spreadsheetId) { try { return inferAllMappings(spreadsheetId, sheets); + } catch (CustomException e) { + throw e; } catch (Exception e) { log.warn("Sheet analysis failed, using default. cause={}", e.getMessage()); return new SheetAnalysisResult(SheetColumnMapping.defaultMapping(), null, null); @@ -90,6 +93,16 @@ private List readAllSheets(String spreadsheetId) { return result; } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while reading spreadsheet info. spreadsheetId={}, cause={}", + spreadsheetId, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "spreadsheetId=" + spreadsheetId + ); + } log.error("Failed to read spreadsheet info. spreadsheetId={}", spreadsheetId, e); return List.of(); } @@ -118,6 +131,17 @@ private List> readSheetRows(String spreadsheetId, String sheetTitle return rows; } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while reading sheet rows. spreadsheetId={}, sheetTitle={}, cause={}", + spreadsheetId, + sheetTitle, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "spreadsheetId=" + spreadsheetId + ", sheetTitle=" + sheetTitle + ); + } log.warn("Failed to read rows from sheet '{}'. cause={}", sheetTitle, e.getMessage()); return List.of(); } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java index d81b43905..ee24bb5b5 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java @@ -243,6 +243,16 @@ private List> readDataRows(String spreadsheetId, SheetColumnMapping return values != null ? values : List.of(); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while reading sheet data. spreadsheetId={}, cause={}", + spreadsheetId, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "spreadsheetId=" + spreadsheetId + ); + } log.error("Failed to read sheet data. spreadsheetId={}", spreadsheetId, e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index c94393097..e78b13bd5 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -226,6 +226,17 @@ private void grantServiceAccountPermission(Drive userDriveService, String fileId role, fileId, serviceAccountEmail ); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while granting service account permission. fileId={}, role={}, cause={}", + fileId, + role, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "fileId=" + fileId + ", role=" + role + ); + } log.error( "Failed to grant service account {} access. fileId={}, cause={}", role, fileId, e.getMessage(), e @@ -328,6 +339,15 @@ private String copyTemplate(Drive driveService, String templateId, String clubNa return newFileId; } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while copying template. cause={}", + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "templateId=" + templateId + ", targetFolderId=" + targetFolderId + ); + } if (newFileId != null) { deleteFile(driveService, newFileId); } @@ -352,6 +372,16 @@ private List> readAllData( return values != null ? values : List.of(); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while reading source data. spreadsheetId={}, cause={}", + spreadsheetId, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "spreadsheetId=" + spreadsheetId + ); + } log.error("Failed to read source data. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); } @@ -395,6 +425,16 @@ private void writeToTemplate( ); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied while writing template data. spreadsheetId={}, cause={}", + newSpreadsheetId, + e.getMessage() + ); + throw GoogleSheetApiExceptionHelper.accessDenied( + "spreadsheetId=" + newSpreadsheetId + ); + } log.error("Failed to write data to template. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java b/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java index 5b58d63d8..af00ed913 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java @@ -79,6 +79,15 @@ public void executeWithSort(Integer clubId, ClubSheetSortKey sortKey, boolean as } log.info("Sheet sync done. clubId={}, members={}", clubId, members.size()); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { + log.warn( + "Google Sheets access denied during sheet sync. clubId={}, spreadsheetId={}, cause={}", + clubId, + spreadsheetId, + e.getMessage() + ); + return; + } log.error( "Sheet sync failed. clubId={}, spreadsheetId={}, cause={}", clubId, spreadsheetId, e.getMessage(), e diff --git a/src/main/java/gg/agit/konect/global/code/ApiResponseCode.java b/src/main/java/gg/agit/konect/global/code/ApiResponseCode.java index a39b6598c..f07ebadf5 100644 --- a/src/main/java/gg/agit/konect/global/code/ApiResponseCode.java +++ b/src/main/java/gg/agit/konect/global/code/ApiResponseCode.java @@ -68,6 +68,8 @@ public enum ApiResponseCode { FORBIDDEN_MEMBER_POSITION_CHANGE(HttpStatus.FORBIDDEN, "회원 직책 변경 권한이 없습니다."), FORBIDDEN_POSITION_NAME_CHANGE(HttpStatus.FORBIDDEN, "해당 직책의 이름은 변경할 수 없습니다."), FORBIDDEN_ROLE_ACCESS(HttpStatus.FORBIDDEN, "접근 권한이 없습니다."), + FORBIDDEN_GOOGLE_SHEET_ACCESS(HttpStatus.FORBIDDEN, + "구글 스프레드시트 접근 권한이 없습니다. 서비스 계정을 공유하거나 Google Drive 권한을 연결한 뒤 다시 시도해 주세요."), FORBIDDEN_ORIGIN_ACCESS(HttpStatus.FORBIDDEN, "허용되지 않은 Origin 입니다."), // 404 Not Found (리소스를 찾을 수 없음) diff --git a/src/main/java/gg/agit/konect/global/exception/CustomException.java b/src/main/java/gg/agit/konect/global/exception/CustomException.java index 777c17211..ee19d0a19 100644 --- a/src/main/java/gg/agit/konect/global/exception/CustomException.java +++ b/src/main/java/gg/agit/konect/global/exception/CustomException.java @@ -27,7 +27,7 @@ public static CustomException of(ApiResponseCode errorCode, String detail) { } public String getFullMessage() { - if (StringUtils.hasText(detail)) { + if (!StringUtils.hasText(detail)) { return super.getMessage(); } return String.format("%s: %s", getMessage(), detail); diff --git a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java index 873403e9a..9158de659 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java @@ -24,6 +24,9 @@ class ClubSheetIntegratedServiceTest extends ServiceTestSupport { @Mock private SheetHeaderMapper sheetHeaderMapper; + @Mock + private GoogleSheetPermissionService googleSheetPermissionService; + @Mock private ClubMemberSheetService clubMemberSheetService; @@ -46,6 +49,8 @@ void analyzeAndImportPreMembersSuccess() { new SheetHeaderMapper.SheetAnalysisResult(SheetColumnMapping.defaultMapping(), null, null); SheetImportResponse expected = SheetImportResponse.of(3, 1, List.of("warn")); + given(googleSheetPermissionService.tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId)) + .willReturn(true); given(sheetHeaderMapper.analyzeAllSheets(spreadsheetId)).willReturn(analysis); given(sheetImportService.importPreMembersFromSheet( clubId, @@ -65,11 +70,14 @@ void analyzeAndImportPreMembersSuccess() { // then InOrder inOrder = inOrder( clubPermissionValidator, + googleSheetPermissionService, sheetHeaderMapper, clubMemberSheetService, sheetImportService ); inOrder.verify(clubPermissionValidator).validateManagerAccess(clubId, requesterId); + inOrder.verify(googleSheetPermissionService) + .tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId); inOrder.verify(sheetHeaderMapper).analyzeAllSheets(spreadsheetId); inOrder.verify(clubMemberSheetService).updateSheetId( clubId, diff --git a/src/test/java/gg/agit/konect/integration/domain/club/ClubSheetMigrationApiTest.java b/src/test/java/gg/agit/konect/integration/domain/club/ClubSheetMigrationApiTest.java index f906a6e33..6adff35e5 100644 --- a/src/test/java/gg/agit/konect/integration/domain/club/ClubSheetMigrationApiTest.java +++ b/src/test/java/gg/agit/konect/integration/domain/club/ClubSheetMigrationApiTest.java @@ -16,6 +16,8 @@ import gg.agit.konect.domain.club.dto.SheetImportRequest; import gg.agit.konect.domain.club.dto.SheetImportResponse; import gg.agit.konect.domain.club.service.ClubSheetIntegratedService; +import gg.agit.konect.global.code.ApiResponseCode; +import gg.agit.konect.global.exception.CustomException; import gg.agit.konect.support.IntegrationTestSupport; class ClubSheetMigrationApiTest extends IntegrationTestSupport { @@ -56,5 +58,26 @@ void analyzeAndImportPreMembersSuccess() throws Exception { .andExpect(jsonPath("$.autoRegisteredCount").value(1)) .andExpect(jsonPath("$.warnings[0]").value("전화번호 형식 경고")); } + + @Test + @DisplayName("구글 스프레드시트 403 오류를 response body로 반환한다") + void analyzeAndImportPreMembersForbiddenGoogleSheetAccess() throws Exception { + // given + given(clubSheetIntegratedService.analyzeAndImportPreMembers( + eq(CLUB_ID), + eq(REQUESTER_ID), + eq(SPREADSHEET_URL) + )).willThrow(CustomException.of(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS)); + + SheetImportRequest request = new SheetImportRequest(SPREADSHEET_URL); + + // when & then + performPost("/clubs/" + CLUB_ID + "/sheet/import/integrated", request) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code") + .value(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS.name())) + .andExpect(jsonPath("$.message") + .value(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS.getMessage())); + } } } From 2f721bca39f249a1de1b2a053232451514a7d935 Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 13:46:07 +0900 Subject: [PATCH 2/7] =?UTF-8?q?chore:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20=EC=B2=B4=ED=81=AC=EC=8A=A4=ED=83=80=EC=9D=BC=20?= =?UTF-8?q?=EB=B3=B4=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service/GoogleSheetApiExceptionHelper.java | 7 +++++-- .../service/GoogleSheetPermissionService.java | 15 ++++++++++----- .../club/service/SheetMigrationService.java | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java index 5ed5b0c14..f64a9a0f1 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java @@ -9,14 +9,17 @@ public final class GoogleSheetApiExceptionHelper { + private static final int HTTP_STATUS_FORBIDDEN = 403; + private static final int HTTP_STATUS_NOT_FOUND = 404; + private GoogleSheetApiExceptionHelper() {} public static boolean isAccessDenied(IOException exception) { - return getStatusCode(exception) == 403; + return getStatusCode(exception) == HTTP_STATUS_FORBIDDEN; } public static boolean isNotFound(IOException exception) { - return getStatusCode(exception) == 404; + return getStatusCode(exception) == HTTP_STATUS_NOT_FOUND; } public static CustomException accessDenied(String detail) { diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java index 6cf36d0ce..bd69b8aac 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java @@ -25,6 +25,11 @@ @RequiredArgsConstructor public class GoogleSheetPermissionService { + private static final int ROLE_RANK_NONE = 0; + private static final int ROLE_RANK_READER = 1; + private static final int ROLE_RANK_COMMENTER = 2; + private static final int ROLE_RANK_WRITER = 3; + private final GoogleCredentials googleCredentials; private final GoogleSheetsConfig googleSheetsConfig; private final UserOAuthAccountRepository userOAuthAccountRepository; @@ -162,14 +167,14 @@ private String getServiceAccountEmail() { private int roleRank(String role) { if (role == null) { - return 0; + return ROLE_RANK_NONE; } return switch (role) { - case "reader" -> 1; - case "commenter" -> 2; - case "writer", "fileOrganizer", "organizer", "owner" -> 3; - default -> 0; + case "reader" -> ROLE_RANK_READER; + case "commenter" -> ROLE_RANK_COMMENTER; + case "writer", "fileOrganizer", "organizer", "owner" -> ROLE_RANK_WRITER; + default -> ROLE_RANK_NONE; }; } } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index e78b13bd5..531ea445e 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -228,7 +228,8 @@ private void grantServiceAccountPermission(Drive userDriveService, String fileId } catch (IOException e) { if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { log.warn( - "Google Sheets access denied while granting service account permission. fileId={}, role={}, cause={}", + "Google Sheets access denied while granting service account permission. " + + "fileId={}, role={}, cause={}", fileId, role, e.getMessage() From 9b19f74d872e6cdc53de7ca88ea558662a4acbeb Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 14:09:58 +0900 Subject: [PATCH 3/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20=EA=B6=8C=ED=95=9C=20=EC=98=88=EC=99=B8=20=ED=9B=84?= =?UTF-8?q?=EC=86=8D=20=EC=B2=98=EB=A6=AC=20=EB=B3=B4=EC=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service/ClubSheetIntegratedService.java | 1 + .../club/service/SheetMigrationService.java | 6 +- .../ClubSheetIntegratedServiceTest.java | 63 +++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java b/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java index 1816ef2e5..0f02bdb11 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedService.java @@ -23,6 +23,7 @@ public SheetImportResponse analyzeAndImportPreMembers( clubPermissionValidator.validateManagerAccess(clubId, requesterId); String spreadsheetId = SpreadsheetUrlParser.extractId(spreadsheetUrl); + // Best-effort: OAuth 미연결/권한 부여 실패여도 이미 수동 공유된 시트는 그대로 읽을 수 있다. googleSheetPermissionService.tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId); SheetHeaderMapper.SheetAnalysisResult analysis = diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index 531ea445e..9486abbc4 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -340,6 +340,9 @@ private String copyTemplate(Drive driveService, String templateId, String clubNa return newFileId; } catch (IOException e) { + if (newFileId != null) { + deleteFile(driveService, newFileId); + } if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { log.warn( "Google Sheets access denied while copying template. cause={}", @@ -349,9 +352,6 @@ private String copyTemplate(Drive driveService, String templateId, String clubNa "templateId=" + templateId + ", targetFolderId=" + targetFolderId ); } - if (newFileId != null) { - deleteFile(driveService, newFileId); - } log.error("Failed to copy template. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); } diff --git a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java index 9158de659..b99129e14 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java @@ -1,8 +1,10 @@ package gg.agit.konect.domain.club.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.verifyNoInteractions; import java.util.List; @@ -14,6 +16,8 @@ import gg.agit.konect.domain.club.dto.SheetImportResponse; import gg.agit.konect.domain.club.model.SheetColumnMapping; +import gg.agit.konect.global.code.ApiResponseCode; +import gg.agit.konect.global.exception.CustomException; import gg.agit.konect.support.ServiceTestSupport; class ClubSheetIntegratedServiceTest extends ServiceTestSupport { @@ -93,4 +97,63 @@ void analyzeAndImportPreMembersSuccess() { ); assertThat(actual).isEqualTo(expected); } + + @Test + @DisplayName("자동 권한 부여가 실패해도 기존 공유 권한으로 가져오기를 계속 시도한다") + void analyzeAndImportPreMembersContinuesWhenAutoGrantFails() { + // given + Integer clubId = 1; + Integer requesterId = 2; + String spreadsheetUrl = + "https://docs.google.com/spreadsheets/d/1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms/edit"; + String spreadsheetId = "1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms"; + SheetHeaderMapper.SheetAnalysisResult analysis = + new SheetHeaderMapper.SheetAnalysisResult(SheetColumnMapping.defaultMapping(), null, null); + SheetImportResponse expected = SheetImportResponse.of(1, 0, List.of()); + + given(googleSheetPermissionService.tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId)) + .willReturn(false); + given(sheetHeaderMapper.analyzeAllSheets(spreadsheetId)).willReturn(analysis); + given(sheetImportService.importPreMembersFromSheet( + clubId, + requesterId, + spreadsheetId, + analysis.memberListMapping() + )) + .willReturn(expected); + + // when + SheetImportResponse actual = clubSheetIntegratedService.analyzeAndImportPreMembers( + clubId, + requesterId, + spreadsheetUrl + ); + + // then + assertThat(actual).isEqualTo(expected); + } + + @Test + @DisplayName("자동 권한 부여 중 예외가 발생하면 후속 시트 작업을 진행하지 않는다") + void analyzeAndImportPreMembersStopsWhenAutoGrantThrowsException() { + // given + Integer clubId = 1; + Integer requesterId = 2; + String spreadsheetUrl = + "https://docs.google.com/spreadsheets/d/1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms/edit"; + String spreadsheetId = "1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgVE2upms"; + CustomException expected = CustomException.of(ApiResponseCode.FAILED_INIT_GOOGLE_DRIVE); + + given(googleSheetPermissionService.tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId)) + .willThrow(expected); + + // when & then + assertThatThrownBy(() -> clubSheetIntegratedService.analyzeAndImportPreMembers( + clubId, + requesterId, + spreadsheetUrl + )) + .isSameAs(expected); + verifyNoInteractions(sheetHeaderMapper, clubMemberSheetService, sheetImportService); + } } From f91cf65f373bc350bf20f2dc39aaadbcd3ae1b93 Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 15:01:41 +0900 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20=EC=9E=90=EA=B2=A9=20=EA=B2=80=EC=A6=9D=20=EB=B0=8F?= =?UTF-8?q?=20=EC=8B=A4=ED=8C=A8=20=EA=B0=90=EC=A7=80=20=EB=B3=B4=EC=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../club/event/SheetSyncFailedEvent.java | 28 +++++ .../GoogleSheetApiExceptionHelper.java | 60 +++++++++- .../service/GoogleSheetPermissionService.java | 10 +- .../club/service/SheetHeaderMapper.java | 8 +- .../club/service/SheetImportService.java | 4 +- .../club/service/SheetMigrationService.java | 36 ++---- .../club/service/SheetSyncExecutor.java | 9 ++ .../googlesheets/GoogleSheetsConfig.java | 28 ++++- .../ClubSheetIntegratedServiceTest.java | 23 ++++ .../GoogleSheetApiExceptionHelperTest.java | 54 +++++++++ .../club/service/SheetSyncExecutorTest.java | 108 ++++++++++++++++++ .../support/IntegrationTestSupport.java | 4 + 12 files changed, 321 insertions(+), 51 deletions(-) create mode 100644 src/main/java/gg/agit/konect/domain/club/event/SheetSyncFailedEvent.java create mode 100644 src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java create mode 100644 src/test/java/gg/agit/konect/domain/club/service/SheetSyncExecutorTest.java diff --git a/src/main/java/gg/agit/konect/domain/club/event/SheetSyncFailedEvent.java b/src/main/java/gg/agit/konect/domain/club/event/SheetSyncFailedEvent.java new file mode 100644 index 000000000..76749c328 --- /dev/null +++ b/src/main/java/gg/agit/konect/domain/club/event/SheetSyncFailedEvent.java @@ -0,0 +1,28 @@ +package gg.agit.konect.domain.club.event; + +import java.time.LocalDateTime; + +public record SheetSyncFailedEvent( + Integer clubId, + String spreadsheetId, + boolean accessDenied, + String reason, + LocalDateTime occurredAt +) { + + public static SheetSyncFailedEvent accessDenied( + Integer clubId, + String spreadsheetId, + String reason + ) { + return new SheetSyncFailedEvent(clubId, spreadsheetId, true, reason, LocalDateTime.now()); + } + + public static SheetSyncFailedEvent unexpected( + Integer clubId, + String spreadsheetId, + String reason + ) { + return new SheetSyncFailedEvent(clubId, spreadsheetId, false, reason, LocalDateTime.now()); + } +} diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java index f64a9a0f1..648c56ab1 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java @@ -1,7 +1,10 @@ package gg.agit.konect.domain.club.service; import java.io.IOException; +import java.util.List; +import java.util.Set; +import com.google.api.client.googleapis.json.GoogleJsonError; import com.google.api.client.googleapis.json.GoogleJsonResponseException; import gg.agit.konect.global.code.ApiResponseCode; @@ -9,21 +12,74 @@ public final class GoogleSheetApiExceptionHelper { + private static final Set ACCESS_DENIED_REASONS = Set.of( + "accessDenied", + "forbidden", + "insufficientFilePermissions", + "insufficientPermissions", + "notAuthorized", + "required" + ); + private static final Set AUTH_FAILURE_REASONS = Set.of( + "authError", + "invalidCredentials", + "unauthorized" + ); private static final int HTTP_STATUS_FORBIDDEN = 403; + private static final int HTTP_STATUS_UNAUTHORIZED = 401; private static final int HTTP_STATUS_NOT_FOUND = 404; private GoogleSheetApiExceptionHelper() {} public static boolean isAccessDenied(IOException exception) { + if (exception instanceof GoogleJsonResponseException responseException) { + if (responseException.getStatusCode() != HTTP_STATUS_FORBIDDEN) { + return false; + } + return hasReason(responseException, ACCESS_DENIED_REASONS); + } return getStatusCode(exception) == HTTP_STATUS_FORBIDDEN; } + public static boolean isAuthFailure(IOException exception) { + if (exception instanceof GoogleJsonResponseException responseException) { + if (responseException.getStatusCode() != HTTP_STATUS_UNAUTHORIZED) { + return false; + } + return hasReason(responseException, AUTH_FAILURE_REASONS) + || !hasKnownReasons(responseException); + } + return getStatusCode(exception) == HTTP_STATUS_UNAUTHORIZED; + } + public static boolean isNotFound(IOException exception) { return getStatusCode(exception) == HTTP_STATUS_NOT_FOUND; } - public static CustomException accessDenied(String detail) { - return CustomException.of(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS, detail); + public static CustomException accessDenied() { + return CustomException.of(ApiResponseCode.FORBIDDEN_GOOGLE_SHEET_ACCESS); + } + + private static boolean hasReason( + GoogleJsonResponseException exception, + Set expectedReasons + ) { + return getReasons(exception).stream() + .anyMatch(expectedReasons::contains); + } + + private static boolean hasKnownReasons(GoogleJsonResponseException exception) { + return !getReasons(exception).isEmpty(); + } + + private static List getReasons(GoogleJsonResponseException exception) { + if (exception.getDetails() == null || exception.getDetails().getErrors() == null) { + return List.of(); + } + return exception.getDetails().getErrors().stream() + .map(GoogleJsonError.ErrorInfo::getReason) + .filter(reason -> reason != null && !reason.isBlank()) + .toList(); } private static int getStatusCode(IOException exception) { diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java index bd69b8aac..85632248c 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java @@ -9,7 +9,6 @@ import com.google.api.services.drive.Drive; import com.google.api.services.drive.model.Permission; -import com.google.auth.oauth2.GoogleCredentials; import com.google.auth.oauth2.ServiceAccountCredentials; import gg.agit.konect.domain.user.enums.Provider; @@ -30,7 +29,7 @@ public class GoogleSheetPermissionService { private static final int ROLE_RANK_COMMENTER = 2; private static final int ROLE_RANK_WRITER = 3; - private final GoogleCredentials googleCredentials; + private final ServiceAccountCredentials serviceAccountCredentials; private final GoogleSheetsConfig googleSheetsConfig; private final UserOAuthAccountRepository userOAuthAccountRepository; @@ -62,6 +61,7 @@ public boolean tryGrantServiceAccountWriterAccess(Integer requesterId, String sp return true; } catch (IOException e) { if (GoogleSheetApiExceptionHelper.isAccessDenied(e) + || GoogleSheetApiExceptionHelper.isAuthFailure(e) || GoogleSheetApiExceptionHelper.isNotFound(e)) { log.warn( "Failed to auto-share spreadsheet with service account. requesterId={}, spreadsheetId={}, cause={}", @@ -156,12 +156,6 @@ private Permission findServiceAccountPermission( } private String getServiceAccountEmail() { - if (!(googleCredentials instanceof ServiceAccountCredentials serviceAccountCredentials)) { - throw new IllegalStateException( - "Google credentials is not a ServiceAccountCredentials. actual type=" - + googleCredentials.getClass().getName() - ); - } return serviceAccountCredentials.getClientEmail(); } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java b/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java index 68190ce9c..44c33c309 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetHeaderMapper.java @@ -99,9 +99,7 @@ private List readAllSheets(String spreadsheetId) { spreadsheetId, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "spreadsheetId=" + spreadsheetId - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error("Failed to read spreadsheet info. spreadsheetId={}", spreadsheetId, e); return List.of(); @@ -138,9 +136,7 @@ private List> readSheetRows(String spreadsheetId, String sheetTitle sheetTitle, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "spreadsheetId=" + spreadsheetId + ", sheetTitle=" + sheetTitle - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.warn("Failed to read rows from sheet '{}'. cause={}", sheetTitle, e.getMessage()); return List.of(); diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java index ee24bb5b5..45e10e63d 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetImportService.java @@ -249,9 +249,7 @@ private List> readDataRows(String spreadsheetId, SheetColumnMapping spreadsheetId, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "spreadsheetId=" + spreadsheetId - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error("Failed to read sheet data. spreadsheetId={}", spreadsheetId, e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index 9486abbc4..d976e5ec4 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -22,7 +22,6 @@ import com.google.api.services.drive.model.Permission; import com.google.api.services.sheets.v4.Sheets; import com.google.api.services.sheets.v4.model.ValueRange; -import com.google.auth.oauth2.GoogleCredentials; import com.google.auth.oauth2.ServiceAccountCredentials; import gg.agit.konect.domain.club.model.Club; @@ -53,7 +52,7 @@ public class SheetMigrationService { private String defaultTemplateSpreadsheetId; private final Sheets googleSheetsService; - private final GoogleCredentials googleCredentials; + private final ServiceAccountCredentials serviceAccountCredentials; private final SheetHeaderMapper sheetHeaderMapper; private final ClubRepository clubRepository; private final UserOAuthAccountRepository userOAuthAccountRepository; @@ -159,10 +158,7 @@ public void afterCompletion(int status) { } private void removeServiceAccountPermission(Drive driveService, String fileId) { - if (!(googleCredentials instanceof ServiceAccountCredentials sac)) { - return; - } - String serviceAccountEmail = sac.getClientEmail(); + String serviceAccountEmail = serviceAccountCredentials.getClientEmail(); try { // permissionId 조회 후 삭제 (getPermissions()는 빈 경우 null 반환 가능) List permissions = @@ -206,13 +202,7 @@ private void grantServiceAccountAccess(Drive userDriveService, String fileId) { * 서비스 계정에 지정된 role로 Drive 접근 권한을 부여하는 공통 메서드입니다. */ private void grantServiceAccountPermission(Drive userDriveService, String fileId, String role) { - if (!(googleCredentials instanceof ServiceAccountCredentials sac)) { - throw new IllegalStateException( - "Google credentials is not a ServiceAccountCredentials. actual type=" - + googleCredentials.getClass().getName() - ); - } - String serviceAccountEmail = sac.getClientEmail(); + String serviceAccountEmail = serviceAccountCredentials.getClientEmail(); try { Permission permission = new Permission() .setType("user") @@ -234,9 +224,7 @@ private void grantServiceAccountPermission(Drive userDriveService, String fileId role, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "fileId=" + fileId + ", role=" + role - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error( "Failed to grant service account {} access. fileId={}, cause={}", @@ -345,12 +333,12 @@ private String copyTemplate(Drive driveService, String templateId, String clubNa } if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { log.warn( - "Google Sheets access denied while copying template. cause={}", + "Google Sheets access denied while copying template. templateId={}, targetFolderId={}, cause={}", + templateId, + targetFolderId, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "templateId=" + templateId + ", targetFolderId=" + targetFolderId - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error("Failed to copy template. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); @@ -379,9 +367,7 @@ private List> readAllData( spreadsheetId, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "spreadsheetId=" + spreadsheetId - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error("Failed to read source data. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); @@ -432,9 +418,7 @@ private void writeToTemplate( newSpreadsheetId, e.getMessage() ); - throw GoogleSheetApiExceptionHelper.accessDenied( - "spreadsheetId=" + newSpreadsheetId - ); + throw GoogleSheetApiExceptionHelper.accessDenied(); } log.error("Failed to write data to template. cause={}", e.getMessage(), e); throw CustomException.of(ApiResponseCode.FAILED_SYNC_GOOGLE_SHEET); diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java b/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java index af00ed913..e36759cdb 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetSyncExecutor.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.Map; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -28,6 +29,7 @@ import com.google.api.services.sheets.v4.model.UpdateSheetPropertiesRequest; import com.google.api.services.sheets.v4.model.ValueRange; +import gg.agit.konect.domain.club.event.SheetSyncFailedEvent; import gg.agit.konect.domain.club.enums.ClubSheetSortKey; import gg.agit.konect.domain.club.model.Club; import gg.agit.konect.domain.club.model.ClubMember; @@ -56,6 +58,7 @@ public class SheetSyncExecutor { private final ClubRepository clubRepository; private final ClubMemberRepository clubMemberRepository; private final ObjectMapper objectMapper; + private final ApplicationEventPublisher applicationEventPublisher; @Async("sheetSyncTaskExecutor") @Transactional(readOnly = true) @@ -86,12 +89,18 @@ public void executeWithSort(Integer clubId, ClubSheetSortKey sortKey, boolean as spreadsheetId, e.getMessage() ); + applicationEventPublisher.publishEvent( + SheetSyncFailedEvent.accessDenied(clubId, spreadsheetId, e.getMessage()) + ); return; } log.error( "Sheet sync failed. clubId={}, spreadsheetId={}, cause={}", clubId, spreadsheetId, e.getMessage(), e ); + applicationEventPublisher.publishEvent( + SheetSyncFailedEvent.unexpected(clubId, spreadsheetId, e.getMessage()) + ); } } diff --git a/src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java b/src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java index 1e4ed4269..52cee7d50 100644 --- a/src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java +++ b/src/main/java/gg/agit/konect/infrastructure/googlesheets/GoogleSheetsConfig.java @@ -10,6 +10,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; @@ -21,6 +22,7 @@ import com.google.api.services.sheets.v4.SheetsScopes; import com.google.auth.http.HttpCredentialsAdapter; import com.google.auth.oauth2.GoogleCredentials; +import com.google.auth.oauth2.ServiceAccountCredentials; import com.google.auth.oauth2.UserCredentials; import lombok.RequiredArgsConstructor; @@ -33,16 +35,30 @@ public class GoogleSheetsConfig { private final ResourceLoader resourceLoader; @Bean - public GoogleCredentials googleCredentials() throws IOException { + public ServiceAccountCredentials serviceAccountCredentials() throws IOException { try (InputStream in = openCredentialsStream()) { - return GoogleCredentials.fromStream(in) - .createScoped(Arrays.asList( - SheetsScopes.SPREADSHEETS, - DriveScopes.DRIVE - )); + GoogleCredentials credentials = GoogleCredentials.fromStream(in); + if (!(credentials instanceof ServiceAccountCredentials serviceAccountCredentials)) { + throw new IllegalStateException( + "Google credentials must be ServiceAccountCredentials. actual type=" + + credentials.getClass().getName() + ); + } + return serviceAccountCredentials; } } + @Bean + @Primary + public GoogleCredentials googleCredentials( + ServiceAccountCredentials serviceAccountCredentials + ) { + return serviceAccountCredentials.createScoped(Arrays.asList( + SheetsScopes.SPREADSHEETS, + DriveScopes.DRIVE + )); + } + @Bean public Sheets googleSheetsService( GoogleCredentials googleCredentials diff --git a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java index b99129e14..700737c68 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/ClubSheetIntegratedServiceTest.java @@ -130,6 +130,29 @@ void analyzeAndImportPreMembersContinuesWhenAutoGrantFails() { ); // then + InOrder inOrder = inOrder( + clubPermissionValidator, + googleSheetPermissionService, + sheetHeaderMapper, + clubMemberSheetService, + sheetImportService + ); + inOrder.verify(clubPermissionValidator).validateManagerAccess(clubId, requesterId); + inOrder.verify(googleSheetPermissionService) + .tryGrantServiceAccountWriterAccess(requesterId, spreadsheetId); + inOrder.verify(sheetHeaderMapper).analyzeAllSheets(spreadsheetId); + inOrder.verify(clubMemberSheetService).updateSheetId( + clubId, + requesterId, + spreadsheetId, + analysis + ); + inOrder.verify(sheetImportService).importPreMembersFromSheet( + clubId, + requesterId, + spreadsheetId, + analysis.memberListMapping() + ); assertThat(actual).isEqualTo(expected); } diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java new file mode 100644 index 000000000..e1f0f56ac --- /dev/null +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java @@ -0,0 +1,54 @@ +package gg.agit.konect.domain.club.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; + +class GoogleSheetApiExceptionHelperTest { + + @Test + @DisplayName("권한 관련 403 reason만 access denied로 분류한다") + void isAccessDeniedReturnsTrueOnlyForPermissionReasons() { + GoogleJsonResponseException permissionDenied = + googleException(403, "insufficientPermissions"); + GoogleJsonResponseException quotaExceeded = + googleException(403, "quotaExceeded"); + + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(permissionDenied)).isTrue(); + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(quotaExceeded)).isFalse(); + } + + @Test + @DisplayName("401 auth error는 auth failure로 분류한다") + void isAuthFailureReturnsTrueForAuthReasons() { + GoogleJsonResponseException authFailure = + googleException(401, "authError"); + + assertThat(GoogleSheetApiExceptionHelper.isAuthFailure(authFailure)).isTrue(); + } + + private GoogleJsonResponseException googleException(int statusCode, String reason) { + GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); + errorInfo.setReason(reason); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(statusCode); + error.setErrors(List.of(errorInfo)); + + HttpResponseException.Builder builder = new HttpResponseException.Builder( + statusCode, + null, + new HttpHeaders() + ); + + return new GoogleJsonResponseException(builder, error); + } +} diff --git a/src/test/java/gg/agit/konect/domain/club/service/SheetSyncExecutorTest.java b/src/test/java/gg/agit/konect/domain/club/service/SheetSyncExecutorTest.java new file mode 100644 index 000000000..1ac3d45f0 --- /dev/null +++ b/src/test/java/gg/agit/konect/domain/club/service/SheetSyncExecutorTest.java @@ -0,0 +1,108 @@ +package gg.agit.konect.domain.club.service; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verify; + +import java.util.List; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.springframework.context.ApplicationEventPublisher; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; +import com.google.api.services.sheets.v4.Sheets; +import com.google.api.services.sheets.v4.model.ClearValuesRequest; + +import gg.agit.konect.domain.club.enums.ClubSheetSortKey; +import gg.agit.konect.domain.club.event.SheetSyncFailedEvent; +import gg.agit.konect.domain.club.model.Club; +import gg.agit.konect.domain.club.repository.ClubMemberRepository; +import gg.agit.konect.domain.club.repository.ClubRepository; +import gg.agit.konect.support.ServiceTestSupport; +import gg.agit.konect.support.fixture.ClubFixture; +import gg.agit.konect.support.fixture.UniversityFixture; + +class SheetSyncExecutorTest extends ServiceTestSupport { + + @Mock + private Sheets googleSheetsService; + + @Mock + private Sheets.Spreadsheets spreadsheets; + + @Mock + private Sheets.Spreadsheets.Values values; + + @Mock + private Sheets.Spreadsheets.Values.Clear clearRequest; + + @Mock + private ClubRepository clubRepository; + + @Mock + private ClubMemberRepository clubMemberRepository; + + @Mock + private ObjectMapper objectMapper; + + @Mock + private ApplicationEventPublisher applicationEventPublisher; + + @InjectMocks + private SheetSyncExecutor sheetSyncExecutor; + + @Test + @DisplayName("시트 동기화 권한 오류는 실패 이벤트로 발행한다") + void executeWithSortPublishesFailureEventWhenAccessDenied() throws Exception { + // given + Integer clubId = 1; + String spreadsheetId = "spreadsheet-id"; + Club club = ClubFixture.create(UniversityFixture.create()); + club.updateGoogleSheetId(spreadsheetId); + + given(clubRepository.getById(clubId)).willReturn(club); + given(clubMemberRepository.findAllByClubId(clubId)).willReturn(List.of()); + given(googleSheetsService.spreadsheets()).willReturn(spreadsheets); + given(spreadsheets.values()).willReturn(values); + given(values.clear(eq(spreadsheetId), eq("A:F"), any(ClearValuesRequest.class))) + .willReturn(clearRequest); + given(clearRequest.execute()).willThrow(googleException(403, "accessDenied")); + + // when + sheetSyncExecutor.executeWithSort(clubId, ClubSheetSortKey.NAME, true); + + // then + verify(applicationEventPublisher).publishEvent(argThat((Object event) -> + event instanceof SheetSyncFailedEvent sheetSyncFailedEvent + && sheetSyncFailedEvent.clubId().equals(clubId) + && sheetSyncFailedEvent.spreadsheetId().equals(spreadsheetId) + && sheetSyncFailedEvent.accessDenied() + )); + } + + private GoogleJsonResponseException googleException(int statusCode, String reason) { + GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); + errorInfo.setReason(reason); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(statusCode); + error.setErrors(List.of(errorInfo)); + + HttpResponseException.Builder builder = new HttpResponseException.Builder( + statusCode, + null, + new HttpHeaders() + ); + + return new GoogleJsonResponseException(builder, error); + } +} diff --git a/src/test/java/gg/agit/konect/support/IntegrationTestSupport.java b/src/test/java/gg/agit/konect/support/IntegrationTestSupport.java index 157a152d6..84b44df82 100644 --- a/src/test/java/gg/agit/konect/support/IntegrationTestSupport.java +++ b/src/test/java/gg/agit/konect/support/IntegrationTestSupport.java @@ -27,6 +27,7 @@ import com.google.api.services.drive.Drive; import com.google.api.services.sheets.v4.Sheets; import com.google.auth.oauth2.GoogleCredentials; +import com.google.auth.oauth2.ServiceAccountCredentials; import jakarta.persistence.EntityManager; @@ -77,6 +78,9 @@ public abstract class IntegrationTestSupport { @MockitoBean protected GoogleCredentials googleCredentials; + @MockitoBean + protected ServiceAccountCredentials serviceAccountCredentials; + @MockitoBean protected Sheets googleSheetsService; From 2c0b4babe9a57a76499e7346e4094c1d5b67fec9 Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 15:26:47 +0900 Subject: [PATCH 5/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20=EA=B6=8C=ED=95=9C=20=EB=B6=80=EC=97=AC=20=EB=A9=B1?= =?UTF-8?q?=EB=93=B1=EC=84=B1=20=EB=B0=8F=20Drive=20=EC=9D=B8=EC=A6=9D=20?= =?UTF-8?q?=EC=98=A4=EB=A5=98=20=EC=B2=98=EB=A6=AC=20=EB=B3=B4=EC=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service/GoogleSheetPermissionService.java | 71 +++++++ .../club/service/SheetMigrationService.java | 122 ++++++++++-- .../GoogleSheetApiExceptionHelperTest.java | 37 +++- .../GoogleSheetPermissionServiceTest.java | 178 ++++++++++++++++++ 4 files changed, 394 insertions(+), 14 deletions(-) create mode 100644 src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java index 85632248c..2d5772e3f 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java @@ -24,6 +24,7 @@ @RequiredArgsConstructor public class GoogleSheetPermissionService { + private static final int PERMISSION_APPLY_MAX_ATTEMPTS = 2; private static final int ROLE_RANK_NONE = 0; private static final int ROLE_RANK_READER = 1; private static final int ROLE_RANK_COMMENTER = 2; @@ -88,6 +89,51 @@ private void ensureServiceAccountPermission( String targetRole ) throws IOException { String serviceAccountEmail = getServiceAccountEmail(); + IOException lastException = null; + + for (int attempt = 1; attempt <= PERMISSION_APPLY_MAX_ATTEMPTS; attempt++) { + try { + applyServiceAccountPermission( + userDriveService, + fileId, + targetRole, + serviceAccountEmail + ); + return; + } catch (IOException e) { + lastException = e; + if (hasRequiredPermission( + userDriveService, + fileId, + serviceAccountEmail, + targetRole + )) { + log.info( + "Service account permission reached target role after retry. fileId={}, role={}, email={}", + fileId, + targetRole, + serviceAccountEmail + ); + return; + } + + if (attempt == PERMISSION_APPLY_MAX_ATTEMPTS) { + throw e; + } + } + } + + if (lastException != null) { + throw lastException; + } + } + + private void applyServiceAccountPermission( + Drive userDriveService, + String fileId, + String targetRole, + String serviceAccountEmail + ) throws IOException { Permission existingPermission = findServiceAccountPermission( userDriveService, fileId, @@ -135,6 +181,31 @@ private void ensureServiceAccountPermission( ); } + private boolean hasRequiredPermission( + Drive userDriveService, + String fileId, + String serviceAccountEmail, + String targetRole + ) { + try { + Permission currentPermission = findServiceAccountPermission( + userDriveService, + fileId, + serviceAccountEmail + ); + return currentPermission != null + && roleRank(currentPermission.getRole()) >= roleRank(targetRole); + } catch (IOException e) { + log.debug( + "Failed to re-check service account permission. fileId={}, email={}, cause={}", + fileId, + serviceAccountEmail, + e.getMessage() + ); + return false; + } + } + private Permission findServiceAccountPermission( Drive userDriveService, String fileId, diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index d976e5ec4..ec77154b8 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -46,6 +46,10 @@ public class SheetMigrationService { Pattern.compile("(?:folders/|id=)([a-zA-Z0-9_-]{20,})"); private static final String MIME_TYPE_SPREADSHEET = "application/vnd.google-apps.spreadsheet"; + private static final int ROLE_RANK_NONE = 0; + private static final int ROLE_RANK_READER = 1; + private static final int ROLE_RANK_COMMENTER = 2; + private static final int ROLE_RANK_WRITER = 3; private static final String NEW_SHEET_TITLE_PREFIX = "KONECT_인명부_"; @Value("${google.sheets.template-spreadsheet-id:}") @@ -202,20 +206,19 @@ private void grantServiceAccountAccess(Drive userDriveService, String fileId) { * 서비스 계정에 지정된 role로 Drive 접근 권한을 부여하는 공통 메서드입니다. */ private void grantServiceAccountPermission(Drive userDriveService, String fileId, String role) { - String serviceAccountEmail = serviceAccountCredentials.getClientEmail(); try { - Permission permission = new Permission() - .setType("user") - .setRole(role) - .setEmailAddress(serviceAccountEmail); - userDriveService.permissions().create(fileId, permission) - .setSendNotificationEmail(false) - .execute(); - log.info( - "Service account {} access granted. fileId={}, email={}", - role, fileId, serviceAccountEmail - ); + ensureServiceAccountPermission(userDriveService, fileId, role); } catch (IOException e) { + if (GoogleSheetApiExceptionHelper.isAuthFailure(e)) { + log.warn( + "Google Drive auth failed while granting service account permission. " + + "fileId={}, role={}, cause={}", + fileId, + role, + e.getMessage() + ); + throw CustomException.of(ApiResponseCode.FAILED_INIT_GOOGLE_DRIVE); + } if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { log.warn( "Google Sheets access denied while granting service account permission. " @@ -234,6 +237,92 @@ private void grantServiceAccountPermission(Drive userDriveService, String fileId } } + private void ensureServiceAccountPermission( + Drive userDriveService, + String fileId, + String targetRole + ) throws IOException { + String serviceAccountEmail = serviceAccountCredentials.getClientEmail(); + Permission existingPermission = findServiceAccountPermission( + userDriveService, + fileId, + serviceAccountEmail + ); + + if (existingPermission == null) { + Permission permission = new Permission() + .setType("user") + .setRole(targetRole) + .setEmailAddress(serviceAccountEmail); + userDriveService.permissions().create(fileId, permission) + .setSendNotificationEmail(false) + .execute(); + log.info( + "Service account {} access granted. fileId={}, email={}", + targetRole, + fileId, + serviceAccountEmail + ); + return; + } + + String currentRole = existingPermission.getRole(); + if (roleRank(currentRole) >= roleRank(targetRole)) { + log.info( + "Service account permission already satisfies requested role. fileId={}, role={}, email={}", + fileId, + currentRole, + serviceAccountEmail + ); + return; + } + + Permission updatedPermission = new Permission().setRole(targetRole); + userDriveService.permissions().update(fileId, existingPermission.getId(), updatedPermission) + .execute(); + log.info( + "Service account permission upgraded. fileId={}, fromRole={}, toRole={}, email={}", + fileId, + currentRole, + targetRole, + serviceAccountEmail + ); + } + + private Permission findServiceAccountPermission( + Drive userDriveService, + String fileId, + String serviceAccountEmail + ) throws IOException { + List permissions = userDriveService.permissions().list(fileId) + .setFields("permissions(id,type,emailAddress,role)") + .execute() + .getPermissions(); + + if (permissions == null) { + return null; + } + + return permissions.stream() + .filter(permission -> "user".equals(permission.getType())) + .filter(permission -> serviceAccountEmail.equals(permission.getEmailAddress())) + .findFirst() + .orElse(null); + } + + private int roleRank(String role) { + if (role == null) { + return ROLE_RANK_NONE; + } + + return switch (role) { + case "reader" -> ROLE_RANK_READER; + case "commenter" -> ROLE_RANK_COMMENTER; + case "writer", "fileOrganizer", "organizer", "owner" -> ROLE_RANK_WRITER; + default -> ROLE_RANK_NONE; + }; + } + private void registerDriveRollback(Drive driveService, String fileId) { TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @Override @@ -331,6 +420,15 @@ private String copyTemplate(Drive driveService, String templateId, String clubNa if (newFileId != null) { deleteFile(driveService, newFileId); } + if (GoogleSheetApiExceptionHelper.isAuthFailure(e)) { + log.warn( + "Google Drive auth failed while copying template. templateId={}, targetFolderId={}, cause={}", + templateId, + targetFolderId, + e.getMessage() + ); + throw CustomException.of(ApiResponseCode.FAILED_INIT_GOOGLE_DRIVE); + } if (GoogleSheetApiExceptionHelper.isAccessDenied(e)) { log.warn( "Google Sheets access denied while copying template. templateId={}, targetFolderId={}, cause={}", diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java index e1f0f56ac..3efea4048 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java @@ -15,19 +15,43 @@ class GoogleSheetApiExceptionHelperTest { @Test - @DisplayName("권한 관련 403 reason만 access denied로 분류한다") + @DisplayName("classifies only permission-related 403 reasons as access denied") void isAccessDeniedReturnsTrueOnlyForPermissionReasons() { GoogleJsonResponseException permissionDenied = googleException(403, "insufficientPermissions"); + GoogleJsonResponseException accessDenied = + googleException(403, "accessDenied"); + GoogleJsonResponseException forbidden = + googleException(403, "forbidden"); GoogleJsonResponseException quotaExceeded = googleException(403, "quotaExceeded"); assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(permissionDenied)).isTrue(); + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(accessDenied)).isTrue(); + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(forbidden)).isTrue(); assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(quotaExceeded)).isFalse(); } @Test - @DisplayName("401 auth error는 auth failure로 분류한다") + @DisplayName("returns false for 403 responses without a reason") + void isAccessDeniedReturnsFalseWhenReasonIsMissing() { + GoogleJsonError error = new GoogleJsonError(); + error.setCode(403); + error.setErrors(List.of()); + + HttpResponseException.Builder builder = new HttpResponseException.Builder( + 403, + null, + new HttpHeaders() + ); + + GoogleJsonResponseException exception = new GoogleJsonResponseException(builder, error); + + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(exception)).isFalse(); + } + + @Test + @DisplayName("classifies auth-related 401 as auth failure") void isAuthFailureReturnsTrueForAuthReasons() { GoogleJsonResponseException authFailure = googleException(401, "authError"); @@ -35,6 +59,15 @@ void isAuthFailureReturnsTrueForAuthReasons() { assertThat(GoogleSheetApiExceptionHelper.isAuthFailure(authFailure)).isTrue(); } + @Test + @DisplayName("classifies 404 as not found") + void isNotFoundReturnsTrueFor404() { + GoogleJsonResponseException notFound = + googleException(404, "notFound"); + + assertThat(GoogleSheetApiExceptionHelper.isNotFound(notFound)).isTrue(); + } + private GoogleJsonResponseException googleException(int statusCode, String reason) { GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); errorInfo.setReason(reason); diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java new file mode 100644 index 000000000..00af07ebc --- /dev/null +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java @@ -0,0 +1,178 @@ +package gg.agit.konect.domain.club.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.List; +import java.util.Optional; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; +import com.google.api.services.drive.Drive; +import com.google.api.services.drive.model.Permission; +import com.google.api.services.drive.model.PermissionList; +import com.google.auth.oauth2.ServiceAccountCredentials; + +import gg.agit.konect.domain.user.enums.Provider; +import gg.agit.konect.domain.user.model.UserOAuthAccount; +import gg.agit.konect.domain.user.repository.UserOAuthAccountRepository; +import gg.agit.konect.infrastructure.googlesheets.GoogleSheetsConfig; +import gg.agit.konect.support.ServiceTestSupport; + +class GoogleSheetPermissionServiceTest extends ServiceTestSupport { + + private static final Integer REQUESTER_ID = 1; + private static final String FILE_ID = "spreadsheet-id"; + private static final String REFRESH_TOKEN = "refresh-token"; + private static final String SERVICE_ACCOUNT_EMAIL = "service-account@konect.iam.gserviceaccount.com"; + + @Mock + private ServiceAccountCredentials serviceAccountCredentials; + + @Mock + private GoogleSheetsConfig googleSheetsConfig; + + @Mock + private UserOAuthAccountRepository userOAuthAccountRepository; + + @Mock + private UserOAuthAccount userOAuthAccount; + + @Mock + private Drive userDriveService; + + @Mock + private Drive.Permissions permissions; + + @Mock + private Drive.Permissions.List listRequest; + + @Mock + private Drive.Permissions.Create createRequest; + + @Mock + private Drive.Permissions.Update updateRequest; + + @InjectMocks + private GoogleSheetPermissionService googleSheetPermissionService; + + @Test + @DisplayName("returns true without creating when the service account already has writer access") + void tryGrantServiceAccountWriterAccessReturnsTrueWhenPermissionAlreadyExists() + throws IOException, GeneralSecurityException { + // given + mockConnectedDriveAccount(); + given(permissions.list(FILE_ID)).willReturn(listRequest); + given(listRequest.setFields("permissions(id,emailAddress,role)")).willReturn(listRequest); + given(listRequest.execute()).willReturn(permissionList(permission("perm-1", "writer"))); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isTrue(); + verify(permissions, never()).create(eq(FILE_ID), any(Permission.class)); + verify(permissions, never()).update(eq(FILE_ID), eq("perm-1"), any(Permission.class)); + } + + @Test + @DisplayName("returns true when create fails but the permission is visible on re-check") + void tryGrantServiceAccountWriterAccessReturnsTrueAfterConcurrentGrant() + throws IOException, GeneralSecurityException { + // given + mockConnectedDriveAccount(); + given(permissions.list(FILE_ID)).willReturn(listRequest); + given(listRequest.setFields("permissions(id,emailAddress,role)")).willReturn(listRequest); + given(listRequest.execute()).willReturn( + permissionList(), + permissionList(permission("perm-1", "writer")) + ); + given(permissions.create(eq(FILE_ID), any(Permission.class))).willReturn(createRequest); + given(createRequest.setSendNotificationEmail(false)).willReturn(createRequest); + given(createRequest.execute()).willThrow(new IOException("already granted")); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isTrue(); + verify(permissions).create(eq(FILE_ID), any(Permission.class)); + } + + @Test + @DisplayName("returns false when Google Drive auth fails during permission lookup") + void tryGrantServiceAccountWriterAccessReturnsFalseWhenAuthFails() + throws IOException, GeneralSecurityException { + // given + mockConnectedDriveAccount(); + given(permissions.list(FILE_ID)).willReturn(listRequest); + given(listRequest.setFields("permissions(id,emailAddress,role)")).willReturn(listRequest); + given(listRequest.execute()).willThrow(googleException(401, "authError")); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isFalse(); + } + + private void mockConnectedDriveAccount() throws IOException, GeneralSecurityException { + given(userOAuthAccountRepository.findByUserIdAndProvider(REQUESTER_ID, Provider.GOOGLE)) + .willReturn(Optional.of(userOAuthAccount)); + given(userOAuthAccount.getGoogleDriveRefreshToken()).willReturn(REFRESH_TOKEN); + given(googleSheetsConfig.buildUserDriveService(REFRESH_TOKEN)).willReturn(userDriveService); + given(serviceAccountCredentials.getClientEmail()).willReturn(SERVICE_ACCOUNT_EMAIL); + given(userDriveService.permissions()).willReturn(permissions); + } + + private Permission permission(String id, String role) { + return new Permission() + .setId(id) + .setType("user") + .setEmailAddress(SERVICE_ACCOUNT_EMAIL) + .setRole(role); + } + + private PermissionList permissionList(Permission... permissions) { + return new PermissionList().setPermissions(List.of(permissions)); + } + + private GoogleJsonResponseException googleException(int statusCode, String reason) { + GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); + errorInfo.setReason(reason); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(statusCode); + error.setErrors(List.of(errorInfo)); + + HttpResponseException.Builder builder = new HttpResponseException.Builder( + statusCode, + null, + new HttpHeaders() + ); + + return new GoogleJsonResponseException(builder, error); + } +} From a9c0b3158c390212ab504b7686f0cb84a1f7ff7e Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 15:51:24 +0900 Subject: [PATCH 6/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20=EA=B6=8C=ED=95=9C=20=EC=A0=95=EB=A6=AC=20=EA=B8=B0?= =?UTF-8?q?=EC=A4=80=20=EB=B0=8F=20=EC=98=88=EC=99=B8=20=EB=B6=84=EB=A5=98?= =?UTF-8?q?=20=EB=B3=B4=EC=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service/GoogleDrivePermissionHelper.java | 34 ++++++++ .../GoogleSheetApiExceptionHelper.java | 4 + .../service/GoogleSheetPermissionService.java | 27 +------ .../club/service/SheetMigrationService.java | 47 +++++------ .../club/service/GoogleApiTestUtils.java | 38 +++++++++ .../GoogleSheetApiExceptionHelperTest.java | 25 +++--- .../GoogleSheetPermissionServiceTest.java | 81 ++++++++++++++----- 7 files changed, 171 insertions(+), 85 deletions(-) create mode 100644 src/main/java/gg/agit/konect/domain/club/service/GoogleDrivePermissionHelper.java create mode 100644 src/test/java/gg/agit/konect/domain/club/service/GoogleApiTestUtils.java diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleDrivePermissionHelper.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleDrivePermissionHelper.java new file mode 100644 index 000000000..8f41fc16d --- /dev/null +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleDrivePermissionHelper.java @@ -0,0 +1,34 @@ +package gg.agit.konect.domain.club.service; + +final class GoogleDrivePermissionHelper { + + private static final int ROLE_RANK_NONE = 0; + private static final int ROLE_RANK_READER = 1; + private static final int ROLE_RANK_COMMENTER = 2; + private static final int ROLE_RANK_WRITER = 3; + + private GoogleDrivePermissionHelper() {} + + enum PermissionApplyStatus { + CREATED, + UPGRADED, + UNCHANGED + } + + static boolean hasRequiredRole(String currentRole, String targetRole) { + return roleRank(currentRole) >= roleRank(targetRole); + } + + private static int roleRank(String role) { + if (role == null) { + return ROLE_RANK_NONE; + } + + return switch (role) { + case "reader" -> ROLE_RANK_READER; + case "commenter" -> ROLE_RANK_COMMENTER; + case "writer", "fileOrganizer", "organizer", "owner" -> ROLE_RANK_WRITER; + default -> ROLE_RANK_NONE; + }; + } +} diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java index 648c56ab1..e7a650ea2 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelper.java @@ -6,6 +6,7 @@ import com.google.api.client.googleapis.json.GoogleJsonError; import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.api.client.http.HttpResponseException; import gg.agit.konect.global.code.ApiResponseCode; import gg.agit.konect.global.exception.CustomException; @@ -86,6 +87,9 @@ private static int getStatusCode(IOException exception) { if (exception instanceof GoogleJsonResponseException responseException) { return responseException.getStatusCode(); } + if (exception instanceof HttpResponseException responseException) { + return responseException.getStatusCode(); + } return -1; } } diff --git a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java index 2d5772e3f..33906432c 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionService.java @@ -25,10 +25,6 @@ public class GoogleSheetPermissionService { private static final int PERMISSION_APPLY_MAX_ATTEMPTS = 2; - private static final int ROLE_RANK_NONE = 0; - private static final int ROLE_RANK_READER = 1; - private static final int ROLE_RANK_COMMENTER = 2; - private static final int ROLE_RANK_WRITER = 3; private final ServiceAccountCredentials serviceAccountCredentials; private final GoogleSheetsConfig googleSheetsConfig; @@ -89,7 +85,6 @@ private void ensureServiceAccountPermission( String targetRole ) throws IOException { String serviceAccountEmail = getServiceAccountEmail(); - IOException lastException = null; for (int attempt = 1; attempt <= PERMISSION_APPLY_MAX_ATTEMPTS; attempt++) { try { @@ -101,7 +96,6 @@ private void ensureServiceAccountPermission( ); return; } catch (IOException e) { - lastException = e; if (hasRequiredPermission( userDriveService, fileId, @@ -122,10 +116,6 @@ private void ensureServiceAccountPermission( } } } - - if (lastException != null) { - throw lastException; - } } private void applyServiceAccountPermission( @@ -159,7 +149,7 @@ private void applyServiceAccountPermission( } String currentRole = existingPermission.getRole(); - if (roleRank(currentRole) >= roleRank(targetRole)) { + if (GoogleDrivePermissionHelper.hasRequiredRole(currentRole, targetRole)) { log.info( "Service account permission already satisfies requested role. fileId={}, role={}, email={}", fileId, @@ -194,7 +184,7 @@ private boolean hasRequiredPermission( serviceAccountEmail ); return currentPermission != null - && roleRank(currentPermission.getRole()) >= roleRank(targetRole); + && GoogleDrivePermissionHelper.hasRequiredRole(currentPermission.getRole(), targetRole); } catch (IOException e) { log.debug( "Failed to re-check service account permission. fileId={}, email={}, cause={}", @@ -229,17 +219,4 @@ private Permission findServiceAccountPermission( private String getServiceAccountEmail() { return serviceAccountCredentials.getClientEmail(); } - - private int roleRank(String role) { - if (role == null) { - return ROLE_RANK_NONE; - } - - return switch (role) { - case "reader" -> ROLE_RANK_READER; - case "commenter" -> ROLE_RANK_COMMENTER; - case "writer", "fileOrganizer", "organizer", "owner" -> ROLE_RANK_WRITER; - default -> ROLE_RANK_NONE; - }; - } } diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index ec77154b8..1fbae6747 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -46,10 +46,6 @@ public class SheetMigrationService { Pattern.compile("(?:folders/|id=)([a-zA-Z0-9_-]{20,})"); private static final String MIME_TYPE_SPREADSHEET = "application/vnd.google-apps.spreadsheet"; - private static final int ROLE_RANK_NONE = 0; - private static final int ROLE_RANK_READER = 1; - private static final int ROLE_RANK_COMMENTER = 2; - private static final int ROLE_RANK_WRITER = 3; private static final String NEW_SHEET_TITLE_PREFIX = "KONECT_인명부_"; @Value("${google.sheets.template-spreadsheet-id:}") @@ -101,7 +97,11 @@ public String migrateToTemplate( // 소스 파일에 서비스 계정 reader 권한을 먼저 부여해야 readAllData()가 성공함 grantServiceAccountReadAccess(userDriveService, sourceSpreadsheetId); // 트랜잭션 실패 / 완료 후 소스 파일 서비스 계정 권한 제거 (보상 처리) - registerSourceFilePermissionCleanup(userDriveService, sourceSpreadsheetId); + GoogleDrivePermissionHelper.PermissionApplyStatus sourcePermissionStatus = + grantServiceAccountReadAccess(userDriveService, sourceSpreadsheetId); + if (sourcePermissionStatus == GoogleDrivePermissionHelper.PermissionApplyStatus.CREATED) { + registerSourceFilePermissionCleanup(userDriveService, sourceSpreadsheetId); + } String newSpreadsheetId = copyTemplate(userDriveService, templateId, club.getName(), folderId); registerDriveRollback(userDriveService, newSpreadsheetId); @@ -144,8 +144,11 @@ public String migrateToTemplate( * 소스 파일에 서비스 계정 reader 권한을 부여합니다. * migrate 시 서비스 계정 Sheets API로 소스 데이터를 읽어야 하므로 필요합니다. */ - private void grantServiceAccountReadAccess(Drive userDriveService, String fileId) { - grantServiceAccountPermission(userDriveService, fileId, "reader"); + private GoogleDrivePermissionHelper.PermissionApplyStatus grantServiceAccountReadAccess( + Drive userDriveService, + String fileId + ) { + return grantServiceAccountPermission(userDriveService, fileId, "reader"); } /** @@ -205,9 +208,13 @@ private void grantServiceAccountAccess(Drive userDriveService, String fileId) { /** * 서비스 계정에 지정된 role로 Drive 접근 권한을 부여하는 공통 메서드입니다. */ - private void grantServiceAccountPermission(Drive userDriveService, String fileId, String role) { + private GoogleDrivePermissionHelper.PermissionApplyStatus grantServiceAccountPermission( + Drive userDriveService, + String fileId, + String role + ) { try { - ensureServiceAccountPermission(userDriveService, fileId, role); + return ensureServiceAccountPermission(userDriveService, fileId, role); } catch (IOException e) { if (GoogleSheetApiExceptionHelper.isAuthFailure(e)) { log.warn( @@ -237,7 +244,7 @@ private void grantServiceAccountPermission(Drive userDriveService, String fileId } } - private void ensureServiceAccountPermission( + private GoogleDrivePermissionHelper.PermissionApplyStatus ensureServiceAccountPermission( Drive userDriveService, String fileId, String targetRole @@ -263,18 +270,18 @@ private void ensureServiceAccountPermission( fileId, serviceAccountEmail ); - return; + return GoogleDrivePermissionHelper.PermissionApplyStatus.CREATED; } String currentRole = existingPermission.getRole(); - if (roleRank(currentRole) >= roleRank(targetRole)) { + if (GoogleDrivePermissionHelper.hasRequiredRole(currentRole, targetRole)) { log.info( "Service account permission already satisfies requested role. fileId={}, role={}, email={}", fileId, currentRole, serviceAccountEmail ); - return; + return GoogleDrivePermissionHelper.PermissionApplyStatus.UNCHANGED; } Permission updatedPermission = new Permission().setRole(targetRole); @@ -287,6 +294,7 @@ private void ensureServiceAccountPermission( targetRole, serviceAccountEmail ); + return GoogleDrivePermissionHelper.PermissionApplyStatus.UPGRADED; } private Permission findServiceAccountPermission( @@ -310,19 +318,6 @@ private Permission findServiceAccountPermission( .orElse(null); } - private int roleRank(String role) { - if (role == null) { - return ROLE_RANK_NONE; - } - - return switch (role) { - case "reader" -> ROLE_RANK_READER; - case "commenter" -> ROLE_RANK_COMMENTER; - case "writer", "fileOrganizer", "organizer", "owner" -> ROLE_RANK_WRITER; - default -> ROLE_RANK_NONE; - }; - } - private void registerDriveRollback(Drive driveService, String fileId) { TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @Override diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleApiTestUtils.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleApiTestUtils.java new file mode 100644 index 000000000..8cb42187a --- /dev/null +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleApiTestUtils.java @@ -0,0 +1,38 @@ +package gg.agit.konect.domain.club.service; + +import java.util.List; + +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; + +final class GoogleApiTestUtils { + + private GoogleApiTestUtils() {} + + static GoogleJsonResponseException googleException(int statusCode, String reason) { + GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); + errorInfo.setReason(reason); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(statusCode); + error.setErrors(List.of(errorInfo)); + + HttpResponseException.Builder builder = new HttpResponseException.Builder( + statusCode, + null, + new HttpHeaders() + ); + + return new GoogleJsonResponseException(builder, error); + } + + static HttpResponseException httpResponseException(int statusCode) { + return new HttpResponseException.Builder( + statusCode, + null, + new HttpHeaders() + ).build(); + } +} diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java index 3efea4048..644b6da78 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetApiExceptionHelperTest.java @@ -1,6 +1,8 @@ package gg.agit.konect.domain.club.service; import static org.assertj.core.api.Assertions.assertThat; +import static gg.agit.konect.domain.club.service.GoogleApiTestUtils.googleException; +import static gg.agit.konect.domain.club.service.GoogleApiTestUtils.httpResponseException; import java.util.List; @@ -68,20 +70,15 @@ void isNotFoundReturnsTrueFor404() { assertThat(GoogleSheetApiExceptionHelper.isNotFound(notFound)).isTrue(); } - private GoogleJsonResponseException googleException(int statusCode, String reason) { - GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); - errorInfo.setReason(reason); - - GoogleJsonError error = new GoogleJsonError(); - error.setCode(statusCode); - error.setErrors(List.of(errorInfo)); - - HttpResponseException.Builder builder = new HttpResponseException.Builder( - statusCode, - null, - new HttpHeaders() - ); + @Test + @DisplayName("classifies non-json http response exceptions with their status code") + void classifiesNonJsonHttpResponseExceptionByStatusCode() { + HttpResponseException forbidden = httpResponseException(403); + HttpResponseException unauthorized = httpResponseException(401); + HttpResponseException notFound = httpResponseException(404); - return new GoogleJsonResponseException(builder, error); + assertThat(GoogleSheetApiExceptionHelper.isAccessDenied(forbidden)).isTrue(); + assertThat(GoogleSheetApiExceptionHelper.isAuthFailure(unauthorized)).isTrue(); + assertThat(GoogleSheetApiExceptionHelper.isNotFound(notFound)).isTrue(); } } diff --git a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java index 00af07ebc..552aa8952 100644 --- a/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java +++ b/src/test/java/gg/agit/konect/domain/club/service/GoogleSheetPermissionServiceTest.java @@ -1,6 +1,7 @@ package gg.agit.konect.domain.club.service; import static org.assertj.core.api.Assertions.assertThat; +import static gg.agit.konect.domain.club.service.GoogleApiTestUtils.googleException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; @@ -17,10 +18,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; -import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.client.googleapis.json.GoogleJsonResponseException; -import com.google.api.client.http.HttpHeaders; -import com.google.api.client.http.HttpResponseException; import com.google.api.services.drive.Drive; import com.google.api.services.drive.model.Permission; import com.google.api.services.drive.model.PermissionList; @@ -69,6 +66,23 @@ class GoogleSheetPermissionServiceTest extends ServiceTestSupport { @InjectMocks private GoogleSheetPermissionService googleSheetPermissionService; + @Test + @DisplayName("returns false when the requester has no Google Drive OAuth account") + void tryGrantServiceAccountWriterAccessReturnsFalseWhenOAuthAccountIsMissing() { + // given + given(userOAuthAccountRepository.findByUserIdAndProvider(REQUESTER_ID, Provider.GOOGLE)) + .willReturn(Optional.empty()); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isFalse(); + } + @Test @DisplayName("returns true without creating when the service account already has writer access") void tryGrantServiceAccountWriterAccessReturnsTrueWhenPermissionAlreadyExists() @@ -118,6 +132,29 @@ void tryGrantServiceAccountWriterAccessReturnsTrueAfterConcurrentGrant() verify(permissions).create(eq(FILE_ID), any(Permission.class)); } + @Test + @DisplayName("returns true when an existing permission needs to be upgraded to writer") + void tryGrantServiceAccountWriterAccessUpgradesExistingPermission() + throws IOException, GeneralSecurityException { + // given + mockConnectedDriveAccount(); + given(permissions.list(FILE_ID)).willReturn(listRequest); + given(listRequest.setFields("permissions(id,emailAddress,role)")).willReturn(listRequest); + given(listRequest.execute()).willReturn(permissionList(permission("perm-x", "reader"))); + given(permissions.update(eq(FILE_ID), eq("perm-x"), any(Permission.class))).willReturn(updateRequest); + given(updateRequest.execute()).willReturn(permission("perm-x", "writer")); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isTrue(); + verify(permissions).update(eq(FILE_ID), eq("perm-x"), any(Permission.class)); + } + @Test @DisplayName("returns false when Google Drive auth fails during permission lookup") void tryGrantServiceAccountWriterAccessReturnsFalseWhenAuthFails() @@ -138,6 +175,26 @@ void tryGrantServiceAccountWriterAccessReturnsFalseWhenAuthFails() assertThat(granted).isFalse(); } + @Test + @DisplayName("returns false when Google Drive reports access denied while listing permissions") + void tryGrantServiceAccountWriterAccessReturnsFalseWhenAccessIsDenied() + throws IOException, GeneralSecurityException { + // given + mockConnectedDriveAccount(); + given(permissions.list(FILE_ID)).willReturn(listRequest); + given(listRequest.setFields("permissions(id,emailAddress,role)")).willReturn(listRequest); + given(listRequest.execute()).willThrow(googleException(403, "forbidden")); + + // when + boolean granted = googleSheetPermissionService.tryGrantServiceAccountWriterAccess( + REQUESTER_ID, + FILE_ID + ); + + // then + assertThat(granted).isFalse(); + } + private void mockConnectedDriveAccount() throws IOException, GeneralSecurityException { given(userOAuthAccountRepository.findByUserIdAndProvider(REQUESTER_ID, Provider.GOOGLE)) .willReturn(Optional.of(userOAuthAccount)); @@ -159,20 +216,4 @@ private PermissionList permissionList(Permission... permissions) { return new PermissionList().setPermissions(List.of(permissions)); } - private GoogleJsonResponseException googleException(int statusCode, String reason) { - GoogleJsonError.ErrorInfo errorInfo = new GoogleJsonError.ErrorInfo(); - errorInfo.setReason(reason); - - GoogleJsonError error = new GoogleJsonError(); - error.setCode(statusCode); - error.setErrors(List.of(errorInfo)); - - HttpResponseException.Builder builder = new HttpResponseException.Builder( - statusCode, - null, - new HttpHeaders() - ); - - return new GoogleJsonResponseException(builder, error); - } } From 7ffd52a81b2993912def035d3f309ff058f0f1d2 Mon Sep 17 00:00:00 2001 From: JanooGwan Date: Wed, 1 Apr 2026 16:14:49 +0900 Subject: [PATCH 7/7] =?UTF-8?q?fix:=20=EA=B5=AC=EA=B8=80=20=EC=8B=9C?= =?UTF-8?q?=ED=8A=B8=20reader=20=EA=B6=8C=ED=95=9C=20=EC=A4=91=EB=B3=B5=20?= =?UTF-8?q?=ED=98=B8=EC=B6=9C=20=EC=A0=9C=EA=B1=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../agit/konect/domain/club/service/SheetMigrationService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java index 1fbae6747..616d3873f 100644 --- a/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java +++ b/src/main/java/gg/agit/konect/domain/club/service/SheetMigrationService.java @@ -95,10 +95,9 @@ public String migrateToTemplate( String folderId = resolveFolderId(userDriveService, sourceSpreadsheetUrl, sourceSpreadsheetId); // 소스 파일에 서비스 계정 reader 권한을 먼저 부여해야 readAllData()가 성공함 - grantServiceAccountReadAccess(userDriveService, sourceSpreadsheetId); - // 트랜잭션 실패 / 완료 후 소스 파일 서비스 계정 권한 제거 (보상 처리) GoogleDrivePermissionHelper.PermissionApplyStatus sourcePermissionStatus = grantServiceAccountReadAccess(userDriveService, sourceSpreadsheetId); + // 트랜잭션 실패 / 완료 후 이번 요청에서 추가한 소스 파일 권한만 정리한다. if (sourcePermissionStatus == GoogleDrivePermissionHelper.PermissionApplyStatus.CREATED) { registerSourceFilePermissionCleanup(userDriveService, sourceSpreadsheetId); }