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: Wrong authentication property key for Segment event in app/core/SecureKeychain.js #8860

Closed
1 of 9 tasks
NicolasMassart opened this issue Mar 6, 2024 · 1 comment · Fixed by #8861
Closed
1 of 9 tasks
Assignees
Labels
next release release-7.18.0 Issue or pull request that will be included in release 7.18.0 release-7.19.0 Issue or pull request that will be included in release 7.19.0 release-blocker This bug is blocking the next release team-mobile-platform

Comments

@NicolasMassart
Copy link
Contributor

NicolasMassart commented Mar 6, 2024

What is this about?

Wrong authentication property key for Segment event in app/core/SecureKeychain.js discovered while doing QA:
The event props should look like:

    await MetaMetrics.getInstance().addTraitsToUser({
      [UserProfileProperty.AUTHENTICATION_TYPE]: AUTHENTICATION_TYPE.PASSWORD,
    });

instead of

    await MetaMetrics.getInstance().addTraitsToUser({
      [NATIVE_AUTH_TYPE]: AUTHENTICATION_TYPE.PASSWORD,
    });

Also, fix typo on UserProfilePropery to UserProfileProperty

Scenario

No response

Design

No response

Technical Details

In app/core/SecureKeychain.js, replace NATIVE_AUTH_TYPE by UserProfileProperty.AUTHENTICATION_TYPE

Threat Modeling Framework

No response

Acceptance Criteria

  • Auth events are sent with the correct auth property key in Segment:
    Traits should look like:
      "traits": {
        "$city": "",
        "$country_code": "FR",
        "$region": "NOR",
        "$timezone": "Europe/Paris",
        "Authentication Type": "password",
      },
    Instead of :
      "traits": {
        "$city": "",
        "$country_code": "FR",
        "$region": "NOR",
        "$timezone": "Europe/Paris",
        "[object Object]": "password"
      },
  • typo fixed

Stakeholder review needed before the work gets merged

  • Engineering (needed in most cases)
  • Design
  • Product
  • QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
  • Security
  • Legal
  • Marketing
  • Management (please specify)
  • Other (please specify)

References

No response

@NicolasMassart NicolasMassart self-assigned this Mar 6, 2024
@NicolasMassart NicolasMassart added next release release-blocker This bug is blocking the next release labels Mar 7, 2024
@metamaskbot metamaskbot added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 7, 2024
NicolasMassart added a commit that referenced this issue Mar 7, 2024
- type for auth events
- fix typo

Fixes #8860
@github-actions github-actions bot removed the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Mar 7, 2024
metamaskbot pushed a commit that referenced this issue Mar 7, 2024
- type for auth events
- fix typo

Fixes #8860
@metamaskbot metamaskbot added release-7.19.0 Issue or pull request that will be included in release 7.19.0 release-7.18.0 Issue or pull request that will be included in release 7.18.0 labels Mar 7, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-7.18.0 on issue. Adding release label release-7.18.0 on issue, as issue is linked to PR #8861 which has this release label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release release-7.18.0 Issue or pull request that will be included in release 7.18.0 release-7.19.0 Issue or pull request that will be included in release 7.19.0 release-blocker This bug is blocking the next release team-mobile-platform
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants