Skip to content

Improved ExternalSessionFeedback audit - #37

Merged
sgeulette merged 2 commits into
mainfrom
PARAF-431/audit_external_session_feedback
Jul 14, 2026
Merged

Improved ExternalSessionFeedback audit#37
sgeulette merged 2 commits into
mainfrom
PARAF-431/audit_external_session_feedback

Conversation

@chris-adam

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

Copy link
Copy Markdown
Contributor

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

    • Improved audit logging for session feedback by recording more detailed context, including the full incoming request payload.
  • Documentation

    • Updated the changelog for version 1.0b9 to reflect the audit logging improvement for external session feedback, with author attribution.

@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

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d7bbc409-6597-467a-81d1-e8b6b0ffd0ac

📥 Commits

Reviewing files that changed from the base of the PR and between 25062b8 and 95113b4.

📒 Files selected for processing (1)
  • CHANGES.rst

📝 Walkthrough

Walkthrough

This PR expands ExternalSessionFeedback audit entries with the incoming request payload, documents its token-based permission override, and adds a changelog entry for the unreleased 1.0b9 version.

Changes

Audit Enhancement and Documentation

Layer / File(s) Summary
Audit Enhancement & Permission Documentation
src/imio/esign/services/external_session_feedback.py, CHANGES.rst
The session feedback audit now includes the full request payload. check_permission documents its token-based authentication override, and the 1.0b9 changelog records the audit improvement.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

  • IMIO/​imio.esign#35: Modifies the same session-feedback handler while changing state/code handling and returned values.

Suggested reviewers: sgeulette

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: improving the ExternalSessionFeedback audit logging.
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.
✨ Finishing Touches
🧪 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.

@coveralls

coveralls commented May 8, 2026

Copy link
Copy Markdown

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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.

@sgeulette
sgeulette merged commit 9334cd6 into main Jul 14, 2026
5 checks passed
@sgeulette
sgeulette deleted the PARAF-431/audit_external_session_feedback branch July 14, 2026 13:43
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.

3 participants