Skip to content

[AC-1638] Disallow Secrets Manager for MSP-managed organizations#6392

Merged
r-tome merged 7 commits intomasterfrom
ac/ac-1638/disallow-secrets-manager-for-msp-managed-organizations
Oct 12, 2023
Merged

[AC-1638] Disallow Secrets Manager for MSP-managed organizations#6392
r-tome merged 7 commits intomasterfrom
ac/ac-1638/disallow-secrets-manager-for-msp-managed-organizations

Conversation

@eliykat
Copy link
Copy Markdown
Member

@eliykat eliykat commented Sep 25, 2023

Type of change

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

Objective

We do not support MSP access to Secrets Manager, so we need to prevent:

  • an org with SM being added to an MSP, or
  • an MSP adding SM to one of their client organizations.

The internal Bitwarden Portal will not have this restriction, so that Customer Success can make case-by-case exceptions if required.

Server changes: bitwarden/server#3297

Code changes

  • OrganizationPlansComponent - hide sm-subscribe component if accessed via the provider portal
  • OrganizationSubscriptionCloudComponent - hide sm-subscribe component if the org has a provider

Note that there is no client-side block on adding an existing org with SM to a provider. We just rely on the server-side error message via toast.

Screenshots

Before you submit

  • 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
  • Ensure that all UI additions follow WCAG AA requirements

@github-actions github-actions Bot added the needs-qa Marks a PR as requiring QA approval label Sep 25, 2023
@bitwarden-bot
Copy link
Copy Markdown

bitwarden-bot commented Sep 25, 2023

Logo
Checkmarx One – Scan Summary & Detailse25e38ec-2a6e-4331-b44a-e0dd446f8b66

New Issues

Severity Issue Source File / Package Checkmarx Insight
LOW Use_Of_Hardcoded_Password /apps/cli/src/models/response/string.response.ts: 8 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/models/response/string.response.ts: 8 Attack Vector
LOW Use_Of_Hardcoded_Password /apps/cli/src/models/response/string.response.ts: 8 Attack Vector

…-secrets-manager-for-msp-managed-organizations
@eliykat eliykat marked this pull request as ready for review October 4, 2023 04:50
@eliykat eliykat requested a review from a team as a code owner October 4, 2023 04:50
cyprain-okeke
cyprain-okeke previously approved these changes Oct 4, 2023
Copy link
Copy Markdown
Contributor

@cyprain-okeke cyprain-okeke left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot temporarily deployed to Web Vault - QA October 11, 2023 16:50 Inactive
@eliykat
Copy link
Copy Markdown
Member Author

eliykat commented Oct 12, 2023

Fixed some logic - no need to check the providerType, only whether the org has a provider at all.

Copy link
Copy Markdown
Contributor

@cyprain-okeke cyprain-okeke 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

@github-actions github-actions Bot temporarily deployed to Web Vault - QA October 12, 2023 14:06 Inactive
@r-tome r-tome removed the needs-qa Marks a PR as requiring QA approval label Oct 12, 2023
@r-tome r-tome merged commit a6b725d into master Oct 12, 2023
@r-tome r-tome deleted the ac/ac-1638/disallow-secrets-manager-for-msp-managed-organizations branch October 12, 2023 14:56
BlackDex pushed a commit to BlackDex/bitwarden-clients that referenced this pull request Nov 21, 2023
…warden#6392)

* Hide Add SM component on sub page for MSPs

* Hide Add SM component on create org page for MSPs

* Use hasProvider instead of providerType
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.

4 participants