Support multi-account

[nextcloud]

Support multi-account

[tobiasKaminsky]

From #8 (comment):

Right now I can think of two use cases when a user wants to use multiple accounts:

1. Just as in the Files App, the user can switch between different accounts, but only one account is active at all times. This is very "light" version of multi-account usage. I think this is already achieved since the client app can use the FindAccounts method to get all accounts and then select one with the SetCurrentAccount method.

2. Just as I'm using it in my task-sync app, I let the user add multiple calendars from multiple caldav endpoints / servers. Therefore all accounts will be accessed somewhat simultaneously. Therefore we can't just use the SetCurrentAccount method to switch between the accounts. We need to hold a reference to the accounts. Not sure if this should be part of this sso library or if it's the responsibility of the client app itself. Any thoughts on this?

I think we can and should make this lib as slim as possible: the app using this library should take care of their account(s).
If it is needed, the app could have multiple NexctloudAPIs with each different account.

The app then would also have to check if the account is still valid prior using it.

Another problem, which also affects case 1 is, that the FindAccounts will only return accounts that the user have granted access to (due to the new security restrictions on Android 8+)

This is done when installing & using NC files app, or? As we require to first have NC files setup correctly, this should be no real problem, or?

[David-Development]

If it is needed, the app could have multiple NexctloudAPIs with each different account.

If the app takes care of storing the accounts is uses, then I completely agree. Multi-account setups are somewhat special anyways. I think most apps only use one account.

This is done when installing & using NC files app, or? As we require to first have NC files setup correctly, this should be no real problem, or?

As far as I understand, each account will be "private" or lets say not visible for any other app. The app has to use the android AccountManager.newChooseAccountIntent() method for each account before being able to access all accounts (and I think it's on a per account basis). I'll run some tests now.

[tobiasKaminsky]

Then I will remove all account related functions within this lib (and also adjust news-app for PoC).

Regarding the second one: let us discuss this in another issue: #16

[David-Development]

Yeah? I don't know. I think the way it is, by letting the library handle the "default" account / letting the user open the dialog chooser is pretty good. All the code required to use the network stack is in this lib. I think if we remove this functionality, it'll be a repetitive work for developers to implement this "store default account" thing. Maybe we can extract it into a different class?

Give me a few minutes. I'll push some updates (for better error handling)

[tobiasKaminsky]

Hmm. Right, removing the account handling would lead to the same code on multiple apps.
So we keep this in and if some app wants to use multi-account, then they have to do it on their own.
So nothing is needed from lib side, except a bit of clarification?

[David-Development]

Yes, that sounds great. Maybe we can refactor the "single-account"-related functions into a separate class to make it more "obvious" and to have a clear separation between them?

I still need some time to implement the "error" handling properly, so you can go ahead and do the refactoring if you're free. I'll merge my changes into yours later than! :)

[David-Development]

@tobiasKaminsky Are you already working on this? I'm currently doing some heavy refactoring (both for single account use and for the error handling)

[tobiasKaminsky]

No, not yet.
Looking forward to your refactoring :-)

[David-Development]

Alright, I just pushed my changes to all three repositories.

I added custom exceptions for different cases that might occur. If you can think of some more exceptions, let me know. Also I'm thinking about a better naming convention for the exceptions.

I added custom messages to the exceptions. This should make it easier for devs to get a "human readable" error texts. I also added this in this library so that we can provide some translations. I think that's better as if every developer has to provide error messages for all common exceptions.

I'm not sure if I'm happy with the way I implemented it. See: https://github.com/nextcloud/Android-SingleSignOn/blob/master/src/main/java/com/nextcloud/android/sso/exceptions/SSOException.java#L39

I couldn't "inject" the messages in the constructor since a context is required to load the translated error messages. It's working pretty good right now but I think it's not a perfectly clean solution.

[tobiasKaminsky]

Now the 3rd party apps store the account in their own database, so multi account will work, right? @David-Development

[David-Development]

Yes, it would be up to the developer to store multiple tokens by him/her-self but multi-account support should be possible! :)