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

getAllItems() return value inconsistent on different platforms #8

Open
randycoulman opened this issue Oct 18, 2016 · 22 comments
Open

Comments

@randycoulman
Copy link
Contributor

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.

@paulief
Copy link

paulief commented Mar 15, 2017

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.

@ricardofavero
Copy link

ricardofavero commented May 11, 2017

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!

@AlanFoster
Copy link

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

@randycoulman
Copy link
Contributor Author

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.

mCodex pushed a commit that referenced this issue Aug 4, 2017
return error code auth fail
@YPCrumble
Copy link

YPCrumble commented Oct 26, 2017

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.

@randycoulman
Copy link
Contributor Author

@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.

@mCodex
Copy link
Owner

mCodex commented Oct 28, 2017

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.

@illright
Copy link

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.

@mromarreyes
Copy link

Has this been resolved?

@SamRaymer
Copy link
Contributor

opened a PR for this at #168

@stale
Copy link

stale bot commented Jun 10, 2020

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 stale bot added the wontfix label Jun 10, 2020
@stale
Copy link

stale bot commented Jul 10, 2020

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 stale bot added the 🚧 stale label Jul 10, 2020
@sasurau4
Copy link

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?

@stale stale bot removed the 🚧 stale label Aug 29, 2020
@stale
Copy link

stale bot commented Sep 28, 2020

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 stale bot added the 🚧 stale label Sep 28, 2020
@mikewoudenberg
Copy link

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

@stale stale bot removed the 🚧 stale label Oct 1, 2020
@stale
Copy link

stale bot commented Oct 31, 2020

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 stale bot added the 🚧 stale label Oct 31, 2020
@hapablap21
Copy link

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

@stale stale bot removed the 🚧 stale label Nov 15, 2020
@mCodex
Copy link
Owner

mCodex commented Nov 16, 2020

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.

@stale stale bot added the 🚧 stale label Dec 16, 2020
Repository owner deleted a comment from stale bot Dec 17, 2020
@stale stale bot removed the 🚧 stale label Dec 17, 2020
@jmalStorm
Copy link

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

@ingvardm
Copy link

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

@m-ueta
Copy link

m-ueta commented Jul 6, 2022

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

For now, I decided not to use getAllItems.

@Matiassimone
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests