Skip to content

Improved ExternalSessionFeedback audit#37

Open
chris-adam wants to merge 1 commit intomainfrom
PARAF-431/audit_external_session_feedback
Open

Improved ExternalSessionFeedback audit#37
chris-adam wants to merge 1 commit intomainfrom
PARAF-431/audit_external_session_feedback

Conversation

@chris-adam
Copy link
Copy Markdown
Contributor

@chris-adam chris-adam commented May 8, 2026

Testé en staging (avant d'afficher data au lieu de juste value et message):

260508 114815 I (fpa_esign) user=- ip=91.121.254.225 action=session_feedback session=19 code=21 db_state=to_sign value="{u'sign_session_url': u'https://simplycosi-1-test.trustsigneurope.com/login?tenantName=IMIO&sessionId=25671&step=195073'}" message="Sign session created successfully."
260508 115301 I (fpa_esign) user=- ip=91.121.254.225 action=session_feedback session=19 code=23 db_state=completed value="{}" message="All documents {'Modele de base S0018 test PARAF 431__9c0c391df5b04d8291392b518350e917.pdf': '012999700000014__9c0c391df5b04d8291392b518350e917'} were successfully uploaded to Webservice GED."
260508 115305 I (fpa_esign) user=- ip=91.121.254.225 action=session_feedback session=19 code=22 db_state=completed value="{u'remaining_users': [], u'signed_users': [u'chris.adam@imio.be']}" message="One new user amongst ['chris.adam@imio.be'] has signed the document(s)"

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced audit logging for session feedback operations to capture complete request data and context.
  • Documentation

    • Updated changelog for version 1.0b9 with audit improvement entry.

@chris-adam chris-adam force-pushed the PARAF-431/audit_external_session_feedback branch from 9df0a38 to 25062b8 Compare May 8, 2026 09:56
@chris-adam chris-adam requested a review from sgeulette May 8, 2026 09:57
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR enhances the ExternalSessionFeedback audit logging to capture the complete incoming request payload alongside existing session identifiers and state information, adds clarifying documentation to the permission override method, and updates the changelog for the upcoming 1.0b9 release.

Changes

Audit Enhancement and Documentation

Layer / File(s) Summary
Audit Enhancement & Permission Documentation
src/imio/esign/services/external_session_feedback.py
The audit("session_feedback", ...) call expands to include the full request data payload. A docstring is added to check_permission documenting the token-based authentication override of the default permission check.
Changelog
CHANGES.rst
Version 1.0b9 release notes include a new entry documenting the ExternalSessionFeedback audit improvement.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • IMIO/imio.esign#35: Both PRs modify the session-feedback handling in external_session_feedback.py's reply logic; this PR adds enhanced audit data capture while the related PR modifies how feedback tuples and state/code are recorded.

Suggested reviewers

  • sgeulette

Poem

🐰 With audit trails now bold and bright,
The feedback path records the light,
Full payloads logged with care and grace,
Each token finds its rightful place!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'Improved ExternalSessionFeedback audit' directly and specifically describes the main change: enhancing audit logging for the ExternalSessionFeedback component.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch PARAF-431/audit_external_session_feedback

Comment @coderabbitai help to get the list of available commands and usage tips.

@coveralls
Copy link
Copy Markdown

coveralls commented May 8, 2026

Coverage Report for CI Build 25549275773

Coverage remained the same at 84.566%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 1 of 1 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1257
Covered Lines: 1063
Line Coverage: 84.57%
Coverage Strength: 0.85 hits per line

💛 - Coveralls

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/imio/esign/services/external_session_feedback.py`:
- Around line 88-91: The audit call is logging the raw request payload (data)
which may contain PII or tokens; update the code around the
audit("session_feedback", ...) call to build a sanitized payload instead: define
an allowlist of safe keys and copy only those from data (or redact sensitive
values like emails, tokens, and URLs by masking parts), then format that
sanitized_data for the audit message instead of the original data; keep use of
session_id, code, and db_state as-is and ensure the symbol audit is called with
the redacted/safe representation rather than the raw data.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26a7833e-e8cf-4a52-97ac-63f2ed842b6f

📥 Commits

Reviewing files that changed from the base of the PR and between 563200d and 25062b8.

📒 Files selected for processing (2)
  • CHANGES.rst
  • src/imio/esign/services/external_session_feedback.py

Comment on lines +88 to +91
audit(
"session_feedback",
'session={} code={} db_state={} data="{}"'.format(session_id, code, db_state, data),
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid storing raw request payload in audit records.

data may contain PII (emails) and sensitive URLs/tokens; writing the full payload to audit increases privacy/compliance risk. Prefer an allowlist + redaction before logging.

Proposed fix
+            safe_data = {
+                "app_session_id": data.get("app_session_id"),
+                "code": data.get("code"),
+                "session_state": data.get("session_state"),
+                "value_keys": sorted((data.get("value") or {}).keys()),
+            }
             audit(
                 "session_feedback",
-                'session={} code={} db_state={} data="{}"'.format(session_id, code, db_state, data),
+                'session={} code={} db_state={} data="{}"'.format(session_id, code, db_state, safe_data),
             )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/services/external_session_feedback.py` around lines 88 - 91,
The audit call is logging the raw request payload (data) which may contain PII
or tokens; update the code around the audit("session_feedback", ...) call to
build a sanitized payload instead: define an allowlist of safe keys and copy
only those from data (or redact sensitive values like emails, tokens, and URLs
by masking parts), then format that sanitized_data for the audit message instead
of the original data; keep use of session_id, code, and db_state as-is and
ensure the symbol audit is called with the redacted/safe representation rather
than the raw data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants