chore: add firebase appcheck dep #1269
Conversation
WalkthroughThe pull request introduces several significant changes, including the addition of a new Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (48)
android/app/src/main/java/com/converse/xmtp/Client.kt (1)
43-45:
⚠️ Potential issueAvoid throwing
Error; use an appropriateExceptioninsteadThrowing
Erroris discouraged as it represents unrecoverable JVM errors. Instead, use anExceptionlikeIllegalStateExceptionto indicate an issue with the application's state whendbEncryptionKeyis missing.Apply this diff to fix the issue:
if (dbEncryptionKey == null) { - throw Error("Missing dbEncryptionKey") + throw IllegalStateException("Missing dbEncryptionKey") }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (dbEncryptionKey == null) { throw IllegalStateException("Missing dbEncryptionKey") }components/Chat/ConsentPopup/ConsentPopup.tsx (2)
66-71: 🛠️ Refactor suggestion
Handle potential errors in the blocking action
Consider handling any potential errors when calling
consentToInboxIdsOnProtocolByAccount. If this function is asynchronous and returns a Promise, you shouldawaitit and include error handling to improve reliability.
84-89: 🛠️ Refactor suggestion
Handle potential errors in the acceptance action
Similarly, in the accept action, ensure that you're handling any potential errors from
consentToInboxIdsOnProtocolByAccount. If it's an asynchronous function, considerawaiting it and adding error handling to maintain consistency and robustness.android/app/src/main/java/com/converse/xmtp/Conversations.kt (2)
19-25:
⚠️ Potential issueHandle nullable parameters correctly to prevent potential issues
The
paramsmap is defined with nullable keys and values (MutableMap<String?, String?>), which can lead to problems when converting it to aJSONObject, especially if any key or value isnull. It's better to use non-nullable types and includehmacKeysonly when it's notnull.Here's how you can update the code:
val params: MutableMap<String, String> = HashMap() params["topic"] = topic params["nativeToken"] = pushToken params["account"] = account if (hmacKeys != null) { params["hmacKeys"] = hmacKeys } val parameters = JSONObject(params)This ensures all keys and values in the map are non-null, preventing potential
NullPointerExceptions when creating theJSONObject.
30-32: 🛠️ Refactor suggestion
⚠️ Potential issueReplace
printStackTrace()with proper logging and implement error handlingUsing
error.printStackTrace()is discouraged in Android applications as it doesn't provide control over log output and can expose sensitive information. Instead, useLog.eto log the error. Additionally, the//TODO: handle failurecomment indicates incomplete error handling, which should be addressed.Update the code to log the error and handle the failure appropriately:
Log.e("PushNotificationsService", "Subscribe to new topic error", error) // Implement necessary error handling hereConsider providing user feedback or retry mechanisms to handle the failure effectively.
components/Chat/Attachment/AddAttachmentButton.tsx (1)
124-124:
⚠️ Potential issueAwait async calls to
handleAttachmentSelected()
handleAttachmentSelectedis an async function, but it's being called without usingawait. This might lead to unhandled promise rejections or unexpected behavior. Consider addingawaitto these calls to ensure proper sequencing and error handling.Apply this change to await the calls:
For
pickMedia():if (!asset) return; - handleAttachmentSelected(asset); + await handleAttachmentSelected(asset);For
openCamera():if (!asset) return; - handleAttachmentSelected(asset); + await handleAttachmentSelected(asset);Also applies to: 130-130
App.tsx (2)
89-95:
⚠️ Potential issueUpdate
useEffectdependencies to trackcurrentAccountchangesThe
useEffecthook sets the authentication status based oncurrentAccountbut has an empty dependency array, meaning it only runs once on mount. IfcurrentAccountchanges during the app's lifecycle, the auth status won't update accordingly. To fix this, you should:
- Use the
useAccountsStorehook to subscribe tocurrentAccountchanges.- Add
currentAccountto the dependency array of theuseEffecthook.Here's the adjusted code:
- useEffect(() => { - const currentAccount = useAccountsStore.getState().currentAccount; - if (currentAccount && currentAccount !== TEMPORARY_ACCOUNT_NAME) { - setAuthStatus("signedIn"); - } else { - setAuthStatus("signedOut"); - } - }, []); + const currentAccount = useAccountsStore((state) => state.currentAccount); + useEffect(() => { + if (currentAccount && currentAccount !== TEMPORARY_ACCOUNT_NAME) { + setAuthStatus("signedIn"); + } else { + setAuthStatus("signedOut"); + } + }, [currentAccount]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const currentAccount = useAccountsStore((state) => state.currentAccount); useEffect(() => { if (currentAccount && currentAccount !== TEMPORARY_ACCOUNT_NAME) { setAuthStatus("signedIn"); } else { setAuthStatus("signedOut"); } }, [currentAccount]);
61-61: 🛠️ Refactor suggestion
Ensure
xmtpEngine.start()is called only onceCalling
xmtpEngine.start()at the top level might lead to multiple initializations, especially during development with hot reloading. To prevent potential issues, consider moving this call inside auseEffecthook with an empty dependency array to ensure it's executed only once.Apply this change:
- xmtpEngine.start(); + useEffect(() => { + xmtpEngine.start(); + }, []);Committable suggestion skipped: line range outside the PR's diff.
components/Chat/ChatGroupUpdatedMessage.tsx (1)
14-14:
⚠️ Potential issueFix typo in 'GroupUpdatedMetadatEntry' import
GroupUpdatedMetadatEntryis misspelled in the import statement; it should beGroupUpdatedMetadataEntry.Apply this diff to correct the typo:
import { GroupUpdatedContent, - GroupUpdatedMetadatEntry, + GroupUpdatedMetadataEntry, InboxId, } from "@xmtp/react-native-sdk";Committable suggestion skipped: line range outside the PR's diff.
components/Chat/ChatDumb.tsx (2)
153-154:
⚠️ Potential issueAttach 'messageListRef' to 'ReanimatedFlashList' to enable scrolling functionality.
The
messageListRefis defined and used inscrollToMessage, but it's not attached to theReanimatedFlashList. Without attaching the ref, scrolling methods likescrollToIndexwon't work.Apply this diff to attach the
messageListRef:<ReanimatedFlashList + ref={messageListRef} contentContainerStyle={styles.chat}Committable suggestion skipped: line range outside the PR's diff.
186-186:
⚠️ Potential issueAvoid passing 'key' prop to a View component; 'key' should not be used as a prop.
The
keyprop is a special attribute used by React to identify elements in a list and should not be passed to components or HTML elements. It's not accessible viaprops.keyinside the component.Apply this diff to remove the
keyprop:- <View style={styles.chatContainer} key={key}> + <View style={styles.chatContainer}>If you need to pass a unique identifier, consider using a different prop name like
viewId.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<View style={styles.chatContainer}>android/app/src/main/java/com/converse/Spam.kt (4)
94-96: 🛠️ Refactor suggestion
Log exceptions to prevent silent failures
The catch block is empty, which means any exceptions thrown within the
tryblock are silently ignored. Swallowing exceptions without logging can make debugging difficult and hide potential issues. Consider logging the exception to aid in troubleshooting.Add logging to the catch block:
} catch (e: Exception) { e.printStackTrace() }Alternatively, use a logging framework like Timber for better logging practices.
158-160: 🛠️ Refactor suggestion
Log exceptions to prevent silent failures
In the catch block, the exception is caught and a default value is returned, but the exception is not logged. This can make debugging challenging if errors occur. Consider logging the exception to provide more visibility into potential issues.
Add logging to the catch block:
} catch (e: Exception) { e.printStackTrace() return 0.0 }Alternatively, use a logging framework to log the exception more effectively.
🧰 Tools
🪛 detekt (1.23.7)
[warning] 158-158: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
148-149:
⚠️ Potential issueEnsure
member.addressesis not empty before accessingfirst()Using
first()on a list without checking if it's not empty can lead to aNoSuchElementExceptionif the list is empty. To prevent potential crashes, ensure thatmember.addressesis not empty before callingfirst().Modify the code to check for emptiness:
val firstAddress = member.addresses.firstOrNull() if (firstAddress != null) { val senderSpamScore = getSenderSpamScore( appContext = appContext, address = Keys.toChecksumAddress(firstAddress), apiURI = apiURI ) return senderSpamScore }
57-59: 🛠️ Refactor suggestion
Avoid using
runBlockingin non-suspending functionsUsing
runBlockingin a non-suspending function can block the calling thread, which may lead to performance issues, especially on the main thread. Consider makingcomputeDmSpamScorea suspending function and invokinggetSenderSpamScoredirectly withoutrunBlocking.Apply this diff to make the function suspend:
-fun computeDmSpamScore(address: String, message: String?, contentType: String, apiURI: String?, appContext: Context): Double { - var senderScore = runBlocking { +suspend fun computeDmSpamScore(address: String, message: String?, contentType: String, apiURI: String?, appContext: Context): Double { + val senderScore = getSenderSpamScore(appContext, address, apiURI) - } return senderScore + getMessageSpamScore(message, contentType) }Committable suggestion skipped: line range outside the PR's diff.
components/Chat/Message/MessageContextMenu.tsx (1)
63-70: 🛠️ Refactor suggestion
Remove unnecessary dependencies from the
useEffecthookIncluding
SharedValues (activeValue,opacityValue,intensityValue) in the dependency array of theuseEffecthook is unnecessary and may lead to unexpected behaviors. SinceSharedValues are mutable and their references do not change, they don't need to be listed as dependencies. You only need to includeisActive, which is the prop that triggers the effect.Proposed change:
useEffect(() => { activeValue.value = isActive; opacityValue.value = withTiming(isActive ? 1 : 0, { duration: animation.contextMenuHoldDuration, }); intensityValue.value = withTiming(isActive ? 50 : 0, { duration: animation.contextMenuHoldDuration, }); - }, [activeValue, isActive, opacityValue, intensityValue]); + }, [isActive]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { activeValue.value = isActive; opacityValue.value = withTiming(isActive ? 1 : 0, { duration: animation.contextMenuHoldDuration, }); intensityValue.value = withTiming(isActive ? 50 : 0, { duration: animation.contextMenuHoldDuration, }); }, [isActive]);components/Chat/Attachment/AttachmentMessagePreview.tsx (2)
310-310:
⚠️ Potential issueEnsure
getCurrentAccount()returns a valid accountAt line 310, you're using a non-null assertion
!ongetCurrentAccount(). This assumes that a current account is always available. To prevent potential runtime errors, ensure thatgetCurrentAccount()cannot returnnullorundefined, or include error handling to manage cases where no current account exists.
306-308: 🛠️ Refactor suggestion
Validate
content.contentLengthbefore parsingWhen using
parseFloatoncontent.contentLength, there's a risk it might returnNaNif the content is not a valid number. Before comparing the parsed value toMAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE, ensure that it is a valid number to prevent unintended behavior.Apply this diff to add validation:
if ( content.contentLength && + !isNaN(parseFloat(content.contentLength)) && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.content.contentLength && !isNaN(parseFloat(content.contentLength)) && parseFloat(content.contentLength) <= MAX_AUTOMATIC_DOWNLOAD_ATTACHMENT_SIZE ) {android/app/src/main/java/com/converse/xmtp/Messages.kt (7)
62-62:
⚠️ Potential issueAdd error handling for
conversation.sync()The call to
conversation.sync()may throw exceptions if the synchronization fails. Currently, there is no error handling in place, which could lead to unhandled exceptions and application crashes.Wrap the
conversation.sync()call in atry-catchblock to handle potential exceptions:try { conversation.sync() } catch (e: Exception) { Log.e(TAG, "Error syncing conversation: ${e.localizedMessage}") return NotificationDataResult() }
69-69:
⚠️ Potential issueHandle exceptions during message processing
The method
conversation.processMessage(envelope.message.toByteArray()).decode()can throw exceptions if message processing fails. Without proper error handling, this could cause the application to crash.Consider wrapping the message processing in a
try-catchblock:val decodedMessage = try { conversation.processMessage(envelope.message.toByteArray()).decode() } catch (e: Exception) { Log.e(TAG, "Error processing message: ${e.localizedMessage}") return NotificationDataResult() }
70-79: 🛠️ Refactor suggestion
Ensure exhaustive handling of conversation types
The
whenexpression handlesConversation.GroupandConversation.Dmtypes but lacks anelseclause. If a new type ofConversationis introduced, this could lead to unexpected behavior or runtime exceptions.Add an
elsebranch to handle any unforeseen conversation types:when (conversation) { is Conversation.Group -> { // Handle the Group case return handleGroupMessage(appContext, conversation, decodedMessage, xmtpClient, remoteMessage) } is Conversation.Dm -> { // Handle the Dm case return handleDmMessage(appContext, conversation, decodedMessage, xmtpClient, remoteMessage) } else -> { Log.e(TAG, "Unhandled conversation type: ${conversation::class.simpleName}") return NotificationDataResult() } }
90-94:
⚠️ Potential issueSafely handle potential null values in DM message processing
Accessing
addresses.get(0)without checking ifaddressesis null or empty can lead to aNullPointerException.Modify the code to safely access the first address:
dm.members() .firstOrNull { it.inboxId == decodedMessage.senderAddress } ?.addresses ?.firstOrNull() ?.let { senderAddress -> decodedMessage.senderAddress = senderAddress }
143-146:
⚠️ Potential issueSafely handle potential null values in group message processing
Similar to the DM message handling, ensure that accessing
addresses.get(0)is safe to prevent possibleNullPointerException.Update the code to:
group.members() .firstOrNull { it.inboxId == decodedMessage.senderAddress } ?.addresses ?.firstOrNull() ?.let { senderAddress -> decodedMessage.senderAddress = senderAddress }
369-369:
⚠️ Potential issueAdd error handling for synchronous operations
The calls to
xmtpClient.syncConsent()andconversation.sync()may throw exceptions. Without error handling, exceptions could lead to crashes or unhandled errors.Wrap these calls in
try-catchblocks:try { xmtpClient.syncConsent() } catch (e: Exception) { Log.e(TAG, "Error syncing consent: ${e.localizedMessage}") // Handle exception as needed }try { conversation.sync() } catch (e: Exception) { Log.e(TAG, "Error syncing conversation: ${e.localizedMessage}") // Handle exception as needed }Also applies to: 408-408
356-359:
⚠️ Potential issueHandle exceptions in
handleV3WelcomefunctionThe
tryblock inhandleV3Welcomeis empty in thecatchclause. Swallowing exceptions without logging can make debugging difficult.Add logging inside the
catchblock to capture exceptions:} catch (e: Exception) { Log.e(TAG, "Error in handleV3Welcome: ${e.localizedMessage}") }components/Chat/Frame/urlProtocols.ts (1)
3-6: 🛠️ Refactor suggestion
Add type safety and validation
Consider adding type annotations and validation to ensure runtime safety.
-export const AUTHORIZED_URL_PROTOCOLS = new Set([ +export const AUTHORIZED_URL_PROTOCOLS: Set<string> = new Set([ + // Validate config properties exist + if (!config.scheme || !config.framesAllowedSchemes) { + throw new Error('Missing required config properties: scheme or framesAllowedSchemes'); + } + // Validate scheme format + if (!/^[a-zA-Z][a-zA-Z0-9+.-]*$/.test(config.scheme)) { + throw new Error('Invalid scheme format'); + } `${config.scheme}:`, - ...config.framesAllowedSchemes.map((s) => `${s}:`), + ...config.framesAllowedSchemes.map((s: string) => { + if (!/^[a-zA-Z][a-zA-Z0-9+.-]*$/.test(s)) { + throw new Error(`Invalid scheme format: ${s}`); + } + return `${s}:`; + }), ]); + +// Consider freezing the set to prevent modifications +Object.freeze(AUTHORIZED_URL_PROTOCOLS);Committable suggestion skipped: line range outside the PR's diff.
android/google-services.json (1)
18-18:
⚠️ Potential issueCRITICAL: Remove exposed API key from version control
The Google Cloud API key is exposed in the repository. This poses a significant security risk as it could allow unauthorized access to your Google Cloud services.
Recommendations:
- Immediately rotate this API key in the Google Cloud Console
- Move sensitive configurations to secure environment management
- Add
google-services.jsonto.gitignore- Consider using encrypted secrets management for CI/CD
Would you like assistance in setting up a secure configuration management system?
🧰 Tools
🪛 Gitleaks (8.21.2)
18-18: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
components/Chat/ActionButton.tsx (1)
15-15: 💡 Codebase verification
Breaking change detected in ActionButton usage
The verification reveals that there is at least one active usage of ActionButton that needs to be updated:
- In
screens/ShareProfile.tsx:<ActionButton picto="xmark" .../>passes a string instead of an IPicto objectThe other usages found are commented out code (in SendMoneyButton.tsx, FramePreview.tsx, and Message.tsx), so they don't require immediate attention.
🔗 Analysis chain
Verify all usage sites of ActionButton component.
The change from
stringtoIPictotype for thepictoprop is a breaking change that requires verification of all component usage sites to ensure they're updated accordingly.Let's verify all usage sites:
Let me gather more information by searching for actual usage patterns of ActionButton component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usage sites of ActionButton component to verify they pass IPicto objects # Expected results: All usage sites should pass an IPicto object, not a string # Search for ActionButton imports and usages rg -A 3 "import.*ActionButton" # Search for direct ActionButton usage in JSX/TSX files ast-grep --pattern '<ActionButton picto={$_}'Length of output: 1162
Script:
#!/bin/bash # Search for ActionButton usage with different patterns to catch various ways it might be used rg -U -A 5 '<ActionButton' # Also search for any assignments or spread of picto prop to catch indirect usage rg -U 'picto[=:]'Length of output: 6712
components/Chat/Message/MessageContext.tsx (2)
19-44: 🛠️ Refactor suggestion
Add cleanup for shared value and error handling.
While the implementation is efficient with proper memoization, consider adding:
- Cleanup for the shared value to prevent memory leaks
- Error handling for edge cases
export const MessageContextProvider = (props: IMessageContextProps) => { const { children } = props; const showTimeAV = useSharedValue(false); + React.useEffect(() => { + return () => { + // Cleanup shared value + showTimeAV.value = false; + }; + }, []); const showTime = useCallback(() => { + try { showTimeAV.value = true; + } catch (error) { + console.error('Failed to show time:', error); + } }, [showTimeAV]);Committable suggestion skipped: line range outside the PR's diff.
46-48: 🛠️ Refactor suggestion
Add runtime safety check to the custom hook.
The hook should verify that it's being used within a provider context.
export function useMessageContext(): IMessageContextType { - return useContext(MessageContext); + const context = useContext(MessageContext); + if (!context.showTimeAV) { + throw new Error( + 'useMessageContext must be used within a MessageContextProvider' + ); + } + return context; }Committable suggestion skipped: line range outside the PR's diff.
components/Chat/Message/MessageReactions/MessageReactionsDrawer/MessageReactionsDrawer.service.ts (1)
11-28: 🛠️ Refactor suggestion
Improve state management and error handling
- Accessing Zustand store with
getState()outside React components can lead to stale state. Consider passing the setter function as a parameter.- The error handling could be more specific and the error should be propagated to the caller.
Here's a suggested improvement:
-export function openMessageReactionsDrawer( - rolledUpReactions: RolledUpReactions +export function openMessageReactionsDrawer( + rolledUpReactions: RolledUpReactions, + setReactions?: (reactions: RolledUpReactions) => void ) { try { if (!bottomSheetModalRef.current) { throw new Error( "Bottom sheet modal reference is not initialized. Ensure the component is mounted." ); } - const setReactions = - useMessageReactionsStore.getState().setRolledUpReactions; - setReactions(rolledUpReactions); + const storeSetReactions = setReactions ?? + useMessageReactionsStore.getState().setRolledUpReactions; + storeSetReactions(rolledUpReactions); bottomSheetModalRef.current.present(); } catch (error) { console.error("Failed to open message reactions drawer:", error); resetMessageReactionsDrawer(); + throw error; // Propagate the error } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function openMessageReactionsDrawer( rolledUpReactions: RolledUpReactions, setReactions?: (reactions: RolledUpReactions) => void ) { try { if (!bottomSheetModalRef.current) { throw new Error( "Bottom sheet modal reference is not initialized. Ensure the component is mounted." ); } const storeSetReactions = setReactions ?? useMessageReactionsStore.getState().setRolledUpReactions; storeSetReactions(rolledUpReactions); bottomSheetModalRef.current.present(); } catch (error) { console.error("Failed to open message reactions drawer:", error); resetMessageReactionsDrawer(); throw error; // Propagate the error } }android/app/src/main/java/com/converse/Preferences.kt (3)
5-19: 🛠️ Refactor suggestion
Add KDoc documentation and improve error handling
The conversation-related functions lack documentation and error handling. Consider the following improvements:
+/** + * Checks if a conversation is blocked in the consent list + * @param conversation The conversation to check + * @param consentList The consent list to check against + * @return true if the conversation is blocked, false otherwise + * @throws IllegalStateException if the consent list operation fails + */ suspend fun isConversationBlocked(conversation: Conversation, consentList: ConsentList): Boolean { + conversation.id ?: throw IllegalArgumentException("Conversation ID cannot be null") return isConversationIdBlocked(conversation.id, consentList) }Similar documentation should be added to all functions in this group.
Committable suggestion skipped: line range outside the PR's diff.
21-27: 🛠️ Refactor suggestion
Add address validation and documentation
The address-related functions should validate the input format and include documentation.
+/** + * Checks if an address is allowed in the consent list + * @param address The Ethereum address to check + * @param consentList The consent list to check against + * @return true if the address is allowed, false otherwise + * @throws IllegalArgumentException if the address format is invalid + */ suspend fun isAddressAllowed(address: String, consentList: ConsentList): Boolean { + require(address.matches(Regex("^0x[a-fA-F0-9]{40}$"))) { "Invalid Ethereum address format" } return consentList.addressState(address) == ConsentState.ALLOWED }Committable suggestion skipped: line range outside the PR's diff.
1-35: 💡 Codebase verification
Add unit tests for consent management functions
The consent management functions in
Preferences.ktare extensively used throughout the spam prevention system (as seen inSpam.ktandMessages.kt), but lack dedicated test coverage. While there's documentation about consent-related UI components, the core consent management logic remains untested.
- Create a test file at
android/app/src/test/java/com/converse/PreferencesTest.ktto test all consent states- Add test cases for each function in
Preferences.kt, covering ALLOWED, DENIED, and default states- Include edge cases like empty strings and special characters for conversationId, address, and inboxId parameters
🔗 Analysis chain
Consider adding unit tests and documentation
Given that these consent management functions are critical for spam prevention:
- Add comprehensive unit tests covering all consent states and edge cases
- Consider adding a README.md in the same directory explaining the consent management system
- Document the relationship between these functions and the spam scoring system
Let's verify the test coverage:
Let's search for existing tests and documentation to make a more informed decision about the review comment's recommendations.
Let's try to find test directories and any existing consent-related tests or documentation to make a more informed decision.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for existing tests fd -e kt "Preferences" -x echo "Found potential test file: {}"Length of output: 145
Script:
#!/bin/bash # Search for test files in test directories fd -e kt -t f . "test" -x echo "Found test file: {}" # Search for any documentation files fd -e md . -x echo "Found documentation: {}" # Search for test references to Preferences rg -l "Preferences" --type kotlin | grep -i "test"Length of output: 732
Script:
#!/bin/bash # Search for test directories in android path fd -t d "test" android/ # Look for any existing test files for consent-related functionality rg -i "consent" --type kotlin android/ # Check if there's any existing documentation about consent management rg -i "consent" --type mdLength of output: 7177
components/Chat/Message/MessageBubble.tsx (1)
37-60: 🛠️ Refactor suggestion
Optimize style calculations and memoization.
Several improvements could enhance performance and maintainability:
- Extract style constants
- Optimize style object creation
- Validate memoization dependencies
+const BORDER_STYLES = { + base: { + paddingHorizontal: theme.spacing.xs, + paddingVertical: theme.spacing.xxs, + }, +} as const; + const bubbleStyle = useMemo(() => { - const baseStyle = { + const baseStyle = { + ...BORDER_STYLES.base, backgroundColor: fromMe ? theme.colors.bubbles.bubble : theme.colors.bubbles.received.bubble, borderRadius: theme.borderRadius.sm, - paddingHorizontal: theme.spacing.xs, - paddingVertical: theme.spacing.xxs, }; + // Early return if we don't need series-specific styling if (!hasNextMessageInSeries) { + const cornerRadius = fromMe ? theme.spacing["4xs"] : theme.borderRadius.sm; return { ...baseStyle, - borderBottomLeftRadius: fromMe - ? theme.borderRadius.sm - : theme.spacing["4xs"], - borderBottomRightRadius: fromMe - ? theme.spacing["4xs"] - : theme.borderRadius.sm, + borderBottomLeftRadius: fromMe ? theme.borderRadius.sm : cornerRadius, + borderBottomRightRadius: fromMe ? cornerRadius : theme.borderRadius.sm, }; } return baseStyle; - }, [fromMe, hasNextMessageInSeries, theme]); + }, [fromMe, hasNextMessageInSeries, theme.borderRadius.sm, theme.spacing["4xs"], theme.colors.bubbles]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const BORDER_STYLES = { base: { paddingHorizontal: theme.spacing.xs, paddingVertical: theme.spacing.xxs, }, } as const; const bubbleStyle = useMemo(() => { const baseStyle = { ...BORDER_STYLES.base, backgroundColor: fromMe ? theme.colors.bubbles.bubble : theme.colors.bubbles.received.bubble, borderRadius: theme.borderRadius.sm, }; // Early return if we don't need series-specific styling if (!hasNextMessageInSeries) { const cornerRadius = fromMe ? theme.spacing["4xs"] : theme.borderRadius.sm; return { ...baseStyle, borderBottomLeftRadius: fromMe ? theme.borderRadius.sm : cornerRadius, borderBottomRightRadius: fromMe ? cornerRadius : theme.borderRadius.sm, }; } return baseStyle; }, [fromMe, hasNextMessageInSeries, theme.borderRadius.sm, theme.spacing["4xs"], theme.colors.bubbles]);app.config.ts (2)
43-44:
⚠️ Potential issueUse environment-specific bundle identifier
The bundle identifier is hardcoded to "com.converse.dev" which doesn't align with the environment-based naming used elsewhere in the config. Consider using the environment variables:
- bundleIdentifier: "com.converse.dev", + bundleIdentifier: isDev + ? "com.converse.dev" + : isPreview + ? "com.converse.preview" + : "com.converse",Committable suggestion skipped: line range outside the PR's diff.
43-43: 💡 Codebase verification
⚠️ Service configuration files exposed in repositoryThe sensitive Firebase configuration files are currently tracked in the repository and not gitignored:
ios/GoogleService-Info.plist- Multiple variants of
google-services.jsonunder android directoriesThese files typically contain sensitive information like API keys and should not be committed to version control.
Recommended actions:
- Remove these files from git tracking using
git rm --cached- Add the following patterns to
.gitignore:**/GoogleService-Info.plist **/google-services.json- Store these files securely and document their setup in README
- Consider using environment variables for sensitive values
🔗 Analysis chain
Verify security of service configuration files
Ensure that the sensitive configuration files are properly handled:
- Verify that production GoogleService-Info.plist and google-services.json are gitignored
- Consider using environment variables or secure storage for production credentials
Also applies to: 56-56
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if sensitive files are gitignored cat .gitignore | grep -E "GoogleService-Info.plist|google-services.json" # Check if these files exist in the repository fd "GoogleService-Info.plist|google-services.json"Length of output: 347
android/app/src/prod/AndroidManifest.xml (2)
14-28:
⚠️ Potential issueImplement additional security measures for sensitive deep links
The manifest includes app links for sensitive operations (
/coinbase,/desktopconnect). Consider:
- Implementing additional verification for sensitive deep links
- Adding rate limiting for deep link handling
- Ensuring proper validation of deep link parameters
- Documenting the domain verification process for app links
Since this PR is about adding Firebase AppCheck, consider integrating it with the deep link handling to add an extra layer of security. Would you like assistance in implementing these security measures?
1-31:
⚠️ Potential issueAdd Firebase AppCheck configuration to manifest
The manifest is missing the necessary Firebase AppCheck configurations. Consider:
- Adding the required Firebase AppCheck metadata
- Including SafetyNet/Play Integrity provider configurations
- Documenting the AppCheck initialization process
Would you like me to provide the necessary manifest entries for Firebase AppCheck integration?
android/app/src/preview/AndroidManifest.xml (2)
6-6: 🛠️ Refactor suggestion
Configure MainActivity attributes for proper behavior
The activity declaration is missing essential attributes for proper functionality and security:
- <activity android:name=".MainActivity"> + <activity + android:name=".MainActivity" + android:exported="true" + android:launchMode="singleTask" + android:configChanges="keyboard|keyboardHidden|orientation|screenSize|uiMode" + android:windowSoftInputMode="adjustResize">Also applies to: 29-29
1-5: 🛠️ Refactor suggestion
Add essential security and configuration attributes
The manifest is missing several important security and configuration attributes that should be explicitly defined:
<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools"> <application tools:replace="android:icon" android:icon="@mipmap/ic_launcher" - android:label="@string/app_name"> + android:label="@string/app_name" + android:allowBackup="false" + android:fullBackupContent="false" + android:supportsRtl="true" + android:usesCleartextTraffic="false" + android:theme="@style/AppTheme">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<manifest xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools"> <application tools:replace="android:icon" android:icon="@mipmap/ic_launcher" android:label="@string/app_name" android:allowBackup="false" android:fullBackupContent="false" android:supportsRtl="true" android:usesCleartextTraffic="false" android:theme="@style/AppTheme">README.md (1)
81-100: 🛠️ Refactor suggestion
Enhance testing documentation with required details
The testing section needs more specific information to be truly helpful.
## Running Jest tests -Before running the tests make sure that you have a `.env` file setup with the variables variable set +Before running the tests, ensure you have a `.env` file with the following required variables: +```env +# Add list of required environment variables here +# Example: +# FIREBASE_API_KEY=xxx +# FIREBASE_APP_ID=xxx +``` ## Running Performance Tests Capture baselines for performance tests ```sh yarn test:perf:baseline+### Understanding Performance Tests
+Performance tests measure [describe what aspects are being measured].
+Baseline captures establish a reference point for:
+- [List key metrics being tracked]
+
Make changes to the code to see the performance impactRun the performance tests again to see the changes
yarn test:perf
+### Interpreting Results
+The test output will show:
+- [Explain key output metrics]
+- [Explain what constitutes a significant change]<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [uncategorized] ~82-~82: Possible missing comma found. Context: ...# Running Jest tests Before running the tests make sure that you have a `.env` file s... (AI_HYDRA_LEO_MISSING_COMMA) --- [style] ~94-~94: Consider shortening or rephrasing this to strengthen your wording. Context: ...ests ```sh yarn test:perf:baseline ``` Make changes to the code to see the performance impact ... (MAKE_CHANGES) </details> </details> </blockquote></details> <details> <summary>components/Chat/Message/Message.utils.tsx (2)</summary><blockquote> 105-124: _:hammer_and_wrench: Refactor suggestion_ **Address architectural TODO and improve error handling** The code has two areas for improvement: 1. The TODO comments suggest uncertainty about the architectural placement of these functions 2. The `getCurrentUserAccountInboxId` function lacks error handling Consider: 1. Moving these functions to the account store as suggested in the TODO 2. Adding error handling: ```diff export function getCurrentUserAccountInboxId() { - return queryClient.getQueryData( + const data = queryClient.getQueryData( getCurrentAccountInboxIdQueryOptions().queryKey ); + if (!data) { + throw new Error('Inbox ID not found. Ensure account is properly initialized.'); + } + return data; }Would you like me to help create a GitHub issue to track the architectural refactor?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// TMP until we move this into an account store or something like that // Maybe instead worth moving into account store? export function useCurrentAccountInboxId() { return useQuery(getCurrentAccountInboxIdQueryOptions()); } function getCurrentAccountInboxIdQueryOptions() { const currentAccount = getCurrentAccount(); return { queryKey: ["inboxId", currentAccount], queryFn: () => getInboxId(currentAccount!), enabled: !!currentAccount, }; } export function getCurrentUserAccountInboxId() { const data = queryClient.getQueryData( getCurrentAccountInboxIdQueryOptions().queryKey ); if (!data) { throw new Error('Inbox ID not found. Ensure account is properly initialized.'); } return data; }
39-93: 🛠️ Refactor suggestion
⚠️ Potential issueReplace
anywith proper type definitionsAll type guard functions are using
anytype for the message parameter, which defeats TypeScript's type safety. The commented-outDecodedMessageWithCodecsTypehints at the intended type.Consider this refactor to improve type safety and reduce duplication:
- export function isTextMessage( - message: any - ): message is DecodedMessage<[TextCodec]> { - return getMessageContentType(message.contentTypeId) === "text"; - } + type MessageTypeMap = { + text: TextCodec; + reaction: ReactionCodec; + readReceipt: ReadReceiptCodec; + groupUpdated: GroupUpdatedCodec; + reply: ReplyCodec; + remoteAttachment: RemoteAttachmentCodec; + attachment: StaticAttachmentCodec; + transactionReference: TransactionReferenceCodec; + coinbasePayment: CoinbaseMessagingPaymentCodec; + }; + + function createTypeGuard<T extends keyof MessageTypeMap>(type: T) { + return function(message: DecodedMessageWithCodecsType): + message is DecodedMessage<[MessageTypeMap[T]]> { + return getMessageContentType(message.contentTypeId) === type; + }; + } + + export const isTextMessage = createTypeGuard('text'); + export const isReactionMessage = createTypeGuard('reaction'); + // ... repeat for other message typesThis refactor:
- Adds proper type safety
- Reduces code duplication
- Makes adding new message types easier
- Maintains the same functionality
Committable suggestion skipped: line range outside the PR's diff.
android/app/src/main/java/com/converse/MessageNotification.kt (1)
1-1: 💡 Codebase verification
Package name inconsistency detected across Android configuration files
The package name change from
com.converse.devtocom.converseis incomplete. Several configuration files still reference the old package name:
android/app/build.gradle: ContainsapplicationId 'com.converse.dev'android/app/src/main/AndroidManifest.xml: Containsandroid:scheme="com.converse.dev"- Multiple
google-services.jsonfiles have"package_name": "com.converse.dev"These inconsistencies need to be addressed to prevent build and runtime issues.
🔗 Analysis chain
Verify package restructuring consistency
The package has been updated from
com.converse.devtocom.converse. While this change aligns with the PR's package restructuring, we should verify the consistency across related configurations.Also applies to: 17-17
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify package name consistency across Android configuration files # Check AndroidManifest.xml for package name echo "Checking AndroidManifest.xml for package name:" fd -e xml "AndroidManifest.xml" android/ -x grep -l "package=\"com.converse" # Check build.gradle files for applicationId echo "Checking build.gradle files for applicationId:" fd -e gradle "build.gradle" android/ -x grep -l "applicationId.*com.converse" # Check for any remaining references to old package echo "Checking for old package references:" rg "com\.converse\.dev" android/Length of output: 968
.github/workflows/build-app-release.yml (2)
57-57: 💡 Codebase verification
Build profile changes are properly configured but require cleanup
The 'production' build profile is correctly defined in eas.json with appropriate configurations for both Android and iOS platforms. However, there are legacy profiles ('production-android') that should be removed to maintain consistency with the new standardized approach.
- The 'production' profile includes all necessary configurations:
- Android: Uses app-bundle build type with
:app:bundleProdReleasecommand- iOS: Uses large resource class with cocoapods 1.14.3
- Legacy profiles still exist:
- 'production-android' profile extends 'production' but only differs in autoIncrement
- Similar legacy profiles exist in the submit configuration
🔗 Analysis chain
Verify EAS configuration for the simplified build profile
The build profiles have been standardized to use 'production' for both platforms. Let's ensure the EAS configuration properly defines this profile for both Android and iOS builds.
Also applies to: 100-100
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check EAS configuration files echo "Checking eas.json configuration:" if [ -f "eas.json" ]; then cat eas.json | jq '.build.production' else echo "Warning: eas.json not found" fi # Check for any other profile definitions find . -type f -name "eas*.json" -o -name "app.config.js" -o -name "app.json" | while read -r file; do echo "Checking $file:" cat "$file" doneLength of output: 3156
51-51:
⚠️ Potential issueFix invalid character in script path
The script path contains a special character 'Ø' which could cause execution failures across different operating systems. This appears to be a typo.
Apply this fix:
- run: node scripts/Øbuild/eas.js --env production + run: node scripts/build/eas.js --env production📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.run: node scripts/build/eas.js --env productionandroid/app/build.gradle (1)
127-141: 💡 Codebase verification
Missing Firebase configuration for dev flavor and potential configuration duplication
- The
devflavor is missing its owngoogle-services.jsoninandroid/app/src/dev/- A base
google-services.jsonexists inandroid/app/, but using it directly may cause configuration conflicts- The existing configurations for
prodandpreviewflavors are identical, which might not be the intended setup for different environmentsRecommendations:
- Create a separate
google-services.jsonfor thedevflavor inandroid/app/src/dev/- Review and ensure each flavor's configuration is environment-specific
- Consider removing the base
google-services.jsonto avoid potential conflicts- Verify that the AppCheck configuration is properly set up for each environment
🔗 Analysis chain
Verify Firebase configuration for each flavor
Since this PR adds Firebase AppCheck support, ensure that:
- Each flavor has its own
google-services.jsonconfiguration file in the appropriate source set directory:
src/dev/google-services.jsonsrc/prod/google-services.jsonsrc/preview/google-services.json- Each flavor's Firebase configuration includes the necessary AppCheck settings.
Let me verify if there's a base
google-services.jsonfile that might be used for the dev flavor, and check the content of the existing files for AppCheck configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if google-services.json exists for each flavor for flavor in dev prod preview; do if [ -f "android/app/src/$flavor/google-services.json" ]; then echo "Found google-services.json for $flavor" # Check if the file contains AppCheck configuration jq -r '.client[].api_key[]' "android/app/src/$flavor/google-services.json" 2>/dev/null else echo "Missing google-services.json for $flavor" fi doneLength of output: 789
Script:
#!/bin/bash # Check for base google-services.json echo "Checking for base google-services.json:" if [ -f "android/app/google-services.json" ]; then echo "Found base google-services.json" jq -r '.client[].api_key[]' "android/app/google-services.json" 2>/dev/null else echo "No base google-services.json found" fi echo -e "\nChecking AppCheck configuration in existing files:" for flavor in prod preview; do echo -e "\nChecking $flavor/google-services.json:" jq -r '.client[].services.appinvite_service // empty' "android/app/src/$flavor/google-services.json" 2>/dev/null doneLength of output: 1233
utils/appCheck.ts
Outdated
| @@ -0,0 +1,22 @@ | |||
| // export const rnFirebaseProvider = firebase | |||
There was a problem hiding this comment.
in which PR does this get implemented?
Note: - only used for address checksum and we now standardize on lowercase
Note: - only used for address checksum and we now standardize on lowercase
…rse-app into kc/33-converse-jwt-auth
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
ios/ConverseNotificationExtension/Profile.swift (1)
13-22: Consider standardizing address case handlingThe code mixes usage of original and lowercased addresses which could lead to inconsistencies in profile storage and retrieval. Consider standardizing the address case handling throughout the profile management flow.
Suggestions:
- Decide on a consistent case format (e.g., always lowercase) for storing addresses
- Convert addresses to the chosen format as early as possible in the data flow
- Update all profile storage/retrieval operations to use the standardized format
components/Chat/Message/MessageSenderAvatar.tsx (1)
8-13: Clean up commented importsThe file contains commented-out imports that should either be removed or restored based on their usage. This affects code maintainability.
If these imports are no longer needed, remove them entirely:
-import { - // getPreferredAvatar, - getPreferredInboxAvatar, - // getPreferredName, - // getProfile, -} from "../../../utils/profile"; +import { getPreferredInboxAvatar } from "../../../utils/profile";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
components/Chat/Message/MessageSenderAvatar.tsx(1 hunks)ios/Converse.xcodeproj/project.pbxproj(8 hunks)ios/ConverseNotificationExtension/Profile.swift(2 hunks)ios/ConverseNotificationExtension/Spam.swift(1 hunks)ios/Podfile(1 hunks)package.json(2 hunks)queries/queryClient.constants.ts(1 hunks)queries/queryClient.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- queries/queryClient.constants.ts
🔇 Additional comments (10)
queries/queryClient.ts (2)
3-3: LGTM! Good modularization practice.
Moving the constant to a dedicated constants file improves code organization and maintainability.
3-3: Verify the constants file and its exports.
Let's ensure the constants file exists and maintains the expected 24-hour value for GC_TIME.
✅ Verification successful
GC_TIME constant is correctly defined and exported
The constant is properly defined in queryClient.constants.ts with the expected 24-hour value (1000 * 60 * 60 * 24) and is exported. No duplicate definitions were found in other files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the queryClient.constants.ts file exists and contains the correct GC_TIME value
# Check if the constants file exists and show its content
rg --type ts "export.*GC_TIME.*=.*1000.*60.*60.*24" ./queries/queryClient.constants.ts
# Verify no other definitions of GC_TIME exist to avoid conflicts
rg --type ts "GC_TIME.*=.*1000.*60.*60.*24" -g '!queryClient.constants.ts'
Length of output: 221
components/Chat/Message/MessageSenderAvatar.tsx (2)
8-17: Verify impact of profile utility changes
The commented-out profile utilities and reorganized imports suggest a shift in profile handling logic. This change might affect the Firebase AppCheck integration being added in this PR.
Let's check for any other files that might need similar updates:
Line range hint 1-1: Clarify relationship to Firebase AppCheck integration
The changes in these files appear to focus on profile handling improvements rather than Firebase AppCheck integration. Could you clarify how these changes relate to the PR's primary objective of adding Firebase AppCheck dependencies?
Let's check for Firebase AppCheck related changes:
✅ Verification successful
The changes in MessageSenderAvatar.tsx are part of a broader Firebase AppCheck integration
Based on the search results, there is clear evidence of Firebase AppCheck integration throughout the codebase:
- Firebase AppCheck configuration exists in both iOS and Android (
GoogleService-Info.plistandgoogle-services.json) - AppCheck dependencies are properly configured in
Podfile.lock - A dedicated
utils/appCheck.tsfile handles AppCheck initialization
The Avatar component changes appear to be part of necessary UI adjustments to support the secure storage access patterns that AppCheck enables, particularly for user profile images that may be stored in Firebase Storage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Firebase AppCheck related imports and configurations
echo "Files with Firebase AppCheck references:"
rg -i "firebase.*appcheck|appcheck.*firebase"
echo "\nFiles with recent Firebase configuration changes:"
rg "google-services.json|GoogleService-Info.plist"
Length of output: 13989
ios/Podfile (1)
31-31: LGTM: Firebase static framework configuration is correct.
The setting $RNFirebaseAsStaticFramework = true is the recommended configuration for React Native Firebase integration, ensuring proper framework linkage.
✅ Verification successful
LGTM: Firebase static framework configuration is correctly set
The $RNFirebaseAsStaticFramework = true setting is correctly placed in the Podfile and is the only instance of this configuration. This setting ensures Firebase is linked as a static framework, which is the recommended approach for React Native Firebase integration to prevent potential conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that no other Firebase framework linkage configurations exist
rg -l "RNFirebaseAsStaticFramework\s*=\s*(true|false)" --type ruby
Length of output: 68
Script:
#!/bin/bash
# Let's first check if the Podfile exists and see its content
fd Podfile --type f --exec cat {}
Length of output: 65634
Script:
#!/bin/bash
# Let's check the actual Podfile content specifically
fd Podfile$ --type f --exec cat {}
Length of output: 3861
ios/ConverseNotificationExtension/Spam.swift (1)
183-183: LGTM: Clean condition check for denied addresses.
The simplified condition maintains the same functionality while improving code readability.
package.json (2)
243-243: LGTM: expo-build-properties version is compatible.
The version ~0.12.5 is appropriate for the project's Expo version.
62-63: LGTM: Firebase dependencies are properly aligned.
Both Firebase packages are using the same version (21.4.0), which is recommended for compatibility.
✅ Verification successful
Firebase dependencies are correctly aligned at version 21.4.0
The verification confirms that both Firebase dependencies (@react-native-firebase/app and @react-native-firebase/app-check) are using the same version (^21.4.0), which is the recommended practice for compatibility. No other Firebase dependencies were found in the project that could cause version mismatches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify Firebase dependencies are aligned across the project
rg '"@react-native-firebase/[^"]+": "\^[^"]+"' package.json
Length of output: 157
ios/Converse.xcodeproj/project.pbxproj (2)
455-458: LGTM: Firebase privacy bundles correctly configured
The necessary Firebase-related privacy bundles have been properly added to both input and output paths in the Copy Pods Resources phase:
- FirebaseCore_Privacy.bundle
- FirebaseCoreInternal_Privacy.bundle
- GoogleUtilities_Privacy.bundle
- FBLPromises_Privacy.bundle
Also applies to: 486-489
10-10: Verify GoogleService-Info.plist file location
The GoogleService-Info.plist file is correctly referenced in the project, but let's verify:
- The file exists in the correct location
- Consider using an absolute path instead of a relative path for better reliability
Also applies to: 66-66, 191-191
✅ Verification successful
GoogleService-Info.plist is correctly integrated and present
The file exists at the expected location in the iOS project directory, confirming proper integration. The relative path reference in the Xcode project file is actually a common and acceptable practice for iOS projects, as Xcode handles path resolution internally.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if GoogleService-Info.plist exists
if [ -f "ios/GoogleService-Info.plist" ]; then
echo "GoogleService-Info.plist found"
else
echo "Warning: GoogleService-Info.plist not found"
fi
Length of output: 112
| let formattedAddress = address.lowercased() | ||
| if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[formattedAddress] { | ||
| let lowercasedAddress = address.lowercased() | ||
| if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] ?? profileState?.profiles?[lowercasedAddress] { |
There was a problem hiding this comment.
Remove redundant profile lookup conditions
The current implementation checks lowercasedAddress twice in the same condition which is redundant and impacts performance.
Simplify the condition:
- if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] ?? profileState?.profiles?[lowercasedAddress] {
+ if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] ?? profileState?.profiles?[lowercasedAddress] { | |
| if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] { |
| "@react-native-firebase/app": "^21.4.0", | ||
| "@react-native-firebase/app-check": "^21.4.0", |
There was a problem hiding this comment.
💡 Codebase verification
AppCheck implementation is incomplete and commented out
The AppCheck dependency has been added to package.json, but the implementation in utils/appCheck.ts is entirely commented out. The current state:
- No active AppCheck provider configuration
- No error handling implementation
- No replay protection setup
- Debug/production environment handling is defined but not active
Action items:
- Uncomment and properly configure the AppCheck provider in
utils/appCheck.ts - Set up proper debug tokens from Firebase Console
- Implement error handling for verification failures
- Configure replay protection for production environment
- Initialize the provider in your app's entry point
🔗 Analysis chain
Consider implementing AppCheck best practices.
Since you're adding Firebase AppCheck, consider implementing these security best practices:
- Use debug provider only in development
- Implement proper error handling for AppCheck verification failures
- Configure replay protection for production
Let's examine the content of these files to understand the AppCheck implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for AppCheck configuration files
rg -l "AppCheck|app-check" --type ts --type js
Length of output: 80
Script:
#!/bin/bash
# Check the content of AppCheck related files
echo "=== app.config.ts ==="
cat app.config.ts
echo -e "\n=== utils/appCheck.ts ==="
cat utils/appCheck.ts
Length of output: 2887
| 91E43602CF513BC4F6064F94 /* [CP-User] [RNFB] Core Configuration */ = { | ||
| isa = PBXShellScriptBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| ); | ||
| inputPaths = ( | ||
| "$(BUILT_PRODUCTS_DIR)/$(INFOPLIST_PATH)", | ||
| ); | ||
| name = "[CP-User] [RNFB] Core Configuration"; | ||
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "#!/usr/bin/env bash\n#\n# Copyright (c) 2016-present Invertase Limited & Contributors\n#\n# Licensed under the Apache License, Version 2.0 (the \"License\");\n# you may not use this library except in compliance with the License.\n# You may obtain a copy of the License at\n#\n# http://www.apache.org/licenses/LICENSE-2.0\n#\n# Unless required by applicable law or agreed to in writing, software\n# distributed under the License is distributed on an \"AS IS\" BASIS,\n# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n# See the License for the specific language governing permissions and\n# limitations under the License.\n#\n\n##########################################################################\n##########################################################################\n#\n# NOTE THAT IF YOU CHANGE THIS FILE YOU MUST RUN pod install AFTERWARDS\n#\n# This file is installed as an Xcode build script in the project file\n# by cocoapods, and you will not see your changes until you pod install\n#\n##########################################################################\n##########################################################################\n\nset -e\n\n_MAX_LOOKUPS=2;\n_SEARCH_RESULT=''\n_RN_ROOT_EXISTS=''\n_CURRENT_LOOKUPS=1\n_JSON_ROOT=\"'react-native'\"\n_JSON_FILE_NAME='firebase.json'\n_JSON_OUTPUT_BASE64='e30=' # { }\n_CURRENT_SEARCH_DIR=${PROJECT_DIR}\n_PLIST_BUDDY=/usr/libexec/PlistBuddy\n_TARGET_PLIST=\"${BUILT_PRODUCTS_DIR}/${INFOPLIST_PATH}\"\n_DSYM_PLIST=\"${DWARF_DSYM_FOLDER_PATH}/${DWARF_DSYM_FILE_NAME}/Contents/Info.plist\"\n\n# plist arrays\n_PLIST_ENTRY_KEYS=()\n_PLIST_ENTRY_TYPES=()\n_PLIST_ENTRY_VALUES=()\n\nfunction setPlistValue {\n echo \"info: setting plist entry '$1' of type '$2' in file '$4'\"\n ${_PLIST_BUDDY} -c \"Add :$1 $2 '$3'\" $4 || echo \"info: '$1' already exists\"\n}\n\nfunction getFirebaseJsonKeyValue () {\n if [[ ${_RN_ROOT_EXISTS} ]]; then\n ruby -Ku -e \"require 'rubygems';require 'json'; output=JSON.parse('$1'); puts output[$_JSON_ROOT]['$2']\"\n else\n echo \"\"\n fi;\n}\n\nfunction jsonBoolToYesNo () {\n if [[ $1 == \"false\" ]]; then\n echo \"NO\"\n elif [[ $1 == \"true\" ]]; then\n echo \"YES\"\n else echo \"NO\"\n fi\n}\n\necho \"info: -> RNFB build script started\"\necho \"info: 1) Locating ${_JSON_FILE_NAME} file:\"\n\nif [[ -z ${_CURRENT_SEARCH_DIR} ]]; then\n _CURRENT_SEARCH_DIR=$(pwd)\nfi;\n\nwhile true; do\n _CURRENT_SEARCH_DIR=$(dirname \"$_CURRENT_SEARCH_DIR\")\n if [[ \"$_CURRENT_SEARCH_DIR\" == \"/\" ]] || [[ ${_CURRENT_LOOKUPS} -gt ${_MAX_LOOKUPS} ]]; then break; fi;\n echo \"info: ($_CURRENT_LOOKUPS of $_MAX_LOOKUPS) Searching in '$_CURRENT_SEARCH_DIR' for a ${_JSON_FILE_NAME} file.\"\n _SEARCH_RESULT=$(find \"$_CURRENT_SEARCH_DIR\" -maxdepth 2 -name ${_JSON_FILE_NAME} -print | /usr/bin/head -n 1)\n if [[ ${_SEARCH_RESULT} ]]; then\n echo \"info: ${_JSON_FILE_NAME} found at $_SEARCH_RESULT\"\n break;\n fi;\n _CURRENT_LOOKUPS=$((_CURRENT_LOOKUPS+1))\ndone\n\nif [[ ${_SEARCH_RESULT} ]]; then\n _JSON_OUTPUT_RAW=$(cat \"${_SEARCH_RESULT}\")\n _RN_ROOT_EXISTS=$(ruby -Ku -e \"require 'rubygems';require 'json'; output=JSON.parse('$_JSON_OUTPUT_RAW'); puts output[$_JSON_ROOT]\" || echo '')\n\n if [[ ${_RN_ROOT_EXISTS} ]]; then\n if ! python3 --version >/dev/null 2>&1; then echo \"python3 not found, firebase.json file processing error.\" && exit 1; fi\n _JSON_OUTPUT_BASE64=$(python3 -c 'import json,sys,base64;print(base64.b64encode(bytes(json.dumps(json.loads(open('\"'${_SEARCH_RESULT}'\"', '\"'rb'\"').read())['${_JSON_ROOT}']), '\"'utf-8'\"')).decode())' || echo \"e30=\")\n fi\n\n _PLIST_ENTRY_KEYS+=(\"firebase_json_raw\")\n _PLIST_ENTRY_TYPES+=(\"string\")\n _PLIST_ENTRY_VALUES+=(\"$_JSON_OUTPUT_BASE64\")\n\n # config.app_data_collection_default_enabled\n _APP_DATA_COLLECTION_ENABLED=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"app_data_collection_default_enabled\")\n if [[ $_APP_DATA_COLLECTION_ENABLED ]]; then\n _PLIST_ENTRY_KEYS+=(\"FirebaseDataCollectionDefaultEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_APP_DATA_COLLECTION_ENABLED\")\")\n fi\n\n # config.analytics_auto_collection_enabled\n _ANALYTICS_AUTO_COLLECTION=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_auto_collection_enabled\")\n if [[ $_ANALYTICS_AUTO_COLLECTION ]]; then\n _PLIST_ENTRY_KEYS+=(\"FIREBASE_ANALYTICS_COLLECTION_ENABLED\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_AUTO_COLLECTION\")\")\n fi\n\n # config.analytics_collection_deactivated\n _ANALYTICS_DEACTIVATED=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_collection_deactivated\")\n if [[ $_ANALYTICS_DEACTIVATED ]]; then\n _PLIST_ENTRY_KEYS+=(\"FIREBASE_ANALYTICS_COLLECTION_DEACTIVATED\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_DEACTIVATED\")\")\n fi\n\n # config.analytics_idfv_collection_enabled\n _ANALYTICS_IDFV_COLLECTION=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_idfv_collection_enabled\")\n if [[ $_ANALYTICS_IDFV_COLLECTION ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_IDFV_COLLECTION_ENABLED\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_IDFV_COLLECTION\")\")\n fi\n\n # config.analytics_default_allow_analytics_storage\n _ANALYTICS_STORAGE=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_default_allow_analytics_storage\")\n if [[ $_ANALYTICS_STORAGE ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_DEFAULT_ALLOW_ANALYTICS_STORAGE\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_STORAGE\")\")\n fi\n\n # config.analytics_default_allow_ad_storage\n _ANALYTICS_AD_STORAGE=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_default_allow_ad_storage\")\n if [[ $_ANALYTICS_AD_STORAGE ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_DEFAULT_ALLOW_AD_STORAGE\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_AD_STORAGE\")\")\n fi\n\n # config.analytics_default_allow_ad_user_data\n _ANALYTICS_AD_USER_DATA=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_default_allow_ad_user_data\")\n if [[ $_ANALYTICS_AD_USER_DATA ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_DEFAULT_ALLOW_AD_USER_DATA\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_AD_USER_DATA\")\")\n fi\n\n # config.analytics_default_allow_ad_personalization_signals\n _ANALYTICS_PERSONALIZATION=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"analytics_default_allow_ad_personalization_signals\")\n if [[ $_ANALYTICS_PERSONALIZATION ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_DEFAULT_ALLOW_AD_PERSONALIZATION_SIGNALS\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_PERSONALIZATION\")\")\n fi\n\n # config.analytics_registration_with_ad_network_enabled\n _ANALYTICS_REGISTRATION_WITH_AD_NETWORK=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"google_analytics_registration_with_ad_network_enabled\")\n if [[ $_ANALYTICS_REGISTRATION_WITH_AD_NETWORK ]]; then\n _PLIST_ENTRY_KEYS+=(\"GOOGLE_ANALYTICS_REGISTRATION_WITH_AD_NETWORK_ENABLED\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_REGISTRATION_WITH_AD_NETWORK\")\")\n fi\n\n # config.google_analytics_automatic_screen_reporting_enabled\n _ANALYTICS_AUTO_SCREEN_REPORTING=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"google_analytics_automatic_screen_reporting_enabled\")\n if [[ $_ANALYTICS_AUTO_SCREEN_REPORTING ]]; then\n _PLIST_ENTRY_KEYS+=(\"FirebaseAutomaticScreenReportingEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_ANALYTICS_AUTO_SCREEN_REPORTING\")\")\n fi\n\n # config.perf_auto_collection_enabled\n _PERF_AUTO_COLLECTION=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"perf_auto_collection_enabled\")\n if [[ $_PERF_AUTO_COLLECTION ]]; then\n _PLIST_ENTRY_KEYS+=(\"firebase_performance_collection_enabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_PERF_AUTO_COLLECTION\")\")\n fi\n\n # config.perf_collection_deactivated\n _PERF_DEACTIVATED=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"perf_collection_deactivated\")\n if [[ $_PERF_DEACTIVATED ]]; then\n _PLIST_ENTRY_KEYS+=(\"firebase_performance_collection_deactivated\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_PERF_DEACTIVATED\")\")\n fi\n\n # config.messaging_auto_init_enabled\n _MESSAGING_AUTO_INIT=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"messaging_auto_init_enabled\")\n if [[ $_MESSAGING_AUTO_INIT ]]; then\n _PLIST_ENTRY_KEYS+=(\"FirebaseMessagingAutoInitEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_MESSAGING_AUTO_INIT\")\")\n fi\n\n # config.in_app_messaging_auto_colllection_enabled\n _FIAM_AUTO_INIT=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"in_app_messaging_auto_collection_enabled\")\n if [[ $_FIAM_AUTO_INIT ]]; then\n _PLIST_ENTRY_KEYS+=(\"FirebaseInAppMessagingAutomaticDataCollectionEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_FIAM_AUTO_INIT\")\")\n fi\n\n # config.app_check_token_auto_refresh\n _APP_CHECK_TOKEN_AUTO_REFRESH=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"app_check_token_auto_refresh\")\n if [[ $_APP_CHECK_TOKEN_AUTO_REFRESH ]]; then\n _PLIST_ENTRY_KEYS+=(\"FirebaseAppCheckTokenAutoRefreshEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"$(jsonBoolToYesNo \"$_APP_CHECK_TOKEN_AUTO_REFRESH\")\")\n fi\n\n # config.crashlytics_disable_auto_disabler - undocumented for now - mainly for debugging, document if becomes useful\n _CRASHLYTICS_AUTO_DISABLE_ENABLED=$(getFirebaseJsonKeyValue \"$_JSON_OUTPUT_RAW\" \"crashlytics_disable_auto_disabler\")\n if [[ $_CRASHLYTICS_AUTO_DISABLE_ENABLED == \"true\" ]]; then\n echo \"Disabled Crashlytics auto disabler.\" # do nothing\n else\n _PLIST_ENTRY_KEYS+=(\"FirebaseCrashlyticsCollectionEnabled\")\n _PLIST_ENTRY_TYPES+=(\"bool\")\n _PLIST_ENTRY_VALUES+=(\"NO\")\n fi\nelse\n _PLIST_ENTRY_KEYS+=(\"firebase_json_raw\")\n _PLIST_ENTRY_TYPES+=(\"string\")\n _PLIST_ENTRY_VALUES+=(\"$_JSON_OUTPUT_BASE64\")\n echo \"warning: A firebase.json file was not found, whilst this file is optional it is recommended to include it to configure firebase services in React Native Firebase.\"\nfi;\n\necho \"info: 2) Injecting Info.plist entries: \"\n\n# Log out the keys we're adding\nfor i in \"${!_PLIST_ENTRY_KEYS[@]}\"; do\n echo \" -> $i) ${_PLIST_ENTRY_KEYS[$i]}\" \"${_PLIST_ENTRY_TYPES[$i]}\" \"${_PLIST_ENTRY_VALUES[$i]}\"\ndone\n\nfor plist in \"${_TARGET_PLIST}\" \"${_DSYM_PLIST}\" ; do\n if [[ -f \"${plist}\" ]]; then\n\n # paths with spaces break the call to setPlistValue. temporarily modify\n # the shell internal field separator variable (IFS), which normally\n # includes spaces, to consist only of line breaks\n oldifs=$IFS\n IFS=\"\n\"\n\n for i in \"${!_PLIST_ENTRY_KEYS[@]}\"; do\n setPlistValue \"${_PLIST_ENTRY_KEYS[$i]}\" \"${_PLIST_ENTRY_TYPES[$i]}\" \"${_PLIST_ENTRY_VALUES[$i]}\" \"${plist}\"\n done\n\n # restore the original internal field separator value\n IFS=$oldifs\n else\n echo \"warning: A Info.plist build output file was not found (${plist})\"\n fi\ndone\n\necho \"info: <- RNFB build script finished\"\n"; | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Missing firebase.json configuration file
The firebase.json configuration file is missing in the repository. This file is required to configure Firebase services including AppCheck token auto refresh. Without this file:
- Firebase services may use default configurations which might not be optimal
- AppCheck token auto refresh settings won't be properly configured
- Other Firebase features may not work as expected
Please add a firebase.json file in the root directory with the necessary configuration:
{
"react-native": {
"app_check_token_auto_refresh": true
}
}🔗 Analysis chain
Verify Firebase configuration for AppCheck
The RNFB Core Configuration script is correctly added, but we should verify:
- The firebase.json file exists with proper AppCheck configuration
- The AppCheck token auto refresh setting is properly configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if firebase.json exists and contains AppCheck configuration
if [ -f "firebase.json" ]; then
echo "Checking AppCheck configuration in firebase.json:"
cat firebase.json | jq '.react-native.app_check_token_auto_refresh'
else
echo "Warning: firebase.json not found"
fi
Length of output: 101
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (15)
ios/Converse.xcodeproj/xcshareddata/xcschemes/Converse.xcscheme (1)
63-68: Consider making Firebase debugging configurableWhile Firebase debugging is useful during development, having it always enabled in debug builds could impact performance and generate excessive logs. Consider:
- Making it disabled by default
- Using a development-specific scheme for Firebase debugging
<CommandLineArguments> <CommandLineArgument argument = "FIRDebugEnabled" - isEnabled = "YES"> + isEnabled = "NO"> </CommandLineArgument> </CommandLineArguments>ios/Converse/AppDelegate.mm (2)
3-4: Consider using more specific Firebase imports.While
<Firebase.h>works, it's an umbrella header that includes all Firebase modules. For better compilation times and clearer dependencies, consider importing only the specific Firebase headers needed:#import "RNFBAppCheckModule.h" -#import <Firebase.h> +#import <FirebaseCore/FirebaseCore.h> +#import <FirebaseAppCheck/FirebaseAppCheck.h>
19-21: Consider native alternatives to Firebase App Check.Given the PR objectives and concerns about dependency management, consider these alternatives:
iOS 14+: Use
DeviceCheckframework directly
- Lighter weight
- Native to iOS
- No external dependencies
iOS 14+: Use
AppAttestframework
- Enhanced security
- Direct API access
- Better control over token lifecycle
Both alternatives would reduce dependency complexity while achieving similar security goals.
Would you like me to provide example implementations using these native alternatives?
scripts/build/ios/production.js (4)
4-16: Enhance documentation to include Firebase-related changesThe documentation block clearly describes the iOS configuration changes but should be updated to mention Firebase-related modifications, given that this PR introduces Firebase AppCheck integration.
* This script modifies various iOS configuration files to set up * the production environment build variant of the Converse app. * It updates bundle IDs, display names, URL schemes and -* entitlements to use production-specific values. +* entitlements to use production-specific values. It also ensures +* proper configuration for Firebase services in production.
Line range hint
21-34: Consider refactoring for maintainability and error handlingWhile the bundle ID updates are correct, there are a few improvements that could make the code more maintainable and robust:
- Extract string literals into constants
- Add error handling for file operations
+const BUNDLE_ID = { + DEV: 'com.converse.dev', + PROD: 'com.converse.native' +}; +const ICON_NAMES = { + PREVIEW: 'AppIconPreview', + PROD: 'AppIcon' +}; const newProjContent = projContent .replace( - /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g, - "PRODUCT_BUNDLE_IDENTIFIER = com.converse.native" + new RegExp(`PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_ID.DEV}`, 'g'), + `PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_ID.PROD}` ) .replace( - /ASSETCATALOG_COMPILER_APPICON_NAME = AppIconPreview/g, - "ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon" + new RegExp(`ASSETCATALOG_COMPILER_APPICON_NAME = ${ICON_NAMES.PREVIEW}`, 'g'), + `ASSETCATALOG_COMPILER_APPICON_NAME = ${ICON_NAMES.PROD}` ); -fs.writeFileSync(PROJ_PATH, newProjContent); +try { + fs.writeFileSync(PROJ_PATH, newProjContent); +} catch (error) { + console.error('Failed to update project configuration:', error); + process.exit(1); +}
98-99: Document the purpose of notifications filteringThe
usernotifications.filteringentitlement is being set without any explanation of its purpose or implications. Please add a comment explaining why this setting is needed.+// Enable notifications filtering capability for the extension +// This is required for processing notifications before they are displayed entitlementsExtension["com.apple.developer.usernotifications.filtering"] = true;
Line range hint
61-107: Add safety checks for array accessThe code directly accesses array indices without checking if they exist. This could lead to runtime errors if the entitlements file structure changes.
-entitlementsApp["keychain-access-groups"][0] = entitlementsApp[ - "keychain-access-groups" -][0].replace("com.converse.dev", "com.converse.native"); +const updateEntitlementArray = (obj, key, oldValue, newValue) => { + if (!obj[key] || !Array.isArray(obj[key]) || obj[key].length === 0) { + console.error(`Invalid or empty array for ${key}`); + process.exit(1); + } + obj[key] = obj[key].map(value => + value.replace(oldValue, newValue) + ); +}; + +updateEntitlementArray( + entitlementsApp, + "keychain-access-groups", + "com.converse.dev", + "com.converse.native" +);scripts/build/ios/preview.js (5)
1-13: Enhance documentation with additional JSDoc annotations.The documentation is well-structured, but could be improved by adding:
@returnsannotation to document successful completion@throwsannotation to document potential errors@exampleshowing usage/** * iOS Preview Environment Configuration Script * * This script modifies various iOS configuration files to set up * the preview environment build variant of the Converse app. * It updates bundle IDs, display names, URL schemes and * entitlements to use preview-specific values. * * @sideEffects * - Updates Xcode project config * - Updates Info.plist files * - Updates entitlements files + * + * @throws {Error} If any file operations fail + * @returns {Promise<void>} Resolves when all updates are complete + * + * @example + * $ node scripts/build/ios/preview.js */
Line range hint
33-58: Add safety measures for plist modifications.The plist modifications lack validation and backup mechanisms.
Consider these improvements:
+const PLIST_CONFIG = { + app: { + displayName: 'Converse PREVIEW', + urlSchemes: ['converse-preview', 'com.converse.preview'] + }, + extension: { + bundleId: 'com.converse.preview', + env: 'preview' + } +}; + +const backupFile = (filePath) => { + const backupPath = `${filePath}.backup`; + fs.copyFileSync(filePath, backupPath); + return backupPath; +}; + +const validatePlist = (data, requiredKeys) => { + for (const key of requiredKeys) { + if (!(key in data)) { + throw new Error(`Required key "${key}" missing in plist`); + } + } +}; // Update main app Info.plist +backupFile(PLIST_APP_PATH); const appInfo = plist.parse(fs.readFileSync(PLIST_APP_PATH, "utf8")); -appInfo.CFBundleDisplayName = "Converse PREVIEW"; -appInfo.CFBundleURLSchemes = ["converse-preview", "com.converse.preview"]; +validatePlist(appInfo, ['CFBundleURLTypes']); +appInfo.CFBundleDisplayName = PLIST_CONFIG.app.displayName; +appInfo.CFBundleURLSchemes = PLIST_CONFIG.app.urlSchemes;
Line range hint
59-111: Improve entitlements modification safety and maintainability.The entitlements modifications involve complex nested property access without proper validation.
Consider these improvements:
+const DOMAINS = { + dev: { + converse: 'dev.converse.xyz', + getconverse: 'dev.getconverse.app' + }, + preview: { + converse: 'preview.converse.xyz', + getconverse: 'preview.getconverse.app' + } +}; + +const updateEntitlements = (entitlements, environment) => { + const requiredKeys = [ + 'keychain-access-groups', + 'com.apple.security.application-groups', + 'com.apple.developer.associated-domains' + ]; + + // Validate structure + for (const key of requiredKeys) { + if (!Array.isArray(entitlements[key])) { + throw new Error(`Invalid entitlements structure: ${key} should be an array`); + } + } + + // Update bundle IDs + const updateBundleIds = (arr) => { + return arr.map(item => + item.replace(BUNDLE_IDS.dev, BUNDLE_IDS[environment]) + ); + }; + + entitlements['keychain-access-groups'] = + updateBundleIds(entitlements['keychain-access-groups']); + + entitlements['com.apple.security.application-groups'] = + updateBundleIds(entitlements['com.apple.security.application-groups']); + + // Update domains + const domains = entitlements['com.apple.developer.associated-domains']; + domains[0] = domains[0].replace( + `applinks:${DOMAINS.dev.converse}`, + `applinks:${DOMAINS.preview.converse}` + ); + domains[1] = domains[1].replace( + `applinks:${DOMAINS.dev.getconverse}`, + `applinks:${DOMAINS.preview.getconverse}` + ); + + return entitlements; +};
Line range hint
113-113: Improve script execution handling.The script execution lacks proper async/await handling and process management.
-go(); +go() + .catch(error => { + console.error('Script failed:', error); + process.exit(1); + }); + +// Handle process signals +process.on('SIGINT', () => { + console.log('\nReceived SIGINT. Cleaning up...'); + process.exit(0); +});
Firebase AppCheck configuration needs to be added
The script correctly handles preview environment configuration, but Firebase AppCheck configuration is missing. The following items need attention:
- The
Info.plistfile doesn't contain any Firebase AppCheck related entries- While
GoogleService-Info.plistexists and contains Firebase configuration, it lacks AppCheck specific settings- The project has Firebase build scripts and bundles, but no AppCheck specific configurations
🔗 Analysis chain
Line range hint
1-113: Verify Firebase AppCheck configuration.While the script handles preview environment configuration well, ensure that Firebase AppCheck is properly configured in the modified files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Firebase AppCheck configuration exists in relevant files echo "Checking Info.plist for Firebase AppCheck configuration..." rg -l "FirebaseAppCheck" ios/Converse/Info.plist echo "Checking for Firebase configuration files..." find . -name "GoogleService-Info.plist" -o -name "google-services.json"Length of output: 576
Script:
#!/bin/bash # Let's check the content of Info.plist and GoogleService-Info.plist for Firebase configuration echo "Checking Info.plist content..." cat ios/Converse/Info.plist echo -e "\nChecking GoogleService-Info.plist content..." cat ios/GoogleService-Info.plist # Also check if there are any Firebase-related configurations in the project file echo -e "\nChecking Xcode project for Firebase configurations..." rg "Firebase" ios/Converse.xcodeproj/project.pbxprojLength of output: 17176
utils/appCheck.ts (1)
6-37: Add TypeScript types and documentationThe implementation lacks type definitions and documentation.
Consider adding:
- Return type for the run function
- Type for the configuration object
- JSDoc comments explaining the purpose and usage
interface AppCheckConfig { android: { provider: 'debug' | 'playIntegrity'; debugToken?: string; }; apple: { provider: 'debug' | 'appAttestWithDeviceCheckFallback'; debugToken?: string; }; } /** * Initializes and configures Firebase AppCheck * @throws {Error} If token verification fails */ async function run(): Promise<void> { // ... existing implementation }screens/Onboarding/OnboardingEphemeraScreen.tsx (1)
Line range hint
1-48: Consider centralizing AppCheck initializationThe current implementation has several architectural concerns:
- AppCheck initialization is scattered across different screens
- No clear error handling strategy
- Environment-specific configuration is hardcoded
Consider:
- Initializing AppCheck during app startup
- Creating a dedicated AppCheck service/context
- Implementing proper error boundaries
- Using environment configuration management
Would you like help implementing these architectural improvements?
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
queries/useV3ConversationListQuery.ts (1)
33-34: Enhance debug logging for AppCheck verification.The current debug logs don't provide meaningful context about the AppCheck verification process. Consider:
- Adding structured logging with relevant AppCheck verification details
- Including error context when verification fails
- logger.debug(foo()); - logger.debug("v3ConversationListQueryFn---------------------------"); + const appCheckResult = await foo(); + logger.debug({ + event: 'app_check_verification', + result: appCheckResult, + context: 'v3ConversationListQueryFn' + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ios/Converse.xcodeproj/xcshareddata/xcschemes/Converse.xcscheme(1 hunks)ios/Converse/AppDelegate.mm(2 hunks)ios/Podfile(2 hunks)queries/useV3ConversationListQuery.ts(2 hunks)screens/NewAccount/NewAccountEphemeraScreen.tsx(2 hunks)screens/Onboarding/OnboardingEphemeraScreen.tsx(2 hunks)scripts/build/build.js(2 hunks)scripts/build/ios/preview.js(4 hunks)scripts/build/ios/production.js(5 hunks)utils/appCheck.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/Podfile
🔇 Additional comments (5)
ios/Converse.xcodeproj/xcshareddata/xcschemes/Converse.xcscheme (1)
63-68: Verify Firebase debugging is disabled in release builds
The addition of FIRDebugEnabled flag is correctly placed in the LaunchAction section which only affects debug builds. However, let's verify that release builds don't have this flag enabled.
✅ Verification successful
Firebase debugging configuration is correctly scoped to debug builds
The FIRDebugEnabled flag is properly configured:
- Only present in the
LaunchActionsection which is used for debug builds - Not present in
ProfileActionorArchiveActionwhich use Release configuration - Not present in any build settings that could affect release builds
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if Firebase debugging is enabled in release configuration
# Look for FIRDebugEnabled in other scheme configurations or build settings
# Search for FIRDebugEnabled in all xcscheme files
fd -e xcscheme . ios/ --exec grep -l "FIRDebugEnabled" {} \;
# Search for any Firebase debug related settings in build configurations
fd . ios/ -e pbxproj --exec grep -l "FIR.*Debug" {} \;
Length of output: 187
Script:
#!/bin/bash
# Let's examine the full content of the xcscheme file to verify if FIRDebugEnabled
# is only present in LaunchAction and not in Release/Profile configurations
cat ios/Converse.xcodeproj/xcshareddata/xcschemes/Converse.xcscheme
Length of output: 3544
scripts/build/ios/production.js (2)
Line range hint 4-107: Verify integration with deployment workflow
Since this script is crucial for production deployment and Firebase AppCheck integration, please ensure it's properly integrated into the CI/CD pipeline and deployment workflow.
✅ Verification successful
Script is properly integrated in deployment workflows
The script is correctly integrated into both internal builds (.github/workflows/build-internal.yml) and app release builds (.github/workflows/build-app-release.yml), ensuring proper configuration for production deployments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for references to this script in workflow files
fd -e yml -e yaml . .github/workflows | xargs rg "scripts/build/ios/production.js"
# Check for other Firebase-related configuration files
fd -g "*google-services*" -g "*GoogleService-Info*"
Length of output: 420
Line range hint 36-59: Verify URL scheme security implications
The URL schemes converse and com.converse.native are being added to the app's URL types. This could have security implications if not properly validated in the app's deep linking implementation.
Additionally, consider adding validation for the plist parsing results:
-const appInfo = plist.parse(fs.readFileSync(PLIST_APP_PATH, "utf8"));
+let appInfo;
+try {
+ appInfo = plist.parse(fs.readFileSync(PLIST_APP_PATH, "utf8"));
+ if (!appInfo || typeof appInfo !== 'object') {
+ throw new Error('Invalid plist format');
+ }
+} catch (error) {
+ console.error('Failed to parse app Info.plist:', error);
+ process.exit(1);
+}✅ Verification successful
Let me analyze the URL scheme handling implementation based on the search results.
Let me also check for any security-related configurations:
Based on the search results, I can now provide a final response about the URL scheme security implications:
URL scheme implementation is properly secured
The codebase shows a well-structured and secure implementation of URL schemes:
- URL handling is centralized in
utils/navigation.tswith proper validation throughisDMLinkand other similar checks - Deep link handling is properly integrated using React Native's
RCTLinkingManager - The URL schemes (
converseandcom.converse.native) are used consistently across different build configurations (dev/preview/production) - There's a validation layer that:
- Validates universal links against configured prefixes
- Transforms URLs through
getSchemedURLFromUniversalURL - Implements specific handlers for different types of deep links (DM, group invites)
The URL scheme configuration in the Info.plist is safe to proceed with as the implementation shows proper validation and handling of deep links.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for URL scheme handling in the codebase
rg -A 5 "openURL|continueUserActivity|handleOpenURL"
Length of output: 8150
Script:
#!/bin/bash
# Search for URL scheme validation and deep linking implementation
rg -A 10 "isDMLink|getSchemedURLFromUniversalURL|RCTLinkingManager"
Length of output: 5566
Script:
#!/bin/bash
# Look for URL scheme validation or security configurations
rg -A 5 "AUTHORIZED_URL_PROTOCOLS|validateURL|URLScheme"
Length of output: 2742
scripts/build/build.js (2)
12-37: LGTM! Well-implemented EAS CLI installation check.
The implementation includes:
- Proper error handling
- User-friendly interactive installation
- Clear error messages and instructions
192-193: LGTM! Clear file extension determination.
The conditional logic for determining file extensions is well-formatted and easy to understand.
| [RNFBAppCheckModule sharedInstance]; | ||
|
|
||
| [FIRApp configure]; |
There was a problem hiding this comment.
💡 Codebase verification
Fix Firebase initialization order and ensure main thread execution
The environment-specific configuration suggestion should be removed as there's only a single GoogleService-Info.plist file in the project. However, the other concerns about initialization order and thread safety remain valid.
Here's the corrected suggestion:
- [RNFBAppCheckModule sharedInstance];
- [FIRApp configure];
+ dispatch_async(dispatch_get_main_queue(), ^{
+ NSError *error;
+ [FIRApp configure];
+ if ([FIRApp defaultApp]) {
+ [RNFBAppCheckModule sharedInstance];
+ } else {
+ NSLog(@"Failed to initialize Firebase");
+ }
+ });🔗 Analysis chain
Fix Firebase initialization order and add error handling.
There are several issues with the current initialization:
- Firebase must be configured before initializing any Firebase services (including App Check)
- Missing error handling for Firebase configuration
- Missing environment-specific configuration (mentioned in PR objectives)
- Firebase initialization should explicitly run on main thread
Here's the suggested fix:
- [RNFBAppCheckModule sharedInstance];
- [FIRApp configure];
+ dispatch_async(dispatch_get_main_queue(), ^{
+ NSString *environment = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"FIREBASE_ENVIRONMENT"];
+ NSString *plistName = [NSString stringWithFormat:@"GoogleService-Info-%@", environment];
+ NSString *plistPath = [[NSBundle mainBundle] pathForResource:plistName ofType:@"plist"];
+
+ if (plistPath) {
+ NSError *error;
+ FIROptions *options = [[FIROptions alloc] initWithContentsOfFile:plistPath];
+ if (options) {
+ [FIRApp configureWithOptions:options];
+ [RNFBAppCheckModule sharedInstance];
+ } else {
+ NSLog(@"Failed to initialize Firebase with config: %@", error);
+ }
+ } else {
+ NSLog(@"Missing GoogleService-Info plist for environment: %@", environment);
+ }
+ });Let's verify the environment configuration setup:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if environment-specific Firebase configs exist
fd "GoogleService-Info-.+\.plist" ios/
# Check if FIREBASE_ENVIRONMENT is defined in Info.plist
plutil -p ios/Converse/Info.plist | grep FIREBASE_ENVIRONMENT
Length of output: 201
Script:
#!/bin/bash
# Check if environment-specific Firebase configs exist
find ios/ -name "GoogleService-Info-*.plist"
# Check Info.plist content for environment configuration
cat ios/Converse/Info.plist | grep -i "FIREBASE\|environment"
# Check for any Firebase configuration files
find ios/ -name "GoogleService-Info*.plist"
# Look for environment configuration in build settings or xcconfig files
find ios/ -name "*.xcconfig" -exec grep -l "FIREBASE\|environment" {} \;
Length of output: 263
|
|
||
| const fs = require("fs"); | ||
| const plist = require("plist"); | ||
|
|
||
| const go = async () => { |
There was a problem hiding this comment.
Add error handling and improve robustness.
The script lacks error handling and input validation which could lead to silent failures.
Consider these improvements:
const fs = require("fs");
const plist = require("plist");
+const path = require("path");
+
+// Configuration
+const CONFIG = {
+ paths: {
+ project: path.resolve(__dirname, "../../../ios/Converse.xcodeproj/project.pbxproj"),
+ // ... other paths
+ }
+};
-const go = async () => {
+const go = async () => {
+ try {
+ // Validate paths exist before proceeding
+ for (const [key, value] of Object.entries(CONFIG.paths)) {
+ if (!fs.existsSync(value)) {
+ throw new Error(`Required path not found: ${key} (${value})`);
+ }
+ }
+
+ // ... rest of the function
+ } catch (error) {
+ console.error('Failed to update iOS preview configuration:', error);
+ process.exit(1);
+ }
};Committable suggestion skipped: line range outside the PR's diff.
| // | ||
| // Update Xcode Project Bundle ID | ||
| // ---------------------------------------- | ||
| const PROJ_PATH = "ios/Converse.xcodeproj/project.pbxproj"; | ||
| const projContent = fs.readFileSync(PROJ_PATH, "utf-8"); | ||
|
|
||
| const newProjContent = projContent | ||
| .replace( | ||
| /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g, | ||
| "PRODUCT_BUNDLE_IDENTIFIER = com.converse.preview" | ||
| ); | ||
| // Replace dev bundle ID with preview bundle ID | ||
| const newProjContent = projContent.replace( | ||
| /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g, | ||
| "PRODUCT_BUNDLE_IDENTIFIER = com.converse.preview" | ||
| ); | ||
|
|
||
| fs.writeFileSync(PROJ_PATH, newProjContent); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve bundle ID replacement reliability.
The current implementation uses a broad regex replacement without verification and contains hard-coded values.
Consider these improvements:
+const BUNDLE_IDS = {
+ dev: 'com.converse.dev',
+ preview: 'com.converse.preview'
+};
+
+const updateBundleId = (content) => {
+ const newContent = content.replace(
+ new RegExp(`PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_IDS.dev.replace('.', '\\.')}`, 'g'),
+ `PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_IDS.preview}`
+ );
+
+ // Verify replacement occurred
+ if (newContent === content) {
+ throw new Error('Bundle ID replacement failed - no matches found');
+ }
+
+ return newContent;
+};
+
- const newProjContent = projContent.replace(
- /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g,
- "PRODUCT_BUNDLE_IDENTIFIER = com.converse.preview"
- );
+ const newProjContent = updateBundleId(projContent);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // | |
| // Update Xcode Project Bundle ID | |
| // ---------------------------------------- | |
| const PROJ_PATH = "ios/Converse.xcodeproj/project.pbxproj"; | |
| const projContent = fs.readFileSync(PROJ_PATH, "utf-8"); | |
| const newProjContent = projContent | |
| .replace( | |
| /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g, | |
| "PRODUCT_BUNDLE_IDENTIFIER = com.converse.preview" | |
| ); | |
| // Replace dev bundle ID with preview bundle ID | |
| const newProjContent = projContent.replace( | |
| /PRODUCT_BUNDLE_IDENTIFIER = com\.converse\.dev/g, | |
| "PRODUCT_BUNDLE_IDENTIFIER = com.converse.preview" | |
| ); | |
| fs.writeFileSync(PROJ_PATH, newProjContent); | |
| // | |
| // Update Xcode Project Bundle ID | |
| // ---------------------------------------- | |
| const PROJ_PATH = "ios/Converse.xcodeproj/project.pbxproj"; | |
| const projContent = fs.readFileSync(PROJ_PATH, "utf-8"); | |
| const BUNDLE_IDS = { | |
| dev: 'com.converse.dev', | |
| preview: 'com.converse.preview' | |
| }; | |
| const updateBundleId = (content) => { | |
| const newContent = content.replace( | |
| new RegExp(`PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_IDS.dev.replace('.', '\\.')}`, 'g'), | |
| `PRODUCT_BUNDLE_IDENTIFIER = ${BUNDLE_IDS.preview}` | |
| ); | |
| // Verify replacement occurred | |
| if (newContent === content) { | |
| throw new Error('Bundle ID replacement failed - no matches found'); | |
| } | |
| return newContent; | |
| }; | |
| const newProjContent = updateBundleId(projContent); | |
| fs.writeFileSync(PROJ_PATH, newProjContent); |
| debugToken: | ||
| "some token you have configured for your project firebase web console", | ||
| }, |
There was a problem hiding this comment.
Security Risk: Remove hardcoded debug tokens
Debug tokens should not be committed to the codebase as they are sensitive credentials.
Consider:
- Moving tokens to environment variables
- Using a secure secret management system
- Adding these tokens to .gitignore
- debugToken:
- "some token you have configured for your project firebase web console",
+ debugToken: process.env.FIREBASE_DEBUG_TOKEN_ANDROID,Similar change needed for the Apple debug token.
Also applies to: 19-21
| try { | ||
| const { token } = await firebase.appCheck().getToken(true); | ||
|
|
||
| if (token.length > 0) { | ||
| console.log("AppCheck verification passed"); | ||
| } | ||
| } catch (error) { | ||
| console.log(`AppCheck verification failed: ${error}`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and logging
The error handling is incomplete and uses console.log instead of the logger utility.
Consider:
try {
const { token } = await firebase.appCheck().getToken(true);
if (token.length > 0) {
- console.log("AppCheck verification passed");
+ logger.info("AppCheck verification passed");
}
} catch (error) {
- console.log(`AppCheck verification failed: ${error}`);
+ logger.error("AppCheck verification failed:", error);
+ throw error;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const { token } = await firebase.appCheck().getToken(true); | |
| if (token.length > 0) { | |
| console.log("AppCheck verification passed"); | |
| } | |
| } catch (error) { | |
| console.log(`AppCheck verification failed: ${error}`); | |
| } | |
| try { | |
| const { token } = await firebase.appCheck().getToken(true); | |
| if (token.length > 0) { | |
| logger.info("AppCheck verification passed"); | |
| } | |
| } catch (error) { | |
| logger.error("AppCheck verification failed:", error); | |
| throw error; | |
| } |
| export const foo = () => { | ||
| console.log("foo"); | ||
| run() | ||
| .then((res) => { | ||
| console.log("res", res); | ||
| }) | ||
| .catch((err) => { | ||
| console.log("err", err); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve function naming and implementation
The function name 'foo' is non-descriptive and the implementation has several issues:
- Uses console.log instead of the logger utility
- No return type specification
- Doesn't handle the promise result
Consider this implementation:
-export const foo = () => {
- console.log("foo");
- run()
- .then((res) => {
- console.log("res", res);
- })
- .catch((err) => {
- console.log("err", err);
- });
-};
+export const initializeAppCheck = async (): Promise<void> => {
+ try {
+ await run();
+ logger.debug('AppCheck initialized successfully');
+ } catch (error) {
+ logger.error('AppCheck initialization failed:', error);
+ throw error;
+ }
+};Committable suggestion skipped: line range outside the PR's diff.
| foo(); | ||
| return; |
There was a problem hiding this comment.
Critical: Early return breaks wallet generation
The early return after calling foo() prevents the wallet generation functionality from executing. This appears to be temporary debugging code that shouldn't be committed.
Remove the early return and properly integrate AppCheck:
- foo();
- return;
+ await initializeAppCheck();
try {
const signer = new Wallet(utils.randomPrivateKey());Committable suggestion skipped: line range outside the PR's diff.
| @@ -21,6 +21,7 @@ import { VStack } from "../../design-system/VStack"; | |||
| import { useRouter } from "../../navigation/useNavigation"; | |||
| import { sentryTrackError } from "../../utils/sentry"; | |||
| import { NavigationParamList } from "../Navigation/Navigation"; | |||
| import { foo } from "@/utils/appCheck"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update import after renaming foo function
Once the foo function is renamed to initializeAppCheck, update the import and usage.
-import { foo } from "@/utils/appCheck";
+import { initializeAppCheck } from "@/utils/appCheck";Also applies to: 34-34
| foo(); | ||
| return; |
There was a problem hiding this comment.
Critical: Early return breaks onboarding flow
The early return after calling foo() prevents the onboarding flow from executing. This appears to be temporary debugging code that shouldn't be committed.
Remove the early return and properly integrate AppCheck:
- foo();
- return;
+ setLoading(true);
+ try {
+ await initializeAppCheck();
const signer = new Wallet(utils.randomPrivateKey());Committable suggestion skipped: line range outside the PR's diff.
| @@ -15,6 +15,7 @@ import { setGroupNameQueryData } from "./useGroupNameQuery"; | |||
| import { setGroupPhotoQueryData } from "./useGroupPhotoQuery"; | |||
| import { ConversationTopic, ConversationVersion } from "@xmtp/react-native-sdk"; | |||
| import { useAppStore } from "@/data/store/appStore"; | |||
| import { foo } from "@/utils/appCheck"; | |||
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Rename foo to initializeAppCheck to accurately reflect its Firebase AppCheck initialization purpose
The function foo initializes Firebase AppCheck with platform-specific providers and handles token verification. The current name doesn't convey its critical security functionality. Renaming suggestions:
- In
utils/appCheck.ts: RenamefootoinitializeAppCheck - Update imports in:
queries/useV3ConversationListQuery.tsscreens/Onboarding/OnboardingEphemeraScreen.tsxscreens/NewAccount/NewAccountEphemeraScreen.tsx
🔗 Analysis chain
Rename the foo function to better describe its purpose.
The function name foo is non-descriptive. Consider renaming it to reflect its Firebase AppCheck verification purpose, such as verifyAppCheckToken or validateAppOrigin.
Let's check how this function is used across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all usages of the foo function to understand its purpose
rg "foo\(" --type ts
Length of output: 203
Script:
#!/bin/bash
# Let's check the implementation of foo to understand its purpose
ast-grep --pattern 'export function foo() {
$$$
}'
# Also check the imports of foo in these files to see the context
rg "import.*foo" --type ts
Length of output: 337
Script:
#!/bin/bash
# Let's check the content of appCheck.ts to understand the implementation
rg -A 5 "export function foo" utils/appCheck.ts
# Also check if there are any related AppCheck functions in the same file
rg "function.*AppCheck|AppCheck.*function" utils/appCheck.ts
Length of output: 112
Script:
#!/bin/bash
# Let's try finding the file first as the path might be different
fd appCheck.ts
# Also check for any AppCheck related files
fd -i appcheck
Length of output: 69
Script:
#!/bin/bash
# Let's check the content of the file directly
cat utils/appCheck.ts
# Also check for any Firebase AppCheck related imports
rg "import.*firebase.*appCheck|import.*AppCheck" --type ts
Length of output: 1464
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
ios/Converse.xcodeproj/project.pbxproj (1)
Line range hint
1-1000: Consider trade-offs between Firebase AppCheck and native APIsThe PR author raised valid concerns about using Firebase AppCheck versus native APIs (AppAttest/DeviceCheck/AppIntegrity). Consider the following trade-offs:
Pros of Firebase AppCheck:
- Unified API across platforms
- Managed token lifecycle
- Integration with other Firebase services
Pros of native APIs:
- Direct platform integration
- Reduced dependency complexity
- More control over implementation
For environment handling, consider:
- Using different GoogleService-Info.plist files per environment
- Configuring AppCheck rules in Firebase Console per environment
- Implementing proper debug provider handling for development
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
features/conversation/conversation-message-sender-avatar.tsx(0 hunks)ios/Converse.xcodeproj/project.pbxproj(8 hunks)package.json(2 hunks)queries/useV3ConversationListQuery.ts(2 hunks)utils/logout/index.tsx(2 hunks)utils/mmkv.ts(1 hunks)
💤 Files with no reviewable changes (1)
- features/conversation/conversation-message-sender-avatar.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- queries/useV3ConversationListQuery.ts
- package.json
🔇 Additional comments (5)
ios/Converse.xcodeproj/project.pbxproj (2)
424-436: Firebase Core Configuration script requires firebase.json
The Firebase Core Configuration build phase is properly added, but as noted in a previous review, the firebase.json configuration file is required for proper setup of AppCheck token auto refresh.
455-459: LGTM: Firebase privacy bundles properly integrated
The required Firebase privacy bundles are correctly added to the Copy Pods Resources build phases.
Also applies to: 485-489
utils/mmkv.ts (2)
14-17: LGTM: Well-structured auth storage setup
Good separation of concerns by creating a dedicated MMKV instance for auth storage with clear type definitions for token keys.
18-35: Consider adding token expiration handling
The auth storage implementation looks solid with proper memory leak prevention. However, since this is being added for Firebase AppCheck integration, consider adding methods to handle token expiration:
- Add expiration timestamp storage alongside tokens
- Add method to check if tokens are expired
- Add method to refresh tokens before expiry
This would help prevent authentication issues and provide a smoother user experience.
Let's check if token expiration is handled elsewhere:
utils/logout/index.tsx (1)
16-20: LGTM: Clean import organization
The imports are well-organized, keeping related storage utilities together.
| @@ -7,6 +7,7 @@ | |||
| objects = { | |||
|
|
|||
| /* Begin PBXBuildFile section */ | |||
| 11C4BAB72CE3B75400915C95 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 11C4BAB62CE3B75400915C95 /* GoogleService-Info.plist */; }; | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing AppCheck configuration in GoogleService-Info.plist
The GoogleService-Info.plist file exists but does not contain the required AppCheck configuration (AppCheckTokenAutoRefreshEnabled key). This configuration is necessary for Firebase AppCheck to function properly.
- Add the AppCheck configuration to
ios/GoogleService-Info.plist:<key>AppCheckTokenAutoRefreshEnabled</key> <true/>
🔗 Analysis chain
Verify GoogleService-Info.plist configuration and location
The GoogleService-Info.plist file is properly integrated into the Xcode project, but we should verify:
- The file exists at the correct location
- The file contains proper AppCheck configuration
Also applies to: 66-66, 191-191, 348-348
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if GoogleService-Info.plist exists and contains AppCheck configuration
if [ -f "ios/GoogleService-Info.plist" ]; then
echo "GoogleService-Info.plist found"
/usr/libexec/PlistBuddy -c "Print :AppCheckTokenAutoRefreshEnabled" "ios/GoogleService-Info.plist" 2>/dev/null || echo "AppCheck configuration not found in plist"
else
echo "Warning: GoogleService-Info.plist not found"
fi
Length of output: 304
| @@ -231,7 +235,7 @@ export const logoutAccount = async ( | |||
| saveLogoutTask(account, apiHeaders, [], pkPath); | |||
|
|
|||
| setTimeout(() => { | |||
| executeLogoutTasks(); | |||
| executeLogoutTasks().then(authMMKVStorage.clear); | |||
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
Consider error handling for auth storage clearing
The review suggestion is valid. Looking at the codebase:
authMMKVStorage.clear()directly callsauthMMKV.clearAll()without any error handling- Other MMKV operations in the codebase (like
clearSecureMmkvForAccount) implement error handling with logger executeLogoutTasksalready has error handling for its operations
The suggested implementation would improve error handling and maintain consistency with other MMKV operations in the codebase.
- executeLogoutTasks().then(authMMKVStorage.clear);
+ try {
+ await executeLogoutTasks();
+ await authMMKVStorage.clear();
+ } catch (error) {
+ logger.error(error, {
+ context: '[Logout] Failed to clear auth storage',
+ });
+ }🔗 Analysis chain
Consider error handling for auth storage clearing
The auth storage clearing is chained to executeLogoutTasks() but lacks error handling. Consider:
- Adding error handling for the auth storage clearing
- Moving the auth storage clearing inside
executeLogoutTasksfor atomic operations
Here's a suggested implementation:
- executeLogoutTasks().then(authMMKVStorage.clear);
+ try {
+ await executeLogoutTasks();
+ await authMMKVStorage.clear();
+ } catch (error) {
+ logger.error(error, {
+ context: '[Logout] Failed to clear auth storage',
+ });
+ }Let's check if there are similar patterns elsewhere in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar error handling patterns in logout-related code
rg "catch.*error.*context.*logout" -A 3
Length of output: 41
Script:
#!/bin/bash
# Let's first check the implementation of executeLogoutTasks to understand the current error handling
ast-grep --pattern 'const executeLogoutTasks = async () => { $$$ }'
# Also search for any error handling patterns in logout-related files
rg "catch.*error" --type ts --type tsx
# Let's also check how authMMKVStorage is used elsewhere
rg "authMMKVStorage" -A 2
Length of output: 3468
Script:
#!/bin/bash
# Let's check the implementation of authMMKVStorage.clear to understand its error handling
ast-grep --pattern 'clear() { $$$ }' utils/mmkv.ts
# Let's also check for any existing error handling patterns in mmkv.ts
rg "catch" utils/mmkv.ts -A 3
# And check if clearSecureMmkvForAccount has error handling since it's related
ast-grep --pattern 'clearSecureMmkvForAccount = async (account: string) => { $$$ }'
Length of output: 248
Script:
#!/bin/bash
# Let's check the entire authMMKVStorage object implementation
rg -A 10 "export const authMMKVStorage = {" utils/mmkv.ts
# And check if there are any other clear operations with error handling
rg "clear.*{" -A 5 utils/mmkv.ts
Length of output: 302
Script:
#!/bin/bash
# Let's try a simpler approach to view the file content
cat utils/mmkv.ts
# And also check for any logger usage patterns in mmkv operations
rg "logger\." utils/mmkv.ts
Length of output: 3022
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
ios/ConverseNotificationExtension/ConverseNotificationExtension.entitlements (1)
11-12: Ensure keychain migration strategy for existing usersThe keychain access group change from dev to preview environment might affect existing users' stored credentials.
Consider implementing a migration strategy if there's sensitive data in the old keychain access group that needs to be preserved.
ios/Converse/Converse.entitlements (1)
4-23: Comprehensive environment transition checklistPlease ensure all these items are addressed for a successful transition to the preview environment:
- Update provisioning profiles with new bundle IDs
- Configure Firebase project for preview environment
- Deploy apple-app-site-association files to preview domains
- Update push notification certificates
- Test deep linking with new URLs
Consider documenting the environment setup process to facilitate future environment additions or transitions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
eas.json(2 hunks)ios/Converse.xcodeproj/project.pbxproj(14 hunks)ios/Converse/Converse.entitlements(1 hunks)ios/Converse/Info.plist(1 hunks)ios/ConverseNotificationExtension/ConverseNotificationExtension.entitlements(1 hunks)ios/ConverseNotificationExtension/Info.plist(1 hunks)
🔇 Additional comments (7)
ios/ConverseNotificationExtension/ConverseNotificationExtension.entitlements (1)
7-8: Verify application group identifier consistency across the app
The application group identifier has been updated to group.com.converse.preview. This change needs to be consistent across all app targets and configurations.
✅ Verification successful
App group identifier is consistent across entitlements files
The app group identifier group.com.converse.preview and keychain access group $(AppIdentifierPrefix)com.converse.preview are consistently configured in both entitlements files:
- ios/Converse/Converse.entitlements
- ios/ConverseNotificationExtension/ConverseNotificationExtension.entitlements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old dev group identifier
rg "group\.com\.converse\.dev" --type xml
Length of output: 43
Script:
#!/bin/bash
# Search for any app group identifiers in entitlements and configuration files
rg "group\.com\.converse\." --type xml
# Also search for keychain access groups since they were mentioned
rg "keychain-access-group" --type xml
Length of output: 78
Script:
#!/bin/bash
# Let's try finding all entitlements files first
fd "\.entitlements$"
# And then directly look at their contents
fd "\.entitlements$" -x cat {}
Length of output: 1397
ios/ConverseNotificationExtension/Info.plist (1)
13-15: Verify bundle identifier consistency across build configurations
The bundle identifier has been updated to com.converse.preview. This change must be reflected in:
- Xcode project configuration
- Firebase configuration
- Push notification certificates
✅ Verification successful
Bundle identifier is consistently set to preview environment
Based on the verification results:
- The notification extension's Info.plist has
com.converse.previewbundle ID - The main app's Firebase configuration (GoogleService-Info.plist) matches with
com.converse.previewbundle ID - The main app's Info.plist correctly references the preview bundle ID in URL schemes
- Only one legacy reference to
com.converse.devremains in Android configuration, which is unrelated to iOS bundle identifiers
The bundle identifier change has been properly implemented across the iOS configuration files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old dev bundle identifier
rg "com\.converse\.dev" --type xml
Length of output: 127
Script:
#!/bin/bash
# Search for all bundle identifier variations in the iOS configuration
rg "com\.converse\." --type xml ios/
# Search for Firebase configuration files
fd "GoogleService-Info\.plist" ios/
Length of output: 105
Script:
#!/bin/bash
# Check the bundle identifier in Firebase configuration
cat ios/GoogleService-Info.plist
# Check for any other bundle identifiers in iOS project files
fd ".pbxproj|.xcconfig" ios/ --exec cat {}
Length of output: 66502
Script:
#!/bin/bash
# Check for bundle identifier in main app Info.plist
cat ios/Converse/Info.plist
# Check for any other potential bundle identifier references in project files
rg "com\.converse\." --type=plist
Length of output: 4178
ios/Converse/Converse.entitlements (1)
5-6: Review APS environment configuration
The aps-environment is set to "development". Given this is a preview environment:
- Verify if this is intentional
- Ensure proper push notification certificates are configured
✅ Verification successful
APS environment configuration is correctly set for preview environment
The aps-environment being set to "development" is correct for a preview environment. This setting is consistent with other preview-related configurations in the entitlements file, such as the app group (group.com.converse.preview), keychain access group, and associated domains (preview.converse.xyz, preview.getconverse.app). No changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other APS environment configurations
rg "aps-environment" --type xml
Length of output: 31
Script:
#!/bin/bash
# Search for entitlements files which typically contain APS configurations
fd -e entitlements -x cat {}
Length of output: 1260
eas.json (1)
49-58:
Review sensitive information in environment variables
Several environment variables contain sensitive information that should be handled more securely:
- API keys (WALLETCONNECT_PROJECT_ID, THIRDWEB_CLIENT_ID)
- Contract addresses
- RPC endpoints
- Project identifiers
Consider:
- Moving sensitive values to a secure secret management system
- Using placeholder values in the configuration file
ios/Converse.xcodeproj/project.pbxproj (3)
10-10: Missing AppCheck configuration in GoogleService-Info.plist
Also applies to: 66-66, 191-191, 348-348
670-670: Bundle identifier changes look good
The bundle identifier updates for the preview environment are consistent across all targets.
Also applies to: 708-708, 752-752, 796-796
424-436: Verify Firebase Core Configuration script
The RNFB Core Configuration script is added, but additional Firebase setup steps are needed:
- Ensure firebase.json exists with proper AppCheck configuration
- Configure AppCheck providers in the application code
- Set up proper error handling for AppCheck token verification
| <string>applinks:preview.converse.xyz</string> | ||
| <string>applinks:preview.getconverse.app</string> |
There was a problem hiding this comment.
💡 Codebase verification
Domain configuration inconsistency detected between iOS and Android
The iOS app has been configured with preview domains:
applinks:preview.converse.xyzapplinks:preview.getconverse.app
However, the Android app still contains references to dev domains in AndroidManifest.xml:
dev.converse.xyz/dmdev.converse.xyz/group-invitedev.converse.xyz/groupdev.converse.xyz/coinbasedev.converse.xyz/desktopconnect
Please update the Android configuration to match the iOS preview domains for consistent deep linking behavior across platforms.
🔗 Analysis chain
Verify associated domains configuration
The associated domains have been updated to:
applinks:preview.converse.xyzapplinks:preview.getconverse.app
Ensure that:
- The domains are properly configured in the Apple Developer portal
- The corresponding
apple-app-site-associationfiles are deployed
Let me check if there are any other domain configurations in the iOS app to ensure consistency.
Let me try a different approach to check the iOS configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to dev domains
rg "dev\.converse\.(xyz|app)" --type xml
Length of output: 758
Script:
#!/bin/bash
# Search for all domain configurations in iOS entitlements and Info.plist
rg "converse\.(xyz|app)" --type xml ios/
Length of output: 42
Script:
#!/bin/bash
# List and show content of iOS entitlements and Info.plist files
fd -e entitlements -e plist . ios/ -x cat {}
Length of output: 7584
| <key>CFBundleName</key> | ||
| <string>$(PRODUCT_NAME)</string> | ||
| <key>CFBundlePackageType</key> | ||
| <string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string> | ||
| <key>CFBundleShortVersionString</key> | ||
| <string>1.0.0</string> | ||
| <key>CFBundleSignature</key> | ||
| <string>????</string> | ||
| <key>CFBundleURLSchemes</key> | ||
| <array> | ||
| <string>converse-preview</string> | ||
| <string>com.converse.preview</string> | ||
| </array> | ||
| <key>CFBundleURLTypes</key> | ||
| <array> | ||
| <dict> | ||
| <key>CFBundleTypeRole</key> | ||
| <string>Editor</string> | ||
| <key>CFBundleURLSchemes</key> | ||
| <array> | ||
| <string>converse-preview</string> | ||
| <string>com.converse.preview</string> | ||
| </array> | ||
| </dict> | ||
| </array> | ||
| <key>CFBundleVersion</key> | ||
| <string>13</string> | ||
| <key>ITSAppUsesNonExemptEncryption</key> | ||
| <false/> | ||
| <key>LSApplicationQueriesSchemes</key> | ||
| <array> | ||
| <string>cbwallet</string> | ||
| <string>ledgerlive</string> | ||
| <string>rainbow</string> | ||
| <string>metamask</string> | ||
| <string>trust</string> | ||
| <string>uniswap</string> | ||
| <string>zerion</string> | ||
| <string>exodus</string> | ||
| <string>oneinch</string> | ||
| </array> | ||
| <key>LSRequiresIPhoneOS</key> | ||
| <true/> | ||
| <key>NSAppTransportSecurity</key> | ||
| <dict> | ||
| <key>NSAllowsArbitraryLoads</key> | ||
| <false/> | ||
| <key>NSAllowsLocalNetworking</key> | ||
| <true/> | ||
| </dict> | ||
| <key>NSCameraUsageDescription</key> | ||
| <string>We need this so that you can take photos to share.</string> | ||
| <key>NSPhotoLibraryUsageDescription</key> | ||
| <string>We need this so that you can share photos from your library.</string> | ||
| <key>NSContactsUsageDescription</key> | ||
| <string>We need this to suggest contacts you know.</string> | ||
| <key>NSFaceIDUsageDescription</key> | ||
| <string>We need this to use biometrics to secure your data.</string> | ||
| <key>NSMicrophoneUsageDescription</key> | ||
| <string>We need this to record audio messages.</string> | ||
| <key>NSUserActivityTypes</key> | ||
| <array> | ||
| <string>INSendMessageIntent</string> | ||
| </array> | ||
| <key>OSLogPreferences</key> | ||
| <dict> | ||
| <key>Enable-Private-Data</key> | ||
| <true/> | ||
| </dict> | ||
| <key>UIBackgroundModes</key> | ||
| <array> | ||
| <string>fetch</string> | ||
| </array> | ||
| <key>UILaunchStoryboardName</key> | ||
| <string>SplashScreen</string> | ||
| <key>UIRequiredDeviceCapabilities</key> | ||
| <array> | ||
| <string>arm64</string> | ||
| </array> | ||
| <key>UIRequiresFullScreen</key> | ||
| <false/> | ||
| <key>UIStatusBarStyle</key> | ||
| <string>UIStatusBarStyleDefault</string> | ||
| <key>UISupportedInterfaceOrientations</key> | ||
| <array> | ||
| <string>UIInterfaceOrientationPortrait</string> | ||
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | ||
| </array> | ||
| <key>UISupportedInterfaceOrientations~ipad</key> | ||
| <array> | ||
| <string>UIInterfaceOrientationPortrait</string> | ||
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | ||
| <string>UIInterfaceOrientationLandscapeLeft</string> | ||
| <string>UIInterfaceOrientationLandscapeRight</string> | ||
| </array> | ||
| <key>UIUserInterfaceStyle</key> | ||
| <string>Automatic</string> | ||
| <key>UIViewControllerBasedStatusBarAppearance</key> | ||
| <false/> | ||
| </dict> |
There was a problem hiding this comment.
Missing Firebase configuration in Info.plist
While the bundle identifier and URL schemes are properly configured for the preview environment, the Info.plist is missing required Firebase configuration keys:
- FirebaseAppCheckDebugEnabled (for development)
- FirebaseAutomaticScreenReportingEnabled
- FirebaseMessagingAutoInitEnabled
Add the following configuration to the Info.plist:
+ <key>FirebaseAppCheckDebugEnabled</key>
+ <false/>
+ <key>FirebaseAutomaticScreenReportingEnabled</key>
+ <true/>
+ <key>FirebaseMessagingAutoInitEnabled</key>
+ <true/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <dict> | |
| <key>LSMinimumSystemVersion</key> | |
| <string>12.0</string> | |
| <key>AppGroup</key> | |
| <string>group.$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>AppGroupName</key> | |
| <string>group.$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>CADisableMinimumFrameDurationOnPhone</key> | |
| <true/> | |
| <key>CFBundleDevelopmentRegion</key> | |
| <string>$(DEVELOPMENT_LANGUAGE)</string> | |
| <key>CFBundleDisplayName</key> | |
| <string>Converse PREVIEW</string> | |
| <key>CFBundleExecutable</key> | |
| <string>$(EXECUTABLE_NAME)</string> | |
| <key>CFBundleIdentifier</key> | |
| <string>$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>CFBundleInfoDictionaryVersion</key> | |
| <string>6.0</string> | |
| <key>CFBundleName</key> | |
| <string>$(PRODUCT_NAME)</string> | |
| <key>CFBundlePackageType</key> | |
| <string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string> | |
| <key>CFBundleShortVersionString</key> | |
| <string>1.0.0</string> | |
| <key>CFBundleSignature</key> | |
| <string>????</string> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>converse-preview</string> | |
| <string>com.converse.preview</string> | |
| </array> | |
| <key>CFBundleURLTypes</key> | |
| <array> | |
| <dict> | |
| <key>CFBundleTypeRole</key> | |
| <string>Editor</string> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>converse-preview</string> | |
| <string>com.converse.preview</string> | |
| </array> | |
| </dict> | |
| </array> | |
| <key>CFBundleVersion</key> | |
| <string>13</string> | |
| <key>ITSAppUsesNonExemptEncryption</key> | |
| <false/> | |
| <key>LSApplicationQueriesSchemes</key> | |
| <array> | |
| <string>cbwallet</string> | |
| <string>ledgerlive</string> | |
| <string>rainbow</string> | |
| <string>metamask</string> | |
| <string>trust</string> | |
| <string>uniswap</string> | |
| <string>zerion</string> | |
| <string>exodus</string> | |
| <string>oneinch</string> | |
| </array> | |
| <key>LSRequiresIPhoneOS</key> | |
| <true/> | |
| <key>NSAppTransportSecurity</key> | |
| <dict> | |
| <key>NSAllowsArbitraryLoads</key> | |
| <false/> | |
| <key>NSAllowsLocalNetworking</key> | |
| <true/> | |
| </dict> | |
| <key>NSCameraUsageDescription</key> | |
| <string>We need this so that you can take photos to share.</string> | |
| <key>NSPhotoLibraryUsageDescription</key> | |
| <string>We need this so that you can share photos from your library.</string> | |
| <key>NSContactsUsageDescription</key> | |
| <string>We need this to suggest contacts you know.</string> | |
| <key>NSFaceIDUsageDescription</key> | |
| <string>We need this to use biometrics to secure your data.</string> | |
| <key>NSMicrophoneUsageDescription</key> | |
| <string>We need this to record audio messages.</string> | |
| <key>NSUserActivityTypes</key> | |
| <array> | |
| <string>INSendMessageIntent</string> | |
| </array> | |
| <key>OSLogPreferences</key> | |
| <dict> | |
| <key>Enable-Private-Data</key> | |
| <true/> | |
| </dict> | |
| <key>UIBackgroundModes</key> | |
| <array> | |
| <string>fetch</string> | |
| </array> | |
| <key>UILaunchStoryboardName</key> | |
| <string>SplashScreen</string> | |
| <key>UIRequiredDeviceCapabilities</key> | |
| <array> | |
| <string>arm64</string> | |
| </array> | |
| <key>UIRequiresFullScreen</key> | |
| <false/> | |
| <key>UIStatusBarStyle</key> | |
| <string>UIStatusBarStyleDefault</string> | |
| <key>UISupportedInterfaceOrientations</key> | |
| <array> | |
| <string>UIInterfaceOrientationPortrait</string> | |
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | |
| </array> | |
| <key>UISupportedInterfaceOrientations~ipad</key> | |
| <array> | |
| <string>UIInterfaceOrientationPortrait</string> | |
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | |
| <string>UIInterfaceOrientationLandscapeLeft</string> | |
| <string>UIInterfaceOrientationLandscapeRight</string> | |
| </array> | |
| <key>UIUserInterfaceStyle</key> | |
| <string>Automatic</string> | |
| <key>UIViewControllerBasedStatusBarAppearance</key> | |
| <false/> | |
| </dict> | |
| <dict> | |
| <key>LSMinimumSystemVersion</key> | |
| <string>12.0</string> | |
| <key>AppGroup</key> | |
| <string>group.$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>AppGroupName</key> | |
| <string>group.$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>CADisableMinimumFrameDurationOnPhone</key> | |
| <true/> | |
| <key>CFBundleDevelopmentRegion</key> | |
| <string>$(DEVELOPMENT_LANGUAGE)</string> | |
| <key>CFBundleDisplayName</key> | |
| <string>Converse PREVIEW</string> | |
| <key>CFBundleExecutable</key> | |
| <string>$(EXECUTABLE_NAME)</string> | |
| <key>CFBundleIdentifier</key> | |
| <string>$(PRODUCT_BUNDLE_IDENTIFIER)</string> | |
| <key>CFBundleInfoDictionaryVersion</key> | |
| <string>6.0</string> | |
| <key>CFBundleName</key> | |
| <string>$(PRODUCT_NAME)</string> | |
| <key>CFBundlePackageType</key> | |
| <string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string> | |
| <key>CFBundleShortVersionString</key> | |
| <string>1.0.0</string> | |
| <key>CFBundleSignature</key> | |
| <string>????</string> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>converse-preview</string> | |
| <string>com.converse.preview</string> | |
| </array> | |
| <key>CFBundleURLTypes</key> | |
| <array> | |
| <dict> | |
| <key>CFBundleTypeRole</key> | |
| <string>Editor</string> | |
| <key>CFBundleURLSchemes</key> | |
| <array> | |
| <string>converse-preview</string> | |
| <string>com.converse.preview</string> | |
| </array> | |
| </dict> | |
| </array> | |
| <key>CFBundleVersion</key> | |
| <string>13</string> | |
| <key>FirebaseAppCheckDebugEnabled</key> | |
| <false/> | |
| <key>FirebaseAutomaticScreenReportingEnabled</key> | |
| <true/> | |
| <key>FirebaseMessagingAutoInitEnabled</key> | |
| <true/> | |
| <key>ITSAppUsesNonExemptEncryption</key> | |
| <false/> | |
| <key>LSApplicationQueriesSchemes</key> | |
| <array> | |
| <string>cbwallet</string> | |
| <string>ledgerlive</string> | |
| <string>rainbow</string> | |
| <string>metamask</string> | |
| <string>trust</string> | |
| <string>uniswap</string> | |
| <string>zerion</string> | |
| <string>exodus</string> | |
| <string>oneinch</string> | |
| </array> | |
| <key>LSRequiresIPhoneOS</key> | |
| <true/> | |
| <key>NSAppTransportSecurity</key> | |
| <dict> | |
| <key>NSAllowsArbitraryLoads</key> | |
| <false/> | |
| <key>NSAllowsLocalNetworking</key> | |
| <true/> | |
| </dict> | |
| <key>NSCameraUsageDescription</key> | |
| <string>We need this so that you can take photos to share.</string> | |
| <key>NSPhotoLibraryUsageDescription</key> | |
| <string>We need this so that you can share photos from your library.</string> | |
| <key>NSContactsUsageDescription</key> | |
| <string>We need this to suggest contacts you know.</string> | |
| <key>NSFaceIDUsageDescription</key> | |
| <string>We need this to use biometrics to secure your data.</string> | |
| <key>NSMicrophoneUsageDescription</key> | |
| <string>We need this to record audio messages.</string> | |
| <key>NSUserActivityTypes</key> | |
| <array> | |
| <string>INSendMessageIntent</string> | |
| </array> | |
| <key>OSLogPreferences</key> | |
| <dict> | |
| <key>Enable-Private-Data</key> | |
| <true/> | |
| </dict> | |
| <key>UIBackgroundModes</key> | |
| <array> | |
| <string>fetch</string> | |
| </array> | |
| <key>UILaunchStoryboardName</key> | |
| <string>SplashScreen</string> | |
| <key>UIRequiredDeviceCapabilities</key> | |
| <array> | |
| <string>arm64</string> | |
| </array> | |
| <key>UIRequiresFullScreen</key> | |
| <false/> | |
| <key>UIStatusBarStyle</key> | |
| <string>UIStatusBarStyleDefault</string> | |
| <key>UISupportedInterfaceOrientations</key> | |
| <array> | |
| <string>UIInterfaceOrientationPortrait</string> | |
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | |
| </array> | |
| <key>UISupportedInterfaceOrientations~ipad</key> | |
| <array> | |
| <string>UIInterfaceOrientationPortrait</string> | |
| <string>UIInterfaceOrientationPortraitUpsideDown</string> | |
| <string>UIInterfaceOrientationLandscapeLeft</string> | |
| <string>UIInterfaceOrientationLandscapeRight</string> | |
| </array> | |
| <key>UIUserInterfaceStyle</key> | |
| <string>Automatic</string> | |
| <key>UIViewControllerBasedStatusBarAppearance</key> | |
| <false/> | |
| </dict> |
Description
NB: @technoplato is taking this over as of December 11
Adds dependencies required to use firebase AppCheck.
TODOs
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores
package.jsonfor Firebase functionalities.