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

fix: BAD_DECRYPT Exception on invalid cipher #1730

Merged
merged 7 commits into from
May 4, 2022
+23 −20
Merged

fix: BAD_DECRYPT Exception on invalid cipher #1730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
83fbbcb
fix: BAD_DECRYPT Exception on invalid cipher
M123-dev May 2, 2022
4dbd53d
Update user_management_provider.dart
M123-dev May 2, 2022
bc84460
More comments + showing SnackBar
M123-dev May 3, 2022
d6d63f3
Moving cred movment lower in the tree
M123-dev May 3, 2022
af0d5bd
Moved back
M123-dev May 3, 2022
6eaf3ec
Await user credentials on app start
M123-dev May 4, 2022
1a27153
Update packages/smooth_app/lib/data_models/user_management_provider.dart
M123-dev May 4, 2022
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
10 changes: 2 additions & 8 deletions packages/smooth_app/lib/data_models/user_management_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:openfoodfacts/openfoodfacts.dart';
import 'package:openfoodfacts/utils/OpenFoodAPIConfiguration.dart';
import 'package:smooth_app/database/dao_secured_string.dart';
Expand Down Expand Up @@ -40,7 +39,7 @@ class UserManagementProvider with ChangeNotifier {
}

/// Mounts already stored credentials, called at app startup
Future<void> mountCredentials(BuildContext context) async {
Future<void> mountCredentials() async {
String? userId;
String? password;

Expand All @@ -52,12 +51,7 @@ class UserManagementProvider with ChangeNotifier {
/// manually overwritten from an external apk.
DaoSecuredString.remove(key: _USER_ID);
DaoSecuredString.remove(key: _PASSWORD);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(AppLocalizations.of(context)!.you_have_been_logged_out),
duration: const Duration(seconds: 2),
),
);
debugPrint('Credentials query failed, you have been logged out');
}

if (userId == null || password == null) {
Expand Down
4 changes: 0 additions & 4 deletions packages/smooth_app/lib/l10n/app_en.arb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,9 +888,5 @@
"user_list_name_error_same": "That is the same name",
"@user_list_name_error_same": {
"description": "Validation error about the renamed name that is the same as the initial list name"
},
"you_have_been_logged_out": "You have been logged out",
"@you_have_been_logged_out": {
"description": "Hint when the user got automatically logged out for example for security reasons"
}
}
2 changes: 2 additions & 0 deletions packages/smooth_app/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ class SmoothAppGetLanguage extends StatelessWidget {
final LocalDatabase _localDatabase = context.read<LocalDatabase>();
AnalyticsHelper.trackStart(_localDatabase, context);

context.read<UserManagementProvider>().mountCredentials();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not await? Btw as it's in the init part and we do that only once, do we have to notify the listeners in mountCredentials? I don't think so, as it's dangerous to notify while building a Widget.

Copy link
Member Author

Choose a reason for hiding this comment

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

no await intended, since the credentials are not needed directly after start, this can run when there is time for that. Because of that that little more efford with the provider

Copy link
Contributor

Choose a reason for hiding this comment

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

Not convinced. All we do in mountCredentials is get the credentials from the dao and put them in static. Quick and easy.
OK, technically you are allowed not to wait until it finishes and use the provider for the refresh, but:

  • hey, it's init time, let's init once and for all
  • the notifylistener will trigger a display refresh just because you didn't want to spend 20 milliseconds awaiting
  • not everything we do is strictly needed at init time - we also set the query country, which is useless as we haven't queried anything yet
  • btw mountCredentials could be called much earlier, in _init1 or _init2

Copy link
Member Author

Choose a reason for hiding this comment

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

Just applied your suggestions, my first approach was to completely remove the provider, but looking into it again we should keep it to properly reflect logins from for example the login card.


return appWidget;
}
}
7 changes: 0 additions & 7 deletions packages/smooth_app/lib/pages/page_manager.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:provider/provider.dart';
import 'package:smooth_app/data_models/user_management_provider.dart';
import 'package:smooth_app/pages/scan/inherited_data_manager.dart';
import 'package:smooth_app/widgets/tab_navigator.dart';

Expand Down Expand Up @@ -60,11 +58,6 @@ class PageManagerState extends State<PageManager> {
_buildOffstageNavigator(BottomNavigationTab.History),
];

// Mounting the user credentials, needs to be done after build of the first
// Scaffold to be able to show a optional error Snackbar
WidgetsBinding.instance?.addPostFrameCallback((_) =>
context.read<UserManagementProvider>().mountCredentials(context));

final ThemeData themeData = Theme.of(context);
final bool brightnessCheck = themeData.brightness == Brightness.light;
return WillPopScope(
Expand Down