-
Notifications
You must be signed in to change notification settings - Fork 45
Implement new cozy-external-bridge setup #2889
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,8 @@ abstract class AppConfig { | |
| static const String sourceCodeUrl = | ||
| 'https://github.com/linagora/twake-on-matrix'; | ||
| static String supportUrl = 'https://twake.app/support'; | ||
| static String cozyExternalBridgeVersion = '0.16.1'; | ||
| static String cozyExternalBridgeVersion = '1.2.1'; | ||
| static List<String> cozyExternalBridgeAllowlist = ['.twake.app', '.lin-saas.com','.lin-saas.dev']; | ||
| static bool renderHtml = true; | ||
| static bool hideRedactedEvents = false; | ||
| static bool hideUnknownEvents = true; | ||
|
|
@@ -193,6 +194,11 @@ abstract class AppConfig { | |
| defaultValue: ConfigurationSaas.homeserver, | ||
| ); | ||
|
|
||
| static const String _cozyExternalBridgeAllowlistEnv = String.fromEnvironment( | ||
| 'COZY_EXTERNAL_BRIDGE_ALLOWLIST', | ||
| defaultValue: '.twake.app,.lin-saas.com,.lin-saas.dev', | ||
| ); | ||
|
|
||
| static void loadEnvironment() { | ||
| twakeWorkplaceHomeserver = _twakeWorkplaceHomeserverEnv; | ||
|
|
||
|
|
@@ -213,6 +219,14 @@ abstract class AppConfig { | |
| homeserver = _homeserverEnv; | ||
|
|
||
| Logs().i('[Public Platform] AppConfig():: HOME_SERVER $_homeserverEnv'); | ||
|
|
||
| cozyExternalBridgeAllowlist = _cozyExternalBridgeAllowlistEnv | ||
| .split(',') | ||
| .map((i) => i.trim()) | ||
| .where((i) => i.isNotEmpty) | ||
| .toList(); | ||
zatteo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Logs().i('[Public Platform] AppConfig():: COZY_EXTERNAL_BRIDGE_ALLOWLIST $_cozyExternalBridgeAllowlistEnv'); | ||
| } | ||
|
|
||
| static bool get isSaasPlatForm => _platformEnv == 'saas'; | ||
|
|
@@ -302,5 +316,11 @@ abstract class AppConfig { | |
| json['cozy_external_bridge_version'].isNotEmpty) { | ||
| cozyExternalBridgeVersion = json['cozy_external_bridge_version']; | ||
| } | ||
| if (json['cozy_external_bridge_allowlist'] is List && | ||
| json['cozy_external_bridge_allowlist'].isNotEmpty) { | ||
| cozyExternalBridgeAllowlist = json['cozy_external_bridge_allowlist'] | ||
| .whereType<String>() | ||
| .toList(); | ||
| } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The goal is to be able to add new approved domain name for bridge in env var or in config.json. It will help us:
However, for config.json, it seems that it is loaded after CozyBridge initialization. Any thought on this?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create a method to initialize config early, put only the cozy config initialization there, then call as soon as the app start. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,7 +67,7 @@ void main() async { | |
| if (PlatformInfos.isWeb) { | ||
| CozyConfigManager() | ||
| .injectCozyScript(AppConfig.cozyExternalBridgeVersion) | ||
| .then((value) => CozyConfigManager().initialize()); | ||
| .then((value) => CozyConfigManager().initialize(validUrlSuffixes: AppConfig.cozyExternalBridgeAllowlist)); | ||
| } | ||
|
Comment on lines
68
to
71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🐛 Proposed fix — await the initialization chain before rendering if (PlatformInfos.isWeb) {
- CozyConfigManager()
- .injectCozyScript(AppConfig.cozyExternalBridgeVersion)
- .then((value) => CozyConfigManager().initialize(validUrlSuffixes: AppConfig.cozyExternalBridgeAllowlist));
+ await CozyConfigManager()
+ .injectCozyScript(AppConfig.cozyExternalBridgeVersion)
+ .then((_) => CozyConfigManager().initialize(
+ validUrlSuffixes: AppConfig.cozyExternalBridgeAllowlist,
+ ));
}🤖 Prompt for AI Agents |
||
|
|
||
| // Started in foreground mode. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,14 +50,11 @@ class _AdaptiveScaffoldAppBarState extends State<AdaptiveScaffoldAppBar> { | |
| }, | ||
| ); | ||
|
|
||
| return FutureBuilder( | ||
| future: CozyConfigManager().isInsideCozy, | ||
| builder: (context, snapshot) { | ||
| if (!snapshot.hasData || !snapshot.data!) return child; | ||
| if(CozyConfigManager().isInContainer) { | ||
| return const SizedBox(); | ||
| } | ||
|
|
||
| return const SizedBox(); | ||
| }, | ||
| ); | ||
| return child; | ||
|
Comment on lines
+53
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Search for CozyConfigManager class definition
fd "cozy_config_manager" --type fRepository: linagora/twake-on-matrix Length of output: 50 🏁 Script executed: #!/bin/bash
# Search for CozyConfigManager class definition and factory constructor
rg -n "class CozyConfigManager" --type dart -A 10Repository: linagora/twake-on-matrix Length of output: 50 🏁 Script executed: #!/bin/bash
# Search for isInContainer property and initialize method
rg -n "isInContainer" --type dart -B 2 -A 2Repository: linagora/twake-on-matrix Length of output: 1478 🏁 Script executed: #!/bin/bash
# Search for initialize method in CozyConfigManager context
rg -n "initialize\(\)" --type dart -B 2 -A 5 | head -100Repository: linagora/twake-on-matrix Length of output: 4901 🏁 Script executed: #!/bin/bash
# Find where CozyConfigManager is imported from
rg -n "import.*[Cc]ozy|from.*cozy" --type dart | head -20Repository: linagora/twake-on-matrix Length of output: 422 🏁 Script executed: #!/bin/bash
# Search for CozyConfigManager.initialize() calls
rg -n "CozyConfigManager\(\)\.initialize" --type dart -B 2 -A 2Repository: linagora/twake-on-matrix Length of output: 359 🏁 Script executed: #!/bin/bash
# Search main.dart for initialize setup
fd "main.dart" --type fRepository: linagora/twake-on-matrix Length of output: 81 🏁 Script executed: #!/bin/bash
# Read main.dart to understand the initialization sequence and race condition
cat -n lib/main.dart | sed -n '50,100p'Repository: linagora/twake-on-matrix Length of output: 2355 Race condition in main.dart: The initialization chain on lines 68–70 of main.dart is fire-and-forget (chained with Fix: Add if (PlatformInfos.isWeb) {
await CozyConfigManager()
.injectCozyScript(AppConfig.cozyExternalBridgeVersion)
.then((value) => CozyConfigManager().initialize(validUrlSuffixes: AppConfig.cozyExternalBridgeAllowlist));
}🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hoangdat I agree it is an issue but I suppose it was built on purpose to not await it right? Any thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
constshould be defined for these default values, for reuse.