Skip to content

feat(security): Add authorization checks for share deletion#1127

Open
Bhagyashri77777 wants to merge 13 commits into
imDarshanGK:mainfrom
Bhagyashri77777:fix-share-deletion-585
Open

feat(security): Add authorization checks for share deletion#1127
Bhagyashri77777 wants to merge 13 commits into
imDarshanGK:mainfrom
Bhagyashri77777:fix-share-deletion-585

Conversation

@Bhagyashri77777

Copy link
Copy Markdown
Contributor

Description

This PR resolves Issue #585 by securing the share deletion flow to ensure only authorized actors can delete shared analysis results.

Since the current database schema does not explicitly link a user_id to SharedSnippet records, the authorization check ensures that only authenticated users (current_user) can access the deletion endpoint.

Key Changes:

  • Backend: Added the delete_share endpoint in backend/app/routers/share.py, enforcing a get_current_user dependency.
  • Testing: Added test_delete_share_unauthorized and test_delete_share_authorized in backend/tests/test_share.py to verify that unauthorized requests are rejected (401/403) and valid requests succeed (204).

Fixes #585

Checklist

  • I have read the contributing guidelines.
  • I have added tests that prove my fix is effective.
  • New and existing unit tests pass locally with my changes.

@Bhagyashri77777

Copy link
Copy Markdown
Contributor Author

Hi @imDarshanGK!

Just a quick update: I have resolved all the formatting and import sorting issues that the CI pipeline caught (black & isort). All checks are now fully green and passing!

The PR is ready for your review whenever you have the time. Thank you!

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@Bhagyashri77777 Thanks for the contribution.

This PR does not fully resolve Issue #585.

Required changes:

  • Add authorization checks so that only the share owner or an admin can delete a shared result.
  • Add audit logging for share deletions.
  • Add tests covering owner, admin, and unauthorized (403) deletion scenarios.
  • Remove unrelated changes from this PR, as it currently includes modifications outside the scope of this issue.
  • add video demo

Please update the PR accordingly and request another review.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

👋 This PR has had no activity for 7 days.

Please push updates or comment if you still need more time.

Inactive PRs may be closed automatically after 7 more days.

@github-actions github-actions Bot added the stale label Jul 7, 2026
@Bhagyashri77777 Bhagyashri77777 force-pushed the fix-share-deletion-585 branch from 5bdd4b7 to f1845f8 Compare July 7, 2026 12:28
@Bhagyashri77777 Bhagyashri77777 force-pushed the fix-share-deletion-585 branch from d2d3016 to 36388cd Compare July 7, 2026 13:35
@Bhagyashri77777

Bhagyashri77777 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Hi @imDarshanGK,

I have updated the PR to fully resolve Issue #585. Here is what has been addressed:

Removed all unrelated file formatting changes to keep the PR clean.

Added authorization checks to ensure only owners and admins can delete shares.

Added an AuditLog entry upon successful deletion.

Added comprehensive unit tests covering owner, admin, and unauthorized (403) scenarios (passing successfully!).

Attached a video demo of the tests passing below.

The PR is ready for your review. Thank you!

Screen.Recording.2026-07-07.191557.mp4

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add permission checks for share deletion

2 participants