react-native-sensitive-info icon indicating copy to clipboard operation
react-native-sensitive-info copied to clipboard

getAllItems() return value inconsistent on different platforms

Open randycoulman opened this issue 7 years ago • 22 comments

Thanks for this library! It's been super-helpful for us.

We noticed that the getAllItems() function resolves the promise with different data types on iOS vs Android.

On iOS, we get back an array that contains another array. The inner array has objects with the keys service, key, and value:

[
  [
    { service: 'app', key: 'foo', value: 'bar' },
    { service: 'app', key: 'baz', value: 'quux' }
  ]
]

On Android, we get back an object that just has keys and values:

{
  foo: 'bar',
  baz: 'quux'
}

This inconsistency makes it difficult to use the results in a cross-platform way.

randycoulman avatar Oct 18 '16 22:10 randycoulman

I just want to add to this that getItem(key) returns null on Android and undefined on iOS when no value exists at that key. Pretty minor issue, but just wanted to give a heads up to anyone looking at this.

paulief avatar Mar 15 '17 18:03 paulief

Thank you for letting us know. We're glad this is being helpful. Right now, we are pretty busy with other projects, but pull requests are always welcome!

ricardofavero avatar May 11 '17 16:05 ricardofavero

@randycoulman I assume the iOS API should align with android's API in this case?

AlanFoster avatar Jun 02 '17 11:06 AlanFoster

I'd probably go the other way and make them both like the iOS API. That way, we'd be adding information to the Android API rather than removing information from the iOS API. Either way, it's going to be a breaking change somewhere.

randycoulman avatar Jun 02 '17 13:06 randycoulman

Thanks for building this library! Experiencing the same issue. If we're fixing the return values, shouldn't the iOS return value be one array rather than a nested array, i.e., return:

[
    { service: 'app', key: 'foo', value: 'bar' },
    { service: 'app', key: 'baz', value: 'quux' }
]

rather than

[
  [
    { service: 'app', key: 'foo', value: 'bar' },
    { service: 'app', key: 'baz', value: 'quux' }
  ]
]

Happy to work on a PR for that if it's helpful. I don't have as much experience with Android React-Native so probably wouldn't be as helpful with reworking that section.

YPCrumble avatar Oct 26 '17 20:10 YPCrumble

@YPCrumble Yes, I agree with you. I forgot about the nested array in my earlier response.

So, my (non-authoritative) opinion is that we should return an array of objects, where the objects have the service, key, and value keys in it.

randycoulman avatar Oct 26 '17 22:10 randycoulman

Hello guys, sorry for delaying the answer once again. I'll take a look in android in this week. About iOS, I'm afraid I wouldn't because I don't have macOS available.

mCodex avatar Oct 28 '17 12:10 mCodex

Moreover, the example in the README is a bit misleading. From looking at it, I expected values to be an array, and have been cracking my head over this.
It would be nice to have an example output for each command, as well as return type, etc.

illright avatar Oct 30 '17 14:10 illright

Has this been resolved?

mromarreyes avatar Jan 07 '19 21:01 mromarreyes

opened a PR for this at https://github.com/mCodex/react-native-sensitive-info/pull/168

SamRaymer avatar Oct 31 '19 18:10 SamRaymer

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 10 '20 20:06 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 10 '20 20:07 stale[bot]

I find the issue still exist because v6.0.0-alpha.5 doesn't contain #168. It's still use WritableMap and same for master branch https://github.com/mCodex/react-native-sensitive-info/blob/master/android/src/main/java/dev/mcodex/RNSensitiveInfoModule.java#L241 . Something wrong happened in release process?

sasurau4 avatar Aug 29 '20 01:08 sasurau4

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 28 '20 02:09 stale[bot]

The issue still exists in v6.0.0-alpha.6.

mikewoudenberg avatar Oct 01 '20 13:10 mikewoudenberg

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 31 '20 14:10 stale[bot]

It looks like #214 stepped on this fix from #168 Is there a reason that this was regressed?

hapablap21 avatar Nov 15 '20 22:11 hapablap21

Hi guys 👋

Thanks for the report. There were no reason regressing this issue. In fact, it may have happend when I merged the keystore branch within the master branch, because the keystore branch had many differences in android's files and this fix wasn't included there.

So, we should only open a new PR for that using the master branch again. I may have do this later, however feel free doing that.

mCodex avatar Nov 16 '20 12:11 mCodex

I am still having this issue. Wasn't this fixed?

jmalStorm avatar Apr 02 '21 08:04 jmalStorm

const RNSIResponseNormal: SensitiveInfoEntry[] | [SensitiveInfoEntry[]] = APP_FLAGS.isIOS ? RNSIResponse[0] : RNSIResponse

ingvardm avatar Aug 19 '21 23:08 ingvardm

The issue still exists in v6.0.0-alpha.9.

For now, I decided not to use getAllItems.

m-ueta avatar Jul 06 '22 05:07 m-ueta

Hey! its a simple hotFix, but you can use the follow workaround to solve this problem :

export const getAllSecureItem = async () => {
  try {
    const allSecureItems = await SInfo.getAllItems({
      sharedPreferencesName,
      keychainService,
    });

    return Platform.OS === 'ios'
      ? allSecureItems[0]
      : Object.entries(allSecureItems)?.map((secureItem) => {
          const [key, value] = secureItem;
          return { key: key, value: value };
        });
        
  } catch (e) {
    // Error handler here.
  }
};

This function return a simple Array of Secure Items: [ { key: "", value: "" }, { key: "", value: "" } ]

@m-ueta @randycoulman

Matiassimone avatar Aug 04 '22 18:08 Matiassimone