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

Mark nextcloud accounts as visible in AccountManager #4047

Closed
Tomasu opened this issue May 20, 2019 · 20 comments
Closed

Mark nextcloud accounts as visible in AccountManager #4047

Tomasu opened this issue May 20, 2019 · 20 comments
Labels
developer experience enhancement feature: authentication Authentication or accounts related needs info Waiting for info from user(s). Issues with this label will auto-stale.

Comments

@Tomasu
Copy link

Tomasu commented May 20, 2019

Is your feature request related to a problem? Please describe.

Currently existing nextcloud accounts are not visible to other apps when they target API 26 or above due to google's constant changes.

Describe the solution you'd like

It is possible to tell AccountManager to mark the account as visible to all or specific packages.

eg AccountManager.setAccountVisibility(account, PACKAGE_NAME_KEY_LEGACY_NOT_VISIBLE, VISIBILITY_VISIBLE) or a bit more "safely": AccountManager.setAccountVisibility(account, PACKAGE_NAME_KEY_LEGACY_NOT_VISIBLE, VISIBILITY_USER_MANAGED_VISIBLE) or AccountManager.setAccountVisibility(account, "explicit.package.name.here", VISIBILITY_USER_MANAGED_VISIBLE)

Describe alternatives you've considered

I've considered a bunch of different authentication mechanisms but most have various downsides I'd rather not deal with, like storing and sending the users's username/pass directly as some apps want to do.

Additional context

With this change it will be possible for modern apps to authenticate with the nextcloud app directly without having to manage basic auth or OAuth themselves. That said, theres one little problem stopping this from being "perfect" which I'll report in another issue (getAuthToken always leads to nextcloud asking for the authentication info even though it aught to allow a simple yes/no auth dialog when you're already logged in).

@AndyScherzinger
Copy link
Member

AndyScherzinger commented May 20, 2019

Hi @Tomasu, are you aware of https://github.com/nextcloud/Android-SingleSignOn which can be used by apps to work with Nextcloud accounts?

@Tomasu
Copy link
Author

Tomasu commented May 20, 2019

I am indeed. I was reffered to it in #nextcloud-dev earlier today. If your app targets API 26 or above, you can not by default see accounts from most other apps. Currently when getAccounts() or getAccountsByType() is called Nextcloud accounts will not show up.

I've added:

if ( Build.VERSION.SDK_INT >= Build.VERSION_CODES.O )
 {
   mAccountMgr.setAccountVisibility( mAccount, AccountManager.PACKAGE_NAME_KEY_LEGACY_NOT_VISIBLE, AccountManager.VISIBILITY_USER_MANAGED_VISIBLE );
}

to AuthenticatorActivity on line 1714 and now when I call getAccounts or getAccountsByType I can now see my nextcloud account.

@AndyScherzinger
Copy link
Member

Ah, I see. Sorry for my confusion :)

So looping in @tobiasKaminsky and @David-Development

@David-Development
Copy link
Member

Hey @Tomasu!

You said you were already introduced to it earlier today. Does the Single-Sign-On Feature not fit your needs? What are you trying to achieve?

It is possible in Android API Level 26 to requests accounts using the build in AccountManager. However you'll need to call AccountManager.newChooseAccountIntent(...) first. After that you'll be able to see that specific account using the getAccounts() method. (Take a look here for reference)

As for the Nextcloud Files App and the SingleSignOn feature we are planning on moving away from the build in Android AccountManager as it has several downsides and different behavior on pretty much every Android Version (Reference).

I just read through the docs for the code you posted above (Reference). All the prerequisites are deprecated and should not be used anymore. Therefore I'm not sure if it's a good idea to include such a change in the Files App. Especially if we have great authentication solutions out there such as the SingleSignOn feature that provide a great user experience. But maybe I'm not getting the use case here..

Even though it might seem a little complicated at first, setting up the SingleSignOn is rather trivial compared to the amount work you have to deal with when trying to use the AccountManager yourself. Apart from connecting to the nextcloud itself (storing credentials / proxy support / ssl (also self signed ssl / the list goes on). All this is done for you in the SingleSignOn library and it provides a great Retrofit2 compatible API.

@Tomasu
Copy link
Author

Tomasu commented May 20, 2019

Hi @David-Development

I started working on this code before learning about the SSO lib, so I haven't switched over yet.
I did test the SSO lib though, and it's AccountImporter.findAccounts method returns no accounts because my app is compiled for api 26+.

PACKAGE_NAME_KEY_LEGACY_NOT_VISIBLE does not seem to be deprecated. It just tells android that packages requesting accounts should be able to see this account even if the package doesn't have all prerequsites needed for normal use. I currently can't see any harm in it, but another option is to explicitly add some package names to the files app for specific users (eg: passman). But that seems like a headache for everyone involved.

take a look at https://github.com/Tomasu/passman-android for what I'm doing, and what currently works. On clean launch you get a login screen listing any nextcloud accounts currently setup on the device, which you can select from /without/ using the popup dialog that the newChooseAccountIntent method forces on you. You can then select an account and it auto logs in. This does depend on nextcloud having the change I made applied.

@Tomasu
Copy link
Author

Tomasu commented May 20, 2019

Hm, I forgot to mention what branch the code is in, oops. I also just merged it to master so that's not really important now. heh.

@Tomasu
Copy link
Author

Tomasu commented May 21, 2019

If this change is a total no-go, is there any chance of having the new SSO authentication stuff any time soon? I'm not against working on it if theres some kind of plan already in place for it. But I figure this is such a simple change that it would be helpful to have in the interim.

@tobiasKaminsky
Copy link
Member

SSO is already being used by multiple apps.
Also developing speed is rather high.
You can also always use latest master for testing/working on it.

@Tomasu
Copy link
Author

Tomasu commented May 21, 2019

It didn't sound like the future change to move away from AccountManager wasn't something that would happen any time soon. I really only have the option to use this change or having the SSO lib provide me with the available accounts. It currently can't do that since its also limited by AccountManager from doing so without this change.

@David-Development
Copy link
Member

David-Development commented May 21, 2019

@Tomasu I think there is a little misunderstanding here. The SSO library won't help you here if you want to display all accounts.

Your options are these:

Don't use SSO

  1. Passman -> AccountManager.newChooseAccountIntent(...)
  2. Passman -> AccountManager.getAccounts (you'll be able to see the account the user selected in step 1)
  3. Passman -> AccountManager.getAuthToken
  4. Passman -> make request to nextcloud

Use SSO

  1. Passman -> SSO -> AccountImporter.pickNewAccount
  1. Passman -> SSO -> Nextcloud Files App -> make request to Nextcloud

So.. if you use the sso library, you'll also have to use the sso's network stack as you won't get a token that you can use to connect to the server yourself. It also won't help you in any way to find all accounts that are available. You have to show the pickNewAccount dialog where the user can pick whatever account he wants to use.

If you decide to use the sso library, you will have to rewrite the networking part of the passman app. I guess it's only this part here: https://github.com/Tomasu/passman-android/tree/ui-cleanup1/app/src/main/java/es/wolfi/passman/API.

Not sure if that is something you want to look into.

It didn't sound like the future change to move away from AccountManager wasn't something that would happen any time soon

It is correct, that it might take some time for us to do that change as it will be something we need to put more thinking into and I'm only working on this in my spare time beside my full-time job. Feel free to look into it as well. It would be great to provide a unified user-experience across all nextcloud apps. (That's what we are working on right now).

We already have a bunch of apps using it such as the nextcloud-deck app, the nextcloud-news app and some gps tracking app. Feel free to check them out and see how look and feel is.

Let me know if you have any further questions!

@Tomasu
Copy link
Author

Tomasu commented May 21, 2019

What I really want to do is display the available accounts in the app, either through my own activity/fragment or by asking the SSO lib to do it. I'd prefer not to have the stock android account chooser dialog popup. It really doesn't make for a nice user experience.

See: https://cloud.tomasu.org/index.php/s/MaAxaw7NoZArY5R

Right now that is not possible without a (this) small change to the files app.

Not sure if that is something you want to look into.

I am intending on redoing the api stuff yes. If it ends up being better to use the SSO lib I'll do that. (before I learned about the SSO library I was going to just use retrofit and probably rx)

It would be great to provide a unified user-experience across all nextcloud apps. (That's what we are working on right now).

Yeah, it would be great. How much thought has been put into the future authentication bits so far?

We already have a bunch of apps using it such as the nextcloud-deck app, the nextcloud-news app and some gps tracking app. Feel free to check them out and see how look and feel is.

Just loaded up the deck app. It launches right to it's main screen and pops up the account chooser dialog. Its a bit jaring.

@tobiasKaminsky
Copy link
Member

future authentication bits so far?

As Files app uses weblogin flow, we will always support any authentication method that server is capable of, e.g. 2fa via notification, etc.
Even when we came up with e.g. webauthn and a hardware token, this then only needs to be done (if any) in Files app and not in any other "3rd party" app.

@tobiasKaminsky
Copy link
Member

I'd prefer not to have the stock android account chooser dialog popup. It really doesn't make for a nice user experience.

As far is I know this is a standard android component and we cannot change it.

@Tomasu
Copy link
Author

Tomasu commented May 22, 2019

As Files app uses weblogin flow, we will always support any authentication method that server is capable of, e.g. 2fa via notification, etc.
Even when we came up with e.g. webauthn and a hardware token, this then only needs to be done (if any) in Files app and not in any other "3rd party" app.

That is one reason I didn't want to do my own login. Another reason is security reasons. Right now passman as is in appstore stores its passwords. Not exactly great.

As far is I know this is a standard android component and we cannot change it.

We can provide our own alternatives. See my screen shot. That is not a mockup. I am actually getting the account from AccountManager and displaying it in passman. It actually allows you to login and it works (assuming my change is added to the files app). If the files app had an alternative, perhaps a service or activity callback that returned the available accounts, or even just an activity that can be started that shows a similar screen with perhaps some way to customize the look via parameters that would be ideal!

@tobiasKaminsky
Copy link
Member

If the files app had an alternative, perhaps a service or activity callback that returned the available accounts, or even just an activity that can be started that shows a similar screen with perhaps some way to customize the look via parameters that would be ideal!

Maybe this is feasible, @David-Development?

  • add a method that gives username/url to 3rd party app
  • 3rd party app displays it in a custom way
  • 3rd party app gives desired username/url back
  • SSO/Files app presents "grant permission dialog"

This should still be safe, but allows a bit more flexibility?

@Tomasu
Copy link
Author

Tomasu commented May 25, 2019

Bit of an update. I've been working closer with the passman team and it appears there is a strong desire to not force users to install the nextcloud files app. There are passman android users who do not have the nextcloud app installed. This means I need to get the token back, or the SSO lib needs to not depend strictly on the files app.

@David-Development
Copy link
Member

@Tomasu I understand your concerns about forcing users to install the nextcloud files app and I totally agree that this is something to consider. As for the nextcloud news and nextcloud decks app I can say that we wanted to move away from having our custom implementation as it required a lot of work to implement and maintain self-signed certificate support, proxies, safe password storage, etc.
Also the user-experience is shitty as the user has to re-enter the account data in every app. Not to mention that the account chooser dialog looks different in every app.

For devs it's quite easy to use the sso library as it provides an retrofit compatible API which makes the transition from an app that uses retrofit to an app that uses sso very easy! And you can even support both with the same API specs. (that's how I do it in my nextcloud news app; if you want to check it out - Link).

So basically what I'm doing is, I provide a manual login option for users that don't want to use the SSO feature or that don't have the nextcloud files app installed. However if users want to, they can use the SSO feature (check out the attached apk to see how I implemented it in the news app).

News-Android-App-extra-release.apk.zip

Also another note on your current implementation. From what you said it's working right now without any confirmation, right? You open up your app, see all the accounts and then select one. After that the account will be used for synchronization, right? I think that only works when your signing keys of both apps are the same. So if you have different signing keys, you should see the standard android permission dialog (correct me if I'm wrong).

@tobiasKaminsky great idea! I was thinking about something similar. An API (maybe aidl) that has a getAccounts() method. It returns username / url and (if already granted) the sso access token.

@Tomasu
Copy link
Author

Tomasu commented Jul 1, 2019

@David-Development Sorry, I didn't see any notification for your reply.

To answer your question, it does actually request confirmation via the files app the first time you attempt to use it after installing either the files app or my (older) version of the passman app. But it does not show the stock android account selection list which is what I'm trying to avoid.

@joshtrichards joshtrichards added feature: authentication Authentication or accounts related developer experience labels Oct 9, 2023
@joshtrichards
Copy link
Member

Is this still relevant today?

For what it's worth, I see that passman has since integrated SSO: nextcloud/passman-android#136

@joshtrichards joshtrichards added the needs info Waiting for info from user(s). Issues with this label will auto-stale. label Dec 22, 2023
@Tomasu
Copy link
Author

Tomasu commented Jan 11, 2024

Is this still relevant today?

For what it's worth, I see that passman has since integrated SSO: nextcloud/passman-android#136

I havent worked on my version of passman in a while. Its probably safe to close this issue. If i still have issues when i get back to it, i can post a new issue.

@Tomasu Tomasu closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer experience enhancement feature: authentication Authentication or accounts related needs info Waiting for info from user(s). Issues with this label will auto-stale.
Projects
None yet
Development

No branches or pull requests

5 participants