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

[IOPID-2575] Bump rn-keychain and force AES_GCM_NO_AUTH #6587

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

Conversation

shadowsheep1
Copy link
Member

@shadowsheep1 shadowsheep1 commented Jan 7, 2025

Short description

This PR updates the react-native-keychain library from version 4.0.5 to 9.2.2, introducing more robust AES_GCM encryption. This resolves an issue where session information would become corrupted ("whitened") on our test Pixel devices for users authenticated via EIC.

The update impacts only Android users and is seamless for those already authenticated.

Fixes #6400

List of changes proposed in this pull request

  • Upgrade react-native-keychain library to version 9.2.2.
  • Enforce Keychain.STORAGE_TYPE.AES_GCM_NO_AUTH as the Android storage type.
  • Update the setPin function to accommodate the modified return type of setGenericPasswordWithDefaultAccessibleOption.
Android Test

On Android devices, existing session information is retrieved from shared preferences using the previously selected encryption method. Upon the next save operation for session data, a new data store file is created with the updated AES_GCM encryption.

iopid-kc-0107.mov

How to test

Prerequisites

  • Use physical devices for both Android and iOS.

Steps

  • Run the app (App IO) with react-native-keychain library version 4.0.5 and log in successfully using SPID.
  • Close and reopen the app multiple times.
  • Update the app to use react-native-keychain library version 9.2.2, then launch it. Verify that you remain logged in without being disconnected.
  • Close and reopen the app multiple times.
  • Log out and log in again using EIC.
  • Close and reopen the app multiple times.
  • Confirm that you stay logged in without any issues.

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Jira Pull Request Link

This Pull Request refers to the following Jira issue IOPID-2575

@shadowsheep1 shadowsheep1 self-assigned this Jan 7, 2025
@shadowsheep1 shadowsheep1 added dont-merge ✋ IO-A&I IO - Autenticazione e Identità labels Jan 7, 2025
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 49.30%. Comparing base (b207270) to head (bac8cb7).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
ts/utils/keychain.ts 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6587      +/-   ##
==========================================
- Coverage   49.30%   49.30%   -0.01%     
==========================================
  Files        1565     1565              
  Lines       32223    32224       +1     
  Branches     7288     7290       +2     
==========================================
  Hits        15887    15887              
  Misses      16298    16298              
- Partials       38       39       +1     
Files with missing lines Coverage Δ
ts/utils/keychain.ts 35.71% <0.00%> (-2.75%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c8f237...bac8cb7. Read the comment docs.

Copy link
Contributor

@Ladirico Ladirico left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge ✋ IO-A&I IO - Autenticazione e Identità
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Richiesta login (quasi) ad ogni avvio
2 participants