From e4b550ab527a9e87e49514896826933108d7e3f2 Mon Sep 17 00:00:00 2001 From: Roland Geider Date: Fri, 28 Mar 2025 17:21:31 +0100 Subject: [PATCH] Refactor app version handling and update authentication flow Previously, this was only triggered when logging in to the application. If a user just opened the app, it would just stop working. We also now always check this min version and have removed the option from the android manifest file since disabling this doesn't make much sense and we have many other platforms as well (iOS, flatpak) --- android/app/src/main/AndroidManifest.xml | 4 - lib/helpers/consts.dart | 3 - lib/main.dart | 33 ++++--- lib/providers/auth.dart | 93 +++++++++++++------ lib/screens/auth_screen.dart | 7 -- lib/screens/home_tabs_screen.dart | 53 ++++++----- lib/screens/update_app_screen.dart | 5 +- test/auth/auth_provider_test.dart | 12 +-- test/auth/auth_screen_test.dart | 2 +- .../nutritional_plan_screen_test.mocks.dart | 27 ++++-- .../nutritional_plans_screen_test.mocks.dart | 27 ++++-- test/utils.dart | 2 +- 12 files changed, 161 insertions(+), 107 deletions(-) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 1e198ad9..4e50bad4 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -29,10 +29,6 @@ android:enableOnBackInvokedCallback="true" android:networkSecurityConfig="@xml/network_security_config"> - - + authResultSnapshot.connectionState == ConnectionState.waiting + ? const SplashScreen() + : const AuthScreen(), + ); + } + } @override Widget build(BuildContext context) { @@ -157,15 +174,7 @@ class MyApp extends StatelessWidget { highContrastTheme: wgerLightThemeHc, highContrastDarkTheme: wgerDarkThemeHc, themeMode: user.themeMode, - home: auth.isAuth - ? HomeTabsScreen() - : FutureBuilder( - future: auth.tryAutoLogin(), - builder: (ctx, authResultSnapshot) => - authResultSnapshot.connectionState == ConnectionState.waiting - ? const SplashScreen() - : const AuthScreen(), - ), + home: _getHomeScreen(auth), routes: { DashboardScreen.routeName: (ctx) => const DashboardScreen(), FormScreen.routeName: (ctx) => const FormScreen(), diff --git a/lib/providers/auth.dart b/lib/providers/auth.dart index f4ca6f39..a419d25d 100644 --- a/lib/providers/auth.dart +++ b/lib/providers/auth.dart @@ -38,6 +38,12 @@ enum LoginActions { proceed, } +enum AuthState { + updateRequired, + loggedIn, + loggedOut, +} + class AuthProvider with ChangeNotifier { final _logger = Logger('AuthProvider'); @@ -46,6 +52,7 @@ class AuthProvider with ChangeNotifier { String? serverVersion; PackageInfo? applicationVersion; Map metadata = {}; + AuthState state = AuthState.loggedOut; static const MIN_APP_VERSION_URL = 'min-app-version'; static const SERVER_VERSION_URL = 'version'; @@ -54,7 +61,7 @@ class AuthProvider with ChangeNotifier { late http.Client client; - AuthProvider([http.Client? client, bool? checkMetadata]) { + AuthProvider([http.Client? client]) { this.client = client ?? http.Client(); } @@ -83,23 +90,16 @@ class AuthProvider with ChangeNotifier { } /// Checking if there is a new version of the application. - Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) async { - metadata ??= this.metadata; - - if (!metadata.containsKey(MANIFEST_KEY_CHECK_UPDATE) || - metadata[MANIFEST_KEY_CHECK_UPDATE] == 'false') { - return false; - } - + Future applicationUpdateRequired([String? version]) async { final applicationCurrentVersion = version ?? applicationVersion!.version; final response = await client.get(makeUri(serverUrl!, MIN_APP_VERSION_URL)); final currentVersion = Version.parse(applicationCurrentVersion); final requiredAppVersion = Version.parse(jsonDecode(response.body)); - return requiredAppVersion > currentVersion; + final result = requiredAppVersion > currentVersion; + _logger.fine('Application update required: $result'); + + return result; } /// Registers a new user @@ -160,15 +160,13 @@ class AuthProvider with ChangeNotifier { await initVersions(serverUrl); // If update is required don't log in user - if (await applicationUpdateRequired( - applicationVersion!.version, - {MANIFEST_KEY_CHECK_UPDATE: 'true'}, - )) { + if (await applicationUpdateRequired()) { return {'action': LoginActions.update}; } // Log user in token = responseData['token']; + state = AuthState.loggedIn; notifyListeners(); // store login data in shared preferences @@ -195,23 +193,58 @@ class AuthProvider with ChangeNotifier { return userData['serverUrl'] as String; } - Future tryAutoLogin() async { + /// Tries to auto-login the user with the stored token + Future tryAutoLogin() async { final prefs = await SharedPreferences.getInstance(); if (!prefs.containsKey(PREFS_USER)) { - _logger.info('autologin failed'); - return false; + _logger.info('autologin failed, no saved user data'); + state = AuthState.loggedOut; + return; } - final extractedUserData = json.decode(prefs.getString(PREFS_USER)!); - token = extractedUserData['token']; - serverUrl = extractedUserData['serverUrl']; + final userData = json.decode(prefs.getString(PREFS_USER)!); + + if (!userData.containsKey('token') || !userData.containsKey('serverUrl')) { + _logger.info('autologin failed, no token or serverUrl'); + state = AuthState.loggedOut; + return; + } + + token = userData['token']; + serverUrl = userData['serverUrl']; + + if (token == null || serverUrl == null) { + _logger.info('autologin failed, token or serverUrl is null'); + state = AuthState.loggedOut; + return; + } + + // // Try to talk to a URL using the token, if this doesn't work, log out + final response = await client.head( + makeUri(serverUrl!, 'routine'), + headers: { + HttpHeaders.contentTypeHeader: 'application/json; charset=UTF-8', + HttpHeaders.userAgentHeader: getAppNameHeader(), + HttpHeaders.authorizationHeader: 'Token $token' + }, + ); + if (response.statusCode != 200) { + _logger.info('autologin failed, statusCode: ${response.statusCode}'); + await logout(); + return; + } + + await initVersions(serverUrl!); + + // If update is required don't log in user + if (await applicationUpdateRequired()) { + state = AuthState.updateRequired; + } else { + state = AuthState.loggedIn; + _logger.info('autologin successful'); + } - _logger.info('autologin successful'); - setApplicationVersion(); - setServerVersion(); notifyListeners(); - //_autoLogout(); - return true; } Future logout({bool shouldNotify = true}) async { @@ -219,6 +252,7 @@ class AuthProvider with ChangeNotifier { token = null; serverUrl = null; dataInit = false; + state = AuthState.loggedOut; if (shouldNotify) { notifyListeners(); @@ -236,7 +270,8 @@ class AuthProvider with ChangeNotifier { if (applicationVersion != null) { out = '/${applicationVersion!.version} ' '(${applicationVersion!.packageName}; ' - 'build: ${applicationVersion!.buildNumber})'; + 'build: ${applicationVersion!.buildNumber})' + ' - https://github.com/wger-project'; } return 'wger App$out'; } diff --git a/lib/screens/auth_screen.dart b/lib/screens/auth_screen.dart index c4c73eca..085f29e2 100644 --- a/lib/screens/auth_screen.dart +++ b/lib/screens/auth_screen.dart @@ -89,12 +89,6 @@ class AuthScreen extends StatelessWidget { ), ), ), - // Positioned( - // top: 0.4 * deviceSize.height, - // left: 15, - // right: 15, - // child: const , - // ), ], ), ); @@ -166,7 +160,6 @@ class _AuthCardState extends State { void _submit(BuildContext context) async { if (!_formKey.currentState!.validate()) { - // Invalid! return; } _formKey.currentState!.save(); diff --git a/lib/screens/home_tabs_screen.dart b/lib/screens/home_tabs_screen.dart index b038a942..22ac70c5 100644 --- a/lib/screens/home_tabs_screen.dart +++ b/lib/screens/home_tabs_screen.dart @@ -146,27 +146,8 @@ class _HomeTabsScreenState extends State with SingleTickerProvid future: _initialData, builder: (context, snapshot) { if (snapshot.connectionState != ConnectionState.done) { - return Scaffold( - body: Center( - child: Column( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - const Center( - child: SizedBox( - height: 70, - child: RiveAnimation.asset( - 'assets/animations/wger_logo.riv', - animations: ['idle_loop2'], - ), - ), - ), - Text( - AppLocalizations.of(context).loadingText, - style: Theme.of(context).textTheme.headlineSmall, - ), - ], - ), - ), + return const Scaffold( + body: LoadingWidget(), ); } else { return Scaffold( @@ -204,3 +185,33 @@ class _HomeTabsScreenState extends State with SingleTickerProvid ); } } + +class LoadingWidget extends StatelessWidget { + const LoadingWidget({ + super.key, + }); + + @override + Widget build(BuildContext context) { + return Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const Center( + child: SizedBox( + height: 70, + child: RiveAnimation.asset( + 'assets/animations/wger_logo.riv', + animations: ['idle_loop2'], + ), + ), + ), + Text( + AppLocalizations.of(context).loadingText, + style: Theme.of(context).textTheme.headlineSmall, + ), + ], + ), + ); + } +} diff --git a/lib/screens/update_app_screen.dart b/lib/screens/update_app_screen.dart index 8b1996c7..2b01eeec 100644 --- a/lib/screens/update_app_screen.dart +++ b/lib/screens/update_app_screen.dart @@ -30,10 +30,7 @@ class UpdateAppScreen extends StatelessWidget { AppLocalizations.of(context).appUpdateTitle, style: Theme.of(context).textTheme.headlineSmall, ), - content: Column( - mainAxisSize: MainAxisSize.min, - children: [Text(AppLocalizations.of(context).appUpdateContent)], - ), + content: Text(AppLocalizations.of(context).appUpdateContent), actions: null, ), ); diff --git a/test/auth/auth_provider_test.dart b/test/auth/auth_provider_test.dart index 8c7d9db4..cea67ef8 100644 --- a/test/auth/auth_provider_test.dart +++ b/test/auth/auth_provider_test.dart @@ -15,11 +15,9 @@ void main() { path: 'api/v2/min-app-version/', ); - final testMetadata = {'wger.check_min_app_version': 'true'}; - setUp(() { mockClient = MockClient(); - authProvider = AuthProvider(mockClient, false); + authProvider = AuthProvider(mockClient); authProvider.serverUrl = 'http://localhost'; }); @@ -27,7 +25,7 @@ void main() { test('app version higher than min version', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.2.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0'); // assert expect(updateNeeded, false); @@ -36,7 +34,7 @@ void main() { test('app version higher than min version - 1', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.1', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.1'); // assert expect(updateNeeded, true); @@ -45,7 +43,7 @@ void main() { test('app version higher than min version - 2', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.1', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.1'); // assert expect(updateNeeded, true); @@ -54,7 +52,7 @@ void main() { test('app version equal as min version', () async { // arrange when(mockClient.get(tVersionUri)).thenAnswer((_) => Future(() => Response('"1.3.0"', 200))); - final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0', testMetadata); + final updateNeeded = await authProvider.applicationUpdateRequired('1.3.0'); // assert expect(updateNeeded, false); diff --git a/test/auth/auth_screen_test.dart b/test/auth/auth_screen_test.dart index 953fd535..ece5cfaa 100644 --- a/test/auth/auth_screen_test.dart +++ b/test/auth/auth_screen_test.dart @@ -73,7 +73,7 @@ void main() { setUp(() { mockClient = MockClient(); - authProvider = AuthProvider(mockClient, false); + authProvider = AuthProvider(mockClient); authProvider.serverUrl = 'https://wger.de'; SharedPreferences.setMockInitialValues({}); diff --git a/test/nutrition/nutritional_plan_screen_test.mocks.dart b/test/nutrition/nutritional_plan_screen_test.mocks.dart index 09ca8fcb..694d7827 100644 --- a/test/nutrition/nutritional_plan_screen_test.mocks.dart +++ b/test/nutrition/nutritional_plan_screen_test.mocks.dart @@ -202,6 +202,18 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { returnValueForMissingStub: null, ); + @override + _i2.AuthState get state => (super.noSuchMethod( + Invocation.getter(#state), + returnValue: _i2.AuthState.updateRequired, + ) as _i2.AuthState); + + @override + set state(_i2.AuthState? _state) => super.noSuchMethod( + Invocation.setter(#state, _state), + returnValueForMissingStub: null, + ); + @override _i3.Client get client => (super.noSuchMethod( Invocation.getter(#client), @@ -253,12 +265,8 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { ) as _i5.Future); @override - _i5.Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) => - (super.noSuchMethod( - Invocation.method(#applicationUpdateRequired, [version, metadata]), + _i5.Future applicationUpdateRequired([String? version]) => (super.noSuchMethod( + Invocation.method(#applicationUpdateRequired, [version]), returnValue: _i5.Future.value(false), ) as _i5.Future); @@ -308,10 +316,11 @@ class MockAuthProvider extends _i1.Mock implements _i2.AuthProvider { ) as _i5.Future); @override - _i5.Future tryAutoLogin() => (super.noSuchMethod( + _i5.Future tryAutoLogin() => (super.noSuchMethod( Invocation.method(#tryAutoLogin, []), - returnValue: _i5.Future.value(false), - ) as _i5.Future); + returnValue: _i5.Future.value(), + returnValueForMissingStub: _i5.Future.value(), + ) as _i5.Future); @override _i5.Future logout({bool? shouldNotify = true}) => (super.noSuchMethod( diff --git a/test/nutrition/nutritional_plans_screen_test.mocks.dart b/test/nutrition/nutritional_plans_screen_test.mocks.dart index dff174c1..a135ff8e 100644 --- a/test/nutrition/nutritional_plans_screen_test.mocks.dart +++ b/test/nutrition/nutritional_plans_screen_test.mocks.dart @@ -94,6 +94,18 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { returnValueForMissingStub: null, ); + @override + _i3.AuthState get state => (super.noSuchMethod( + Invocation.getter(#state), + returnValue: _i3.AuthState.updateRequired, + ) as _i3.AuthState); + + @override + set state(_i3.AuthState? _state) => super.noSuchMethod( + Invocation.setter(#state, _state), + returnValueForMissingStub: null, + ); + @override _i2.Client get client => (super.noSuchMethod( Invocation.getter(#client), @@ -145,12 +157,8 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { ) as _i5.Future); @override - _i5.Future applicationUpdateRequired([ - String? version, - Map? metadata, - ]) => - (super.noSuchMethod( - Invocation.method(#applicationUpdateRequired, [version, metadata]), + _i5.Future applicationUpdateRequired([String? version]) => (super.noSuchMethod( + Invocation.method(#applicationUpdateRequired, [version]), returnValue: _i5.Future.value(false), ) as _i5.Future); @@ -200,10 +208,11 @@ class MockAuthProvider extends _i1.Mock implements _i3.AuthProvider { ) as _i5.Future); @override - _i5.Future tryAutoLogin() => (super.noSuchMethod( + _i5.Future tryAutoLogin() => (super.noSuchMethod( Invocation.method(#tryAutoLogin, []), - returnValue: _i5.Future.value(false), - ) as _i5.Future); + returnValue: _i5.Future.value(), + returnValueForMissingStub: _i5.Future.value(), + ) as _i5.Future); @override _i5.Future logout({bool? shouldNotify = true}) => (super.noSuchMethod( diff --git a/test/utils.dart b/test/utils.dart index 9244b4da..db590b38 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -23,7 +23,7 @@ import 'measurements/measurement_provider_test.mocks.dart'; import 'other/base_provider_test.mocks.dart'; // Test Auth provider -final AuthProvider testAuthProvider = AuthProvider(MockClient(), false) +final AuthProvider testAuthProvider = AuthProvider(MockClient()) ..token = 'FooBar' ..serverUrl = 'https://localhost';