Skip to content

Commit 2070b6f

Browse files
Merge branch 'main' into authenticated-header
2 parents b97eefa + 7c49f5c commit 2070b6f

9 files changed

Lines changed: 55 additions & 25 deletions

File tree

Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ RUN \
209209
nano \
210210
sqlite3 \
211211
zip \
212+
gettext \
212213
libasound2t64 \
213214
libxtst6 \
214215
libxrandr2 \

cps/admin.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2062,6 +2062,7 @@ def _configuration_update_helper():
20622062

20632063
# security configuration
20642064
_config_checkbox(to_save, "config_disable_standard_login")
2065+
_config_checkbox(to_save, "config_enable_oauth_group_admin_management")
20652066
_config_checkbox(to_save, "config_check_extensions")
20662067
_config_checkbox(to_save, "config_password_policy")
20672068
_config_checkbox(to_save, "config_password_number")

cps/config_sql.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ class _Settings(_Base):
155155
config_ldap_auto_create_users = Column(Boolean, default=True)
156156
config_oauth_redirect_host = Column(String, default='')
157157
config_disable_standard_login = Column(Boolean, default=False)
158+
config_enable_oauth_group_admin_management = Column(Boolean, default=True)
158159

159160
schedule_start_time = Column(Integer, default=4)
160161
schedule_duration = Column(Integer, default=10)

cps/cw_babel.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ def get_locale():
3030
preferred = list()
3131
if has_request_context() and request.accept_languages:
3232
for x in request.accept_languages.values():
33+
# Skip wildcard '*' from Accept-Language headers (common in internal API requests)
34+
if x == '*':
35+
continue
3336
try:
3437
preferred.append(str(Locale.parse(x.replace('-', '_'))))
3538
except (UnknownLocaleError, ValueError) as e:

cps/oauth_bb.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,16 @@ def register_user_from_generic_oauth(token=None):
322322
# Apply default configuration settings for new OAuth users (Issue #660)
323323
# Match the same pattern as normal user creation in admin.py
324324

325-
# Set role: admin group overrides default role, otherwise use configured default
326-
if should_be_admin:
325+
# Set role: admin group overrides default role (only if group management enabled), otherwise use configured default
326+
if should_be_admin and config.config_enable_oauth_group_admin_management:
327327
user.role = constants.ROLE_ADMIN
328-
log.info("New OAuth user '%s' granted admin role via group '%s'", provider_username, admin_group)
328+
log.info("New OAuth user '%s' granted admin role via group '%s' (groups: %s)",
329+
provider_username, admin_group, user_groups)
329330
else:
330331
user.role = config.config_default_role
332+
if should_be_admin and not config.config_enable_oauth_group_admin_management:
333+
log.debug("New OAuth user '%s' not granted admin role - group-based management disabled",
334+
provider_username)
331335

332336
# Apply default user settings (same as normal user registration)
333337
user.sidebar_view = getattr(config, 'config_default_show', 1)
@@ -360,16 +364,23 @@ def register_user_from_generic_oauth(token=None):
360364
else:
361365
# Existing user: update admin role based on current group membership (Issue #715)
362366
# This ensures that users who are added to or removed from admin groups get proper access
367+
# Only enforce if group-based admin management is enabled (global setting)
363368
current_is_admin = user.role_admin()
364369

365-
if should_be_admin and not current_is_admin:
366-
# User was added to admin group - grant admin role
367-
user.role |= constants.ROLE_ADMIN
368-
log.info("OAuth user '%s' will be granted admin role via group '%s'", provider_username, admin_group)
369-
elif not should_be_admin and current_is_admin:
370-
# User was removed from admin group - revoke admin role (but keep other roles)
371-
user.role &= ~constants.ROLE_ADMIN
372-
log.info("OAuth user '%s' admin role will be revoked (not in group '%s')", provider_username, admin_group)
370+
if config.config_enable_oauth_group_admin_management:
371+
if should_be_admin and not current_is_admin:
372+
# User was added to admin group - grant admin role
373+
user.role |= constants.ROLE_ADMIN
374+
log.info("OAuth user '%s' granted admin role via group '%s' (groups: %s)",
375+
provider_username, admin_group, user_groups)
376+
elif not should_be_admin and current_is_admin:
377+
# User was removed from admin group - revoke admin role (but keep other roles)
378+
user.role &= ~constants.ROLE_ADMIN
379+
log.warning("OAuth user '%s' admin role revoked - not in required group '%s' (user groups: %s)",
380+
provider_username, admin_group, user_groups)
381+
else:
382+
log.debug("OAuth group-based admin management disabled - preserving manual role assignments for '%s'",
383+
provider_username)
373384
# Note: Changes are not committed yet - will be committed with OAuth entry below
374385

375386
oauth = ub.session.query(ub.OAuth).filter_by(
@@ -394,8 +405,8 @@ def register_user_from_generic_oauth(token=None):
394405
# Commit all changes together: OAuth entry + Token + User + Role updates
395406
try:
396407
ub.session_commit()
397-
# Log role changes after successful commit
398-
if user.role_admin() and should_be_admin:
408+
# Log role changes after successful commit (only if group management is enabled)
409+
if user.role_admin() and should_be_admin and config.config_enable_oauth_group_admin_management:
399410
log.info("OAuth user '%s' has admin role via group '%s'", provider_username, admin_group)
400411
except Exception as ex:
401412
log.error("Failed to save OAuth session for user '%s': %s", provider_username, ex)

cps/templates/config_edit.html

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ <h5>{{_('Field Mapping Configuration')}}</h5>
489489
<div class="form-group">
490490
<label for="config_generic_oauth_admin_group">{{_('OAuth group for Admin')}}</label>
491491
<input type="text" class="form-control" id="config_generic_oauth_admin_group" name="config_generic_oauth_admin_group" value="{% if generic['oauth_admin_group'] != None %}{{ generic['oauth_admin_group'] }}{% endif %}" autocomplete="off">
492+
<div class="help-block">{{_('Name of the OAuth group that grants admin privileges. Leave empty to disable group-based admin management, or use the global setting in Security Settings for more control.')}}</div>
492493
</div>
493494
<div class="form-group">
494495
<label for="config_generic_oauth_login_button">{{_('Login Button Text')}}</label>
@@ -552,6 +553,11 @@ <h4 class="settings-section-header">{{_('Security Settings')}}</h4>
552553
<label for="config_disable_standard_login">{{_('Disable Standard Login (Username/Password)')}}</label>
553554
<div class="help-block">{{_('Hides the standard login form. Users must log in via OAuth or LDAP. Ensure you have a working alternative login method before enabling.')}}</div>
554555
</div>
556+
<div class="form-group">
557+
<input type="checkbox" id="config_enable_oauth_group_admin_management" name="config_enable_oauth_group_admin_management" {% if config.config_enable_oauth_group_admin_management %}checked{% endif %}>
558+
<label for="config_enable_oauth_group_admin_management">{{_('Enable OAuth Group-Based Admin Role Management')}}</label>
559+
<div class="help-block">{{_('When enabled, admin privileges are automatically granted or revoked based on OAuth group membership. Disable this to manually manage admin roles in the user management panel, preventing OAuth logins from overriding local role assignments.')}}</div>
560+
</div>
555561
<div class="form-group">
556562
<input type="checkbox" id="config_ratelimiter" name="config_ratelimiter" {% if config.config_ratelimiter %}checked{% endif %}>
557563
<label for="config_ratelimiter">{{_('Limit failed login attempts')}}</label>
@@ -639,6 +645,16 @@ <h4 class="settings-section-header">{{_('Security Settings')}}</h4>
639645

640646
// Trigger change on load to set initial state
641647
$('#config_login_type').trigger('change');
648+
649+
// Warn about potential lockout when disabling standard login without OAuth (Issue #715)
650+
$('#config_disable_standard_login').on('change', function() {
651+
var isDisabled = $(this).is(':checked');
652+
var loginType = $('#config_login_type').val();
653+
654+
if (isDisabled && loginType === '0') {
655+
alert('Warning: You are about to disable standard login without enabling OAuth or LDAP authentication. This may lock you out of the system. Ensure you have configured and tested an alternative login method before saving.');
656+
}
657+
});
642658

643659
$('#test_oidc_connection').on('click', function() {
644660
var serverUrl = $('#config_generic_oauth_server_url_test').val() || $('#config_generic_oauth_server_url').val();

cps/web.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,6 +1952,9 @@ def render_login(username="", password=""):
19521952
if url_for("web.logout") == next_url:
19531953
next_url = url_for("web.index")
19541954

1955+
# Get OAuth check status
1956+
oauth_check = oauth_bb.oauth_check if feature_support['oauth'] else {}
1957+
19551958
# Get generic OAuth login button text for display
19561959
generic_login_button = None
19571960
if feature_support['oauth']:

tests/docker/test_container_startup.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,21 +90,18 @@ def test_port_mapping(self, cwa_api_client):
9090
class TestDockerHealthChecks:
9191
"""Test container health and readiness."""
9292

93-
def test_container_stays_running(self):
93+
def test_container_stays_running(self, cwa_api_client):
9494
"""Verify container doesn't crash immediately after startup."""
9595
# Wait 10 seconds and verify it's still running
9696
time.sleep(10)
9797

9898
# Try to access the web interface - if container crashed, this will fail
99-
# Default to 8085 to avoid conflicts with production CWA on 8083
100-
test_port = os.getenv('CWA_TEST_PORT', '8085')
101-
99+
# The cwa_api_client fixture ensures the container is ready before this test runs
102100
try:
103-
response = requests.get(f"http://localhost:{test_port}", timeout=5)
101+
response = cwa_api_client["session"].get(cwa_api_client["base_url"], timeout=5)
104102
assert response.status_code == 200
105103
except requests.exceptions.ConnectionError:
106-
pytest.skip(f"No CWA container available on port {test_port}")
107-
assert response.status_code == 200
104+
pytest.fail("Container crashed after initial startup")
108105

109106
def test_logs_directory_created(self, test_volumes):
110107
"""Verify log directory structure is created."""

tests/integration/test_ingest_checksums.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ def test_multi_format_book_has_multiple_checksums(
150150
assert len(checksums) >= 1
151151

152152
def test_checksum_persists_after_container_restart(
153-
self, cwa_container, ingest_folder, library_folder, test_epub
153+
self, container_name, ingest_folder, library_folder, test_epub
154154
):
155155
"""Test that checksums survive container restart."""
156156
from conftest import volume_copy, get_db_path
@@ -172,7 +172,6 @@ def test_checksum_persists_after_container_restart(
172172
# Restart container (if we have container control)
173173
# This might not work in all test environments
174174
try:
175-
container_name = cwa_container.name if hasattr(cwa_container, 'name') else 'calibre-web-automated'
176175
subprocess.run(['docker', 'restart', container_name], check=True, timeout=30)
177176
time.sleep(20) # Wait for restart
178177

@@ -314,14 +313,12 @@ def test_checksums_generated_on_first_startup(
314313
assert len(checksum) == 32
315314
assert all(c in '0123456789abcdef' for c in checksum.lower())
316315

317-
def test_sentinel_file_prevents_regeneration(self, cwa_container):
316+
def test_sentinel_file_prevents_regeneration(self, container_name):
318317
"""Test that checksum generation only runs once per library."""
319318
import subprocess
320319

321320
# Check if sentinel file exists
322321
try:
323-
container_name = cwa_container.name if hasattr(cwa_container, 'name') else 'calibre-web-automated'
324-
325322
result = subprocess.run(
326323
['docker', 'exec', container_name, 'test', '-f', '/config/.checksums_generated'],
327324
capture_output=True

0 commit comments

Comments
 (0)