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.

Sign up for GitHub

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

Jump to bottom

[PM-17668] Move DeviceTrustService to KM ownership #13194

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[PM-17668] Move DeviceTrustService to KM ownership #13194

wants to merge 9 commits into from

Conversation

Thomas-Avery
Copy link
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-17668

📔 Objective

The purpose of this PR is to move DeviceTrustService to KM ownership.

⏰ 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

@Thomas-Avery Thomas-Avery self-assigned this Jan 31, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 31, 2025 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – 58e649b7-6dca-4d26-b032-3ebb09b0011b

Fixed Issues (81)

Great job! The following issues were fixed in this Pull Request

Severity Issue Source File / Package
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /libs/vault/src/components/download-attachment/download-attachment.component.ts: 29
MEDIUM Client_Privacy_Violation /libs/vault/src/components/download-attachment/download-attachment.component.ts: 29
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 106
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
MEDIUM Client_Privacy_Violation /apps/web/src/app/vault/individual-vault/view.component.ts: 80
LOW Angular_Usage_of_Unsafe_DOM_Sanitizer /libs/components/src/avatar/avatar.component.ts: 87

@codecov Codecov
Copy link

codecov bot commented Jan 31, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 45.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 35.05%. Comparing base (28d9202) to head (dd6b95e).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
libs/angular/src/services/jslib-services.module.ts 0.00% 2 Missing ⚠️
.../auth/popup/login-via-auth-request-v1.component.ts 0.00% 1 Missing ⚠️
apps/browser/src/background/main.background.ts 0.00% 1 Missing ⚠️
.../auth/login/login-via-auth-request-v1.component.ts 0.00% 1 Missing ⚠️
...ents/base-login-decryption-options-v1.component.ts 0.00% 1 Missing ⚠️
.../components/login-via-auth-request-v1.component.ts 0.00% 1 Missing ⚠️
libs/angular/src/auth/guards/redirect.guard.ts 0.00% 1 Missing ⚠️
...r/src/auth/guards/tde-decryption-required.guard.ts 0.00% 1 Missing ⚠️
...tion-options/login-decryption-options.component.ts 0.00% 1 Missing ⚠️
...a-auth-request/login-via-auth-request.component.ts 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #13194   +/-   ##
=======================================
  Coverage   35.05%   35.05%           
=======================================
  Files        3036     3036           
  Lines       92723    92723           
  Branches    16890    16890           
=======================================
  Hits        32502    32502           
  Misses      57757    57757           
  Partials     2464     2464

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Thomas-Avery Thomas-Avery marked this pull request as ready for review February 3, 2025 22:05
@Thomas-Avery Thomas-Avery requested review from a team as code owners February 3, 2025 22:05
@Thomas-Avery Thomas-Avery requested a review from rr-bw February 3, 2025 22:05
rr-bw
rr-bw reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@rr-bw rr-bw 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, just check the failing tests.

quexten
quexten previously approved these changes Feb 4, 2025
View reviewed changes
@quexten
Copy link
Contributor

quexten commented Feb 4, 2025

I think the test failures are unrelated so LGTM.

@Thomas-Avery
Copy link
Contributor Author

Looks good, just check the failing tests.

This is failures with code signing the macOS pacakge and not related to the change. Looks to be happening on main for a bit now.

@Thomas-Avery Thomas-Avery requested a review from rr-bw February 4, 2025 15:49
rr-bw
rr-bw previously approved these changes Feb 4, 2025
View reviewed changes
@Thomas-Avery Thomas-Avery dismissed stale reviews from rr-bw and quexten via 85a977e February 5, 2025 23:10
@mzieniukbw
Copy link
Contributor

Could you update https://github.com/bitwarden/clients/blob/main/.github/codecov.yml ?
I assume it falls under Lock ?

quexten
quexten previously approved these changes Feb 6, 2025
View reviewed changes
Copy link
Contributor

@quexten quexten left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Aside from @mzieniukbw's comment on codecov, this LGTM, approving and will re-approve if/when you address that comment too.

@Thomas-Avery
Copy link
Contributor Author

Could you update https://github.com/bitwarden/clients/blob/main/.github/codecov.yml ? I assume it falls under Lock ?

Added dd6b95e

@Thomas-Avery Thomas-Avery requested review from quexten and rr-bw February 6, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quexten quexten quexten approved these changes

@rr-bw rr-bw rr-bw approved these changes

@mzieniukbw mzieniukbw mzieniukbw approved these changes

Assignees

@Thomas-Avery Thomas-Avery

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Thomas-Avery @quexten @mzieniukbw @rr-bw