search

xamarin / Xamarin.Auth

Xamarin.Auth
Apache License 2.0
541 stars 351 forks source link

UWPAccountStore uses username as filename #325

Closed csteeg closed 5 years ago

csteeg commented 6 years ago

Xamarin.Auth Issue

If you store an Account with a Username that contains invalid path chars, an error occurs and nothing is saved.

Version

Steps to reproduce

  1. CreateAccountStore() in UWP 2.Create an Account with username set to "asdf:/@!(&*#(" 3.Save the account

Platform:

Expected behaviour

It should at least strip the invalid characters so it can Save.

Actual behaviour

Crash, no file created

newky2k commented 6 years ago

@csteeg Is this likely situation in the real world? Can you save an email address as the user? Is it one specific character that causes the issue?

It maybe better to fail gracefully with an invalid username error, rather than striping out characters which could provide unexpected results.

Regards

newky2k commented 6 years ago

@csteeg Additionally, does this function as expected on iOS or android?

csteeg commented 6 years ago

@newky2k it is in my real world situation. I use generated usernames for accessing an api. The accounts are just stored in the AccountStore, the user doesn't have to enter them manually. iOS and Android use KeyChain and KeyStore, so dictionary like storage instead of seperate files per account. So they work. I also committed a pullrequest for UWP to use a dictionary in #328 to make it work more flawlessly on UWP

newky2k commented 6 years ago

@csteeg thanks for the quick update. I'll get @moljac to look into and get him to review the PR as well.

Regads

Redth commented 5 years ago

Appreciate the PR, however we are deprecating AccountStore and recommending users move to Xamarin.Essentials' SecureStorage API's. https://docs.microsoft.com/en-us/xamarin/essentials/secure-storage

jcmanke commented 5 years ago

@Redth Is this bug present in Xamarin.Essentials.SecureStorage?

Redth commented 5 years ago

@jcmanke no, in Xamarin.Essentials, SecureStorage takes a key/value pair and uses a predetermined UWP settings container based on your application id:

https://github.com/xamarin/Essentials/blob/master/Xamarin.Essentials/SecureStorage/SecureStorage.uwp.cs#L66-L70 and https://github.com/xamarin/Essentials/blob/master/Xamarin.Essentials/SecureStorage/SecureStorage.shared.cs#L9