One key for audit retention, and stop the SAML page claiming a feature exists
Wave 6: two settings pages that told an administrator the site was configured when it was not. That is worse than a missing feature, because nobody goes looking for it. Audit retention had TWO keys. shopdb/core/services/auditretention.py reads `audit_retentiondays` (category site, default 0). A second `audit_retention_days` was seeded under category audit with a default of 90, and the Audit & Logging page wrote to that one - so an administrator set a period, reloaded, saw it persisted, and nothing ever consulted it. Retention stayed off. The page now writes the key that is read, the duplicate is out of the seed list, and migration 7d39 carries a value an operator actually set on the dead key over to the live one before deleting it: their intent was "keep 30 days", and the fix should not silently discard it. The page's copy also claimed logs "will be automatically purged", which nothing does - purging is manual, from the site settings page or `flask audit purge --apply`. The Authentication page offered a full SAML configuration - metadata URL, entity id, ACS URL, auto-create, admin group - and reported "Configured - SAML login available" once two fields held text. There is no SAML anywhere in the codebase: no login route, no ACS, no assertion handling. SAML IS planned, so the settings stay and the page keeps them; what changes is that it now says plainly that nothing consumes them yet, and the status line never claims availability. The dangerous control there was "Allow Local Login". Turning it off did nothing at all, so an administrator could believe they had closed local password login while it stayed wide open. It is locked on until there is another way in, and says why. CONFIG.md marks the whole block reserved rather than describing it as working configuration. Scope: product
This commit is contained in:
95
migrations/versions/7d39_audit_retention_key_merge.py
Normal file
95
migrations/versions/7d39_audit_retention_key_merge.py
Normal file
@@ -0,0 +1,95 @@
|
||||
"""Merge the second audit-retention setting into the one that is actually read.
|
||||
|
||||
Two keys existed for one setting. shopdb/core/services/auditretention.py reads
|
||||
`audit_retentiondays` (seeded under category 'site', default 0). A second key
|
||||
`audit_retention_days` was seeded under category 'audit' with a default of 90,
|
||||
and the Audit & Logging settings page wrote to that one - so an administrator
|
||||
set a retention period, reloaded, saw it persisted, and nothing ever consulted
|
||||
it. Retention stayed off while the page said otherwise.
|
||||
|
||||
The page now writes the key that is read, and the duplicate is out of the seed
|
||||
list. This handles the sites that already have it: if an operator set a real
|
||||
value on the dead key and never set the live one, that intent is carried over
|
||||
rather than dropped. Their intent was "keep 30 days", and the fix should not
|
||||
silently discard it.
|
||||
|
||||
Not carried over when the live key already holds a non-default value - that one
|
||||
was set through the page that worked, and it wins.
|
||||
|
||||
The dead key is then deleted. Leaving it would preserve the ambiguity that
|
||||
caused this, and nothing reads it.
|
||||
|
||||
Revision ID: 7d39_audit_retention_key_merge
|
||||
Revises: 7d38_widen_importbatch_status
|
||||
"""
|
||||
from alembic import op
|
||||
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
revision = '7d39_audit_retention_key_merge'
|
||||
down_revision = '7d38_widen_importbatch_status'
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
DEAD = 'audit_retention_days'
|
||||
LIVE = 'audit_retentiondays'
|
||||
|
||||
|
||||
def _value(bind, key):
|
||||
return bind.execute(
|
||||
sa.text('SELECT value FROM settings WHERE `key` = :key'
|
||||
if bind.dialect.name == 'mysql'
|
||||
else 'SELECT value FROM settings WHERE "key" = :key'),
|
||||
{'key': key}).scalar()
|
||||
|
||||
|
||||
def upgrade():
|
||||
bind = op.get_bind()
|
||||
if not sa.inspect(bind).has_table('settings'):
|
||||
return
|
||||
|
||||
dead = _value(bind, DEAD)
|
||||
if dead is None:
|
||||
return
|
||||
|
||||
live = _value(bind, LIVE)
|
||||
# '90' was the dead key's seeded default, so it is not evidence of intent.
|
||||
operator_set_dead = dead not in (None, '', '90')
|
||||
live_is_default = live in (None, '', '0')
|
||||
|
||||
if operator_set_dead and live_is_default:
|
||||
if live is None:
|
||||
bind.execute(sa.text(
|
||||
'INSERT INTO settings (`key`, value, valuetype, category, '
|
||||
'description, createddate, modifieddate, isactive) VALUES '
|
||||
'(:key, :value, :valuetype, :category, :description, '
|
||||
'NOW(), NOW(), 1)'
|
||||
if bind.dialect.name == 'mysql' else
|
||||
'INSERT INTO settings ("key", value, valuetype, category, '
|
||||
'description, createddate, modifieddate, isactive) VALUES '
|
||||
'(:key, :value, :valuetype, :category, :description, '
|
||||
"datetime('now'), datetime('now'), 1)"),
|
||||
{'key': LIVE, 'value': dead, 'valuetype': 'integer',
|
||||
'category': 'site',
|
||||
'description': 'Delete audit log entries older than this many '
|
||||
'days. 0 keeps everything forever.'})
|
||||
else:
|
||||
bind.execute(sa.text(
|
||||
'UPDATE settings SET value = :value WHERE `key` = :key'
|
||||
if bind.dialect.name == 'mysql' else
|
||||
'UPDATE settings SET value = :value WHERE "key" = :key'),
|
||||
{'value': dead, 'key': LIVE})
|
||||
|
||||
bind.execute(sa.text(
|
||||
'DELETE FROM settings WHERE `key` = :key'
|
||||
if bind.dialect.name == 'mysql' else
|
||||
'DELETE FROM settings WHERE "key" = :key'), {'key': DEAD})
|
||||
|
||||
|
||||
def downgrade():
|
||||
"""Deliberately a no-op.
|
||||
|
||||
Re-creating a key that nothing reads, to sit beside the one that works,
|
||||
would restore the defect. The live setting keeps whatever value it holds.
|
||||
"""
|
||||
Reference in New Issue
Block a user