Skip to content

Added logging for certificate verification when linked and added testing#178

Merged
dimoschi merged 3 commits intohackthebox:mainfrom
janssensjelle:feature/log-certificate-verification
Apr 3, 2026
Merged

Added logging for certificate verification when linked and added testing#178
dimoschi merged 3 commits intohackthebox:mainfrom
janssensjelle:feature/log-certificate-verification

Conversation

@janssensjelle
Copy link
Copy Markdown
Contributor

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply.

  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality not to work as expected).
  • Documentation Update (if none of the other choices applies).

Proposed changes

Sends a message to the verification channel of saying which user used which cert id

Checklist

Put an x in the boxes that apply.

  • I have read and followed the CONTRIBUTING.md
    doc.
  • Lint and unit tests pass locally with my changes.
  • I have added the necessary documentation (if appropriate).

Additional context

Add any other context or screenshots here.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.10%. Comparing base (0f08c66) to head (da620c1).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/webhooks/handlers/academy.py 75.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   61.74%   62.10%   +0.35%     
==========================================
  Files          50       50              
  Lines        2941     2953      +12     
==========================================
+ Hits         1816     1834      +18     
+ Misses       1125     1119       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@dimoschi dimoschi left a comment

Choose a reason for hiding this comment

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

Good feature overall — the audit logging is a useful addition and the tests are well-structured. Two things to address before merging:

  1. The channel send needs its own try/except (see inline). As-is, a Discord API failure on the log message will be misreported as a role assignment error and cause an incorrect failure response to the webhook caller, even though the role was already applied. account.py has the right pattern.
  2. Missing test for the verify_channel = None path — one uncovered line flagged by Codecov.

Copy link
Copy Markdown
Contributor

@dimoschi dimoschi left a comment

Choose a reason for hiding this comment

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

@janssensjelle thanks for addressing my comments. The PR looks good.

@dimoschi dimoschi merged commit b0eb0e1 into hackthebox:main Apr 3, 2026
3 checks passed
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