Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add firebase appcheck dep #1269

Closed
wants to merge 27 commits into from
Closed

chore: add firebase appcheck dep #1269

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7e76a17
Install firebase deps
Koleok Nov 7, 2024
081a4a1
IOS setup
Koleok Nov 12, 2024
33cf1da
Android setup
Koleok Nov 13, 2024
e7f8d16
Add expo build properties to dev deps
Koleok Nov 14, 2024
5020135
Update RN sdk version to get signature method
Koleok Nov 19, 2024
9efe115
Resolve bad rebase results
Koleok Nov 20, 2024
da3b245
WIP auth header conversion
Koleok Nov 27, 2024
d41ec83
Remove web3 module
Koleok Dec 2, 2024
4d01e3a
Update sdk usage for client/consent/conversation
Koleok Dec 3, 2024
07f339e
Bring back token refresh/retry logic
Koleok Dec 6, 2024
ecd6480
Break import dependency cycle
Koleok Dec 9, 2024
3b405a1
Update RN sdk version to get signature method
Koleok Nov 19, 2024
c81b6ee
Resolve bad rebase results
Koleok Nov 20, 2024
88d08c1
WIP auth header conversion
Koleok Nov 27, 2024
b2b680b
Remove web3 module
Koleok Dec 2, 2024
580f152
Update sdk usage for client/consent/conversation
Koleok Dec 3, 2024
7719deb
Bring back token refresh/retry logic
Koleok Dec 6, 2024
4d9c3f8
Break import dependency cycle
Koleok Dec 9, 2024
5f6e52e
Merge branch 'kc/33-converse-jwt-auth' of github.com:ephemeraHQ/conve…
technoplato Dec 10, 2024
90ecb17
Merge branch 'kc/33-converse-jwt-auth' into kc/33-add-firebase-appcheck
technoplato Dec 11, 2024
aeb5055
app builds after merge
technoplato Dec 11, 2024
739d194
comments/debugging
technoplato Dec 11, 2024
f00427e
feat: improve build script to install eas if user didn't have installed
technoplato Dec 11, 2024
6fcf2f5
temp test
technoplato Dec 12, 2024
470dff4
Merge branch 'release/3.0.0' into kc/33-add-firebase-appcheck
technoplato Dec 12, 2024
92af7ce
clean and pod install
technoplato Dec 12, 2024
72e4120
[revertme] yarn build > select preview ios > get these changes
technoplato Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions android/google-services.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"project_info": {
"project_number": "23052121755",
"project_id": "converse-appcheck",
"storage_bucket": "converse-appcheck.firebasestorage.app"
},
"client": [
{
"client_info": {
"mobilesdk_app_id": "1:23052121755:android:1787e5971de11f3d13f83e",
"android_client_info": {
"package_name": "com.converse.dev"
}
},
"oauth_client": [],
"api_key": [
{
"current_key": "AIzaSyCYQ3mVhPmGWFYvX9SpEKN4cA6XpdsOC9I"
}
],
"services": {
"appinvite_service": {
"other_platform_oauth_client": []
}
}
}
],
"configuration_version": "1"
}
8 changes: 8 additions & 0 deletions app.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@

import appBuildNumbers from "./app.json";

const env = process.env as any;

Check warning on line 6 in app.config.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
const isDev = env.EXPO_ENV === "dev";

warnOnce(
isDev && !(process.env as any).EXPO_PUBLIC_DEV_API_URI,

Check warning on line 10 in app.config.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
"\n\n🚧 Running the app without EXPO_PUBLIC_DEV_API_URI setup\n\n"
);

const isPreview = env.EXPO_ENV === "preview";
const isProduction = !isDev && !isPreview;

