Skip to content

Commit 46bba64

Browse files
authored
Merge pull request #17 from headlines-toolkit/fix_authentication
Fix authentication
2 parents deb906e + ad65ef2 commit 46bba64

File tree

2 files changed

+43
-22
lines changed

2 files changed

+43
-22
lines changed

lib/src/services/auth_service.dart

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -75,25 +75,23 @@ class AuthService {
7575
// For dashboard login, first validate the user exists and has permissions.
7676
if (isDashboardLogin) {
7777
final user = await _findUserByEmail(email);
78+
79+
// For dashboard login, the user must exist AND have permission.
80+
// If either condition fails, throw the appropriate exception.
7881
if (user == null) {
7982
_log.warning('Dashboard login failed: User $email not found.');
8083
throw const UnauthorizedException(
8184
'This email address is not registered for dashboard access.',
8285
);
83-
}
84-
85-
// Use the PermissionService to check for the specific dashboard login permission.
86-
if (!_permissionService.hasPermission(
87-
user,
88-
Permissions.dashboardLogin,
89-
)) {
86+
} else if (!_permissionService.hasPermission(user, Permissions.dashboardLogin)) {
9087
_log.warning(
9188
'Dashboard login failed: User ${user.id} lacks required permission (${Permissions.dashboardLogin}).',
9289
);
9390
throw const ForbiddenException(
9491
'Your account does not have the required permissions to sign in.',
9592
);
9693
}
94+
9795
_log.info('Dashboard user ${user.id} verified successfully.');
9896
}
9997

@@ -105,11 +103,13 @@ class AuthService {
105103
await _emailRepository.sendOtpEmail(recipientEmail: email, otpCode: code);
106104
_log.info('Initiated email sign-in for $email, code sent.');
107105
} on HtHttpException {
108-
// Propagate known exceptions from dependencies
106+
// Propagate known exceptions from dependencies or from this method's logic.
107+
// This ensures that specific errors like ForbiddenException are not
108+
// masked as a generic server error.
109109
rethrow;
110-
} catch (e) {
111-
// Catch unexpected errors during orchestration
112-
_log.severe('Error during initiateEmailSignIn for $email: $e');
110+
} catch (e, s) {
111+
// Catch unexpected errors during orchestration.
112+
_log.severe('Error during initiateEmailSignIn for $email: $e', e, s);
113113
throw const OperationFailedException(
114114
'Failed to initiate email sign-in process.',
115115
);
@@ -166,13 +166,25 @@ class AuthService {
166166
// This closes the loophole where a non-admin user could request a code
167167
// via the app flow and then use it to log into the dashboard.
168168
if (isDashboardLogin) {
169+
if (user.email != email) {
170+
// This is a critical security check. If the user found by email
171+
// somehow has a different email than the one provided, it's a
172+
// sign of a serious issue (like the data layer bug we fixed).
173+
// We throw a generic error to avoid revealing information.
174+
_log.severe(
175+
'CRITICAL: Mismatch between requested email ($email) and found '
176+
'user email (${user.email}) during dashboard login for user '
177+
'ID ${user.id}.',
178+
);
179+
throw const UnauthorizedException('User account does not exist.');
180+
}
169181
if (!_permissionService.hasPermission(
170182
user,
171183
Permissions.dashboardLogin,
172184
)) {
173185
_log.warning(
174-
'Dashboard login failed: User ${user.id} lacks required permission '
175-
'during code verification.',
186+
'Dashboard login failed: User ${user.id} lacks required '
187+
'permission during code verification.',
176188
);
177189
throw const ForbiddenException(
178190
'Your account does not have the required permissions to sign in.',
@@ -254,14 +266,14 @@ class AuthService {
254266
'Created default UserContentPreferences for user: ${user.id}',
255267
);
256268
}
257-
} on HtHttpException catch (e) {
258-
_log.severe('Error finding/creating user for $email: $e');
259-
throw const OperationFailedException(
260-
'Failed to find or create user account.',
261-
);
262-
} catch (e) {
269+
} on HtHttpException {
270+
// Propagate known exceptions from dependencies or from this method's logic.
271+
// This ensures that specific errors like ForbiddenException are not
272+
// masked as a generic server error.
273+
rethrow;
274+
} catch (e, s) {
263275
_log.severe(
264-
'Unexpected error during user lookup/creation for $email: $e',
276+
'Unexpected error during user lookup/creation for $email: $e', e, s,
265277
);
266278
throw const OperationFailedException('Failed to process user account.');
267279
}

routes/api/v1/auth/request-code.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,17 @@ Future<Response> onRequest(RequestContext context) async {
4343
);
4444
}
4545

46-
// Check for the optional dashboard login flag. Default to false if not present.
47-
final isDashboardLogin = (body['isDashboardLogin'] as bool?) ?? false;
46+
// Check for the optional dashboard login flag. This handles both boolean
47+
// `true` and string `"true"` values to prevent type cast errors.
48+
// It defaults to `false` if the key is missing or the value is not
49+
// recognized as true.
50+
final isDashboardLoginRaw = body['isDashboardLogin'];
51+
var isDashboardLogin = false;
52+
if (isDashboardLoginRaw is bool) {
53+
isDashboardLogin = isDashboardLoginRaw;
54+
} else if (isDashboardLoginRaw is String) {
55+
isDashboardLogin = isDashboardLoginRaw.toLowerCase() == 'true';
56+
}
4857

4958
// Basic email format check (more robust validation can be added)
5059
// Using a slightly more common regex pattern

0 commit comments

Comments
 (0)