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

Question: Drop use of androids build in AccountManager #49

Open
David-Development opened this issue Mar 19, 2019 · 12 comments
Open

Question: Drop use of androids build in AccountManager #49

David-Development opened this issue Mar 19, 2019 · 12 comments

Comments

@David-Development
Copy link
Member

@tobiasKaminsky Just throwing in some thoughts here. As we had a bunch of issues already when trying to figure out which accounts exist, I was thinking about if we can drop the need to use the Android "AccountManager" at all. As of Android 9 getting accounts from another app is even more difficult. Therefore dropping it makes things easier for all involved parties.

Advantages:

  • reauthentication works without the need to request the account permission again (way easier for client apps as we don't need to restart the login flow)
  • Custom dialog for choosing the account (nextcloud design / theme)
  • reduce complexity of account handling (in the sso library)

Disadvantages:

  • None?

So basically what I'm proposing is:

  • Instead of calling the account manager to show us available accounts, we create a new custom activity in the files app for that. (Reference)
  • The files app lists all accounts available (as the files app has the required permission for that)
  • The users selects an account here

This also might be interesting for @desperateCoder @stefan-niedermann

@tobiasKaminsky
Copy link
Member

Hm. I am not sure if I understand it.
Do you want to get rid of the account handling in files app? And implement our own one?
Or only within SSO? E.g.

// Find all currently installed nextcloud accounts on the phone
public static List<Account> findAccounts(final Context context) {
final AccountManager accMgr = AccountManager.get(context);
final Account[] accounts = accMgr.getAccounts();
List<Account> accountsAvailable = new ArrayList<>();
for (final Account account : accounts) {
if ("nextcloud".equals(account.type)) {
accountsAvailable.add(account);
}
}
return accountsAvailable;
}

@David-Development
Copy link
Member Author

David-Development commented Mar 19, 2019

@tobiasKaminsky The method you mentioned uses the internal Android AccountManager. the call to getAccounts() will only return accounts, that the app currently has permission to access. So if we reinstall the files app, the permission is revoked and therefore the user has to request that permission again. Right now the only purpose of using the Android build in Account Manager is to figure out which accounts are installed and to select one of the accounts. The whole part with requesting / exchanging tokens is handled elsewhere.

That's why I was wondering.. maybe we can throw the Android Account Manager out so we don't have to work around those permission handling stuff? What do you think?

Reference: https://developer.android.com/about/versions/oreo/android-8.0-changes.html#aaad

@mario
Copy link

mario commented Mar 19, 2019

Personally I find using my own account management/storage compared to Android Account Manager significantly better, even if Android Account Manager has a tighter integration into the OS (which I don't really use or care about).

@tobiasKaminsky
Copy link
Member

So if we reinstall the files app, the permission is revoked and therefore the user has to request that permission again.

Even if we use our own account management system, after a reinstallation these infos will be gone, or?

@AndyScherzinger
Copy link
Member

So if we don't rely on Android's account manager but rather our very own we wouldn't have all the issues we currently face with the login loops (my guess). So shouldn't we try to drop Android's account manager anyways?

@David-Development
Copy link
Member Author

David-Development commented Mar 19, 2019

@tobiasKaminsky Not necessarily. Right now, it's required that if you want to authenticate an account, you have to send the account object. (See: https://github.com/nextcloud/Android-SingleSignOn/blob/master/src/main/java/com/nextcloud/android/sso/AccountImporter.java#L259)

The problem is, that if you reinstall the files app, you won't be able to "find" that account anymore as you don't have the permission to access it anymore (on Android 8 and above). What we could do (using the Android Account Manager or our custom Account Manager) is to only send the account name (instead of the whole account object). This way we can work around the permission issue when reinstalling the app as we know the account name and we don't have to "recieve" the account object. But that has some serious side effects later on (if you want to list all accounts available, no accounts will be shown as you don't have the permission anymore.. etc..)

If we create our own solution, we need to provide some mechanism to return a list of available accounts to the sso library (e.g. if you plan on using multiple accounts) -> something similar to what the AccountManager provides right now (https://github.com/nextcloud/Android-SingleSignOn/blob/master/src/main/java/com/nextcloud/android/sso/AccountImporter.java#L96)

@AndyScherzinger yes, there are definitely some advantages to it.

My only concern right now is: How do we provide a list of all accounts to the client app?

@David-Development David-Development changed the title Question: Drop Account Manager Question: Drop use of androids build in AccountManager Mar 19, 2019
@tobiasKaminsky
Copy link
Member

Is this urgent? I would love to talk about this on conf/hackweek, as I think this is a fundamental change and we should think really well about this…

@David-Development
Copy link
Member Author

Would like to share some ideas here:

3PA = Third Party App (such as news / deck / ...)


Provide a getAccounts() api that returns all nextcloud accounts

  1. 3PA -> SSO -> Nextcloud Files App -> getAccounts()
  2. 3PA -> SSO -> Show Account List (we can combine dev/normal files app accounts)
    or
  3. 3PA -> SSO -> Nextcloud Files App -> Show Account List (only show accounts of dev or normal app)
  4. 3PA -> SSO -> Nextcloud Files App -> requestAccountPermission

This works because the Nextcloud Files App owns all accounts and can list them. Basically we're creating a workaround for the new android security features (in favor of better usability). We can also integrate multi-account support pretty well this way.


3PA -> SSO -> Nextcloud Files App -> Show Account List
3PA -> SSO -> Nextcloud Files App -> requestAccountPermission

This one provides more security as we don't show any accounts to third party apps (Kind of similar how it is on Android 26+ now).


I personally prefer option one as it is more flexible. I think providing the "account list" to all apps isn't too bad as it doesn't contain much sensitive information. (Apps need to use the custom sso api to get this information). So if an apps really wants to spy on you.. they have to use the sso api and even then they will only be able to read the account names (username and url).

What do you guys think?

@tobiasKaminsky
Copy link
Member

they have to use the sso api and even then they will only be able to read the account names (username and url).

Currently this is shown in account list anyway.
Also we would have to expose this infos on account chooser, as otherwise people could not distinguish same username on different users, or?

@David-Development
Copy link
Member Author

@tobiasKaminsky

Currently this is shown in account list anyway.

That is true - but only if apps have permission to list all accounts, right?


Just to clarify:

  • SSO -> Nextcloud Files App -> getAccounts()
  • SSO -> Show Account List (we can combine dev/normal files app accounts)

or

  • SSO -> Nextcloud Files App -> Show Account List (only show accounts of dev or normal app)
  • SSO -> Nextcloud Files App -> requestAccountPermission

Both apps (nextcloud files app dev and prod) are signed with the same key, right? So the main nextcloud files app (prod) would be able to request accounts from the dev app as well, right?


Trying to wrap my head around it if it would work if we always use the production / or beta version of the nextcloud files app to show the new account chooser dialog. Because it would need to pass on the information to the sso library to tell which app it needs to connect to when making requests..

@tobiasKaminsky
Copy link
Member

let us move this to a brainstorming session on conf …

@David-Development
Copy link
Member Author

Okay, sounds good 👍

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

4 participants