Skip to content

Fix authentication #16

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.

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

Merged
merged 3 commits into from
Jul 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion lib/src/config/app_dependencies.dart
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,12 @@ class AppDependencies {
);
verificationCodeStorageService =
InMemoryVerificationCodeStorageService();
permissionService = const PermissionService();
authService = AuthService(
userRepository: userRepository,
authTokenService: authTokenService,
verificationCodeStorageService: verificationCodeStorageService,
permissionService: permissionService,
emailRepository: emailRepository,
userAppSettingsRepository: userAppSettingsRepository,
userContentPreferencesRepository: userContentPreferencesRepository,
Expand All @@ -193,9 +195,9 @@ class AppDependencies {
topicRepository: topicRepository,
sourceRepository: sourceRepository,
);
permissionService = const PermissionService();
userPreferenceLimitService = DefaultUserPreferenceLimitService(
remoteConfigRepository: remoteConfigRepository,
permissionService: permissionService,
log: Logger('DefaultUserPreferenceLimitService'),
);

Expand Down
7 changes: 7 additions & 0 deletions lib/src/rbac/permissions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,11 @@ abstract class Permissions {
static const String remoteConfigRead = 'remote_config.read';
static const String remoteConfigUpdate = 'remote_config.update';
static const String remoteConfigDelete = 'remote_config.delete';

// Dashboard Permissions
static const String dashboardLogin = 'dashboard.login';

// User Preference Permissions
static const String userPreferenceBypassLimits =
'user_preference.bypass_limits';
}
2 changes: 2 additions & 0 deletions lib/src/rbac/role_permissions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ final Set<String> _dashboardPublisherPermissions = {
Permissions.headlineCreate,
Permissions.headlineUpdate,
Permissions.headlineDelete,
Permissions.dashboardLogin,
};

final Set<String> _dashboardAdminPermissions = {
Expand All @@ -50,6 +51,7 @@ final Set<String> _dashboardAdminPermissions = {
Permissions.remoteConfigCreate,
Permissions.remoteConfigUpdate,
Permissions.remoteConfigDelete,
Permissions.userPreferenceBypassLimits,
};

/// Defines the mapping between user roles (both app and dashboard) and the
Expand Down
39 changes: 31 additions & 8 deletions lib/src/services/auth_service.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'package:ht_api/src/rbac/permission_service.dart';
import 'package:ht_api/src/rbac/permissions.dart';
import 'package:ht_api/src/services/auth_token_service.dart';
import 'package:ht_api/src/services/verification_code_storage_service.dart';
import 'package:ht_data_repository/ht_data_repository.dart';
Expand All @@ -21,12 +23,14 @@ class AuthService {
required HtEmailRepository emailRepository,
required HtDataRepository<UserAppSettings> userAppSettingsRepository,
required HtDataRepository<UserContentPreferences>
userContentPreferencesRepository,
userContentPreferencesRepository,
required PermissionService permissionService,
required Uuid uuidGenerator,
required Logger log,
}) : _userRepository = userRepository,
_authTokenService = authTokenService,
_verificationCodeStorageService = verificationCodeStorageService,
_permissionService = permissionService,
_emailRepository = emailRepository,
_userAppSettingsRepository = userAppSettingsRepository,
_userContentPreferencesRepository = userContentPreferencesRepository,
Expand All @@ -39,7 +43,8 @@ class AuthService {
final HtEmailRepository _emailRepository;
final HtDataRepository<UserAppSettings> _userAppSettingsRepository;
final HtDataRepository<UserContentPreferences>
_userContentPreferencesRepository;
_userContentPreferencesRepository;
final PermissionService _permissionService;
final Logger _log;
final Uuid _uuid;

Expand Down Expand Up @@ -77,13 +82,13 @@ class AuthService {
);
}

final hasRequiredRole =
user.dashboardRole == DashboardUserRole.admin ||
user.dashboardRole == DashboardUserRole.publisher;

if (!hasRequiredRole) {
// Use the PermissionService to check for the specific dashboard login permission.
if (!_permissionService.hasPermission(
user,
Permissions.dashboardLogin,
)) {
_log.warning(
'Dashboard login failed: User ${user.id} lacks required roles.',
'Dashboard login failed: User ${user.id} lacks required permission (${Permissions.dashboardLogin}).',
);
throw const ForbiddenException(
'Your account does not have the required permissions to sign in.',
Expand Down Expand Up @@ -157,6 +162,24 @@ class AuthService {
final existingUser = await _findUserByEmail(email);
if (existingUser != null) {
user = existingUser;
// If this is a dashboard login, re-verify the user's dashboard role.
// This closes the loophole where a non-admin user could request a code
// via the app flow and then use it to log into the dashboard.
if (isDashboardLogin) {
if (!_permissionService.hasPermission(
user,
Permissions.dashboardLogin,
)) {
_log.warning(
'Dashboard login failed: User ${user.id} lacks required permission '
'during code verification.',
);
throw const ForbiddenException(
'Your account does not have the required permissions to sign in.',
);
}
_log.info('Dashboard user ${user.id} re-verified successfully.');
}
} else {
// User not found.
if (isDashboardLogin) {
Expand Down
19 changes: 15 additions & 4 deletions lib/src/services/default_user_preference_limit_service.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'package:ht_api/src/rbac/permission_service.dart';
import 'package:ht_api/src/rbac/permissions.dart';
import 'package:ht_api/src/services/user_preference_limit_service.dart';
import 'package:ht_data_repository/ht_data_repository.dart';
import 'package:ht_shared/ht_shared.dart';
Expand All @@ -11,11 +13,14 @@ class DefaultUserPreferenceLimitService implements UserPreferenceLimitService {
/// {@macro default_user_preference_limit_service}
const DefaultUserPreferenceLimitService({
required HtDataRepository<RemoteConfig> remoteConfigRepository,
required PermissionService permissionService,
required Logger log,
}) : _remoteConfigRepository = remoteConfigRepository,
_permissionService = permissionService,
_log = log;

final HtDataRepository<RemoteConfig> _remoteConfigRepository;
final PermissionService _permissionService;
final Logger _log;

// Assuming a fixed ID for the RemoteConfig document
Expand All @@ -34,8 +39,11 @@ class DefaultUserPreferenceLimitService implements UserPreferenceLimitService {
);
final limits = remoteConfig.userPreferenceConfig;

// Admins have no limits.
if (user.dashboardRole == DashboardUserRole.admin) {
// Users with the bypass permission (e.g., admins) have no limits.
if (_permissionService.hasPermission(
user,
Permissions.userPreferenceBypassLimits,
)) {
return;
}
Comment on lines +42 to 48

Choose a reason for hiding this comment

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

medium

The logic for checking user permissions and retrieving remote configuration is duplicated in the checkUpdatePreferences method (lines 105-111). To avoid this duplication, extract this logic into a private helper method.


Expand Down Expand Up @@ -94,8 +102,11 @@ class DefaultUserPreferenceLimitService implements UserPreferenceLimitService {
);
final limits = remoteConfig.userPreferenceConfig;

// Admins have no limits.
if (user.dashboardRole == DashboardUserRole.admin) {
// Users with the bypass permission (e.g., admins) have no limits.
if (_permissionService.hasPermission(
user,
Permissions.userPreferenceBypassLimits,
)) {
return;
}
Comment on lines +105 to 111

Choose a reason for hiding this comment

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

medium

The logic for checking user permissions and retrieving remote configuration is duplicated from the checkAddItem method (lines 42-48). Extract this logic into a private helper method to avoid code duplication.


Expand Down
Loading