export default ({ config }: ConfigContext): ExpoConfig => ({

Check warning on line 17 in app.config.ts

View workflow job for this annotation

GitHub Actions / lint 

Prefer named exports
...config,
name: isDev ? "Converse DEV" : isPreview ? "Converse PREVIEW" : "Converse",
scheme: isDev ? "converse-dev" : isPreview ? "converse-preview" : "converse",
Expand All @@ -33,19 +33,27 @@
},
version: appBuildNumbers.expo.version,
assetBundlePatterns: ["**/*"],
plugins: [
"@react-native-firebase/app-check",
["expo-build-properties", { ios: { useFrameworks: "static" } }],
],
ios: {
supportsTablet: true,
buildNumber: appBuildNumbers.expo.ios.buildNumber,
googleServicesFile: "./ios/GoogleService-Info.plist",
bundleIdentifier: "com.converse.dev",
config: {
usesNonExemptEncryption: false,
},
},
android: {
package: "com.converse.dev",
adaptiveIcon: {
foregroundImage: "./assets/adaptive-icon.png",
backgroundColor: "#FFFFFF",
},
versionCode: appBuildNumbers.expo.android.versionCode,
googleServicesFile: "./android/google-services.json",
},
web: {
favicon: "./assets/favicon.png",
Expand Down
3 changes: 0 additions & 3 deletions features/conversation/conversation-message-sender-avatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
import { useCallback, useMemo } from "react";
import { StyleSheet, TouchableOpacity, View } from "react-native";

import { usePreferredInboxAddress } from "@hooks/usePreferredInboxAddress";
import { usePreferredInboxName } from "@hooks/usePreferredInboxName";
import { useInboxProfileSocialsQuery } from "@queries/useInboxProfileSocialsQuery";
import { useAppTheme } from "@theme/useAppTheme";
import { InboxId } from "@xmtp/react-native-sdk";
import { useCurrentAccount } from "../../data/store/accountsStore";
Expand Down Expand Up @@ -79,12 +76,12 @@
inboxId,
}: V3MessageSenderAvatarProps) => {
const currentAccount = useCurrentAccount();
const { data: senderSocials } = useInboxProfileSocialsQuery(

Check failure on line 79 in features/conversation/conversation-message-sender-avatar.tsx

View workflow job for this annotation

GitHub Actions / tsc 

Cannot find name 'useInboxProfileSocialsQuery'.
currentAccount!,
inboxId
);
const address = usePreferredInboxAddress(inboxId);

Check failure on line 83 in features/conversation/conversation-message-sender-avatar.tsx

View workflow job for this annotation

GitHub Actions / tsc 

Cannot find name 'usePreferredInboxAddress'.
const name = usePreferredInboxName(inboxId);

Check failure on line 84 in features/conversation/conversation-message-sender-avatar.tsx

View workflow job for this annotation

GitHub Actions / tsc 

Cannot find name 'usePreferredInboxName'.
const avatarUri = getPreferredInboxAvatar(senderSocials);

const openProfile = useCallback(() => {
Expand Down
28 changes: 28 additions & 0 deletions ios/Converse.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions ios/Converse.xcodeproj/xcshareddata/xcschemes/Converse.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@
ReferencedContainer = "container:Converse.xcodeproj">
</BuildableReference>
</BuildableProductRunnable>
<CommandLineArguments>
<CommandLineArgument
argument = "FIRDebugEnabled"
isEnabled = "YES">
</CommandLineArgument>
</CommandLineArguments>
</LaunchAction>
<ProfileAction
buildConfiguration = "Release"
Expand Down
7 changes: 7 additions & 0 deletions ios/Converse/AppDelegate.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#import "AppDelegate.h"

#import "RNFBAppCheckModule.h"
#import <Firebase.h>

#import <React/RCTBundleURLProvider.h>
#import <React/RCTLinkingManager.h>

Expand All @@ -13,6 +16,10 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
// They will be passed down to the ViewController used by React Native.
self.initialProps = @{};

[RNFBAppCheckModule sharedInstance];

[FIRApp configure];
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

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:

  1. Firebase must be configured before initializing any Firebase services (including App Check)
  2. Missing error handling for Firebase configuration
  3. Missing environment-specific configuration (mentioned in PR objectives)
  4. 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


return [super application:application didFinishLaunchingWithOptions:launchOptions];
}

Expand Down
2 changes: 1 addition & 1 deletion ios/Converse/Supporting/Expo.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
<key>EXUpdatesURL</key>
<string>https://u.expo.dev/49a65fae-3895-4487-8e8a-5bd8bee3a401</string>
<key>EXUpdatesRuntimeVersion</key>
<string>1.3.10</string>
<string>2.0.7</string>
</dict>
</plist>
9 changes: 5 additions & 4 deletions ios/ConverseNotificationExtension/Profile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import Alamofire

func getProfile(account: String, address: String) async -> Profile? {
var profileState = getProfilesStore(account: account)?.state
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] {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

Suggested change
if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] ?? profileState?.profiles?[lowercasedAddress] {
if let profile = profileState?.profiles?[address] ?? profileState?.profiles?[lowercasedAddress] {

return profile
}

// If profile is nil, let's refresh it
try? await refreshProfileFromBackend(account: account, address: formattedAddress)
try? await refreshProfileFromBackend(account: account, address: lowercasedAddress)

profileState = getProfilesStore(account: account)?.state
if let profile = profileState?.profiles?[formattedAddress] {
if let profile = profileState?.profiles?[lowercasedAddress] {
return profile
}
return nil
Expand Down Expand Up @@ -53,3 +53,4 @@ func refreshProfileFromBackend(account: String, address: String) async throws {
}

}

2 changes: 1 addition & 1 deletion ios/ConverseNotificationExtension/Spam.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func computeSpamScoreV3Message(client: XMTP.Client, conversation: XMTP.Conversat

if let senderAddresses = try await group.members.first(where: {$0.inboxId == senderInboxId})?.addresses {
for address in senderAddresses {
if try await client.preferences.addressState(address:address) == .denied {
if try await client.preferences.addressState(address: address) == .denied {
return 1
}
}
Expand Down
30 changes: 30 additions & 0 deletions ios/GoogleService-Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>API_KEY</key>
<string>AIzaSyABJ4xiJDqhKhu4x0eCE2ub9h0gBSDIRg8</string>
<key>GCM_SENDER_ID</key>
<string>23052121755</string>
<key>PLIST_VERSION</key>
<string>1</string>
<key>BUNDLE_ID</key>
<string>com.converse.preview</string>
<key>PROJECT_ID</key>
<string>converse-appcheck</string>
<key>STORAGE_BUCKET</key>
<string>converse-appcheck.firebasestorage.app</string>
<key>IS_ADS_ENABLED</key>
<false></false>
<key>IS_ANALYTICS_ENABLED</key>
<false></false>
<key>IS_APPINVITE_ENABLED</key>
<true></true>
<key>IS_GCM_ENABLED</key>
<true></true>
<key>IS_SIGNIN_ENABLED</key>
<true></true>
<key>GOOGLE_APP_ID</key>
<string>1:23052121755:ios:832b219466063d0113f83e</string>
</dict>
</plist>
14 changes: 14 additions & 0 deletions ios/Podfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ target 'Converse' do
config = use_native_modules!

use_frameworks! :linkage => :static
$RNFirebaseAsStaticFramework = true

pod 'MMKV', $mmkvVersion, :linkage => :dynamic
pod 'MMKVCore', $mmkvVersion, :linkage => :dynamic
pod 'XMTP', $xmtpVersion, :modular_headers => true
Expand Down Expand Up @@ -80,6 +82,18 @@ target 'Converse' do
end
end

# Recommend in case of build errors by https://rnfirebase.io/app-check/usage
# I'm not seeing any errors, but leaving this here just in case
# installer.aggregate_targets.each do |aggregate_target|
# aggregate_target.user_project.native_targets.each do |target|
# target.build_configurations.each do |config|
# config.build_settings['ONLY_ACTIVE_ARCH'] = 'NO'
# config.build_settings['EXCLUDED_ARCHS'] = 'i386'
# end
# end
# aggregate_target.user_project.save
# end

end

post_integrate do |installer|
Expand Down
Loading
Loading