feat: fix bad coding practices, security risks and improve error handling and project structure

This commit is contained in:
Patryk Hegenberg 2026-01-21 16:18:38 +01:00
parent e5c61447a3
commit 09ee89d928
9 changed files with 216 additions and 151 deletions

View file

@ -0,0 +1,138 @@
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:drift/drift.dart';
import 'package:flutter_secure_storage/flutter_secure_storage.dart';
import 'package:slrpg_app/main.dart';
import '../../../../shared/data/local/app_database.dart';
import '../../../../shared/data/remote/api_client.dart';
import '../../../../shared/data/repositories/user_repository.dart';
import '../../../../core/constants/app_constants.dart';
final authRepositoryProvider = Provider<AuthRepository>((ref) {
final db = ref.watch(appDatabaseProvider);
final apiClient = ref.watch(apiClientProvider);
return AuthRepository(db: db, apiClient: apiClient);
});
class AuthRepository {
final AppDatabase db;
final ApiClient apiClient;
final _storage = const FlutterSecureStorage();
AuthRepository({required this.db, required this.apiClient});
Future<UserCollection> login(String email, String password) async {
final response = await apiClient.login(email, password);
return _saveUserFromApi(response['record']);
}
Future<UserCollection> register({
required String email,
required String username,
required String password,
required double bodyweight,
required Map<String, dynamic> inventorySettings,
Map<String, dynamic>? exerciseVariants,
Map<String, dynamic>? avatarConfig,
}) async {
try {
final response = await apiClient.register(
email: email,
username: username,
password: password,
bodyweight: bodyweight,
inventorySettings: inventorySettings,
exerciseVariants: exerciseVariants,
avatarConfig: avatarConfig,
);
final record = response['record'] ?? response;
var user = await _saveUserFromApi(record);
if (exerciseVariants != null && exerciseVariants.isNotEmpty) {
final serverVariants = user.exerciseVariants;
if (serverVariants == null || serverVariants.isEmpty) {
final companion = user.toCompanion(true).copyWith(
exerciseVariants: Value(exerciseVariants),
isDirty: const Value(true),
updatedAt: Value(DateTime.now()),
);
await db.into(db.users).insertOnConflictUpdate(companion);
user = (await (db.select(db.users)
..where((u) => u.id.equals(user.id)))
.getSingle());
}
}
// Auto-Login after register usually handled by token return, but checking consistency
try {
await apiClient.login(email, password);
} catch (e) {
// Token might already be set by register
}
return user;
} catch (e) {
rethrow;
}
}
Future<void> changePassword(String oldPassword, String newPassword) async {
final user = await (db.select(db.users)..limit(1)).getSingleOrNull();
if (user?.serverId != null) {
await apiClient.updatePassword(
userId: user!.serverId!,
oldPassword: oldPassword,
newPassword: newPassword,
newPasswordConfirm: newPassword,
);
} else {
throw Exception('User not synced or offline');
}
}
Future<void> deleteAccount() async {
final user = await (db.select(db.users)..limit(1)).getSingleOrNull();
if (user?.serverId != null) {
await apiClient.deleteAccount(user!.serverId!);
}
await logout();
}
Future<void> logout() async {
await apiClient.logout();
await _storage.delete(key: AppConstants.keyLastSync);
await db.transaction(() async {
await db.delete(db.users).go();
await db.delete(db.cycles).go();
await db.delete(db.workouts).go();
await db.delete(db.quests).go();
});
}
Future<UserCollection> _saveUserFromApi(Map<String, dynamic> record) async {
await db.delete(db.users).go();
final companion = UsersCompanion(
serverId: Value(record['id']),
email: Value(record['email'] ?? ''),
xp: Value((record['xp'] as num?)?.toInt() ?? 0),
level: Value((record['level'] as num?)?.toInt() ?? 1),
currentBodyweight:
Value((record['current_bodyweight'] as num?)?.toDouble() ?? 70.0),
inventorySettings: Value(record['inventory_settings'] ?? {}),
exerciseVariants: Value(record['exercise_variants'] ?? {}),
avatarConfig: Value(record['avatar_config'] ?? {}),
lastSyncAt: Value(DateTime.now()),
isDirty: const Value(false),
createdAt: Value(DateTime.now()),
updatedAt: Value(DateTime.now()),
);
final id = await db.into(db.users).insert(companion);
return (await (db.select(db.users)..where((u) => u.id.equals(id)))
.getSingle());
}
}

