Conversation
tmail-flutter assumes the OIDC provider lives on the same server as JMAP. When the IdP is separate (e.g. Kanidm), WebFinger discovery fails and the app incorrectly uses the JMAP URL as the OIDC authority. Changes: - Add AppConfig.authorityOidc getter reading AUTHORITY_OIDC from env.file - In OidcConfigurationCacheManager, prioritize AUTHORITY_OIDC over both Hive cache and SharedPreferences KEY_AUTHORITY_OIDC fallback - Bypass WebFinger in LoginController._checkOIDCIsAvailable() and HomeController.checkOIDCIsAvailable() when AUTHORITY_OIDC is set
When a JMAP server (e.g. Stalwart) is behind a reverse proxy, it may return session URLs with the internal scheme/host/port (e.g. http://host:8080/jmap/) instead of the external HTTPS URL. This causes mixed-content errors in the browser. toQualifiedUrl now detects when the source URL's origin differs from the configured baseUrl (SERVER_URL) and rewrites the scheme/host/port while preserving the path, query, and fragment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Only rewrite JMAP session URLs when the hostname matches but the scheme differs (http vs https). This handles the reverse proxy case without affecting legitimate multi-port setups or different-host configurations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add MOBILE_OIDC_CLIENT_ID env var for configurable mobile client ID (falls back to hardcoded 'teammail-mobile' if not set) - Add intent-filter in AndroidManifest for teammail.mobile://oauthredirect callback (was missing — only twakemail.mobile://redirect was registered) - Auto-start OIDC flow on mobile when both AUTHORITY_OIDC and SERVER_URL are configured in env.file (skip DNS lookup form) - Compatible with any OIDC provider (Authentik, Kanidm, Keycloak, etc.) via standard flutter_appauth + PKCE Mobile env.file example: SERVER_URL=https://mail.example.org AUTHORITY_OIDC=https://id.example.org/application/o/mail/ MOBILE_OIDC_CLIENT_ID=tmail-mobile OIDC_SCOPES=openid,profile,email,offline_access Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When WebFinger discovery fails on mobile (JMAP server does not expose /.well-known/openid-configuration), instead of falling back to basic auth directly, show a form asking the user for their identity provider URL. This enables mobile users to sign in via any external OIDC provider (Authentik, Kanidm, Keycloak, etc.) without requiring the JMAP server to proxy OIDC discovery. - New LoginFormType.oidcAuthorityForm with text input + skip link - "Skip, use password instead" link to fall back to basic auth - Back button returns to DNS lookup form - i18n keys: oidcAuthorityHint, skipUsePasswordInstead, oidcAuthorityMessage - MOBILE_OIDC_CLIENT_ID env var support (falls back to teammail-mobile) - AndroidManifest: added teammail.mobile://oauthredirect intent-filter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip the fallback to /.well-known/openid-configuration from the JMAP server URL on mobile. This prevents the app from picking up the JMAP server's built-in OIDC provider (e.g. Stalwart) when the real IdP is on a different domain. Instead, show the authority input form so the user can specify their external identity provider. Also fixes _autoStartOidc to set _baseUri instead of _currentBaseUrl (which is a getter, not a setter). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flutter_appauth handles the teammail.mobile://oauthredirect callback natively via Android Custom Tabs. The extra intent-filter for flutter_web_auth_2 caused a disambiguation dialog and broke the flow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TwakeWelcomeView locks the screen to portrait. When navigating to the login view, restore all orientations so tablets can use landscape. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Many OIDC providers (Authentik, Kanidm, etc.) don't support post_logout_redirect_uri with custom URL schemes (teammail.mobile://). This causes flutter_appauth.endSession() to hang indefinitely as the browser never redirects back to the app. For external OIDC providers (non-TWP), skip the browser-based end_session and perform a local-only logout (clear tokens and session). The TWP (Twake Workplace) flow is unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Chrome Custom Tabs on some devices/versions fail to redirect custom URL schemes (teammail.mobile://) back to the app after OIDC auth, leaving the app stuck on a black screen. Using the system browser instead ensures the redirect is handled by the OS intent system. iOS keeps ephemeralAsWebAuthenticationSession (works correctly). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On some Android devices (notably Nokia), Chrome Custom Tabs don't automatically return focus to the app after redirecting to a custom URL scheme. Override RedirectUriReceiverActivity with launchMode=singleTask to ensure the app regains focus. Reverts the ExternalUserAgent change (not applicable on Android). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These methods were added for env.file-based auto-start but are not called anywhere. The mobile OIDC flow uses the authority input form instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis PR introduces support for manual OIDC authority configuration and environment-based OIDC settings. Changes include: adding a new OIDC authority input form to the login UI with option to skip to password entry; updating OIDC configuration handling to prioritize environment variables ( Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/lib/presentation/extensions/uri_extension.dart`:
- Line 19: The log in SessionUtils::toQualifiedUrl currently prints the full
rewritten URL variable rewritten; change it to redact sensitive parts before
logging by removing or masking query and fragment components (or only logging
origin + pathname) so tokens/identifiers aren't emitted. Locate the
toQualifiedUrl function and replace the direct use of rewritten in the log with
a sanitized representation (e.g., rewritten.withoutQueryOrFragment or a short
redacted string) and keep the original message context (origin and rewritten
summary) while ensuring no query/fragment values are logged.
In `@lib/features/login/data/local/oidc_configuration_cache_manager.dart`:
- Around line 37-47: When AppConfig.authorityOidc is used to override the cached
OIDC configuration, the code currently preserves the cached config.isTWP which
can cause incorrect Twake Workplace logout flows against an external IdP; update
the override in the block that returns a new OIDCConfiguration (the code calling
oidcConfigurationCache.toOIDCConfiguration() and creating OIDCConfiguration) to
explicitly set isTWP to false when AppConfig.authorityOidc is non-empty (or add
a clear comment if preserving the cached value is intentional), so that
OIDCConfiguration created when overriding authority does not retain a previous
TWP flag.
In `@lib/features/login/presentation/login_controller.dart`:
- Around line 424-427: When skipping or backing out of the OIDC authority step,
preserve the previous flow instead of always jumping to
dnsLookupForm/passwordForm: update the handlers around
oidcAuthorityInputController.clear() and assignments to loginFormType (where
LoginFormType.oidcAuthorityForm is handled) to branch based on whether we have a
prior username or prior form context (e.g., check _username != null or store a
previousLoginFormType when entering oidcAuthorityForm) and set loginFormType
back to LoginFormType.baseUrlForm when no _username (or to the stored
previousLoginFormType), otherwise fall back to the existing
dnsLookupForm/passwordForm behavior; apply the same change to the other
occurrences around the OIDC skip/back logic (the blocks at the other referenced
spots).
- Around line 97-101: The onClose() method leaks the newly added
oidcAuthorityInputController and oidcAuthorityFocusNode because they are created
(oidcAuthorityInputController, oidcAuthorityFocusNode) but not disposed; update
onClose() in LoginController to dispose oidcAuthorityInputController.dispose()
and oidcAuthorityFocusNode.dispose() alongside the other disposals
(passFocusNode, baseUrlFocusNode, userNameFocusNode, urlInputController,
usernameInputController, passwordInputController) and ensure any mobile-only
cleanup logic remains unchanged.
- Around line 326-333: The mobile branch is too broad: when
PlatformInfo.isMobile and we get a CheckOIDCIsAvailableFailure we always call
_handleCommonOIDCFailure(), which incorrectly treats transient transport/server
errors as permanent "use manual authority" cases. Update the handler in
LoginController (where CheckOIDCIsAvailableFailure is processed) to distinguish
genuine discovery-missing errors from transient/network errors — only call
_handleCommonOIDCFailure() for failures that indicate explicit
WebFinger/authority-not-found (e.g., WebFingerNotFound or an error flag on
CheckOIDCIsAvailableFailure), and for timeouts/network failures allow the
existing retry/path that triggers discovery to continue (or surface a retry to
the user) instead of forcing the manual authority form.
In `@lib/features/login/presentation/login_view.dart`:
- Around line 125-126: The back button is never shown for the OIDC authority
step because LoginFormType.oidcAuthorityForm is rendered by
_buildOidcAuthorityInput but is not considered by the
controller.isBackButtonActivated check that controls LoginBackButton visibility;
update the visibility logic so the back button is enabled for the authority form
(either add LoginFormType.oidcAuthorityForm to the set of form types checked by
controller.isBackButtonActivated or modify the isBackButtonActivated
implementation to return true when currentForm ==
LoginFormType.oidcAuthorityForm), ensuring LoginBackButton is rendered during
that step.
- Around line 29-30: The call to
SystemChrome.setPreferredOrientations(DeviceOrientation.values) is being invoked
from LoginView.build(), causing repeated platform-side effects on every rebuild;
move that call into LoginController.onInit() so it runs once when the controller
initializes, and if the app requires restoring a different orientation later,
add the corresponding reset in LoginController.onClose(); update LoginView to
remove the SystemChrome call and rely on LoginController lifecycle (methods:
LoginController.onInit and LoginController.onClose) to manage orientation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8c47cd7-4c24-41a7-8348-21925ae37a9a
📒 Files selected for processing (13)
Dockerfileandroid/app/src/main/AndroidManifest.xmlcore/lib/presentation/extensions/uri_extension.dartlib/features/home/presentation/extensions/handle_web_finger_to_get_token_extension.dartlib/features/login/data/local/oidc_configuration_cache_manager.dartlib/features/login/data/network/authentication_client/authentication_client_mobile.dartlib/features/login/data/network/config/oidc_constant.dartlib/features/login/presentation/login_controller.dartlib/features/login/presentation/login_form_type.dartlib/features/login/presentation/login_view.dartlib/features/login/presentation/widgets/login_message_widget.dartlib/main/localizations/app_localizations.dartlib/main/utils/app_config.dart
| query: query.isEmpty ? null : query, | ||
| fragment: fragment.isEmpty ? null : fragment, | ||
| ); | ||
| log('SessionUtils::toQualifiedUrl():rewritten from $origin to ${baseUrl.origin}: $rewritten'); |
There was a problem hiding this comment.
Avoid logging full rewritten URL values.
Line 19 logs $rewritten verbatim; if query/fragment carries tokens or identifiers, this leaks sensitive data into logs. Please sanitize before logging.
🔧 Proposed fix
- log('SessionUtils::toQualifiedUrl():rewritten from $origin to ${baseUrl.origin}: $rewritten');
+ final rewrittenSanitized = rewritten.replace(query: null, fragment: null);
+ log('SessionUtils::toQualifiedUrl():rewritten from $origin to ${baseUrl.origin}: $rewrittenSanitized');Based on learnings: ensure diagnostic logging avoids sensitive data exposure in this repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/lib/presentation/extensions/uri_extension.dart` at line 19, The log in
SessionUtils::toQualifiedUrl currently prints the full rewritten URL variable
rewritten; change it to redact sensitive parts before logging by removing or
masking query and fragment components (or only logging origin + pathname) so
tokens/identifiers aren't emitted. Locate the toQualifiedUrl function and
replace the direct use of rewritten in the log with a sanitized representation
(e.g., rewritten.withoutQueryOrFragment or a short redacted string) and keep the
original message context (origin and rewritten summary) while ensuring no
query/fragment values are logged.
| final config = oidcConfigurationCache.toOIDCConfiguration(); | ||
| // Override cached authority when AUTHORITY_OIDC is explicitly configured | ||
| if (AppConfig.authorityOidc.isNotEmpty) { | ||
| return OIDCConfiguration( | ||
| authority: AppConfig.authorityOidc, | ||
| clientId: config.clientId, | ||
| scopes: config.scopes, | ||
| isTWP: config.isTWP, | ||
| ); | ||
| } | ||
| return config; |
There was a problem hiding this comment.
Consider resetting isTWP when overriding cached authority.
When AppConfig.authorityOidc overrides the cached authority (lines 39-46), the code preserves config.isTWP from the cache. This could cause issues if:
- User previously logged into Twake Workplace (
isTWP=truecached) - Admin later configures
AUTHORITY_OIDCinenv.filepointing to an external IdP
The preserved isTWP=true would incorrectly trigger browser-based end_session for the external IdP during logout, potentially causing the hang that authentication_client_mobile.dart tries to avoid.
Consider setting isTWP: false when authority is explicitly overridden via environment configuration, or add a comment explaining why preserving the cached value is intentional.
🛡️ Suggested fix
if (AppConfig.authorityOidc.isNotEmpty) {
return OIDCConfiguration(
authority: AppConfig.authorityOidc,
clientId: config.clientId,
scopes: config.scopes,
- isTWP: config.isTWP,
+ isTWP: false, // External authority from env.file is not TWP
);
}📝 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.
| final config = oidcConfigurationCache.toOIDCConfiguration(); | |
| // Override cached authority when AUTHORITY_OIDC is explicitly configured | |
| if (AppConfig.authorityOidc.isNotEmpty) { | |
| return OIDCConfiguration( | |
| authority: AppConfig.authorityOidc, | |
| clientId: config.clientId, | |
| scopes: config.scopes, | |
| isTWP: config.isTWP, | |
| ); | |
| } | |
| return config; | |
| final config = oidcConfigurationCache.toOIDCConfiguration(); | |
| // Override cached authority when AUTHORITY_OIDC is explicitly configured | |
| if (AppConfig.authorityOidc.isNotEmpty) { | |
| return OIDCConfiguration( | |
| authority: AppConfig.authorityOidc, | |
| clientId: config.clientId, | |
| scopes: config.scopes, | |
| isTWP: false, // External authority from env.file is not TWP | |
| ); | |
| } | |
| return config; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/data/local/oidc_configuration_cache_manager.dart` around
lines 37 - 47, When AppConfig.authorityOidc is used to override the cached OIDC
configuration, the code currently preserves the cached config.isTWP which can
cause incorrect Twake Workplace logout flows against an external IdP; update the
override in the block that returns a new OIDCConfiguration (the code calling
oidcConfigurationCache.toOIDCConfiguration() and creating OIDCConfiguration) to
explicitly set isTWP to false when AppConfig.authorityOidc is non-empty (or add
a clear comment if preserving the cached value is intentional), so that
OIDCConfiguration created when overriding authority does not retain a previous
TWP flag.
| final TextEditingController oidcAuthorityInputController = TextEditingController(); | ||
| final FocusNode baseUrlFocusNode = FocusNode(); | ||
| final FocusNode userNameFocusNode = FocusNode(); | ||
| final FocusNode passFocusNode = FocusNode(); | ||
| final FocusNode oidcAuthorityFocusNode = FocusNode(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'oidcAuthority(InputController|FocusNode)' lib/features/login/presentation/login_controller.dart
echo
sed -n '717,732p' lib/features/login/presentation/login_controller.dartRepository: linagora/tmail-flutter
Length of output: 1605
Dispose the new authority controller and focus node.
Lines 97–101 allocate oidcAuthorityInputController and oidcAuthorityFocusNode, but onClose() (lines 717–732) disposes only the original fields. Re-entering the login route will leak both objects and any listeners attached to them.
Current onClose() method
void onClose() {
featureFailure = null;
passFocusNode.dispose();
baseUrlFocusNode.dispose();
userNameFocusNode.dispose();
urlInputController.dispose();
usernameInputController.dispose();
passwordInputController.dispose();
// Missing:
// oidcAuthorityInputController.dispose();
// oidcAuthorityFocusNode.dispose();
if (PlatformInfo.isMobile) {
_deepLinkDataStreamSubscription?.cancel();
saveLoginInfoInteractor = null;
removeLoginInfoInteractor = null;
}
super.onClose();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/presentation/login_controller.dart` around lines 97 - 101,
The onClose() method leaks the newly added oidcAuthorityInputController and
oidcAuthorityFocusNode because they are created (oidcAuthorityInputController,
oidcAuthorityFocusNode) but not disposed; update onClose() in LoginController to
dispose oidcAuthorityInputController.dispose() and
oidcAuthorityFocusNode.dispose() alongside the other disposals (passFocusNode,
baseUrlFocusNode, userNameFocusNode, urlInputController,
usernameInputController, passwordInputController) and ensure any mobile-only
cleanup logic remains unchanged.
| if (PlatformInfo.isMobile) { | ||
| // On mobile, when WebFinger fails, show the OIDC authority form | ||
| // instead of trying the base URL's /.well-known/openid-configuration. | ||
| // This avoids picking up the JMAP server's built-in OIDC provider | ||
| // (e.g. Stalwart) when the real IdP is on a different domain. | ||
| _handleCommonOIDCFailure(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Don't treat every mobile discovery failure as “use manual authority”.
Lines 326-333 now route every CheckOIDCIsAvailableFailure into _handleCommonOIDCFailure(). That also covers transient transport/server failures, so mobile users lose the existing retry path and get prompted for an authority even when discovery was only temporarily unavailable.
🐛 Proposed fix
- if (PlatformInfo.isMobile) {
+ if (PlatformInfo.isMobile &&
+ (failure.exception is CanNotFoundOIDCLinks ||
+ failure.exception is InvalidOIDCResponseException)) {
// On mobile, when WebFinger fails, show the OIDC authority form
// instead of trying the base URL's /.well-known/openid-configuration.
// This avoids picking up the JMAP server's built-in OIDC provider
// (e.g. Stalwart) when the real IdP is on a different domain.
_handleCommonOIDCFailure();
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/presentation/login_controller.dart` around lines 326 -
333, The mobile branch is too broad: when PlatformInfo.isMobile and we get a
CheckOIDCIsAvailableFailure we always call _handleCommonOIDCFailure(), which
incorrectly treats transient transport/server errors as permanent "use manual
authority" cases. Update the handler in LoginController (where
CheckOIDCIsAvailableFailure is processed) to distinguish genuine
discovery-missing errors from transient/network errors — only call
_handleCommonOIDCFailure() for failures that indicate explicit
WebFinger/authority-not-found (e.g., WebFingerNotFound or an error flag on
CheckOIDCIsAvailableFailure), and for timeouts/network failures allow the
existing retry/path that triggers discovery to continue (or surface a retry to
the user) instead of forcing the manual authority form.
| case LoginFormType.oidcAuthorityForm: | ||
| oidcAuthorityInputController.clear(); | ||
| loginFormType.value = LoginFormType.dnsLookupForm; | ||
| break; |
There was a problem hiding this comment.
Preserve the previous flow when backing out or skipping OIDC authority.
Lines 425-426 and Lines 656-658/671-672 always jump to dnsLookupForm or passwordForm. When this step was reached from baseUrlForm, _username is still null, so skipping authority lands on passwordForm and the next login attempt fails with CanNotFoundUserName(). Back navigation also skips past the base-URL step entirely.
🐛 Proposed fix
+ void _showPasswordOrCredentialForm() {
+ if (_username == null) {
+ _showCredentialForm();
+ } else {
+ _showPasswordForm();
+ }
+ }
+
void handleBackButtonAction(BuildContext context) {
KeyboardUtils.hideKeyboard(context);
clearState();
switch(loginFormType.value) {
@@
case LoginFormType.oidcAuthorityForm:
oidcAuthorityInputController.clear();
- loginFormType.value = LoginFormType.dnsLookupForm;
+ if (_username == null) {
+ _showBaseUrlForm();
+ } else {
+ loginFormType.value = LoginFormType.dnsLookupForm;
+ }
break;
@@
void handleOidcAuthoritySubmitted() {
final authorityUrl = oidcAuthorityInputController.text.trim();
if (authorityUrl.isEmpty) {
- // Skip OIDC, fall back to password form
- _showPasswordForm();
+ _showPasswordOrCredentialForm();
return;
}
@@
void handleSkipOidcAuthority() {
- _showPasswordForm();
+ _showPasswordOrCredentialForm();
}Also applies to: 654-673
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/presentation/login_controller.dart` around lines 424 -
427, When skipping or backing out of the OIDC authority step, preserve the
previous flow instead of always jumping to dnsLookupForm/passwordForm: update
the handlers around oidcAuthorityInputController.clear() and assignments to
loginFormType (where LoginFormType.oidcAuthorityForm is handled) to branch based
on whether we have a prior username or prior form context (e.g., check _username
!= null or store a previousLoginFormType when entering oidcAuthorityForm) and
set loginFormType back to LoginFormType.baseUrlForm when no _username (or to the
stored previousLoginFormType), otherwise fall back to the existing
dnsLookupForm/passwordForm behavior; apply the same change to the other
occurrences around the OIDC skip/back logic (the blocks at the other referenced
spots).
| // Restore all orientations (TwakeWelcomeView locks to portrait) | ||
| SystemChrome.setPreferredOrientations(DeviceOrientation.values); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Flutter, is it recommended to call SystemChrome.setPreferredOrientations inside a widget build method, or should it be moved to a lifecycle hook like initState/dispose?
💡 Result:
No, it is not recommended to call SystemChrome.setPreferredOrientations inside a widget's build method. It should be moved to lifecycle hooks like initState and dispose. Calling it in build causes performance issues because build can run multiple times (on every rebuild), leading to repeated unnecessary calls and potential lag, as seen when switching screens with different orientations. Best practice: Use a StatefulWidget and call SystemChrome.setPreferredOrientations in initState to set the orientation when the widget initializes, and call it again in dispose (with appropriate orientations, like all or previous) to reset when leaving the screen. Official Flutter API docs show examples using lifecycle methods like initState, didChangeMetrics, and dispose, never build. Stack Overflow answers explicitly warn against build method usage for this reason.
Citations:
- 1: https://api.flutter.dev/flutter/services/SystemChrome/setPreferredOrientations.html
- 2: https://stackoverflow.com/questions/54375011/setting-different-orientations-for-each-screen-in-flutter-app-causes-massive-lag
- 3: https://stackoverflow.com/questions/49418332/flutter-how-to-prevent-device-orientation-changes-and-force-portrait
- 4: https://medium.com/@rk0936626/force-portrait-or-landscape-orientation-in-flutter-easily-84fab7a834de
- 5: https://stackoverflow.com/questions/68883002/how-to-fix-a-screen-orientation-permanently-in-flutter
- 6: SystemChrome.setPreferredOrientations only works the first time on iOS flutter/flutter#121625
- 7: https://docs.flutter.dev/cookbook/design/orientation
🏁 Script executed:
# Check the LoginView widget structure and lifecycle methods
cd lib/features/login/presentation && cat -n login_view.dart | head -50Repository: linagora/tmail-flutter
Length of output: 2998
🏁 Script executed:
# Check if LoginView is StatelessWidget or StatefulWidget, and examine BaseLoginView
grep -n "class LoginView\|class BaseLoginView" lib/features/login/presentation/*.dart
# Also check BaseLoginView structure
cat -n lib/features/login/presentation/base_login_view.dart | head -60Repository: linagora/tmail-flutter
Length of output: 3235
🏁 Script executed:
# Check if GetWidget or LoginController provides lifecycle hooks
grep -n "initState\|dispose\|onInit\|onClose" lib/features/login/presentation/login_controller.dart | head -20
# Also check for didChangeDependencies or other lifecycle methods in LoginView
grep -n "didChangeDependencies\|initState\|dispose\|onInit\|onClose" lib/features/login/presentation/login_view.dartRepository: linagora/tmail-flutter
Length of output: 389
🏁 Script executed:
# Check if LoginController already has orientation reset logic
grep -n "setPreferredOrientations\|DeviceOrientation" lib/features/login/presentation/login_controller.dart
# Check if there's any other orientation-related code in the login module
grep -rn "setPreferredOrientations\|DeviceOrientation" lib/features/login/Repository: linagora/tmail-flutter
Length of output: 185
Move the orientation change to the LoginController lifecycle.
Line 30 currently triggers a platform-side effect from build(), which runs on every widget rebuild. Since LoginView is a stateless GetWidget with a controller, move SystemChrome.setPreferredOrientations(DeviceOrientation.values) to LoginController.onInit() and reset it (if needed) in onClose() to avoid repeated calls and performance degradation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/presentation/login_view.dart` around lines 29 - 30, The
call to SystemChrome.setPreferredOrientations(DeviceOrientation.values) is being
invoked from LoginView.build(), causing repeated platform-side effects on every
rebuild; move that call into LoginController.onInit() so it runs once when the
controller initializes, and if the app requires restoring a different
orientation later, add the corresponding reset in LoginController.onClose();
update LoginView to remove the SystemChrome call and rely on LoginController
lifecycle (methods: LoginController.onInit and LoginController.onClose) to
manage orientation.
| case LoginFormType.oidcAuthorityForm: | ||
| return _buildOidcAuthorityInput(context); |
There was a problem hiding this comment.
Show LoginBackButton for LoginFormType.oidcAuthorityForm.
Line 125 makes the authority step reachable, but controller.isBackButtonActivated still excludes it, so the custom back button never renders while this form is active. On iOS/mobile portrait that removes the only obvious in-app route back to the previous step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/login/presentation/login_view.dart` around lines 125 - 126, The
back button is never shown for the OIDC authority step because
LoginFormType.oidcAuthorityForm is rendered by _buildOidcAuthorityInput but is
not considered by the controller.isBackButtonActivated check that controls
LoginBackButton visibility; update the visibility logic so the back button is
enabled for the authority form (either add LoginFormType.oidcAuthorityForm to
the set of form types checked by controller.isBackButtonActivated or modify the
isBackButtonActivated implementation to return true when currentForm ==
LoginFormType.oidcAuthorityForm), ensuring LoginBackButton is rendered during
that step.
|
@dab246 @hoangdat @tddang-linagora please have a look to this contribution;-) |
chibenwa
left a comment
chibenwa
left a comment
There was a problem hiding this comment.
On my side I am missing a comprehensive documentation on this set up for web and mobile. Can you please provide it?
Thanks
|
Hello @nka11 Thanks again for your interest in our solution. Are you interested in moving this work forward and getting it merged ? This PR mixes numerous concerns , are thos all strictly related to OIDC? Maybe we could writte a little ADR summing up the changes, and justifying:
Best regards, Benoit TELLIER |
2 participants
Cleaned PR from other topics
Summary by CodeRabbit
New Features
Improvements
Bug Fixes