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 failing crypto tests #5948

Merged
merged 8 commits into from
Aug 4, 2023
Merged

Fix failing crypto tests #5948

merged 8 commits into from
Aug 4, 2023

Conversation

MGibson1
Copy link
Member

@MGibson1 MGibson1 commented Aug 3, 2023

Type of change

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

Objective

Fixes failing crypto tests due to jestjs/jest#14379. This is accomplished through converting most of our usage of ArrayBuffer to Uint8Array, which works around the jest-environment-jsdom issue.

Uint8Array downcasts to ArrayBuffer without issues.

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

@MGibson1 MGibson1 requested review from a team as code owners August 3, 2023 12:44
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Aug 3, 2023
@bitwarden-bot
Copy link

bitwarden-bot commented Aug 3, 2023

Logo
Checkmarx One – Scan Summary & Details40475c2f-0c21-4efe-85ae-ff8f37f46b51

New Issues

Severity Issue Source File / Package Checkmarx Insight
LOW Use_Of_Hardcoded_Password /libs/common/src/platform/services/web-crypto-function.service.spec.ts: 368 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/node/src/services/node-crypto-function.service.spec.ts: 285 Attack Vector
LOW Use_of_Broken_or_Risky_Cryptographic_Algorithm /libs/node/src/services/node-crypto-function.service.ts: 23 Attack Vector

Fixed Issues

Severity Issue Source File / Package Checkmarx Insight
LOW Use_Of_Hardcoded_Password /libs/common/src/platform/services/web-crypto-function.service.spec.ts: 368 Attack Vector
LOW Use_Of_Hardcoded_Password /libs/node/src/services/node-crypto-function.service.spec.ts: 305 Attack Vector
LOW Use_of_Broken_or_Risky_Cryptographic_Algorithm /libs/node/src/services/node-crypto-function.service.ts: 23 Attack Vector

Copy link
Member

@differsthecat differsthecat left a comment

Choose a reason for hiding this comment

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

This looks good, I also verified locally the Native Test Runner is working as expected 👍

Copy link
Contributor

@ike-kottlowski ike-kottlowski left a comment

Choose a reason for hiding this comment

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

Pulled branch and all test passed locally.

@MGibson1 MGibson1 enabled auto-merge (squash) August 3, 2023 18:40
@MGibson1 MGibson1 removed the needs-qa Marks a PR as requiring QA approval label Aug 4, 2023
@MGibson1 MGibson1 merged commit 36b7d30 into master Aug 4, 2023
@MGibson1 MGibson1 deleted the fix-failing-crypto-tests branch August 4, 2023 02:13
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.

10 participants