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

fix: [M3-9024] - Mask sensitive data in Managed and Longview #11476

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Jan 3, 2025

Description 📝

We overlooked a couple of older, more niche sections of Cloud Manager with the Mask Sensitive Data feature. This PR masks data in Managed and Longview.

Changes 🔄

  • Name, email, primary phone, and secondary phone are maskable and toggleable on the Managed > Contacts tab
  • User, IP addresses, and SSH keys are maskable and toggleable on the Managed > SSH Access tab
  • API keys, contact information, user, and IP addresses are masked and toggleable on the Longview landing and the Longview client details page

Target release date 🗓️

1/14/25

Preview 📷

Before After
Screenshot 2025-01-06 at 11 06 27 PM Screenshot 2025-01-06 at 11 05 23 PM
Screenshot 2025-01-06 at 11 07 15 PM Screenshot 2025-01-06 at 11 05 50 PM
Screenshot 2025-01-06 at 11 43 14 PM Screenshot 2025-01-06 at 11 30 57 PM Screenshot 2025-01-06 at 11 37 09 PM

How to test 🧪

Prerequisites

(How to setup test environment)

Reproduction steps

(How to reproduce the issue, if applicable)

  • Enable Managed in admin (or use the MSW, Legacy Handlers) and in prod, go to:
  • Go to Longview landing and observe the API key and cURL command is visible; on the details pages, observe the same, and note the User and IP fields in the tables (which may not have data)

Verification steps

(How to verify changes)

  • On this branch, repeat the steps above and observe:
    • Managed SSH Access public key, user, and IP are toggleable masked data. The public key notice resizes gracefully on smaller screens.
    • Managed Contacts cells have toggleable masked data.
  • Longview API key and cURL command are toggleable masked data. If you do not have data in the table for User and IP fields in the Longview client details tabs, look at the diff and confirm the right columns are wrapped in the MaskableText component.
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@mjac0bs mjac0bs self-assigned this Jan 3, 2025
@mjac0bs mjac0bs force-pushed the M3-9024-mask-sensitive-data-in-managed-and-longview branch from 8db8f0c to 878e9fb Compare January 7, 2025 15:14
@mjac0bs mjac0bs marked this pull request as ready for review January 7, 2025 15:14
@mjac0bs mjac0bs requested a review from a team as a code owner January 7, 2025 15:14
@mjac0bs mjac0bs requested review from cpathipa and harsh-akamai and removed request for a team January 7, 2025 15:14
@mjac0bs mjac0bs added Ready for Review Bug Fixes for regressions or bugs labels Jan 7, 2025
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 474 passing tests on test run #2 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing474 Passing2 Skipped90m 40s

Copy link

github-actions bot commented Jan 7, 2025

Coverage Report:
Base Coverage: 86.95%
Current Coverage: 86.94%

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jan 7, 2025
@cpathipa cpathipa removed the Add'tl Approval Needed Waiting on another approval! label Jan 7, 2025
@cpathipa cpathipa added the Approved Multiple approvals and ready to merge! label Jan 7, 2025
@mjac0bs mjac0bs merged commit 6dc152f into linode:develop Jan 7, 2025
23 checks passed
Copy link

cypress bot commented Jan 7, 2025

Cloud Manager E2E    Run #7050

Run Properties:  status check failed Failed #7050  •  git commit 6dc152f9d2: fix: [M3-9024] - Mask sensitive data in Managed and Longview (#11476)
Project Cloud Manager E2E
Branch Review develop
Run status status check failed Failed #7050
Run duration 24m 25s
Commit git commit 6dc152f9d2: fix: [M3-9024] - Mask sensitive data in Managed and Longview (#11476)
Committer Mariah Jacobs
View all properties for this run ↗︎

Test results
Tests that failed  Failures 23
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 455
View all changes introduced in this branch ↗︎

Tests for review

Failed  linodes/clone-linode.spec.ts • 1 failed test

View Output Video

Test Artifacts
clone linode > can clone a Linode from Linode details page Screenshots Video
Failed  stackscripts/create-stackscripts.spec.ts • 1 failed test

View Output Video

Test Artifacts
Create stackscripts > creates a StackScript with Any/All target image Screenshots Video
Failed  linodes/rebuild-linode.spec.ts • 2 failed tests

View Output Video

Test Artifacts
rebuild linode > rebuilds a linode from Image Screenshots Video
rebuild linode > rebuilds a linode from Community StackScript Screenshots Video
Failed  firewalls/update-firewall.spec.ts • 3 failed tests

View Output Video

Test Artifacts
update firewall > updates a firewall's linodes and rules Screenshots Video
update firewall > updates a firewall's status Screenshots Video
update firewall > updates a firewall's label Screenshots Video
Failed  linodes/backup-linode.spec.ts • 2 failed tests

View Output Video

Test Artifacts
linode backups > can enable backups Screenshots Video
linode backups > can capture a manual snapshot Screenshots Video

The first 5 failed specs are shown, see all 13 specs in Cypress Cloud.

Flakiness  linodes/linode-config.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Linode Config management > End-to-End > Clones a config Screenshots Video
Flakiness  linodes/search-linodes.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Search Linodes > create a linode and make sure it shows up in the table and is searchable in main search tool Screenshots Video
Flakiness  images/create-image.spec.ts • 1 flaky test

View Output Video

Test Artifacts
create image (e2e) > create image from a linode Screenshots Video

dmcintyr-akamai pushed a commit to dmcintyr-akamai/manager that referenced this pull request Jan 9, 2025
…11476)

* Add maskable text to Managed Contacts

* Update the docs link URL to a more helpful doc

* Make component for reusable copy

* Use component in Billing Contact Info and SSH Access Pub Key

* Make MaskableText icon styling more flexible; add to Longview command

* Add masking to Longview API Key

* Clean up styles

* Mask IP in Longview

* Mask IP Address on SSH Access Managed tab

* Fix margin for API Key

* Fix spacing for Longview API key

* Mask user in Managed and Longview

* Added changeset: Visibility of sensitive data in Managed and Longview with Mask Sensitive Data setting enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants