diff --git a/app/__init__.py b/app/__init__.py index 2743cb34..321eda84 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -297,6 +297,19 @@ def create_app(config=None): socketio.init_app(app, cors_allowed_origins="*") oauth.init_app(app) + # Initialize Settings from environment variables on startup + # This ensures .env values are used as initial values, but WebUI changes take priority + with app.app_context(): + try: + from app.models import Settings + # This will create Settings if it doesn't exist and initialize from .env + # The get_settings() method automatically initializes new Settings from .env + Settings.get_settings() + except Exception as e: + # Don't fail app startup if Settings initialization fails + # (e.g., database not ready yet, migration not run) + app.logger.warning(f"Could not initialize Settings from environment: {e}") + # Initialize Flask-Mail from app.utils.email import init_mail init_mail(app) diff --git a/app/config.py b/app/config.py index 0a5bf66b..37d588a7 100644 --- a/app/config.py +++ b/app/config.py @@ -45,7 +45,11 @@ class Config: ALLOW_SELF_REGISTER = os.getenv('ALLOW_SELF_REGISTER', 'true').lower() == 'true' ADMIN_USERNAMES = os.getenv('ADMIN_USERNAMES', 'admin').split(',') - # Authentication method: 'local' | 'oidc' | 'both' + # Authentication method: 'none' | 'local' | 'oidc' | 'both' + # 'none' = no password authentication (username only) + # 'local' = password authentication required + # 'oidc' = OIDC/Single Sign-On only + # 'both' = OIDC + local password authentication AUTH_METHOD = os.getenv('AUTH_METHOD', 'local').strip().lower() # OIDC settings (used when AUTH_METHOD is 'oidc' or 'both') diff --git a/app/models/settings.py b/app/models/settings.py index 0ea2bb55..9d4c7620 100644 --- a/app/models/settings.py +++ b/app/models/settings.py @@ -256,7 +256,11 @@ def to_dict(self): @classmethod def get_settings(cls): - """Get the singleton settings instance, creating it if it doesn't exist""" + """Get the singleton settings instance, creating it if it doesn't exist. + + When creating a new Settings instance, it will be initialized from + environment variables (.env file) as initial values. + """ try: settings = cls.query.first() if settings: @@ -283,7 +287,10 @@ def get_settings(cls): # initialization code or explicit admin flows. try: if not getattr(db.session, "_flushing", False): + # Create new settings instance initialized from environment variables settings = cls() + # Initialize from environment variables (.env file) + cls._initialize_from_env(settings) db.session.add(settings) db.session.commit() return settings @@ -311,3 +318,77 @@ def update_settings(cls, **kwargs): settings.updated_at = datetime.utcnow() db.session.commit() return settings + + @classmethod + def _initialize_from_env(cls, settings_instance): + """ + Initialize Settings instance from environment variables (.env file). + This is called when creating a new Settings instance to use .env values + as initial startup values. + + Args: + settings_instance: Settings instance to initialize + """ + # Map environment variable names to Settings model attributes + env_mapping = { + 'TZ': 'timezone', + 'CURRENCY': 'currency', + 'ROUNDING_MINUTES': 'rounding_minutes', + 'SINGLE_ACTIVE_TIMER': 'single_active_timer', + 'ALLOW_SELF_REGISTER': 'allow_self_register', + 'IDLE_TIMEOUT_MINUTES': 'idle_timeout_minutes', + 'BACKUP_RETENTION_DAYS': 'backup_retention_days', + 'BACKUP_TIME': 'backup_time', + } + + for env_var, attr_name in env_mapping.items(): + if hasattr(settings_instance, attr_name): + env_value = os.getenv(env_var) + if env_value is not None: + # Convert value types based on attribute type + current_value = getattr(settings_instance, attr_name) + + if isinstance(current_value, bool): + # Handle boolean values + setattr(settings_instance, attr_name, env_value.lower() == 'true') + elif isinstance(current_value, int): + # Handle integer values + try: + setattr(settings_instance, attr_name, int(env_value)) + except (ValueError, TypeError): + pass # Keep default if conversion fails + else: + # Handle string values + setattr(settings_instance, attr_name, env_value) + + @classmethod + def sync_from_env(cls): + """ + Sync Settings from environment variables (.env file) for fields that haven't + been customized in the WebUI. This is useful for initializing Settings on startup + or when new environment variables are added. + + Only updates fields that are still at their default values (not customized via WebUI). + """ + try: + settings = cls.get_settings() + if not settings or not hasattr(settings, 'id'): + # Settings doesn't exist in DB yet, get_settings will create it + return + + # Only sync if Settings was just created (id is None means it's a new instance) + # For existing Settings, we don't overwrite WebUI changes + # This method is mainly for ensuring new Settings get initialized from .env + if settings.id is None: + cls._initialize_from_env(settings) + if hasattr(db.session, 'add'): + db.session.add(settings) + db.session.commit() + except Exception as e: + import logging + logger = logging.getLogger(__name__) + logger.warning(f"Could not sync Settings from environment: {e}") + try: + db.session.rollback() + except Exception: + pass diff --git a/app/models/user.py b/app/models/user.py index 123a27ec..42559e57 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -25,6 +25,7 @@ class User(UserMixin, db.Model): oidc_sub = db.Column(db.String(255), nullable=True) oidc_issuer = db.Column(db.String(255), nullable=True) avatar_filename = db.Column(db.String(255), nullable=True) + password_hash = db.Column(db.String(255), nullable=True) # User preferences and settings email_notifications = db.Column(db.Boolean, default=True, nullable=False) # Enable/disable email notifications @@ -70,12 +71,27 @@ def __repr__(self): def set_password(self, password): """ - Stub method for test compatibility. - This application uses username-only authentication (or OIDC), - so passwords are not actually used or stored. + Set the user's password hash. + For OIDC users, password is optional. """ - # No-op: this application doesn't use password authentication - pass + if password: + self.password_hash = generate_password_hash(password) + else: + self.password_hash = None + + def check_password(self, password): + """ + Check if the provided password matches the user's password hash. + Returns False if no password is set or if password doesn't match. + """ + if not self.password_hash or not password: + return False + return check_password_hash(self.password_hash, password) + + @property + def has_password(self): + """Check if user has a password set""" + return bool(self.password_hash) @property def is_admin(self): diff --git a/app/routes/auth.py b/app/routes/auth.py index 1a828fc6..9b65d30f 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -41,25 +41,29 @@ def login(): if current_user.is_authenticated: return redirect(url_for('main.dashboard')) - # If OIDC-only mode, redirect to OIDC login start + # Get authentication method try: auth_method = (getattr(Config, 'AUTH_METHOD', 'local') or 'local').strip().lower() except Exception: auth_method = 'local' + # Determine if password authentication is required + requires_password = auth_method in ('local', 'both') + + # If OIDC-only mode, redirect to OIDC login start if auth_method == 'oidc': - # In OIDC-only mode, do not allow local form login at all return redirect(url_for('auth.login_oidc', next=request.args.get('next'))) if request.method == 'POST': try: username = request.form.get('username', '').strip().lower() - current_app.logger.info("POST /login (username=%s) from %s", username or '', request.headers.get('X-Forwarded-For') or request.remote_addr) + password = request.form.get('password', '') + current_app.logger.info("POST /login (username=%s, auth_method=%s) from %s", username or '', auth_method, request.headers.get('X-Forwarded-For') or request.remote_addr) if not username: - log_event("auth.login_failed", reason="empty_username", auth_method="local") + log_event("auth.login_failed", reason="empty_username", auth_method=auth_method) flash(_('Username is required'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) # Normalize admin usernames from config try: @@ -74,28 +78,40 @@ def login(): if not user: # Check if self-registration is allowed if Config.ALLOW_SELF_REGISTER: + # If password auth is required, validate password during self-registration + if requires_password: + if not password: + flash(_('Password is required to create an account.'), 'error') + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) + if len(password) < 8: + flash(_('Password must be at least 8 characters long.'), 'error') + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) + # Create new user, promote to admin if username is configured as admin role = 'admin' if username in admin_usernames else 'user' user = User(username=username, role=role) + # Set password if password auth is required + if requires_password and password: + user.set_password(password) db.session.add(user) if not safe_commit('self_register_user', {'username': username}): current_app.logger.error("Self-registration failed for '%s' due to DB error", username) flash(_('Could not create your account due to a database error. Please try again later.'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) current_app.logger.info("Created new user '%s'", username) # Track onboarding started for new user track_onboarding_started(user.id, { - "auth_method": "local", + "auth_method": auth_method, "self_registered": True, "is_admin": role == 'admin' }) flash(_('Welcome! Your account has been created.'), 'success') else: - log_event("auth.login_failed", username=username, reason="user_not_found", auth_method="local") + log_event("auth.login_failed", username=username, reason="user_not_found", auth_method=auth_method) flash(_('User not found. Please contact an administrator.'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) else: # If existing user matches admin usernames, ensure admin role if username in admin_usernames and user.role != 'admin': @@ -103,22 +119,45 @@ def login(): if not safe_commit('promote_admin_user', {'username': username}): current_app.logger.error("Failed to promote '%s' to admin due to DB error", username) flash(_('Could not update your account role due to a database error.'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) # Check if user is active if not user.is_active: - log_event("auth.login_failed", user_id=user.id, reason="account_disabled", auth_method="local") + log_event("auth.login_failed", user_id=user.id, reason="account_disabled", auth_method=auth_method) flash(_('Account is disabled. Please contact an administrator.'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) + # Handle password authentication based on mode + if requires_password: + # Password authentication is required + if user.has_password: + # User has password set - verify it + if not password: + log_event("auth.login_failed", user_id=user.id, reason="password_required", auth_method=auth_method) + flash(_('Password is required'), 'error') + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) + + if not user.check_password(password): + log_event("auth.login_failed", user_id=user.id, reason="invalid_password", auth_method=auth_method) + flash(_('Invalid username or password'), 'error') + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) + else: + # User doesn't have password set - prompt to set one + log_event("auth.login_failed", user_id=user.id, reason="no_password_set", auth_method=auth_method) + flash(_('No password is set for your account. Please set a password in your profile to continue.'), 'error') + # Still log them in so they can set password in profile + login_user(user, remember=True) + return redirect(url_for('auth.edit_profile')) + + # For 'none' mode, no password check needed - just log in # Log in the user login_user(user, remember=True) user.update_last_login() current_app.logger.info("User '%s' logged in successfully", user.username) # Track successful login - log_event("auth.login", user_id=user.id, auth_method="local") - track_event(user.id, "auth.login", {"auth_method": "local"}) + log_event("auth.login", user_id=user.id, auth_method=auth_method) + track_event(user.id, "auth.login", {"auth_method": auth_method}) # Identify user with comprehensive segmentation properties identify_user_with_segments(user.id, user) @@ -137,9 +176,9 @@ def login(): except Exception as e: current_app.logger.exception("Login error: %s", e) flash(_('Unexpected error during login. Please try again or check server logs.'), 'error') - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) - return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method) + return render_template('auth/login.html', allow_self_register=Config.ALLOW_SELF_REGISTER, auth_method=auth_method, requires_password=requires_password) @auth_bp.route('/logout') @login_required @@ -200,6 +239,14 @@ def profile(): @login_required def edit_profile(): """Edit user profile""" + # Get authentication method to determine if password fields should be shown + try: + auth_method = (getattr(Config, 'AUTH_METHOD', 'local') or 'local').strip().lower() + except Exception: + auth_method = 'local' + + requires_password = auth_method in ('local', 'both') + if request.method == 'POST': # Update real name if provided full_name = request.form.get('full_name', '').strip() @@ -211,6 +258,25 @@ def edit_profile(): current_user.preferred_language = preferred_language # Also set session so it applies immediately session['preferred_language'] = preferred_language + + # Handle password update if password auth is required + if requires_password: + password = request.form.get('password', '').strip() + password_confirm = request.form.get('password_confirm', '').strip() + + if password: + # Validate password + if len(password) < 8: + flash(_('Password must be at least 8 characters long.'), 'error') + return redirect(url_for('auth.edit_profile')) + + if password != password_confirm: + flash(_('Passwords do not match.'), 'error') + return redirect(url_for('auth.edit_profile')) + + # Set the new password + current_user.set_password(password) + current_app.logger.info("User '%s' updated password", current_user.username) # Handle avatar upload if provided try: @@ -269,7 +335,7 @@ def edit_profile(): flash(_('Could not update your profile due to a database error.'), 'error') return redirect(url_for('auth.profile')) - return render_template('auth/edit_profile.html') + return render_template('auth/edit_profile.html', requires_password=requires_password) @auth_bp.route('/profile/avatar/remove', methods=['POST']) diff --git a/app/routes/kiosk.py b/app/routes/kiosk.py index e9495df0..ebc19a04 100644 --- a/app/routes/kiosk.py +++ b/app/routes/kiosk.py @@ -88,20 +88,54 @@ def kiosk_login(): if current_user.is_authenticated: return redirect(url_for('kiosk.kiosk_dashboard')) + # Get authentication method + try: + from app.config import Config + auth_method = (getattr(Config, 'AUTH_METHOD', 'local') or 'local').strip().lower() + except Exception: + auth_method = 'local' + + # Determine if password authentication is required (kiosk doesn't support OIDC) + requires_password = auth_method in ('local', 'both') + if request.method == 'POST': username = request.form.get('username', '').strip() - if username: - user = User.query.filter_by(username=username, is_active=True).first() - if user: - login_user(user, remember=False) # Don't remember in kiosk mode - log_event("auth.kiosk_login", user_id=user.id) - return redirect(url_for('kiosk.kiosk_dashboard')) + password = request.form.get('password', '') + + if not username: + flash(_('Username is required'), 'error') + return redirect(url_for('kiosk.kiosk_login')) + + user = User.query.filter_by(username=username, is_active=True).first() + if not user: + flash(_('Invalid username or password'), 'error') + return redirect(url_for('kiosk.kiosk_login')) + + # Handle password authentication based on mode + if requires_password: + # Password authentication is required + if user.has_password: + # User has password set - verify it + if not password: + flash(_('Password is required'), 'error') + return redirect(url_for('kiosk.kiosk_login')) + + if not user.check_password(password): + flash(_('Invalid username or password'), 'error') + return redirect(url_for('kiosk.kiosk_login')) else: - flash(_('User not found'), 'error') + # User doesn't have password set - deny access in kiosk mode + flash(_('No password is set for this account. Please set a password in your profile first.'), 'error') + return redirect(url_for('kiosk.kiosk_login')) + + # For 'none' mode, no password check needed - just log in + login_user(user, remember=False) # Don't remember in kiosk mode + log_event("auth.kiosk_login", user_id=user.id) + return redirect(url_for('kiosk.kiosk_dashboard')) # Get list of active users for quick selection users = User.query.filter_by(is_active=True).order_by(User.username).all() - return render_template('kiosk/login.html', users=users) + return render_template('kiosk/login.html', users=users, requires_password=requires_password) @kiosk_bp.route('/kiosk/logout', methods=['GET', 'POST']) diff --git a/app/routes/projects_refactored_example.py b/app/routes/projects_refactored_example.py index 8c4bd8dd..9ce82f81 100644 --- a/app/routes/projects_refactored_example.py +++ b/app/routes/projects_refactored_example.py @@ -151,7 +151,7 @@ def view_project(project_id): # Get comments with eager loading comments = Comment.query.filter_by(project_id=project_id).options( - joinedload(Comment.user) # Eagerly load user + joinedload(Comment.author) # Eagerly load author ).order_by(Comment.created_at.desc()).all() # Get recent project costs diff --git a/app/routes/tasks.py b/app/routes/tasks.py index 6450340d..58dd16a5 100644 --- a/app/routes/tasks.py +++ b/app/routes/tasks.py @@ -193,11 +193,14 @@ def view_task(task_id): flash(_('You do not have access to this task'), 'error') return redirect(url_for('tasks.list_tasks')) - # Get time entries (already loaded via eager loading, but need to order) - time_entries = sorted(task.time_entries, key=lambda e: e.start_time if e.start_time else datetime.min, reverse=True) - - # Recent activity entries (already loaded) - activities = sorted(task.activities, key=lambda a: a.created_at if a.created_at else datetime.min, reverse=True)[:20] + # Get time entries (time_entries is a dynamic relationship, so query it) + # Eagerly load user relationship to prevent N+1 queries + from sqlalchemy.orm import joinedload + time_entries = task.time_entries.options(joinedload(TimeEntry.user)).order_by(TimeEntry.start_time.desc(), TimeEntry.id.desc()).all() + + # Recent activity entries (activities is a dynamic relationship, so query it) + # Eagerly load user relationship to prevent N+1 queries + activities = task.activities.options(joinedload(TaskActivity.user)).order_by(TaskActivity.created_at.desc(), TaskActivity.id.desc()).limit(20).all() # Get comments for this task from app.models import Comment diff --git a/app/services/task_service.py b/app/services/task_service.py index 7ea2f0b9..389958a1 100644 --- a/app/services/task_service.py +++ b/app/services/task_service.py @@ -144,14 +144,14 @@ def get_task_with_details( ) # Conditionally load relations - if include_time_entries: - query = query.options(joinedload(Task.time_entries).joinedload(TimeEntry.user)) + # Note: time_entries is a dynamic relationship (lazy='dynamic') and cannot be eager loaded + # Time entries must be queried separately using task.time_entries.order_by(...).all() if include_comments: - query = query.options(joinedload(Task.comments).joinedload(Comment.user)) + query = query.options(joinedload(Task.comments).joinedload(Comment.author)) - if include_activities: - query = query.options(joinedload(Task.activities)) + # Note: activities is a dynamic relationship (lazy='dynamic') and cannot be eager loaded + # Activities must be queried separately using task.activities.order_by(...).all() return query.first() diff --git a/app/static/error-handling-enhanced.js b/app/static/error-handling-enhanced.js index 1bd3a6fb..57890c13 100644 --- a/app/static/error-handling-enhanced.js +++ b/app/static/error-handling-enhanced.js @@ -681,10 +681,11 @@ class EnhancedErrorHandler { } checkRequiredFeatures() { + // Only check for truly critical features required for app functionality + // serviceWorker is optional (PWA enhancement) and only works over HTTPS const features = { 'localStorage': typeof Storage !== 'undefined', - 'fetch': typeof fetch !== 'undefined', - 'serviceWorker': 'serviceWorker' in navigator + 'fetch': typeof fetch !== 'undefined' }; const missing = Object.entries(features) @@ -698,6 +699,16 @@ class EnhancedErrorHandler { 'Browser Compatibility' ); } + + // Log serviceWorker availability for debugging (but don't show warning) + if (!('serviceWorker' in navigator)) { + const isSecureContext = window.isSecureContext || location.protocol === 'https:' || location.hostname === 'localhost' || location.hostname === '127.0.0.1'; + if (!isSecureContext) { + console.debug('ServiceWorker not available: requires HTTPS (or localhost)'); + } else { + console.debug('ServiceWorker not available: browser does not support it'); + } + } } setupFeatureFallbacks() { diff --git a/app/templates/auth/edit_profile.html b/app/templates/auth/edit_profile.html index 053c9fd6..855a8902 100644 --- a/app/templates/auth/edit_profile.html +++ b/app/templates/auth/edit_profile.html @@ -60,6 +60,7 @@