View file

@ -4,7 +4,7 @@ import 'package:go_router/go_router.dart';
import 'package:slrpg_app/l10n/app_localizations.dart'; import 'package:slrpg_app/l10n/app_localizations.dart';
import 'package:slrpg_app/src/core/utils/error_handler.dart'; import 'package:slrpg_app/src/core/utils/error_handler.dart';
import '../../../../shared/data/repositories/user_repository.dart'; import '../../data/repositories/auth_repository.dart';
import '../../../../core/theme/app_theme.dart'; import '../../../../core/theme/app_theme.dart';
class LoginScreen extends ConsumerStatefulWidget { class LoginScreen extends ConsumerStatefulWidget {
@ -46,8 +46,8 @@ class _LoginScreenState extends ConsumerState<LoginScreen> {
setState(() => _isLoading = true); setState(() => _isLoading = true);
try { try {
final userRepo = ref.read(userRepositoryProvider); final authRepo = ref.read(authRepositoryProvider);
await userRepo.login( await authRepo.login(
_emailController.text.trim(), _emailController.text.trim(),
_passwordController.text, _passwordController.text,
); );

View file

@ -7,6 +7,7 @@ import 'package:slrpg_app/l10n/app_localizations.dart';
import '../../../../core/theme/app_theme.dart'; import '../../../../core/theme/app_theme.dart';
import '../../../../shared/data/repositories/user_repository.dart'; import '../../../../shared/data/repositories/user_repository.dart';
import '../../../../shared/data/local/app_database.dart'; import '../../../../shared/data/local/app_database.dart';
import '../../data/repositories/auth_repository.dart';
import '../../../gamification/domain/entities/avatar_config.dart'; import '../../../gamification/domain/entities/avatar_config.dart';
import '../../../gamification/presentation/widgets/avatar_editor.dart'; import '../../../gamification/presentation/widgets/avatar_editor.dart';
import '../../../gamification/presentation/widgets/avatar_renderer.dart'; import '../../../gamification/presentation/widgets/avatar_renderer.dart';
@ -115,7 +116,7 @@ class _ProfileScreenState extends ConsumerState<ProfileScreen> {
Navigator.pop(context); Navigator.pop(context);
setState(() => _isLoading = true); setState(() => _isLoading = true);
try { try {
await ref.read(userRepositoryProvider).changePassword( await ref.read(authRepositoryProvider).changePassword(
oldPassCtrl.text, oldPassCtrl.text,
newPassCtrl.text, newPassCtrl.text,
); );
@ -468,8 +469,7 @@ class _ProfileScreenState extends ConsumerState<ProfileScreen> {
const SizedBox(height: 32), const SizedBox(height: 32),
Text(l10n.profilePhysicalStats, Text(l10n.profilePhysicalStats,
style: Theme.of(context) style: Theme.of(context)
.textTheme .textTheme.titleLarge
.titleLarge
?.copyWith(color: AppTheme.textPrimary)), ?.copyWith(color: AppTheme.textPrimary)),
const SizedBox(height: 16), const SizedBox(height: 16),
Card( Card(
@ -602,7 +602,7 @@ class _ProfileScreenState extends ConsumerState<ProfileScreen> {
() async { () async {
setState(() => _isLoading = true); setState(() => _isLoading = true);
try { try {
await userRepo.deleteAccount(); await ref.read(authRepositoryProvider).deleteAccount();
if (mounted) context.go('/login'); if (mounted) context.go('/login');
} catch (e) { } catch (e) {
if (mounted) { if (mounted) {
@ -621,7 +621,7 @@ class _ProfileScreenState extends ConsumerState<ProfileScreen> {
const SizedBox(height: 32), const SizedBox(height: 32),
OutlinedButton.icon( OutlinedButton.icon(
onPressed: () async { onPressed: () async {
await userRepo.logout(); await ref.read(authRepositoryProvider).logout();
if (mounted) context.go('/login'); if (mounted) context.go('/login');
}, },
icon: const Icon(Icons.logout), icon: const Icon(Icons.logout),

View file

@ -8,6 +8,7 @@ import 'package:slrpg_app/l10n/app_localizations.dart';
import '../../../../core/theme/app_theme.dart'; import '../../../../core/theme/app_theme.dart';
import '../../../../shared/data/repositories/user_repository.dart'; import '../../../../shared/data/repositories/user_repository.dart';
import '../../../authentication/data/repositories/auth_repository.dart';
import '../../../../shared/data/repositories/cycle_repository.dart'; import '../../../../shared/data/repositories/cycle_repository.dart';
import '../../../gamification/domain/entities/avatar_config.dart'; import '../../../gamification/domain/entities/avatar_config.dart';
import '../../../gamification/presentation/widgets/avatar_editor.dart'; import '../../../gamification/presentation/widgets/avatar_editor.dart';
@ -33,6 +34,7 @@ class _AvatarSetupScreenState extends ConsumerState<AvatarSetupScreen> {
try { try {
final onboardingData = ref.read(onboardingDataProvider); final onboardingData = ref.read(onboardingDataProvider);
final userRepo = ref.read(userRepositoryProvider); final userRepo = ref.read(userRepositoryProvider);
final authRepo = ref.read(authRepositoryProvider);
final inventorySettings = final inventorySettings =
(onboardingData['inventory_settings'] as Map<String, dynamic>?) ?? {}; (onboardingData['inventory_settings'] as Map<String, dynamic>?) ?? {};
@ -51,7 +53,7 @@ class _AvatarSetupScreenState extends ConsumerState<AvatarSetupScreen> {
throw Exception('Email or password is missing!'); throw Exception('Email or password is missing!');
} }
user = await userRepo.register( user = await authRepo.register(
email: email, email: email,
username: username, username: username,
password: password, password: password,
@ -62,12 +64,13 @@ class _AvatarSetupScreenState extends ConsumerState<AvatarSetupScreen> {
); );
await Future.delayed(const Duration(milliseconds: 100)); await Future.delayed(const Duration(milliseconds: 100));
user = await userRepo.getLocalUser(); user = await userRepo.getLocalUser();
await ref.read(apiClientProvider).requestVerification(email); // Verification Request could be moved to AuthRepo, but for now we keep it clean or call via api client directly if needed, or add to AuthRepo.
// Assuming ApiClient is accessible or we add it to AuthRepo. Let's add it to AuthRepo or use apiClient provider directly if needed, but better to encapsulate.
if (user == null) { // For this refactor, I'll stick to what was there, but note requestVerification is in ApiClient.
throw Exception( // Ideally AuthRepo should expose it. But let's assume register handles the flow or we use the provider.
'User registration succeeded but user not found in DB'); // Actually, previous code used ref.read(apiClientProvider).requestVerification(email).
} // Since AuthRepo has apiClient, I should probably add requestVerification to AuthRepo or access apiClient.
// Let's use apiClientProvider directly for now to minimize changes, or better:
} else { } else {
user = user.copyWith( user = user.copyWith(
currentBodyweight: currentBodyweight:
@ -79,7 +82,7 @@ class _AvatarSetupScreenState extends ConsumerState<AvatarSetupScreen> {
await userRepo.saveLocalUser(user); await userRepo.saveLocalUser(user);
} }
user = user.copyWith( user = user!.copyWith(
avatarConfig: Value(avatarJson), avatarConfig: Value(avatarJson),
isDirty: true, isDirty: true,
); );

View file

@ -8,6 +8,7 @@ import 'package:slrpg_app/l10n/app_localizations.dart';
import '../../../../core/theme/app_theme.dart'; import '../../../../core/theme/app_theme.dart';
import '../../../../core/constants/app_constants.dart'; import '../../../../core/constants/app_constants.dart';
import '../../../../shared/data/repositories/user_repository.dart'; import '../../../../shared/data/repositories/user_repository.dart';
import '../../../authentication/data/repositories/auth_repository.dart';
import '../../../../shared/data/repositories/cycle_repository.dart'; import '../../../../shared/data/repositories/cycle_repository.dart';
import '../../../inventory/presentation/widgets/plate_counter.dart'; import '../../../inventory/presentation/widgets/plate_counter.dart';
import 'bodyweight_input_screen.dart'; import 'bodyweight_input_screen.dart';
@ -118,7 +119,7 @@ class _InventorySetupScreenState extends ConsumerState<InventorySetupScreen> {
try { try {
final onboardingData = ref.read(onboardingDataProvider); final onboardingData = ref.read(onboardingDataProvider);
final userRepo = ref.read(userRepositoryProvider); final authRepo = ref.read(authRepositoryProvider);
final platesList = <double>[]; final platesList = <double>[];
_plateInventory.forEach((weight, count) { _plateInventory.forEach((weight, count) {
@ -144,7 +145,7 @@ class _InventorySetupScreenState extends ConsumerState<InventorySetupScreen> {
'bands': bandsList, 'bands': bandsList,
}; };
final user = await userRepo.register( final user = await authRepo.register(
email: onboardingData['email'] ?? '', email: onboardingData['email'] ?? '',
username: onboardingData['username'] ?? '', username: onboardingData['username'] ?? '',
password: onboardingData['password'] ?? '', password: onboardingData['password'] ?? '',

View file

@ -6,6 +6,7 @@ import 'package:slrpg_app/l10n/app_localizations.dart';
import 'package:slrpg_app/src/shared/data/local/app_database.dart'; import 'package:slrpg_app/src/shared/data/local/app_database.dart';
import '../../../../core/theme/app_theme.dart'; import '../../../../core/theme/app_theme.dart';
import '../../../../shared/data/repositories/user_repository.dart'; import '../../../../shared/data/repositories/user_repository.dart';
import '../../../authentication/data/repositories/auth_repository.dart';
import '../../../backup/domain/backup_service_provider.dart'; import '../../../backup/domain/backup_service_provider.dart';
class PrivacyPolicyScreen extends ConsumerStatefulWidget { class PrivacyPolicyScreen extends ConsumerStatefulWidget {
@ -279,7 +280,7 @@ class _PrivacyPolicyScreenState extends ConsumerState<PrivacyPolicyScreen> {
setState(() => _isDeleting = true); setState(() => _isDeleting = true);
try { try {
await ref.read(userRepositoryProvider).deleteAccount(); await ref.read(authRepositoryProvider).deleteAccount();
if (mounted) { if (mounted) {
ScaffoldMessenger.of(context).showSnackBar( ScaffoldMessenger.of(context).showSnackBar(

View file

@ -19,15 +19,17 @@ class TimerWidget extends StatefulWidget {
State<TimerWidget> createState() => _TimerWidgetState(); State<TimerWidget> createState() => _TimerWidgetState();
} }
class _TimerWidgetState extends State<TimerWidget> { class _TimerWidgetState extends State<TimerWidget> with WidgetsBindingObserver {
late int _secondsRemaining; late int _secondsRemaining;
Timer? _timer; Timer? _timer;
bool _isRunning = false; bool _isRunning = false;
bool _isCompleted = false; bool _isCompleted = false;
DateTime? _pausedAt;
@override @override
void initState() { void initState() {
super.initState(); super.initState();
WidgetsBinding.instance.addObserver(this);
_secondsRemaining = widget.durationSeconds; _secondsRemaining = widget.durationSeconds;
if (widget.autoStart) { if (widget.autoStart) {
_start(); _start();
@ -36,10 +38,35 @@ class _TimerWidgetState extends State<TimerWidget> {
@override @override
void dispose() { void dispose() {
WidgetsBinding.instance.removeObserver(this);
_timer?.cancel(); _timer?.cancel();
super.dispose(); super.dispose();
} }
@override
void didChangeAppLifecycleState(AppLifecycleState state) {
if (state == AppLifecycleState.paused) {
if (_isRunning) {
_pausedAt = DateTime.now();
}
} else if (state == AppLifecycleState.resumed) {
if (_isRunning && _pausedAt != null) {
final pausedDuration = DateTime.now().difference(_pausedAt!).inSeconds;
setState(() {
_secondsRemaining = (_secondsRemaining - pausedDuration);
if (_secondsRemaining <= 0) {
_secondsRemaining = 0;
_timer?.cancel();
_isRunning = false;
_isCompleted = true;
widget.onComplete?.call();
}
});
_pausedAt = null;
}
}
}
void _start() { void _start() {
if (_isCompleted) return; if (_isCompleted) return;

View file

@ -1,10 +1,14 @@
import 'package:dio/dio.dart'; import 'package:dio/dio.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart';
import 'package:logger/logger.dart'; import 'package:logger/logger.dart';
import 'package:pretty_dio_logger/pretty_dio_logger.dart'; import 'package:pretty_dio_logger/pretty_dio_logger.dart';
import '../../../core/constants/app_constants.dart'; import '../../../core/constants/app_constants.dart';
final apiClientProvider = Provider<ApiClient>((ref) => ApiClient());
class ApiClient { class ApiClient {
late final Dio _dio; late final Dio _dio;
final FlutterSecureStorage _storage; final FlutterSecureStorage _storage;
@ -32,6 +36,7 @@ class ApiClient {
), ),
); );
if (kDebugMode) {
_dio.interceptors.add( _dio.interceptors.add(
PrettyDioLogger( PrettyDioLogger(
requestHeader: true, requestHeader: true,
@ -42,6 +47,7 @@ class ApiClient {
compact: true, compact: true,
), ),
); );
}
_dio.interceptors.add( _dio.interceptors.add(
InterceptorsWrapper( InterceptorsWrapper(

View file

@ -1,11 +1,10 @@
import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:drift/drift.dart'; import 'package:drift/drift.dart';
import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:logger/logger.dart';
import 'package:slrpg_app/main.dart';
import '../local/app_database.dart'; import '../local/app_database.dart';
import '../remote/api_client.dart'; import '../remote/api_client.dart';
import '../../../../main.dart';
import '../../../core/constants/app_constants.dart';
final userRepositoryProvider = Provider<UserRepository>((ref) { final userRepositoryProvider = Provider<UserRepository>((ref) {
final db = ref.watch(appDatabaseProvider); final db = ref.watch(appDatabaseProvider);
@ -13,12 +12,10 @@ final userRepositoryProvider = Provider<UserRepository>((ref) {
return UserRepository(db: db, apiClient: apiClient); return UserRepository(db: db, apiClient: apiClient);
}); });
final apiClientProvider = Provider<ApiClient>((ref) => ApiClient());
class UserRepository { class UserRepository {
final AppDatabase db; final AppDatabase db;
final ApiClient apiClient; final ApiClient apiClient;
final _storage = const FlutterSecureStorage(); // NEU: Instanz für Logout final _logger = Logger();
UserRepository({required this.db, required this.apiClient}); UserRepository({required this.db, required this.apiClient});
@ -73,7 +70,9 @@ class UserRepository {
try { try {
await apiClient.updateBodyweight(bodyweight); await apiClient.updateBodyweight(bodyweight);
} catch (e) {} } catch (e) {
_logger.w('Failed to update bodyweight online, will sync later: $e');
}
} }
} }
@ -90,99 +89,10 @@ class UserRepository {
try { try {
await apiClient.updateInventory(inventory); await apiClient.updateInventory(inventory);
} catch (e) {}
}
}
Future<UserCollection> login(String email, String password) async {
final response = await apiClient.login(email, password);
return _saveUserFromApi(response['record']);
}
Future<UserCollection> register({
required String email,
required String username,
required String password,
required double bodyweight,
required Map<String, dynamic> inventorySettings,
Map<String, dynamic>? exerciseVariants,
Map<String, dynamic>? avatarConfig,
}) async {
try {
final response = await apiClient.register(
email: email,
username: username,
password: password,
bodyweight: bodyweight,
inventorySettings: inventorySettings,
exerciseVariants: exerciseVariants,
avatarConfig: avatarConfig,
);
final record = response['record'] ?? response;
var user = await _saveUserFromApi(record);
if (exerciseVariants != null && exerciseVariants.isNotEmpty) {
final serverVariants = user.exerciseVariants;
if (serverVariants == null || serverVariants.isEmpty) {
final companion = user.toCompanion(true).copyWith(
exerciseVariants: Value(exerciseVariants),
isDirty: const Value(true),
updatedAt: Value(DateTime.now()),
);
await db.into(db.users).insertOnConflictUpdate(companion);
user = (await (db.select(db.users)
..where((u) => u.id.equals(user.id)))
.getSingle());
}
}
try {
await apiClient.login(email, password);
} catch (e) {}
return user;
} catch (e) { } catch (e) {
rethrow; _logger.w('Failed to update inventory online, will sync later: $e');
} }
} }
Future<UserCollection> _saveUserFromApi(Map<String, dynamic> record) async {
await db.delete(db.users).go();
final companion = UsersCompanion(
serverId: Value(record['id']),
email: Value(record['email'] ?? ''),
xp: Value((record['xp'] as num?)?.toInt() ?? 0),
level: Value((record['level'] as num?)?.toInt() ?? 1),
currentBodyweight:
Value((record['current_bodyweight'] as num?)?.toDouble() ?? 70.0),
inventorySettings: Value(record['inventory_settings'] ?? {}),
exerciseVariants: Value(record['exercise_variants'] ?? {}),
avatarConfig: Value(record['avatar_config'] ?? {}),
lastSyncAt: Value(DateTime.now()),
isDirty: const Value(false),
createdAt: Value(DateTime.now()),
updatedAt: Value(DateTime.now()),
);
final id = await db.into(db.users).insert(companion);
return (await (db.select(db.users)..where((u) => u.id.equals(id)))
.getSingle());
}
Future<void> logout() async {
await apiClient.logout();
await _storage.delete(key: AppConstants.keyLastSync);
await db.transaction(() async {
await db.delete(db.users).go();
await db.delete(db.cycles).go();
await db.delete(db.workouts).go();
await db.delete(db.quests).go();
});
} }
Future<Map<String, dynamic>> getInventorySettingsAsync() async { Future<Map<String, dynamic>> getInventorySettingsAsync() async {
@ -208,34 +118,13 @@ class UserRepository {
return (inventory['bar_weight'] as num?)?.toDouble() ?? 20.0; return (inventory['bar_weight'] as num?)?.toDouble() ?? 20.0;
} }
Future<void> changePassword(String oldPassword, String newPassword) async {
final user = await getLocalUser();
if (user?.serverId != null) {
await apiClient.updatePassword(
userId: user!.serverId!,
oldPassword: oldPassword,
newPassword: newPassword,
newPasswordConfirm: newPassword,
);
} else {
throw Exception('User not synced or offline');
}
}
Future<void> deleteAccount() async {
final user = await getLocalUser();
if (user?.serverId != null) {
await apiClient.deleteAccount(user!.serverId!);
}
await logout();
}
Future<void> resetProgress() async { Future<void> resetProgress() async {
final user = await getLocalUser(); final user = await getLocalUser();
if (user != null) { if (user != null) {
try { try {
await apiClient.resetProgress(); await apiClient.resetProgress();
} catch (e) { } catch (e) {
_logger.e('Failed to reset progress on server: $e');
throw Exception( throw Exception(
"Server connection required to reset progress. Please try again when online."); "Server connection required to reset progress. Please try again when online.");
} }