Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion DevLog/App/Delegate/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import Combine
import UIKit
import Firebase
import FirebaseAuth
import FirebaseFirestore
import FirebaseMessaging
import GoogleSignIn
import UserNotifications
Expand Down Expand Up @@ -39,7 +41,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, MessagingDelegate {
}
}
}


// 앱이 온그라운드로 되었을 때, 로그인 세션이 존재한다면 현재 유저의 timeZone 저장
updateUserTimeZone()

// Firebase Messaging 설정
Messaging.messaging().delegate = self

Expand Down Expand Up @@ -76,6 +81,19 @@ class AppDelegate: UIResponder, UIApplicationDelegate, MessagingDelegate {
NotificationCenter.default.post(name: .fcmToken, object: nil, userInfo: ["fcmToken": fcmToken])
}
}

private func updateUserTimeZone() {
Task {
do {
guard let uid = Auth.auth().currentUser?.uid else { return }
let settingsRef = Firestore.firestore().document("users/\(uid)/userData/settings")

try await settingsRef.setData(["timeZone": TimeZone.autoupdatingCurrent.identifier], merge: true)
} catch {
print("Failed to update timeZone: \(error)")
}
Comment on lines +92 to +94
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

에러 발생 시 print 문을 사용하는 것은 프로덕션 환경에서 적절한 로깅 방식이 아닙니다. 에러를 추적하고 디버깅하기 위해 전용 로깅 시스템(예: Crashlytics, Firebase Analytics의 커스텀 이벤트 등)을 사용하는 것이 좋습니다.

Suggested change
} catch {
print("Failed to update timeZone: \(error)")
}
print("Failed to update timeZone: \(error)") // TODO: 프로덕션 환경에 맞는 로깅 시스템으로 교체

}
}
}

extension AppDelegate: UNUserNotificationCenterDelegate {
Expand Down
48 changes: 35 additions & 13 deletions DevLog/Infra/Service/UserService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,17 @@ final class UserService {
logger.error("User not authenticated")
throw AuthError.notAuthenticated
}

let userRef = store.document("users/\(user.uid)")
let infoRef = store.document("users/\(user.uid)/userData/info")
let tokensRef = store.document("users/\(user.uid)/userData/tokens")
let settingsRef = store.document("users/\(user.uid)/userData/settings")

// 사용자 기본 정보
var userField: [String: Any] = [
"statusMsg": "",
"lastLogin": FieldValue.serverTimestamp()
"currentProvider": response.providerID
]
Comment on lines 32 to 34
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

이전 코드에서는 userField 초기화 시 lastLogin 필드를 FieldValue.serverTimestamp()로 설정하여 사용자의 마지막 로그인 시간을 기록했습니다. 현재 변경 사항에서는 이 필드가 완전히 제거되어 더 이상 업데이트되지 않습니다. lastLogin 필드가 중요한 사용자 추적 데이터였다면, 이 변경은 데이터 추적의 회귀로 이어질 수 있습니다. updatedAt 필드와는 다른 목적으로 사용될 수 있으므로, lastLogin 필드의 업데이트를 다시 추가하는 것을 고려해 주세요.

Suggested change
var userField: [String: Any] = [
"statusMsg": "",
"lastLogin": FieldValue.serverTimestamp()
"currentProvider": response.providerID
]
var userField: [String: Any] = [
"currentProvider": response.providerID,
"lastLogin": FieldValue.serverTimestamp()
]


userField["currentProvider"] = response.providerID

// 공급자 이슈로 인한 nil 방지
if let email = user.email {
userField["email"] = email
Expand All @@ -48,22 +47,45 @@ final class UserService {
user.displayName != nil && user.displayName != "" {
userField["appleName"] = user.displayName
}

try await infoRef.setData(userField, merge: true)

let userDocument = try await userRef.getDocument()
if !userDocument.exists {
userField["statusMsg"] = ""
}

var settingField = ["fcmToken": response.fcmToken]

// 깃헙 로그인 시 추가 정보 저장
if response.providerID == "github.com", let accessToken = response.accessToken {
settingField["githubAccessToken"] = accessToken
}

try await tokensRef.setData(settingField, merge: true)

try await settingsRef.setData([
"allowPushNotification": true,
"pushNotificationHour": 9,
"pushNotificationMinute": 0], merge: true)
// Reference to capture ~ in concurrently-executing code; Swift 6 lang mode의 경고 해결
let userFieldSnapshot = userField
let settingFieldSnapshot = settingField
// -----------------------------------------------------

async let userUpdate: Void = userRef.setData(
["updatedAt": FieldValue.serverTimestamp()],
merge: true
)
async let infoUpdate: Void = infoRef.setData(userFieldSnapshot, merge: true)
async let tokensUpdate: Void = tokensRef.setData(settingFieldSnapshot, merge: true)

let settingsDocument = try await settingsRef.getDocument()
var settingsField: [String: Any] = [
"timeZone": TimeZone.autoupdatingCurrent.identifier
]
if !settingsDocument.exists {
settingsField["allowPushNotification"] = true
settingsField["pushNotificationHour"] = 9
settingsField["pushNotificationMinute"] = 0
}
Comment on lines +75 to +83
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

settingsTimeZoneUpdatesettingsReftimeZone을 병합(merge)하여 설정하는 async let으로 실행됩니다. 이후 settingsDocument.exists를 확인하여 문서가 존재하지 않을 경우 allowPushNotification, pushNotificationHour, pushNotificationMinute를 설정합니다. 이 과정에서 settingsTimeZoneUpdate가 먼저 문서를 생성하고, 그 사이에 settingsRef.getDocument()가 호출되면 settingsDocument.existstrue가 되어 기본 푸시 알림 설정이 누락될 수 있는 경쟁 조건(race condition)이 발생합니다. 신규 사용자의 경우 푸시 알림 기본 설정이 적용되지 않을 수 있는 심각한 버그로 이어질 수 있습니다. settingsRef에 대한 초기 쓰기 작업을 하나의 setData 호출로 통합하여 모든 기본 설정이 함께 적용되도록 하는 것이 안전합니다.

Suggested change
let settingsDocument = try await settingsRef.getDocument()
if !settingsDocument.exists {
try await settingsRef.setData([
"allowPushNotification": true,
"pushNotificationHour": 9,
"pushNotificationMinute": 0
], merge: true)
}
let settingsDocument = try await settingsRef.getDocument()
if !settingsDocument.exists {
try await settingsRef.setData([
"allowPushNotification": true,
"pushNotificationHour": 9,
"pushNotificationMinute": 0,
"timeZone": TimeZone.autoupdatingCurrent.identifier
], merge: true)
} else {
// 문서가 이미 존재하면 timeZone만 업데이트
try await settingsRef.setData(["timeZone": TimeZone.autoupdatingCurrent.identifier], merge: true)
}


let settingsFieldSnapshot = settingsField
async let settingsUpdate: Void = settingsRef.setData(settingsFieldSnapshot, merge: true)

_ = try await (userUpdate, infoUpdate, tokensUpdate, settingsUpdate)

logger.info("Successfully upserted user: \(user.uid)")
}
Expand Down
152 changes: 135 additions & 17 deletions Firebase/functions/src/fcm/notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,162 @@ import { onTaskDispatched } from "firebase-functions/v2/tasks";
import * as admin from "firebase-admin";
import * as logger from "firebase-functions/logger";

type TaskPayload = {
userId: string;
todoId: string;
todoKind: string;
dueDateKey: string;
title: string;
body: string;
};

type FirestoreErrorLike = {
code?: unknown;
};

// Cloud Tasks에 의해 트리거되는 함수
export const sendPushNotification = onTaskDispatched({
export const sendPushNotification = onTaskDispatched({
region: "asia-northeast3",
retryConfig: { maxAttempts: 1, minBackoffSeconds: 5 },
rateLimits: { maxDispatchesPerSecond: 500 },
retryConfig: { maxAttempts: 3, minBackoffSeconds: 5 },
rateLimits: { maxDispatchesPerSecond: 200 },
},
async (req) => {
const { userId, title, body } = req.data; // 예약 시 보냈던 데이터
logger.info(`[${userId}]에게 알림 발송 작업을 시작합니다: ${title}`);
const taskId = req.data?.taskId;
if (!isValidTaskId(taskId)) {
logger.warn("유효하지 않은 푸시 알림 payload", req.data);
return;
}

const taskDocRef = admin.firestore().collection("notificationTasks").doc(taskId);
try {
const taskDoc = await taskDocRef.get();
if (!taskDoc.exists) {
logger.warn("notificationTask 문서를 찾을 수 없습니다.", { taskId });
return;
}

const parsed = parseTaskPayload(taskDoc.data());
if (!parsed) {
logger.warn("notificationTask 문서 형식이 올바르지 않습니다.", { taskId });
return;
}
const { userId, todoId, todoKind, dueDateKey, title, body } = parsed;

const settingsDoc = await admin.firestore().doc(`users/${userId}/userData/settings`).get();
const allowPushNotification = settingsDoc.data()?.allowPushNotification ?? true;
if (!allowPushNotification) {
return;
}

const notificationDocId = `${todoId}_${dueDateKey}`;
const notificationDocRef = admin.firestore().doc(`users/${userId}/notifications/${notificationDocId}`);
Comment on lines +46 to +53
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The sendPushNotification function uses userId and todoId from the task payload to construct Firestore document paths without sanitization. If an attacker can trigger this function with arbitrary data (e.g., via a misconfigured Cloud Task or a leaked service account key), they could potentially read from or write to arbitrary Firestore collections by using path traversal characters (like /) in the userId or todoId fields. This is a classic path injection vulnerability in the context of Firestore document paths.


const notificationData = {
title: "Todo 알림",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

이전 코드에서는 notificationDatatitle 필드가 req.data.title에서 동적으로 가져왔습니다. 하지만 현재 변경 사항에서는 "Todo 알림"으로 하드코딩되어 있습니다. 만약 알림 제목이 항상 "Todo 알림"으로 고정되어야 한다면 괜찮지만, 다양한 종류의 알림에 따라 제목을 다르게 설정할 필요가 있다면 req.data.title을 다시 사용하는 것이 좋습니다.

Suggested change
title: "Todo 알림",
title: title, // req.data.title 사용

body,
receivedAt: admin.firestore.FieldValue.serverTimestamp(),
isRead: false,
todoID: todoId,
todoKind: todoKind
};
try {
await notificationDocRef.create(notificationData);
} catch (error) {
if (isAlreadyExistsError(error)) {
return;
}
throw error;
}

// 1. 사용자 FCM 토큰 가져오기
const tokenDoc = await admin.firestore().doc(`users/${userId}/userData/tokens`).get();
const fcmToken = tokenDoc.data()?.fcmToken;

if (!fcmToken) {
logger.warn(`사용자 ${userId}의 fcmToken이 없어 알림을 보낼 수 없습니다.`);
logger.warn(`사용자 ${userId}의 fcmToken이 없어 푸시 발송은 건너뜁니다. Firestore에는 기록했습니다.`);
return;
}

// 2. 알림 발송 및 Firestore에 기록
// 2. 푸시 알림 발송
const message = {
notification: { title, body },
data: {
todoID: todoId,
todoId: todoId,
todoKind: todoKind
},
apns: { payload: { aps: { sound: "default" } } },
token: fcmToken,
};
await admin.messaging().send(message);

const notificationData = {
title, body, sentAt: admin.firestore.FieldValue.serverTimestamp(), isRead: false, type: 'reminder'
};
await admin.firestore().collection(`users/${userId}/notifications`).add(notificationData);

logger.info(`[${userId}]에게 알림을 성공적으로 보내고 저장했습니다.`);
try {
await admin.messaging().send(message);
} catch (sendError) {
logger.warn(`[${userId}] 푸시 발송 실패. Firestore 기록은 유지됩니다.`, sendError);
return;
}

} catch (error) {
logger.error(`[${userId}]에게 알림 발송 중 오류 발생:`, error);
throw error; // 오류를 다시 던져 Cloud Tasks가 재시도하도록 함
logger.error("알림 발송 중 오류 발생", {
taskId,
error
});
} finally {
try {
await taskDocRef.delete();
} catch (cleanupError) {
logger.warn("notificationTask 정리 실패", {
taskId,
cleanupError
});
}
}
}
);

function isValidTaskId(value: unknown): value is string {
return typeof value === "string" && /^[A-Za-z0-9_-]{1,128}$/.test(value);
}

function hasPathSeparator(value: string): boolean {
return value.includes("/");
}

function parseTaskPayload(data: FirebaseFirestore.DocumentData | undefined): TaskPayload | null {
const {
userId,
todoId,
todoKind,
dueDateKey,
title,
body
} = data ?? {};

if (
typeof userId !== "string" ||
typeof todoId !== "string" ||
typeof todoKind !== "string" ||
typeof dueDateKey !== "string" ||
typeof title !== "string" ||
typeof body !== "string"
) {
return null;
}

if (hasPathSeparator(userId) || hasPathSeparator(todoId)) {
return null;
}

return {
userId,
todoId,
todoKind,
dueDateKey,
title,
body
};
}

function isAlreadyExistsError(error: unknown): boolean {
const code = (error as FirestoreErrorLike)?.code;
return code === 6 || code === "6" || code === "already-exists";
}
Loading