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

Enable multiple user/profile support for seedvault #332

Closed
dznsm opened this issue Sep 22, 2020 · 12 comments
Closed

Enable multiple user/profile support for seedvault #332

dznsm opened this issue Sep 22, 2020 · 12 comments
Labels
enhancement New feature or request

Comments

@dznsm
Copy link

dznsm commented Sep 22, 2020

Looks like theres some bugs with auto-restore when installing apps, but possibly worth enabling to allow seedvault backups for multiple profiles.
seedvault-app/seedvault#77 (comment)

@thestinger thestinger added the enhancement New feature or request label Sep 25, 2020
@i90faytn3
Copy link

While it seems to have backups of other secondary users, it cannot be restored when making a new one in the user-facing GUI as of now...

@crass
Copy link

crass commented Mar 14, 2021

As the linked comment shows the backup manager must be activated on a new user in order for the restore to work. I'm wondering if it can't be activated easily on new user creation. Even if there's no UI to restore, there is the Activity Launcher work around for explicitly sending the restore intent. This would make it so that users would not need to go through ADB (with all the security implications that entails) in order to restore a secondary user.

As for @i90faytn3's comment, I understand him to say that restoring is not possible from the UI. Currently the restore UI only runs when the Owner account is being setup, which is right after device initialization. Why can't we have the very same flow on new user setup?

@thestinger
Copy link
Member

It needs to be implemented in Seedvault and in SetupWizard. It's not a viable approach to enable it with adb and to manually run the restore activity, which is an option right now, but not officially supported / tested and involves the attack surface / risks associated with trusting a computer to have adb access.

@crass
Copy link

crass commented Mar 14, 2021

Yes, I'm glad we agree that adb is not a viable approach. Its not clear to me that there is something for Seedvault here, if we're just talking about getting the restore workflow to work for secondary users as it does in Owner, which is only once at account setup. Not being familiar with the code, I would suppose that its SetupWizard that needs to do the equivalent of adb shell bmgr --user [ID] activate true, then optionally launch Seedvault's Restore intent.

It seems like a good idea for now to activate the backup manager for all newly created users. Once implemented, this will mean that there would exist a workflow for restoring to secondary user without using adb. Seems like an obvious win. Then the UI/SetupWizard changes can come later. But the idea is to support workflows that remove the need to use adb.

Aside, I just tested activating the backup manager for a new user, after user creation but before user setup (eg. before login). And it appears that the Seedvault transport is already correctly selected. So adb would not be needed for this as is suggested in OPs link.

@thestinger
Copy link
Member

I think the transport is correctly selected for all profiles but for some reason it's disabled by default in secondary ones.

@dznsm
Copy link
Author

dznsm commented Mar 16, 2021

I was looking at this a couple of weeks back and got the impression that I needed to activate the backup manager and set the transport, maybe its since changed? or I'm confused?
I used the ADB OTG app on another phone to reduce the risk of using adb.

@thestinger
Copy link
Member

You shouldn't need to set the transport. That should already set. We need this to be done properly and that ideally means Seedvault enabling itself on other profiles. It could require doing it ourselves as a hack but that doesn't seem very appropriate.

@crass
Copy link

crass commented Mar 16, 2021

@thestinger: This is what I'm confused about. It appears to me from seedvault-app/seedvault#208 that they do not believe it to be an issue to be resolved by them (since its been closed).

Your comment confuses me a little because I don't believe its a question of Seedvault enabling itself. The issue is that the Backup Manager Service itself is not enabled automatically on newly created profiles. This is not Seedvault.

Are you saying that it should be the role of any backup system app to turn on the Backup Manager Service? This could make sense, in the sense that if there are no backup transports (aside from the local one for debugging which isn't useful for backups) then the OS wouldn't want to blindly turn on the Backup Manager Service and thus confuse users by allowing them to turn on backups in the settings but not be able to backup anything.

As for Seedvault actually implementing this, at what point during the new profile creation process would Seedvault be running in order to do this? My understanding is that the new profile initialization doesn't try to do the setup wizard like setting up the Owner account does. Perhaps Seedvault should listen for new profile creation events?

It would be great to have some more details on what your vision is for how this would be handled.

@thestinger
Copy link
Member

It's a Seedvault issue to be addressed by Seedvault. GrapheneOS isn't making code to work around Seedvault limitations or to enable stuff that's not supported upstream.

@thestinger
Copy link
Member

Commenting on the issue and filing more issues about it will slow down OS development but it won't make progress any faster here. Unless someone is going to submit a patch to Seedvault implementing this or a workaround to do it in GrapheneOS downstream (where it would need to be clear we aren't doing something unsupported) there isn't going to be progress on this.

@thestinger
Copy link
Member

I think it's clear that what's supposed to happen is that Seedvault works for secondary profiles and the restore activity would be added to the setup wizard script. GrapheneOS simply bundles Seedvault and sets it as the default backup provider. Other than bundling a setup wizard with support for Seedvault restore, the rest is up to Seedvault.

@thestinger
Copy link
Member

This is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants