Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-434] Hide Billing screen for Reseller clients #2783

Conversation

r-tome
Copy link
Contributor

@r-tome r-tome commented Mar 8, 2023

Type of change

- [ ] Bug fix
- [X] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

If an organization is managed by a Reseller, any Owner of that organization should only see billing information that is relevant to the organization.

Code changes

This PR is related to this clients PR.

  • src/Api/Models/Response/ProfileOrganizationResponseModel.cs: Added property for ProviderType
  • src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserOrganizationDetails.cs: Added property for ProviderType
  • src/Infrastructure.EntityFramework/Repositories/Queries/OrganizationUserOrganizationDetailsViewQuery.cs: Added property for ProviderType
  • src/Sql/dbo/Views/OrganizationUserOrganizationDetailsView.sql: Retrieving the ProviderType
  • util/Migrator/DbScripts/2023-03-08_OrganizationProviderType.sql: Migration script to update OrganizationUserOrganizationDetailsView

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@r-tome r-tome marked this pull request as ready for review March 13, 2023 14:56
@r-tome r-tome requested review from eliykat and a team March 13, 2023 14:56
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

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

Looks good, but as mentioned in bitwarden/clients#4955 we need to make sure we're not sending any unauthorized info to Reseller clients when they get their subscription info.

@r-tome r-tome requested a review from eliykat March 23, 2023 16:05
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

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

EDIT: deleted comment as not applicable

…y, ViewSubscription, EditSubscription and EditPaymentMethods
…g Subscription.Items = null when User does not have permission
@r-tome r-tome requested a review from eliykat March 27, 2023 09:26
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

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

The permissions stuff looks great!

{
var orgManagedByProvider = await ProviderIdForOrg(orgId) != null;
var orgManagedByMspProvider = (await _providerRepository.GetByOrganizationIdAsync(orgId))?.Type == ProviderType.Msp;
Copy link
Member

@eliykat eliykat Mar 28, 2023

Choose a reason for hiding this comment

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

The benefit of the GetProviderOrganizations helper in this class is that it caches its results.

I suggest adding ProviderType to ProviderUserOrganizationDetails so that you can use the existing helper method and avoid adding another repository dependency. (It also fits in with the other data in that model imo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried using that but it doesn't work if the user has no ProviderUser associated to that Organization. Here we just need to know if the Organization has a provider, it doesn't matter if the user has a ProviderUser.

@r-tome r-tome requested a review from eliykat March 28, 2023 16:21
@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

…creen-for-reseller-clients-web

# Conflicts:
#	src/Infrastructure.Dapper/Repositories/ProviderOrganizationRepository.cs
@r-tome r-tome requested a review from a team March 31, 2023 10:36
@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

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

:shipit:

@r-tome r-tome merged commit 33d9223 into feature/billing-obfuscation Apr 3, 2023
@r-tome r-tome deleted the AC-434-hide-billing-screen-for-reseller-clients-web branch April 3, 2023 09:57
r-tome added a commit that referenced this pull request Apr 14, 2023
* [EC-1014] Create Organization Status (Pending/Created) (#2610)

* [EC-427] Add columns 'Type' and 'BillingPhone' to Provider table

* [EC-427] Provider table Type and BillingPhone MySql migrations

* [EC-427] Provider table Type and BillingPhone Postgres migrations

* [EC-427] Add mysql migration script

* [EC-427] Add mysql migration script

* [EC-427] Updated Provider sql script to include default column value

* [EC-427] Removed default value from Provider.Type column

* [EC-427] Changed migration script to include a default value constraint instead of updating the null type

* [EC-427] Updated Sql project Provider table script

* [EC-427] Changed migration script to use 'Create OR Alter' for views and sprocs

* [EC-427] Added default values for 'BillingPhone' and 'Type' fields on sprocs [dbo].[Provider_Create] and [dbo].[Provider_Update]

* [EC-427] Adjusting metadata in migration script

* [EC-427] Updated Provider sprocs SQL script files

* [EC-427] Fixed migration script

* [EC-427] Added sqlite migration

* [EC-427] Add missing Provider_Update sproc default value

* [EC-427] Added missing GO action to migration script

* [EC-428] Redirect to Edit after creating Provider

* Revert "[EC-428] Redirect to Edit after creating Provider"

This reverts commit 6347bca.

* [EC-1014] Create OrganizationStatusType and add Status column to Organizations table

* [EC-1014] Added EF migrations

* [EC-1014] dotnet format

* [EC-1014] Changed Organization.Status from SMALLINT to TINYINT

* [EC-1014] Set Organization.Status default value = 1

* [EC-1014] Setting Organization.Status default value as 1

* [EC-459 / EC-428] Admin panel: Add Provider Type to list and creation flow (#2593)

* [EC-427] Add columns 'Type' and 'BillingPhone' to Provider table

* [EC-427] Provider table Type and BillingPhone MySql migrations

* [EC-427] Provider table Type and BillingPhone Postgres migrations

* [EC-427] Add mysql migration script

* [EC-427] Add mysql migration script

* [EC-427] Updated Provider sql script to include default column value

* [EC-427] Removed default value from Provider.Type column

* [EC-427] Changed migration script to include a default value constraint instead of updating the null type

* [EC-427] Updated Sql project Provider table script

* [EC-427] Changed migration script to use 'Create OR Alter' for views and sprocs

* [EC-427] Added default values for 'BillingPhone' and 'Type' fields on sprocs [dbo].[Provider_Create] and [dbo].[Provider_Update]

* [EC-427] Adjusting metadata in migration script

* [EC-427] Updated Provider sprocs SQL script files

* [EC-427] Fixed migration script

* [EC-427] Added sqlite migration

* [EC-427] Add missing Provider_Update sproc default value

* [EC-427] Added missing GO action to migration script

* [EC-459] Added Type column to Providers list

* [EC-428] Added Type, BusinessName and BillingEmail to CreateProviderModel

* [EC-428] Updated Create Provider view to include new fields

* [EC-428] Updated ProviderService to not create a ProviderUser for the type Reseller

* [EC-428] Added custom validation for Provider fields depending on selected Type

* [EC-428] Redirect to Edit after creating Provider

* [EC-428] Setting Provider status as Created for Resellers

* [EC-428] Redirect on Provider creation depending if self host server

* [EC-428] Split ProviderService.CreateAsync into two methods: CreateMspAsync and CreateResellerAsync

* [EC-428] Created ICreateProviderCommand and added service for injection on Admin.Startup

* [EC-428] Modified Provider views to use DisplayName attribute values

* [EC-428] Moved ICreateProviderCommand to Core project

* [EC-428] Adding ICreateProviderCommand injection next to IProviderService

* [EC-428] Moved CreateProviderCommand to Commercial.Core project

* [EC-459] Added Type column to Providers list

* [EC-428] Added Type, BusinessName and BillingEmail to CreateProviderModel

* [EC-428] Updated Create Provider view to include new fields

* [EC-428] Updated ProviderService to not create a ProviderUser for the type Reseller

* [EC-428] Added custom validation for Provider fields depending on selected Type

* [EC-428] Redirect to Edit after creating Provider

* [EC-428] Setting Provider status as Created for Resellers

* [EC-428] Redirect on Provider creation depending if self host server

* [EC-428] Split ProviderService.CreateAsync into two methods: CreateMspAsync and CreateResellerAsync

* [EC-428] Created ICreateProviderCommand and added service for injection on Admin.Startup

* [EC-428] Modified Provider views to use DisplayName attribute values

* [EC-428] Moved ICreateProviderCommand to Core project

* [EC-428] Adding ICreateProviderCommand injection next to IProviderService

* [EC-428] Moved CreateProviderCommand to Commercial.Core project

* [EC-428] Moved CreateProviderCommand to namespace Bit.Commercial.Core.Providers

* [EC-429] Provider details screen updated with Type, BillingPhone and Organization details (#2666)

* [EC-430] Admin portal: Update organization information screen (#2672)

* [EC-430] Added ProviderOrganizationProviderDetailsView to get Provider details for an Organization

* [EC-430] Added Provider information to Organization Edit/View on Admin panel

* [EC-430] Remove "Add to Reseller" button

* [EC-430] Removed unused property OrganizationEditModel.ClientOwnerEmail

* [EC-430] Replaced IProviderOrganizationRepository.GetProviderDetailsByOrganizationAsync with IProviderRepository.GetByOrganizationIdAsync

* [EC-430] Deleted ProviderOrganizationProviderDetails and ProviderOrganizationProviderDetailsReadByOrganizationIdQuery

* [EC-429] Only show Create/Add Existing Organization buttons for Reseller providers (#2723)

* [EC-432] Add existing Organizations to Provider (#2683)

* [EC-432] Added ProviderOrganizationUnassignedOrganizationDetails_Search stored procedure

* [EC-432] Added IProviderOrganizationRepository.SearchAsync

* [EC-432] Created controller ProviderOrganizationsController to assign Organizations to a Provider

* [EC-432] Filter existing organizations by plans Enterprise or Team

* [EC-432] Existing Organization name links to edit page

* [EC-432] EF filtering out existing organizations by plan type enterprise or teams

* [EC-432] Creating multiple ProviderOrganization records

* [EC-432] Added ProviderOrganizationUnassignedOrganizationDetails_Search stored procedure

* [EC-432] Added IProviderOrganizationRepository.SearchAsync

* [EC-432] Created controller ProviderOrganizationsController to assign Organizations to a Provider

* [EC-432] Filter existing organizations by plans Enterprise or Team

* [EC-432] Existing Organization name links to edit page

* [EC-432] EF filtering out existing organizations by plan type enterprise or teams

* [EC-432] Creating multiple ProviderOrganization records

* [EC-432] Renamed migration script and added missing sproc

* [EC-432] Saving multiple events for the created ProviderOrganizations

* [EC-432] Included unit testing for ProviderService.AddOrganizations and EventService.LogProviderOrganizationEventsAsync

* [EC-432] Removed async from NoopEventService.LogProviderOrganizationEventsAsync

* [EC-432] Remove unused dependency setup in ProviderServiceTests.AddOrganizations_Success

* [EC-432] Renamed AddOrganizations to AddOrganizationsToReseller and removed addingUserId and key arguments

* [EC-432] Added DisplayName attributes to ProviderOrganizationViewModel and used them in the view

* [EC-432] Reverted changes to input fields

* [EC-432] Moved unassigned organizations search to Organizations repo

* [EC-432] Moved AddExistingOrganization action to ProvidersController

* [EC-432] dotnet format

* [EC-432] Fixed unit test issues

* [EC-432] Removed unnecessary Html.DisplayNameFor for labels

* [EC-432] Renamed OrganizationSearchViewModel to OrganizationUnassignedToProviderSearchViewModel

* [EC-432] Modified IEventService.LogProviderOrganizationEventsAsync to receive an IEnumerable as parameter

* [EC-432] Updated IProviderOrganizationRepository and replaced CreateWithManyOrganizations method with CreateManyAsync

* [EC-432] Deleted ProviderOrganization_CreateWithManyOrganizations

* [AC-432] Simplified Organization_UnassignedToProviderSearch query

* [AC-432] Removed unnecessary setup

* [EC-432] Checking if stored procedure exists before creating

* [EC-432] Renamed migration file to recent date

* [EC-435] Admin Portal: Add new Organization creation flow UI (#2707)

* [EC-435] Created _OrganizationForm partial view. Added actions for creating an Organization assigned to a provider

* [EC-435] Remove logic for creating an organization

* [EC-435] Created partial view _OrganizationFormScripts

* [EC-435] Remove unused ReferenceEventType

* [EC-435] Added TODO comment on Organization Create

* [EC-435] Checking if Provider type is Reseller on creating new assigned organization

* [EC-435] Setting the Organization plan type as TeamsMonthly by default when adding to a provider

* [EC-435] Removing unused buttons

* [EC-435] Switched hidden fields to form submit route value

* [EC-435] Moved _OrganizationForm and _OrganizationFormScripts to Shared folder

* [EC-435] Moved Create organization actions from OrganizationsController to ProvidersController

* [EC-435] Fixing bug on saving Organization that would have BillingEmail as null

* [EC-435] Added null check to Provider

* [EC-435] Moved trial buttons script logic to Edit view

* [AC-431] Add new organization invite process (#2737)

* [EC-435] Created _OrganizationForm partial view. Added actions for creating an Organization assigned to a provider

* [EC-435] Remove logic for creating an organization

* [EC-435] Created partial view _OrganizationFormScripts

* [EC-435] Remove unused ReferenceEventType

* [EC-435] Added TODO comment on Organization Create

* [EC-435] Checking if Provider type is Reseller on creating new assigned organization

* [EC-435] Setting the Organization plan type as TeamsMonthly by default when adding to a provider

* [EC-435] Removing unused buttons

* [EC-435] Switched hidden fields to form submit route value

* [EC-435] Moved _OrganizationForm and _OrganizationFormScripts to Shared folder

* [EC-435] Moved Create organization actions from OrganizationsController to ProvidersController

* [AC-431] Added new ReferenceEventType OrganizationCreatedByAdmin

* [AC-431] Added method IOrganizationService.CreateOrganization

* [AC-431] Creating new Organization with Pending status and assigning to Provider

* [AC-431] Added method to IMailService to send invitation to initialize org

* [AC-431] Added methods CreatePendingOrganization and InitPendingOrganization to IOrganizationService

* [AC-431] Org invite includes initOrganization parameter

* [AC-431] Modified existing Accept organization user action to initialize org

* [AC-431] Updated ProvidersController method name

* [AC-431] Created OrganizationUserInitInvitedViewModel to link to 'accept-init-organization' url

* [AC-431] Added action AcceptInit to OrganizationUsersController

* [AC-431] Resend owner invite

* [AC-431] dotnet format

* [AC-431] Removed unused parameter 'addingUserId' from IProviderService.AddOrganization

* [AC-431] Removed setting manual values for CreationDate and RevisionDate

* [AC-431] Updated OrganizationService.InitPendingOrganization to throw exceptions when the Organization does not meet the required criteria

* [AC-431] Modified OrganizationUserInitInvitedViewModel to inherit properties from OrganizationUserInvitedViewModel

* [AC-431] Removed unecessary parameter check

* [AC-431] Moved method description to IOrganizationService.InitPendingOrganization

* [AC-431] Moved ApplicationCacheService.UpsertOrganizationAbilityAsync and ReferenceEventService.RaiseEventAsync to OrganizationService

* [AC-431] Creating collection after creating organization

* [EC-435] Fixing bug on saving Organization that would have BillingEmail as null

* [AC-431] Deleted OrganizationUserInitInvitedViewModel and added parameter InitOrganization to OrganizationUserInvitedViewModel.cs

* [AC-431] Checking if the user has any existing SingleOrg policies before initializing an Org

* [AC-431] Remove commented code

* [EC-435] Added null check to Provider

* [EC-435] Moved trial buttons script logic to Edit view

* [AC-431] Added EncryptedString attribute to OrganizationUserAcceptInitRequestModel.CollectionName

* [AC-431] Refactored plan check condition

* [AC-431] Remove duplicate _applicationCacheService.UpsertOrganizationAbilityAsync call

* [AC-431] Removed IMailService.SendOrganizationInitInviteEmailAsync

* [AC-431] Added parameters ClaimsPrincipal and IUserService to IOrganizationService.CreatePendingOrganization

* [AC-434] Hide Billing screen for Reseller clients (#2783)

* [AC-434] Added ProviderType to ProfileOrganizationResponseModel

* [AC-434] Migration script

* [AC-434] Fixed indentation on migration script

* [AC-434] Hiding sensitive subscription data if the user does not have permissions

* [AC-434] Fixed missing dependency in unit test

* [AC-434] Altered BillingSubscription.Amount and BillingSubscriptionUpcomingInvoice.Amount to nullable

* [AC-434] Replaced CurrentContext.ManageBilling with ViewBillingHistory, ViewSubscription, EditSubscription and EditPaymentMethods

* [AC-434] Reverted change on BillingSubscription.Amount and now setting Subscription.Items = null when User does not have permission

* [AC-434] Added ProviderOrganizationProviderDetails_ReadByUserId

* [AC-434] Added IProviderOrganizationRepository.GetManyByUserAsync

* [AC-434] Added CurrentContext.GetOrganizationProviderDetails

* [AC-434] Remove unneeded join Organization table

* [AC-1255] Search Existing Organizations by partial Email (#2830)

* [AC-1255] Added email search field input validation

* [AC-1255] Reverted added email pattern

* [AC-1255] Modified Organization search by Email to search using substring

* [AC-1276] Displaying an Organizations pending owners if the Organization is in a Pending status (#2834)

* [AC-432] Checking that an existing Organization is not assigned to any Provider before being assigned (#2840)

* [AC-432] Checking if any of the selected Organizations is already assigned to a Provider

* [AC-432] Changed ProviderOrganization_ReadByOrganizationIds to only get count

* [AC-432] Replaced IProviderOrganizationRepository.GetCountByOrganizationIdsAsync with call to IProviderOrganizationRepository.GetByOrganizationId

* [AC-432] undo new line

* [AC-432] Fixed unit test

* Revert "[AC-432] Replaced IProviderOrganizationRepository.GetCountByOrganizationIdsAsync with call to IProviderOrganizationRepository.GetByOrganizationId"

This reverts commit ee6e095.

# Conflicts:
#	util/Migrator/DbScripts/2023-03-22_00_ProviderAddExistingOrganizations.sql

* [AC-432] Created new migration script for ProviderOrganization_ReadCountByOrganizationIds
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