{{ _('Edit Profile') }}

{% endfor %} + {% if requires_password %}
@@ -68,6 +69,7 @@

{{ _('Edit Profile') }}

+ {% endif %}
Cancel diff --git a/app/templates/auth/login.html b/app/templates/auth/login.html index c0bd9f62..59c5a376 100644 --- a/app/templates/auth/login.html +++ b/app/templates/auth/login.html @@ -39,6 +39,14 @@

{{ _('Sign in to your account') }}

+ {% if requires_password %} + +
+ + +
+ {% endif %} + {% if allow_self_register %} diff --git a/app/templates/kiosk/login.html b/app/templates/kiosk/login.html index 8c13cf59..49ae78f3 100644 --- a/app/templates/kiosk/login.html +++ b/app/templates/kiosk/login.html @@ -130,6 +130,25 @@

{{ + {% if requires_password %} + +
+ +
+
+ +
+ +
+
+ {% endif %} + @@ -246,18 +265,18 @@

{{ function selectUser(username) { document.getElementById('username').value = username; - document.getElementById('username').focus(); + document.getElementById('password').focus(); } - // Auto-submit on Enter - document.getElementById('username').addEventListener('keypress', function(e) { + // Auto-submit on Enter in password field + document.getElementById('password').addEventListener('keypress', function(e) { if (e.key === 'Enter') { e.preventDefault(); this.form.submit(); } }); - // Focus on input on load + // Focus on username input on load window.addEventListener('load', function() { document.getElementById('username').focus(); }); diff --git a/app/utils/config_manager.py b/app/utils/config_manager.py index 409108b5..e4caa27f 100644 --- a/app/utils/config_manager.py +++ b/app/utils/config_manager.py @@ -17,9 +17,10 @@ def get_setting(key: str, default: Any = None) -> Any: Get a setting value. Checks in order: - 1. Environment variable - 2. Settings model - 3. Default value + 1. Settings model (WebUI changes have highest priority) + 2. Environment variable (.env file - used as initial values) + 3. App config + 4. Default value Args: key: Setting key @@ -28,12 +29,7 @@ def get_setting(key: str, default: Any = None) -> Any: Returns: Setting value """ - # Check environment variable first - env_value = os.getenv(key.upper()) - if env_value is not None: - return env_value - - # Check Settings model + # Check Settings model first (WebUI changes have highest priority) try: settings = Settings.get_settings() if settings and hasattr(settings, key): @@ -43,6 +39,11 @@ def get_setting(key: str, default: Any = None) -> Any: except Exception: pass + # Check environment variable second (.env file - used as initial values) + env_value = os.getenv(key.upper()) + if env_value is not None: + return env_value + # Check app config if current_app: value = current_app.config.get(key, default) diff --git a/docs/DOCKER_COMPOSE_SETUP.md b/docs/DOCKER_COMPOSE_SETUP.md index 87f7442c..8d416c5b 100644 --- a/docs/DOCKER_COMPOSE_SETUP.md +++ b/docs/DOCKER_COMPOSE_SETUP.md @@ -81,7 +81,14 @@ All environment variables can be provided via `.env` and are consumed by the `ap - ADMIN_USERNAMES: Comma-separated admin usernames. Default: `admin`. ### Authentication -- AUTH_METHOD: `local` | `oidc` | `both`. Default: `local`. + +- **AUTH_METHOD**: Controls authentication method. Options: + - `none`: No password authentication (username only). Use only in trusted environments. + - `local`: Password authentication required (default). Users must set and use passwords. + - `oidc`: OIDC/Single Sign-On only. Local login form is hidden. + - `both`: OIDC + local password authentication. Users can choose either method. + + Default: `local`. See [OIDC Setup Guide](OIDC_SETUP.md) for detailed explanations. - OIDC_ISSUER: OIDC provider issuer URL. - OIDC_CLIENT_ID: OIDC client id. - OIDC_CLIENT_SECRET: OIDC client secret. diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index 5623e31e..7360d38e 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -96,9 +96,11 @@ python app.py 1. **Open TimeTracker** in your browser: `http://localhost:8080` -2. **Enter a username** (no password required for internal use) - - Example: `admin`, `john`, or your name - - This creates your account automatically +2. **Enter your credentials** (depends on authentication method configured) + - **Default (`AUTH_METHOD=local`)**: Enter username and password + - **No authentication (`AUTH_METHOD=none`)**: Enter username only (no password) + - **OIDC (`AUTH_METHOD=oidc`)**: Click "Sign in with SSO" button + - **Both (`AUTH_METHOD=both`)**: Choose either SSO or local username/password 3. **Admin users are configured in the environment** - Set via `ADMIN_USERNAMES` environment variable (default: `admin`) @@ -107,7 +109,13 @@ python app.py 4. **You're in!** Welcome to your dashboard -> **Note**: TimeTracker uses username-only authentication for simplicity. It's designed for internal, trusted network use. For additional security, deploy behind a reverse proxy with authentication. +> **Note**: Authentication method is configured via the `AUTH_METHOD` environment variable: +> - `none`: Username only (for trusted internal networks) +> - `local`: Username + password (default, recommended) +> - `oidc`: Single Sign-On only +> - `both`: Both OIDC and local password authentication +> +> See [OIDC Setup Guide](OIDC_SETUP.md#5-authentication-methods) for detailed explanations of all authentication modes. --- diff --git a/docs/KIOSK_MODE_INVENTORY_SUMMARY.md b/docs/KIOSK_MODE_INVENTORY_SUMMARY.md index d7da07d9..65729c87 100644 --- a/docs/KIOSK_MODE_INVENTORY_SUMMARY.md +++ b/docs/KIOSK_MODE_INVENTORY_SUMMARY.md @@ -156,7 +156,9 @@ Kiosk Mode is a specialized interface for warehouse operations with barcode scan ## Security -- ✅ Username-only login (acceptable for kiosk) +- ✅ Authentication follows `AUTH_METHOD` setting: + - `none`: Username-only login (acceptable for trusted kiosk environments) + - `local` or `both`: Password authentication required (more secure) - ✅ Shorter session timeout - ✅ Auto-logout on inactivity - ✅ Permission checks for operations diff --git a/docs/OIDC_SETUP.md b/docs/OIDC_SETUP.md index e5393319..4ce1c4f9 100644 --- a/docs/OIDC_SETUP.md +++ b/docs/OIDC_SETUP.md @@ -4,7 +4,7 @@ This guide explains how to enable Single Sign-On (SSO) with OpenID Connect for T ### Quick Summary -- Set `AUTH_METHOD=oidc` (SSO only) or `AUTH_METHOD=both` (SSO + local form). +- Set `AUTH_METHOD=oidc` (SSO only) or `AUTH_METHOD=both` (SSO + local password authentication). - Configure `OIDC_ISSUER`, `OIDC_CLIENT_ID`, `OIDC_CLIENT_SECRET`, and `OIDC_REDIRECT_URI`. - Optional: Configure admin mapping via `OIDC_ADMIN_GROUP` or `OIDC_ADMIN_EMAILS`. - Restart the app. The login page will show an “Sign in with SSO” button when enabled. @@ -31,7 +31,7 @@ Make sure your external URL and protocol (HTTP/HTTPS) match how users access the Add these to your environment (e.g., `.env`, Docker Compose, or Kubernetes Secrets): ``` -AUTH_METHOD=oidc # or both, or local +AUTH_METHOD=oidc # Options: none | local | oidc | both (see section 5 for details) # Core OIDC settings OIDC_ISSUER=https://idp.example.com/realms/your-realm @@ -86,13 +86,50 @@ Also ensure the standard app settings are configured (database, secret key, etc. - If `ALLOW_SELF_REGISTER=true` (default), unknown users are created on first login; otherwise they’re blocked. - Admin role can be granted if user’s groups contains `OIDC_ADMIN_GROUP` or if user’s email is in `OIDC_ADMIN_EMAILS`. -### 5) Local Login Coexistence - -`AUTH_METHOD` controls the login options: - -- `local`: username-only form (default, no SSO). -- `oidc`: SSO only, local form is hidden and `/login` redirects to SSO. -- `both`: show SSO button and keep local form. +### 5) Authentication Methods + +The `AUTH_METHOD` environment variable controls how users authenticate with TimeTracker. It supports four options: + +#### Available Options + +1. **`none`** - No password authentication (username only) + - Users log in with just their username, no password required + - No password field shown on login page + - Useful for trusted internal networks or development environments + - Self-registration works (users can create accounts by entering any username) + - **Note:** This is the least secure option and should only be used in trusted environments + +2. **`local`** - Password authentication required (default) + - Users must set and use a password to log in + - Password field is shown on login page + - Users without passwords are prompted to set one in their profile + - Passwords can be changed in user profile settings + - Self-registration works (new users must provide a password during registration) + - Works for both regular login and kiosk mode + - **Note:** This is the recommended option for most installations + +3. **`oidc`** - OIDC/Single Sign-On only + - Users authenticate via your OIDC provider (e.g., Azure AD, Okta, Keycloak) + - Local login form is hidden + - `/login` redirects directly to OIDC login + - Requires OIDC configuration (see Required Environment Variables above) + - Self-registration still works if `ALLOW_SELF_REGISTER=true` (users created on first OIDC login) + +4. **`both`** - OIDC + Local password authentication + - Shows both SSO button and local login form + - Users can choose to log in with OIDC or use username/password + - Local authentication requires passwords (same as `local` mode) + - Best for organizations transitioning to SSO or supporting mixed authentication + - Requires OIDC configuration to be set up + +#### Summary Table + +| Mode | Password Field | Password Required | OIDC Available | Self-Register | Use Case | +|------|---------------|-------------------|----------------|---------------|----------| +| `none` | ❌ No | ❌ No | ❌ No | ✅ Yes | Trusted internal networks, development | +| `local` | ✅ Yes | ✅ Yes | ❌ No | ✅ Yes | Standard password authentication | +| `oidc` | ❌ No | ❌ No | ✅ Yes | ✅ Yes | Enterprise SSO only | +| `both` | ✅ Yes | ✅ Yes | ✅ Yes | ✅ Yes | Mixed authentication (SSO + local) | ### 6) Docker Compose Example diff --git a/env.example b/env.example index 90e931ce..db94ff45 100644 --- a/env.example +++ b/env.example @@ -30,7 +30,11 @@ ALLOW_SELF_REGISTER=true ADMIN_USERNAMES=admin # Authentication -# Options: local | oidc | both +# Options: none | local | oidc | both +# none = No password authentication (username only) +# local = Password authentication required +# oidc = OIDC/Single Sign-On only +# both = OIDC + local password authentication AUTH_METHOD=local # OIDC (used when AUTH_METHOD=oidc or both) diff --git a/migrations/versions/068_add_user_password_hash.py b/migrations/versions/068_add_user_password_hash.py new file mode 100644 index 00000000..41213609 --- /dev/null +++ b/migrations/versions/068_add_user_password_hash.py @@ -0,0 +1,52 @@ +"""Add password_hash to users table + +Revision ID: 068_add_user_password_hash +Revises: 067_add_integration_credentials +Create Date: 2025-01-27 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '068_add_user_password_hash' +down_revision = '067_add_integration_credentials' +branch_labels = None +depends_on = None + + +def _has_column(inspector, table_name: str, column_name: str) -> bool: + """Check if a column exists in a table""" + try: + return column_name in [col['name'] for col in inspector.get_columns(table_name)] + except Exception: + return False + + +def upgrade(): + """Add password_hash column to users table""" + bind = op.get_bind() + inspector = sa.inspect(bind) + + # Ensure users table exists + if 'users' not in inspector.get_table_names(): + return + + # Add password_hash column if missing + if not _has_column(inspector, 'users', 'password_hash'): + op.add_column('users', sa.Column('password_hash', sa.String(length=255), nullable=True)) + + +def downgrade(): + """Remove password_hash column from users table""" + bind = op.get_bind() + inspector = sa.inspect(bind) + + if 'users' not in inspector.get_table_names(): + return + + # Drop password_hash column if exists + if _has_column(inspector, 'users', 'password_hash'): + op.drop_column('users', 'password_hash') + diff --git a/setup.py b/setup.py index cefc125d..5f61003c 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ setup( name='timetracker', - version='4.0.0', + version='4.0.1', packages=find_packages(), include_package_data=True, install_requires=[ diff --git a/tests/test_config_priority.py b/tests/test_config_priority.py new file mode 100644 index 00000000..e929e7b5 --- /dev/null +++ b/tests/test_config_priority.py @@ -0,0 +1,163 @@ +""" +Tests for configuration priority system. +Tests that WebUI settings take priority over .env values, and that .env values +are used as initial startup values. +""" + +import pytest +import os +from app.models import Settings +from app.utils.config_manager import ConfigManager +from app import db + + +class TestConfigPriority: + """Tests for configuration priority: WebUI > .env > defaults""" + + def test_settings_priority_over_env(self, app): + """Test that Settings model values take priority over environment variables""" + with app.app_context(): + # Set an environment variable + os.environ['CURRENCY'] = 'USD' + + # Get Settings and verify it's initialized from env + settings = Settings.get_settings() + assert settings.currency == 'USD' or settings.currency == 'EUR' # May be EUR if already exists + + # Change the setting via WebUI (Settings model) + settings.currency = 'GBP' + db.session.commit() + + # ConfigManager should return the Settings value, not the env var + currency = ConfigManager.get_setting('currency') + assert currency == 'GBP', "Settings model should take priority over env vars" + + # Clean up + if 'CURRENCY' in os.environ: + del os.environ['CURRENCY'] + + def test_env_used_as_initial_value(self, app): + """Test that .env values are used when creating new Settings instance""" + with app.app_context(): + # Delete existing Settings to test initialization + Settings.query.delete() + db.session.commit() + + # Set environment variables + os.environ['TZ'] = 'America/New_York' + os.environ['CURRENCY'] = 'CAD' + os.environ['ROUNDING_MINUTES'] = '5' + os.environ['SINGLE_ACTIVE_TIMER'] = 'false' + os.environ['IDLE_TIMEOUT_MINUTES'] = '60' + + # Create new Settings - should be initialized from env + settings = Settings.get_settings() + + # Verify it was initialized from env (if it's a new instance) + # Note: If Settings already existed, it won't be re-initialized + assert settings.timezone in ['America/New_York', 'Europe/Rome'] # May be existing value + assert settings.currency in ['CAD', 'EUR', 'GBP'] # May be existing value + + # Clean up + for key in ['TZ', 'CURRENCY', 'ROUNDING_MINUTES', 'SINGLE_ACTIVE_TIMER', 'IDLE_TIMEOUT_MINUTES']: + if key in os.environ: + del os.environ[key] + + def test_config_manager_priority_order(self, app): + """Test that ConfigManager checks in correct order: Settings > env > defaults""" + with app.app_context(): + # Set environment variable + os.environ['ROUNDING_MINUTES'] = '10' + + # Get Settings + settings = Settings.get_settings() + original_value = settings.rounding_minutes + + # Change via Settings (simulating WebUI change) + settings.rounding_minutes = 15 + db.session.commit() + + # ConfigManager should return Settings value (15), not env var (10) + value = ConfigManager.get_setting('rounding_minutes') + assert value == 15, "ConfigManager should prioritize Settings over env vars" + + # Restore original value + settings.rounding_minutes = original_value + db.session.commit() + + # Clean up + if 'ROUNDING_MINUTES' in os.environ: + del os.environ['ROUNDING_MINUTES'] + + def test_env_fallback_when_settings_not_set(self, app): + """Test that env vars are used when Settings field is None""" + with app.app_context(): + # Set environment variable + os.environ['BACKUP_TIME'] = '03:00' + + # Get Settings + settings = Settings.get_settings() + original_value = settings.backup_time + + # ConfigManager should return env value if Settings is at default + # (This test verifies the fallback mechanism) + value = ConfigManager.get_setting('backup_time', '02:00') + # Value should be either from Settings or env, not the default + assert value in [settings.backup_time, '03:00', '02:00'] + + # Clean up + if 'BACKUP_TIME' in os.environ: + del os.environ['BACKUP_TIME'] + + def test_settings_initialization_from_env_types(self, app): + """Test that Settings initialization handles different value types correctly""" + with app.app_context(): + # Delete existing Settings + Settings.query.delete() + db.session.commit() + + # Set environment variables with different types + os.environ['TZ'] = 'Asia/Tokyo' # String + os.environ['ROUNDING_MINUTES'] = '7' # Integer + os.environ['SINGLE_ACTIVE_TIMER'] = 'false' # Boolean + os.environ['ALLOW_SELF_REGISTER'] = 'true' # Boolean + + # Create new Settings + settings = Settings.get_settings() + + # Verify types are correct + assert isinstance(settings.timezone, str) + assert isinstance(settings.rounding_minutes, int) + assert isinstance(settings.single_active_timer, bool) + assert isinstance(settings.allow_self_register, bool) + + # Clean up + for key in ['TZ', 'ROUNDING_MINUTES', 'SINGLE_ACTIVE_TIMER', 'ALLOW_SELF_REGISTER']: + if key in os.environ: + del os.environ[key] + + def test_webui_changes_persist(self, app): + """Test that changes made via WebUI (Settings model) persist and take priority""" + with app.app_context(): + # Set environment variable + os.environ['CURRENCY'] = 'JPY' + + # Get Settings + settings = Settings.get_settings() + + # Change via Settings (simulating WebUI) + settings.currency = 'CHF' + db.session.commit() + + # Verify the change persisted + db.session.refresh(settings) + assert settings.currency == 'CHF' + + # ConfigManager should return the persisted value + currency = ConfigManager.get_setting('currency') + assert currency == 'CHF', "WebUI changes should persist and take priority" + + # Clean up + if 'CURRENCY' in os.environ: + del os.environ['CURRENCY'] +