From 7aee0e0cec96d5aed1273464f2a88aa32022e9ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=BC=ED=98=81?= Date: Sun, 6 Sep 2026 17:09:27 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20=EA=B3=B5=EC=9C=A0=20=EC=BD=94?= =?UTF-8?q?=EB=93=9C=20=EC=86=8C=EC=9C=A0=EA=B6=8C=EA=B3=BC=20=EA=B3=84?= =?UTF-8?q?=EC=A0=95=20=EC=82=AD=EC=A0=9C=20=EC=9D=BC=EA=B4=80=EC=84=B1?= =?UTF-8?q?=EC=9D=84=20=EB=B3=B4=EC=99=84=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #198 공유 등록부를 확인한 뒤 일정을 쓰고 이전 코드의 동일 소유자 복구를 허용한다. 사용자·공개 프로필·등록부 삭제를 한 batch로 처리한다. 검증: Firestore emulator 56개, core:data 단위 41개, profile 23개, addschedule 5개, ktlint, app assembleDebug/lintDebug 통과. Sources: https://firebase.google.com/docs/firestore/security/rules-conditions https://firebase.google.com/docs/reference/android/com/google/firebase/firestore/WriteBatch --- .../remote/repository/ScheduleRepository.kt | 7 + .../remote/repository/ShareCodeRegistry.kt | 32 ++++ .../data/remote/repository/UserRepository.kt | 19 +- .../ScheduleRepositoryAddScheduleTest.kt | 5 + .../ScheduleRepositoryShareCodeTest.kt | 175 ++++++++++++++++++ .../repository/UserRepositoryShareCodeTest.kt | 170 +++++++++++++++++ firestore-tests/rules.test.mjs | 137 +++++++++++++- firestore.rules | 23 ++- 8 files changed, 552 insertions(+), 16 deletions(-) create mode 100644 core/data/src/main/java/com/example/slowclock/data/remote/repository/ShareCodeRegistry.kt create mode 100644 core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt create mode 100644 core/data/src/test/java/com/example/slowclock/data/remote/repository/UserRepositoryShareCodeTest.kt diff --git a/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt index d1826b0..f277e7f 100644 --- a/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt +++ b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt @@ -181,6 +181,8 @@ class ScheduleRepository if (userShareCode.isBlank()) { Log.w("ScheduleRepo", "공유 코드가 비어 있어 이 일정은 가족에게 보이지 않는다") } + // 앱 시작의 비동기 복구보다 저장이 먼저 실행돼도 등록부를 확인한 뒤 쓴다. + firestore.ensureShareCodeOwner(uid, userShareCode) return userShareCode } @@ -218,6 +220,8 @@ class ScheduleRepository docRef.set(scheduleWithId).await() Log.d("ScheduleRepo", "일정 저장 성공: ${docRef.id}") ScheduleResult.Success(docRef.id) + } catch (e: ShareCodeOwnershipException) { + ScheduleResult.Error(AppError.GeneralError(e.message.orEmpty())) } catch (e: FirebaseFirestoreException) { Log.e("ScheduleRepo", "Firestore 저장 에러: ${e.code}", e) val error = @@ -314,6 +318,7 @@ class ScheduleRepository ) return try { + firestore.ensureShareCodeOwner(uid, updatedSchedule.sharedCode) schedulesCollection .document(schedule.id) .set(updatedSchedule) @@ -321,6 +326,8 @@ class ScheduleRepository Log.d("ScheduleRepo", "일정 수정 성공: ${schedule.id}") ScheduleResult.Success(Unit) + } catch (e: ShareCodeOwnershipException) { + ScheduleResult.Error(AppError.GeneralError(e.message.orEmpty())) } catch (e: FirebaseFirestoreException) { Log.e("ScheduleRepo", "일정 수정 실패: ${e.code}", e) val error = diff --git a/core/data/src/main/java/com/example/slowclock/data/remote/repository/ShareCodeRegistry.kt b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ShareCodeRegistry.kt new file mode 100644 index 0000000..0ae9b31 --- /dev/null +++ b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ShareCodeRegistry.kt @@ -0,0 +1,32 @@ +package com.example.slowclock.data.remote.repository + +import com.example.slowclock.data.FirestoreCollections +import com.google.firebase.firestore.FirebaseFirestore +import com.google.firebase.firestore.FirebaseFirestoreException +import kotlinx.coroutines.tasks.await + +/** 기존 코드의 등록부를 보완한다. 같은 소유자의 동일값 쓰기만 규칙이 허용한다. */ +internal suspend fun FirebaseFirestore.ensureShareCodeOwner( + userId: String, + shareCode: String, +) { + if (shareCode.isBlank()) return + try { + collection(FirestoreCollections.SHARE_CODES) + .document(shareCode) + .set(mapOf("userId" to userId)) + .await() + } catch (e: FirebaseFirestoreException) { + if (e.code == FirebaseFirestoreException.Code.PERMISSION_DENIED) { + throw ShareCodeOwnershipException(e) + } + throw e + } +} + +internal class ShareCodeOwnershipException( + cause: Throwable, +) : Exception( + "공유 코드의 소유권을 확인하지 못했습니다. 기존 코드는 유지됩니다. 관리자에게 문의해 주세요.", + cause, + ) diff --git a/core/data/src/main/java/com/example/slowclock/data/remote/repository/UserRepository.kt b/core/data/src/main/java/com/example/slowclock/data/remote/repository/UserRepository.kt index cf9ea7b..846bdf7 100644 --- a/core/data/src/main/java/com/example/slowclock/data/remote/repository/UserRepository.kt +++ b/core/data/src/main/java/com/example/slowclock/data/remote/repository/UserRepository.kt @@ -36,7 +36,7 @@ class UserRepository /** * 아직 임자가 없는 여섯 자리 코드를 하나 만들어 [uid] 앞으로 등록한다. * - * 등록부는 코드 자체가 문서 이름이라, 이미 있는 코드에 대한 만들기는 규칙이 거절한다. + * 등록부는 코드 자체가 문서 이름이라, 다른 소유자의 코드에 대한 쓰기는 규칙이 거절한다. * 확인과 저장이 한 번으로 합쳐지므로 그 사이에 같은 코드를 두 사람이 가져가는 틈이 없다. * 질의로 확인하던 앞의 방식은 컬렉션을 훑을 권한을 함께 열어야 했다(#174). */ @@ -80,11 +80,15 @@ class UserRepository .await() .getString("shareCode") .orEmpty() - if (shareCode.isNotBlank()) { - shareCodesCollection.document(shareCode).delete().await() - } - usersCollection.document(userId).delete().await() - publicProfilesCollection.document(userId).delete().await() + // 셋 중 하나만 지워지면 다음 시도에서 코드를 잃거나 삭제가 막힐 수 있다. + firestore + .batch() + .apply { + if (shareCode.isNotBlank()) delete(shareCodesCollection.document(shareCode)) + delete(usersCollection.document(userId)) + delete(publicProfilesCollection.document(userId)) + }.commit() + .await() true } catch (e: Exception) { Log.e(TAG, "사용자 문서 삭제 실패", e) @@ -116,6 +120,9 @@ class UserRepository usersCollection.document(uid).set(newUser).await() savePublicProfile(newUser.id, newUser.name) } else { + // 등록부 도입 전 발급된 코드도 같은 소유자로 등록한다. 타인 코드와 충돌하면 + // 기존 코드·문서를 그대로 두고 실패한다. 자동 회전은 기존 일정을 분리한다. + firestore.ensureShareCodeOwner(uid, existing.shareCode) val updates = mutableMapOf() if (existing.name != name && name.isNotBlank()) updates["name"] = name if (existing.email != email && email.isNotBlank()) updates["email"] = email diff --git a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryAddScheduleTest.kt b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryAddScheduleTest.kt index 55a4db3..0c010c2 100644 --- a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryAddScheduleTest.kt +++ b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryAddScheduleTest.kt @@ -131,6 +131,11 @@ class ScheduleRepositoryAddScheduleTest { every { firestore.collection("schedules") } returns schedulesCollection every { firestore.collection("users") } returns usersCollection every { firestore.batch() } returns batch + val registry = mockk() + val codeRef = mockk() + every { firestore.collection("shareCodes") } returns registry + every { registry.document(shareCode) } returns codeRef + every { codeRef.set(mapOf("userId" to uid)) } returns Tasks.forResult(null) return ScheduleRepository(auth, firestore) to batch } diff --git a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt new file mode 100644 index 0000000..ee3d753 --- /dev/null +++ b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt @@ -0,0 +1,175 @@ +package com.example.slowclock.data.remote.repository + +import com.example.slowclock.data.model.Schedule +import com.example.slowclock.util.AppError +import com.google.android.gms.tasks.TaskCompletionSource +import com.google.android.gms.tasks.Tasks +import com.google.firebase.auth.FirebaseAuth +import com.google.firebase.auth.FirebaseUser +import com.google.firebase.firestore.CollectionReference +import com.google.firebase.firestore.DocumentReference +import com.google.firebase.firestore.DocumentSnapshot +import com.google.firebase.firestore.FirebaseFirestore +import com.google.firebase.firestore.FirebaseFirestoreException +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.async +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) +class ScheduleRepositoryShareCodeTest { + private val uid = "owner" + private val code = "ABC123" + private val auth = mockk() + private val firestore = mockk() + private val users = mockk() + private val schedules = mockk() + private val registry = mockk() + private val userRef = mockk() + private val scheduleRef = mockk() + private val codeRef = mockk() + private val snapshot = mockk() + private val repository: ScheduleRepository + private val schedule = Schedule(id = "schedule", title = "혈압약", userId = uid, sharedCode = code) + + init { + val user = mockk() + every { user.uid } returns uid + every { auth.currentUser } returns user + every { firestore.collection("users") } returns users + every { firestore.collection("schedules") } returns schedules + every { firestore.collection("shareCodes") } returns registry + every { users.document(uid) } returns userRef + every { schedules.document() } returns scheduleRef + every { schedules.document(schedule.id) } returns scheduleRef + every { scheduleRef.id } returns schedule.id + every { registry.document(code) } returns codeRef + every { userRef.get() } returns Tasks.forResult(snapshot) + every { snapshot.getString("shareCode") } returns code + every { codeRef.set(any()) } returns Tasks.forResult(null) + every { scheduleRef.set(any()) } returns Tasks.forResult(null) + every { scheduleRef.update(any>()) } returns Tasks.forResult(null) + repository = ScheduleRepository(auth, firestore) + } + + @Test + fun `등록부 복구가 완료되기 전에 새 일정을 쓰지 않는다`() = + runTest { + val pending = TaskCompletionSource() + every { codeRef.set(any()) } returns pending.task + + val result = async { repository.addSchedule(schedule) } + runCurrent() + assertFalse(result.isCompleted) + verify(exactly = 0) { scheduleRef.set(any()) } + pending.setResult(null) + + assertEquals(ScheduleRepository.ScheduleResult.Success(schedule.id), result.await()) + verify(exactly = 1) { codeRef.set(mapOf("userId" to uid)) } + verify(exactly = 1) { scheduleRef.set(match { it.sharedCode == code && it.userId == uid }) } + } + + @Test + fun `등록부 확인이 끝나야 편집을 저장한다`() = + runTest { + val pending = TaskCompletionSource() + every { codeRef.set(any()) } returns pending.task + + val result = async { repository.updateSchedule(schedule) } + runCurrent() + assertFalse(result.isCompleted) + verify(exactly = 0) { scheduleRef.set(any()) } + verify(exactly = 0) { scheduleRef.update(any>()) } + pending.setResult(null) + + assertEquals(ScheduleRepository.ScheduleResult.Success(Unit), result.await()) + } + + @Test + fun `다른 계정의 이전 일정을 수정하려 하면 현재 계정으로 확인하고 쓰기를 막는다`() = + runTest { + val nextUser = mockk() + every { nextUser.uid } returns "next-user" + every { auth.currentUser } returns nextUser + every { codeRef.set(mapOf("userId" to "next-user")) } returns Tasks.forException(denied()) + + assertTrue(repository.updateSchedule(schedule) is ScheduleRepository.ScheduleResult.Error) + + verify(exactly = 1) { codeRef.set(mapOf("userId" to "next-user")) } + verify(exactly = 0) { scheduleRef.set(any()) } + verify(exactly = 0) { scheduleRef.update(any>()) } + } + + @Test + fun `등록부 충돌은 저장과 편집을 막고 코드 유지 안내를 돌려준다`() = + runTest { + every { codeRef.set(any()) } returns Tasks.forException(denied()) + + val results = listOf(repository.addSchedule(schedule), repository.updateSchedule(schedule)) + + results.forEach { result -> + val error = (result as ScheduleRepository.ScheduleResult.Error).error + assertTrue(error is AppError.GeneralError) + assertTrue((error as AppError.GeneralError).message.contains("기존 코드는 유지됩니다")) + } + verify(exactly = 0) { scheduleRef.set(any()) } + verify(exactly = 0) { scheduleRef.update(any>()) } + } + + @Test + fun `등록부 네트워크 실패도 일정 저장 없이 결과로 돌아온다`() = + runTest { + every { codeRef.set(any()) } returns + Tasks.forException( + FirebaseFirestoreException("offline", FirebaseFirestoreException.Code.UNAVAILABLE), + ) + + val result = repository.addSchedule(schedule) as ScheduleRepository.ScheduleResult.Error + + assertEquals(AppError.NetworkError, result.error) + verify(exactly = 0) { scheduleRef.set(any()) } + } + + @Test + fun `비공유 저장은 등록부를 호출하지 않는다`() = + runTest { + every { snapshot.getString("shareCode") } returns "" + + assertTrue(repository.addSchedule(schedule) is ScheduleRepository.ScheduleResult.Success) + assertTrue(repository.updateSchedule(schedule.copy(sharedCode = "")) is ScheduleRepository.ScheduleResult.Success) + + verify(exactly = 0) { firestore.collection("shareCodes") } + verify(atLeast = 1) { scheduleRef.set(match { it.sharedCode == "" }) } + } + + @Test + fun `코드 복구가 실패하면 빈 코드 일정 backfill을 시작하지 않는다`() = + runTest { + every { codeRef.set(any()) } returns Tasks.forException(denied()) + + assertEquals(0, repository.fillMissingSharedCode(uid)) + + verify(exactly = 0) { schedules.whereEqualTo(any(), any()) } + verify(exactly = 0) { firestore.batch() } + } + + @Test + fun `로그아웃 뒤에는 등록부와 일정 저장을 시작하지 않는다`() = + runTest { + every { auth.currentUser } returns null + + assertEquals(ScheduleRepository.ScheduleResult.Error(AppError.AuthError), repository.addSchedule(schedule)) + assertEquals(ScheduleRepository.ScheduleResult.Error(AppError.AuthError), repository.updateSchedule(schedule)) + verify(exactly = 0) { codeRef.set(any()) } + verify(exactly = 0) { scheduleRef.set(any()) } + verify(exactly = 0) { scheduleRef.update(any>()) } + } + + private fun denied() = FirebaseFirestoreException("denied", FirebaseFirestoreException.Code.PERMISSION_DENIED) +} diff --git a/core/data/src/test/java/com/example/slowclock/data/remote/repository/UserRepositoryShareCodeTest.kt b/core/data/src/test/java/com/example/slowclock/data/remote/repository/UserRepositoryShareCodeTest.kt new file mode 100644 index 0000000..f2b89fc --- /dev/null +++ b/core/data/src/test/java/com/example/slowclock/data/remote/repository/UserRepositoryShareCodeTest.kt @@ -0,0 +1,170 @@ +package com.example.slowclock.data.remote.repository + +import com.example.slowclock.data.model.PublicProfile +import com.example.slowclock.data.model.User +import com.google.android.gms.tasks.TaskCompletionSource +import com.google.android.gms.tasks.Tasks +import com.google.firebase.auth.FirebaseAuth +import com.google.firebase.firestore.CollectionReference +import com.google.firebase.firestore.DocumentReference +import com.google.firebase.firestore.DocumentSnapshot +import com.google.firebase.firestore.FirebaseFirestore +import com.google.firebase.firestore.FirebaseFirestoreException +import com.google.firebase.firestore.WriteBatch +import com.google.firebase.messaging.FirebaseMessaging +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.async +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) +class UserRepositoryShareCodeTest { + private val uid = "owner" + private val code = "ABC123" + private val firestore = mockk() + private val users = mockk() + private val profiles = mockk() + private val registry = mockk() + private val userRef = mockk() + private val profileRef = mockk() + private val codeRef = mockk() + private val snapshot = mockk() + private val batch = mockk() + private val repository: UserRepository + + init { + every { firestore.collection("users") } returns users + every { firestore.collection("publicProfiles") } returns profiles + every { firestore.collection("shareCodes") } returns registry + every { users.document(uid) } returns userRef + every { profiles.document(uid) } returns profileRef + every { registry.document(any()) } returns codeRef + every { userRef.get() } returns Tasks.forResult(snapshot) + every { snapshot.toObject(User::class.java) } returns User(id = uid, name = "기존 이름", shareCode = code) + every { snapshot.getString("shareCode") } returns code + every { codeRef.set(any()) } returns Tasks.forResult(null) + every { userRef.set(any()) } returns Tasks.forResult(null) + every { userRef.update(any>()) } returns Tasks.forResult(null) + every { profileRef.set(any()) } returns Tasks.forResult(null) + every { firestore.batch() } returns batch + every { batch.delete(any()) } returns batch + every { batch.commit() } returns Tasks.forResult(null) + repository = UserRepository(mockk(), firestore, mockk()) + } + + @Test + fun `이전 계정은 등록부 확인이 끝나야 사용자와 공개 프로필을 갱신한다`() = + runTest { + val pending = TaskCompletionSource() + every { codeRef.set(mapOf("userId" to uid)) } returns pending.task + + val result = async { repository.ensureShareCode(uid, "새 이름", "new@example.com") } + runCurrent() + + assertFalse(result.isCompleted) + verify(exactly = 0) { userRef.set(any()) } + verify(exactly = 0) { userRef.update(any>()) } + verify(exactly = 0) { profileRef.set(any()) } + pending.setResult(null) + assertTrue(result.await()) + verify(exactly = 1) { userRef.update(match> { it["name"] == "새 이름" }) } + verify(exactly = 1) { profileRef.set(PublicProfile(id = uid, name = "새 이름")) } + } + + @Test + fun `기존 코드가 충돌하면 자동 회전하지 않고 사용자와 일정을 보존한다`() = + runTest { + every { codeRef.set(any()) } returns Tasks.forException(denied()) + + assertFalse(repository.ensureShareCode(uid, "새 이름", "new@example.com")) + + verify(exactly = 1) { registry.document(code) } + verify(exactly = 1) { codeRef.set(mapOf("userId" to uid)) } + verify(exactly = 0) { userRef.set(any()) } + verify(exactly = 0) { userRef.update(any>()) } + verify(exactly = 0) { profileRef.set(any()) } + verify(exactly = 0) { firestore.collection("schedules") } + } + + @Test + fun `새 사용자는 등록한 여섯 자리 코드로 문서를 만든다`() = + runTest { + every { snapshot.toObject(User::class.java) } returns null + val claimedCode = slot() + val savedUser = slot() + every { registry.document(capture(claimedCode)) } returns codeRef + every { userRef.set(capture(savedUser)) } returns Tasks.forResult(null) + + assertTrue(repository.ensureShareCode(uid, "이름", "new@example.com")) + + assertTrue(claimedCode.captured.matches(Regex("[A-Z0-9]{6}"))) + assertEquals(claimedCode.captured, savedUser.captured.shareCode) + assertEquals(uid, savedUser.captured.id) + verify(exactly = 1) { codeRef.set(mapOf("userId" to uid)) } + } + + @Test + fun `계정 삭제는 등록부 사용자 공개 프로필을 한 batch에 넣는다`() = + runTest { + assertTrue(repository.deleteUserDocument(uid)) + + verify(exactly = 1) { batch.delete(codeRef) } + verify(exactly = 1) { batch.delete(userRef) } + verify(exactly = 1) { batch.delete(profileRef) } + verify(exactly = 1) { batch.commit() } + verify(exactly = 0) { codeRef.delete() } + verify(exactly = 0) { userRef.delete() } + verify(exactly = 0) { profileRef.delete() } + } + + @Test + fun `공유 코드가 없으면 사용자와 프로필만 batch로 지운다`() = + runTest { + every { snapshot.getString("shareCode") } returns null + + assertTrue(repository.deleteUserDocument(uid)) + + verify(exactly = 2) { batch.delete(any()) } + verify(exactly = 0) { registry.document(any()) } + verify(exactly = 1) { batch.commit() } + } + + @Test + fun `삭제는 batch 응답을 기다리며 실패 뒤 같은 문서들로 재시도한다`() = + runTest { + val pending = TaskCompletionSource() + every { batch.commit() } returns pending.task + val result = async { repository.deleteUserDocument(uid) } + runCurrent() + assertFalse(result.isCompleted) + pending.setException(denied()) + assertFalse(result.await()) + + every { batch.commit() } returns Tasks.forResult(null) + assertTrue(repository.deleteUserDocument(uid)) + verify(exactly = 2) { batch.delete(codeRef) } + verify(exactly = 2) { batch.delete(userRef) } + verify(exactly = 2) { batch.delete(profileRef) } + verify(exactly = 2) { batch.commit() } + } + + @Test + fun `사용자 코드 읽기가 실패하면 어떤 삭제도 시작하지 않는다`() = + runTest { + every { userRef.get() } returns Tasks.forException(denied()) + + assertFalse(repository.deleteUserDocument(uid)) + + verify(exactly = 0) { firestore.batch() } + verify(exactly = 0) { batch.commit() } + } + + private fun denied() = FirebaseFirestoreException("denied", FirebaseFirestoreException.Code.PERMISSION_DENIED) +} diff --git a/firestore-tests/rules.test.mjs b/firestore-tests/rules.test.mjs index 3dfed65..0bcba63 100644 --- a/firestore-tests/rules.test.mjs +++ b/firestore-tests/rules.test.mjs @@ -25,6 +25,7 @@ import { setDoc, updateDoc, where, + writeBatch, } from "firebase/firestore"; const OWNER = "uid-owner"; @@ -180,12 +181,13 @@ describe("publicProfiles", () => { }); it("계정을 삭제할 때 본인 공개 프로필도 지운다", async () => { - // UserRepository.deleteUserDocument 와 같은 순서다. 사용자 문서가 없어도 본인 확인은 - // 인증 uid 로 해야 하며, 삭제에는 request.resource 의 새 필드가 없다. + // UserRepository.deleteUserDocument 와 같은 원자 batch다. const db = owner(); - await assertSucceeds(deleteDoc(doc(db, "shareCodes", SHARE_CODE))); - await assertSucceeds(deleteDoc(doc(db, "users", OWNER))); - await assertSucceeds(deleteDoc(doc(db, "publicProfiles", OWNER))); + const batch = writeBatch(db); + batch.delete(doc(db, "shareCodes", SHARE_CODE)); + batch.delete(doc(db, "users", OWNER)); + batch.delete(doc(db, "publicProfiles", OWNER)); + await assertSucceeds(batch.commit()); assert.equal((await getDoc(doc(db, "publicProfiles", OWNER))).exists(), false); await assertSucceeds(deleteDoc(doc(db, "publicProfiles", OWNER))); }); @@ -246,8 +248,131 @@ describe("shareCodes", () => { }); it("본인 코드만 반납한다", async () => { - await assertSucceeds(deleteDoc(doc(owner(), "shareCodes", SHARE_CODE))); await assertFails(deleteDoc(doc(other(), "shareCodes", SHARE_CODE))); + await assertSucceeds(deleteDoc(doc(owner(), "shareCodes", SHARE_CODE))); + }); +}); + +describe("공유 등록부 복구와 삭제 재시도", () => { + it("기존 소유자는 동일한 값으로만 등록부를 확인한다", async () => { + const ref = doc(owner(), "shareCodes", SHARE_CODE); + await assertSucceeds(setDoc(ref, { userId: OWNER })); + await assertFails(setDoc(ref, { userId: OTHER })); + await assertFails(setDoc(ref, { userId: OWNER, extra: true })); + await assertFails(getDoc(ref)); + await assertFails(getDocs(collection(owner(), "shareCodes"))); + }); + + it("등록부가 없는 이전 계정은 본인 코드를 복구한 뒤 공유 일정을 쓴다", async () => { + await testEnv.withSecurityRulesDisabled(async (context) => { + await deleteDoc(doc(context.firestore(), "shareCodes", SHARE_CODE)); + }); + const db = owner(); + await assertFails(updateDoc(doc(db, "schedules", "shared-1"), { title: "수정" })); + await assertSucceeds(setDoc(doc(db, "shareCodes", SHARE_CODE), { userId: OWNER })); + await assertSucceeds(updateDoc(doc(db, "schedules", "shared-1"), { title: "수정" })); + }); + + it("없는 등록부 삭제는 로그인한 재시도에만 허용한다", async () => { + await assertSucceeds(deleteDoc(doc(owner(), "shareCodes", "MISSING"))); + await assertSucceeds(deleteDoc(doc(owner(), "shareCodes", "MISSING"))); + await assertFails(deleteDoc(doc(anonymous(), "shareCodes", "MISSING"))); + }); + + it("등록부가 먼저 삭제된 계정도 나머지 문서를 한 번에 지우고 재시도한다", async () => { + const db = owner(); + await assertSucceeds(deleteDoc(doc(db, "shareCodes", SHARE_CODE))); + for (let attempt = 0; attempt < 2; attempt += 1) { + const batch = writeBatch(db); + batch.delete(doc(db, "shareCodes", SHARE_CODE)); + batch.delete(doc(db, "users", OWNER)); + batch.delete(doc(db, "publicProfiles", OWNER)); + await assertSucceeds(batch.commit()); + } + assert.equal((await getDoc(doc(db, "users", OWNER))).exists(), false); + assert.equal((await getDoc(doc(db, "publicProfiles", OWNER))).exists(), false); + }); + + it("등록부 삭제가 거절되면 사용자와 공개 프로필도 보존한다", async () => { + const db = owner(); + const batch = writeBatch(db); + batch.delete(doc(db, "shareCodes", "ZZZ999")); + batch.delete(doc(db, "users", OWNER)); + batch.delete(doc(db, "publicProfiles", OWNER)); + await assertFails(batch.commit()); + assert.equal((await getDoc(doc(db, "users", OWNER))).exists(), true); + assert.equal((await getDoc(doc(db, "publicProfiles", OWNER))).exists(), true); + await testEnv.withSecurityRulesDisabled(async (context) => { + assert.equal((await getDoc(doc(context.firestore(), "shareCodes", "ZZZ999"))).data().userId, OTHER); + }); + }); +}); + +describe("공유 일정의 코드 소유권", () => { + for (const sharedCode of ["", undefined]) { + it(`비공유 일정(${String(sharedCode)})은 등록부 없이 저장한다`, async () => { + const data = { userId: STRANGER, title: "비공유" }; + if (sharedCode !== undefined) data.sharedCode = sharedCode; + const ref = doc(stranger(), "schedules", "private-new"); + await assertSucceeds(setDoc(ref, data)); + await assertSucceeds(updateDoc(ref, { title: "수정" })); + }); + } + + it("본인 등록부의 코드로 생성하고 비공유 일정을 채운다", async () => { + const db = owner(); + await assertSucceeds(setDoc(doc(db, "schedules", "shared-new"), { + userId: OWNER, title: "공유", sharedCode: SHARE_CODE, + })); + await assertSucceeds(updateDoc(doc(db, "schedules", "own-1"), { sharedCode: SHARE_CODE })); + await assertSucceeds(updateDoc(doc(db, "schedules", "shared-1"), { title: "수정" })); + }); + + for (const sharedCode of ["ZZZ999", "MISSING", null, 123]) { + it(`소유권 없는 코드(${String(sharedCode)})로 생성하거나 수정하지 못한다`, async () => { + const db = owner(); + await assertFails(setDoc(doc(db, "schedules", "invalid-new"), { + userId: OWNER, title: "공유", sharedCode, + })); + await assertFails(updateDoc(doc(db, "schedules", "own-1"), { sharedCode })); + }); + } + + it("같은 코드의 일정 batch는 등록부 조회 한도 안에서 채운다", async () => { + await testEnv.withSecurityRulesDisabled(async (context) => { + const batch = writeBatch(context.firestore()); + for (let i = 0; i < 30; i += 1) { + batch.set(doc(context.firestore(), "schedules", `backfill-${i}`), { + userId: OWNER, title: "비공유 일정", sharedCode: "", + }); + } + await batch.commit(); + }); + const db = owner(); + const batch = writeBatch(db); + for (let i = 0; i < 30; i += 1) { + batch.update(doc(db, "schedules", `backfill-${i}`), { sharedCode: SHARE_CODE }); + } + await assertSucceeds(batch.commit()); + assert.equal((await getDoc(doc(db, "schedules", "backfill-29"))).data().sharedCode, SHARE_CODE); + }); + + it("사용자 문서의 코드만 바꿔서는 일정 공유 소유권을 얻지 못한다", async () => { + const db = owner(); + await assertSucceeds(updateDoc(doc(db, "users", OWNER), { shareCode: "ZZZ999" })); + await assertFails(setDoc(doc(db, "schedules", "invalid-new"), { + userId: OWNER, title: "공유", sharedCode: "ZZZ999", + })); + await assertFails(updateDoc(doc(db, "schedules", "own-1"), { sharedCode: "ZZZ999" })); + }); + + it("등록부가 없어도 기존 감시자는 완료만 수정하고 소유자는 삭제할 수 있다", async () => { + await testEnv.withSecurityRulesDisabled(async (context) => { + await deleteDoc(doc(context.firestore(), "shareCodes", SHARE_CODE)); + }); + await assertSucceeds(updateDoc(doc(other(), "schedules", "shared-1"), { completed: true })); + await assertFails(updateDoc(doc(other(), "schedules", "shared-1"), { title: "수정" })); + await assertSucceeds(deleteDoc(doc(owner(), "schedules", "shared-1"))); }); }); diff --git a/firestore.rules b/firestore.rules index 7ed1cdc..41105c8 100644 --- a/firestore.rules +++ b/firestore.rules @@ -19,6 +19,14 @@ service cloud.firestore { ); } + // 비공유 일정은 그대로 허용하고, 공유할 때는 등록부의 소유자를 확인한다. + function ownsRequestedShareCode() { + return !('sharedCode' in request.resource.data) + || request.resource.data.sharedCode == '' + || (request.resource.data.sharedCode is string + && get(/databases/$(database)/documents/shareCodes/$(request.resource.data.sharedCode)).data.userId == request.auth.uid); + } + // 사용자 문서. 이메일과 FCM 토큰이 함께 있어 본인만 읽고 쓴다. // 다른 사람에게 보여야 하는 이름은 publicProfiles 에 따로 둔다(#93). match /users/{userId} { @@ -51,8 +59,14 @@ service cloud.firestore { allow create: if isSignedIn() && request.resource.data.keys().hasOnly(['userId']) && request.resource.data.userId == request.auth.uid; - allow update: if false; - allow delete: if isSignedIn() && resource.data.userId == request.auth.uid; + // 기존 코드도 조회 권한을 열지 않고 같은 값으로 등록·확인할 수 있다. + allow update: if isSignedIn() + && resource.data.userId == request.auth.uid + && request.resource.data == resource.data; + // 이전 부분 삭제나 재시도에서 이미 없는 등록부는 성공으로 처리한다. + allow delete: if isSignedIn() + && (!exists(/databases/$(database)/documents/shareCodes/$(shareCode)) + || resource.data.userId == request.auth.uid); } // 일정 @@ -60,9 +74,10 @@ service cloud.firestore { // 생성할 때 소유자를 확인하고, 기존 일정은 저장된 소유자를 기준으로 확인한다. // 일정을 고칠 때도 소유자는 그대로 유지한다. allow read, delete: if isSelf(resource.data.userId); - allow create: if isSelf(request.resource.data.userId); + allow create: if isSelf(request.resource.data.userId) && ownsRequestedShareCode(); allow update: if isSelf(resource.data.userId) - && request.resource.data.userId == resource.data.userId; + && request.resource.data.userId == resource.data.userId + && ownsRequestedShareCode(); // 공유 일정은 그 코드의 감시자로 등록한 사람만 읽는다. allow read: if resource.data.sharedCode is string && From d7b2fcf3d0dcc13d56f453304bffddbfed1fb51c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=BC=ED=98=81?= Date: Sun, 6 Sep 2026 17:10:57 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20=EC=B5=9C=EC=8B=A0=20=EC=9D=BC?= =?UTF-8?q?=EC=A0=95=20=EB=B6=80=EB=B6=84=20=EC=88=98=EC=A0=95=EA=B3=BC=20?= =?UTF-8?q?=EB=93=B1=EB=A1=9D=EB=B6=80=20=EA=B2=80=EC=A6=9D=EC=9D=84=20?= =?UTF-8?q?=ED=86=B5=ED=95=A9=ED=95=9C=EB=8B=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #198 공유 코드 사전 확인 뒤 기존 부분 update를 유지한다. 서버 완료 상태와 공유 코드·소유자 보존 회귀를 함께 검증했다. 검증: main 통합 data/profile/addschedule/app 단위, ktlint, app assembleDebug/lintDebug 통과. Source: https://firebase.google.com/docs/firestore/security/rules-conditions --- .../slowclock/data/remote/repository/ScheduleRepository.kt | 2 +- .../remote/repository/ScheduleRepositoryShareCodeTest.kt | 2 +- .../data/remote/repository/ScheduleRepositoryUpdateTest.kt | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt index 46f7ee7..1ac6ddb 100644 --- a/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt +++ b/core/data/src/main/java/com/example/slowclock/data/remote/repository/ScheduleRepository.kt @@ -323,7 +323,7 @@ class ScheduleRepository ) return try { - firestore.ensureShareCodeOwner(uid, updatedSchedule.sharedCode) + firestore.ensureShareCodeOwner(uid, schedule.sharedCode) schedulesCollection .document(schedule.id) .update(changes) diff --git a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt index ee3d753..2728dba 100644 --- a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt +++ b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryShareCodeTest.kt @@ -145,7 +145,7 @@ class ScheduleRepositoryShareCodeTest { assertTrue(repository.updateSchedule(schedule.copy(sharedCode = "")) is ScheduleRepository.ScheduleResult.Success) verify(exactly = 0) { firestore.collection("shareCodes") } - verify(atLeast = 1) { scheduleRef.set(match { it.sharedCode == "" }) } + verify(exactly = 1) { scheduleRef.set(match { it.sharedCode == "" }) } } @Test diff --git a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryUpdateTest.kt b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryUpdateTest.kt index 663d095..2d23817 100644 --- a/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryUpdateTest.kt +++ b/core/data/src/test/java/com/example/slowclock/data/remote/repository/ScheduleRepositoryUpdateTest.kt @@ -38,6 +38,11 @@ class ScheduleRepositoryUpdateTest { val firestore = mockk() every { firestore.collection("schedules") } returns schedules every { firestore.collection("users") } returns mockk() + val registry = mockk() + val codeRef = mockk() + every { firestore.collection("shareCodes") } returns registry + every { registry.document("OLD123") } returns codeRef + every { codeRef.set(mapOf("userId" to "owner")) } returns Tasks.forResult(null) return ScheduleRepository(auth, firestore) }