PM-23358 removing phishing blocker code
đī¸ Tracking
đ Objective
Phishing Blocker feature is now is frontend only and the BEEP server side code needs to be removed.
đ¸ Screenshots
â° Reminders before review
- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team
đĻŽ Reviewer guidelines
- đ (
:+1:) or similar for great changes - đ (
:memo:) or âšī¸ (:information_source:) for notes or general info - â (
:question:) for questions - đ¤ (
:thinking:) or đ (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - đ¨ (
:art:) for suggestions / improvements - â (
:x:) or â ī¸ (:warning:) for more significant problems or concerns needing attention - đą (
:seedling:) or âģī¸ (:recycle:) for future improvements or indications of technical debt - â (
:pick:) for minor or nitpick changes
Claude finished @prograhamming's task in 1m 30s ââ View job
Review Complete
The code review has been posted with detailed findings.
Summary: The PR successfully removes server-side BEEP implementation code (controllers, jobs, Azure services), but database schema and entity model cleanup remains incomplete.
Key findings:
- Database
UsePhishingBlockerfield still present in Organization table and related stored procedures/views - Entity properties still exist across 43+ files
- Requires rollback migration and entity model cleanup
Checkmarx One â Scan Summary & Details â bc61c948-bf42-4bd1-9936-ccd0dcb9257c
Great job! No new security vulnerabilities introduced in this pull request
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 54.38%. Comparing base (acfe0d7) to head (b530b73).
:warning: Report is 2 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #6668 +/- ##
==========================================
+ Coverage 54.19% 54.38% +0.19%
==========================================
Files 1922 1916 -6
Lines 85095 84769 -326
Branches 7610 7579 -31
==========================================
- Hits 46114 46105 -9
+ Misses 37221 36903 -318
- Partials 1760 1761 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Deletions look good to me. Do we need any additional migrations to remove tables or cleanup any jobs?
Yeah good callout. It looks like from the old code they added a JSON blob to a Azure Storage service and nothing to the relational databases we support. The only migration script related to Phishing Blocker I can find is Vijay's migrations (completed on 11/20/2025) adding theUsePhishingBlocker field to the Organization table and view related to that table.
Overall Assessment: REQUEST CHANGES
Critical Issues:
Finding 1: UsePhishingBlocker database field still present (src/Sql/dbo/Tables/Organization.sql:64)
Finding 2: UsePhishingBlocker property retained in Organization entity (src/Core/AdminConsole/Entities/Organization.cs:140)
Finding 3: Licensing code still references UsePhishingBlocker
Finding 1: Database Field Cleanup Required
â ī¸ IMPORTANT: The UsePhishingBlocker field was added to the Organization table in migration 2025-11-21_00_AddUsePhishingBlockerToOrganization.sql. Since phishing blocker is now frontend-only, this database field should be removed.
Details and fix
Files requiring cleanup:
- SQL Schema:
src/Sql/dbo/Tables/Organization.sql(line 64) - Stored Procedures:
Organization_CreateOrganization_UpdateOrganization_ReadAbilities
- Views:
OrganizationView.sqlOrganizationUserOrganizationDetailsView.sqlProviderUserProviderOrganizationDetailsView.sql
- EF Migrations: All three database providers (MySQL, PostgreSQL, SQLite)
- Admin Portal:
src/Admin/AdminConsole/Views/Shared/_OrganizationForm.cshtml(lines 160-161)
Required actions:
- Create a rollback migration script (e.g.,
2025-12-XX_RemoveUsePhishingBlockerFromOrganization.sql) - Drop the column:
ALTER TABLE [dbo].[Organization] DROP CONSTRAINT [DF_Organization_UsePhishingBlocker]; ALTER TABLE [dbo].[Organization] DROP COLUMN [UsePhishingBlocker]; - Update all stored procedures to remove the
@UsePhishingBlockerparameter - Update all views to remove the column from SELECT statements
- Create corresponding Entity Framework migrations for rollback
- Remove the checkbox from the admin portal form
The field defaults to 0 (false), so removal should not impact existing organizations.
Finding 2: Organization Entity Properties Cleanup
â ī¸ IMPORTANT: The UsePhishingBlocker property remains in the Organization entity and related models.
Details and fix
Locations still referencing UsePhishingBlocker:
src/Core/AdminConsole/Entities/Organization.cs(line 140) - Entity propertysrc/Core/Billing/Organizations/Models/OrganizationLicense.cs(line 146) - License modelsrc/Core/Billing/Licenses/Services/Implementations/OrganizationLicenseClaimsFactory.cs(line 60) - Claims factorysrc/Core/Billing/Licenses/LicenseConstants.cs(line 47) - Constants- Multiple response models, DTOs, and views across AdminConsole
Recommended approach:
Since phishing blocker is now frontend-only with no server-side functionality, remove the property entirely:
- Remove from
Organizationentity - Remove from
OrganizationLicensemodel - Remove from license claims factory
- Remove from license constants
- Update
Organization.UpdateFromLicense()method (line 342) to remove UsePhishingBlocker assignment - Update all repository implementations, views, and queries
- Update test fixtures that set this property
This ensures clean separation between server and client responsibilities.
Finding 3: Feature Flag Cleanup
âģī¸ DEBT: The PhishingDetection feature flag constant is still defined in src/Core/Constants.cs (line 246).
Details and cleanup needed
public const string PhishingDetection = "phishing-detection";
Action required:
Before removing, verify:
- No client-side code (web vault, browser extensions) references this flag server-side
- If clients use it for frontend-only functionality, update documentation to clarify it's client-managed
- If completely unused, remove the constant definition
This prevents technical debt from unused constants accumulating in the codebase.
Review Summary
The PR successfully removes the server-side BEEP implementation (API controller, jobs, Azure storage services, HTTP clients, and configuration). However, the database schema and entity model still contain the UsePhishingBlocker field, which creates inconsistency.
Since the feature is now frontend-only, the server should not maintain any references to phishing blocker configuration. A complete cleanup requires:
- Database migration to drop the column
- Remove properties from entity models
- Remove from licensing system
- Verify feature flag usage
Completeness: Code deletions are thorough â
Database cleanup: Incomplete - UsePhishingBlocker field remains â
Entity model cleanup: Incomplete - Properties still present â
Configuration cleanup: Complete â
Job cleanup: Complete â
Please address the database and entity model cleanup to complete the server-side removal.