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

WIP: make upgrading demo accounts available via a config option. #3614

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zenhack
Copy link
Collaborator

@zenhack zenhack commented Apr 3, 2022

This fixes #3239 by providing a config option to re-enable the ability
to upgrade demo accounts to full accounts, which was removed in order to
allow alpha to act as the demo server when oasis was shut down.

Unfortunately, this seems to break other tests. The handful I examined
resulted in permission errors trying to upload the test app; presumably
when this is enabled our test accounts are somehow getting marked as
not allowed to upload their own apps.


Accordingly, I'm marking this as a draft until that can be debugged.

Doing this is the source of sandstorm-io#3584.

Instead, make sure we've fully entered a new user namespace before
we have to do anything that would require the capabilities that are
dropped on exec(). We also need to be in a new pid namespace, since
we try to mount /proc so it needs to be a procfs that we own. We use
clone() instead of unshare() for this so we don't have to disturb
the process hierarchy.
This fixes sandstorm-io#3239 by providing a config option to re-enable the ability
to upgrade demo accounts to full accounts, which was removed in order to
allow alpha to act as the demo server when oasis was shut down.

Unfortunately, this seems to break *other* tests. The handful I examined
resulted in permission errors trying to upload the test app; presumably
when this is enabled our test accounts are somehow getting marked as
not allowed to upload their own apps.
@zenhack zenhack marked this pull request as draft April 3, 2022 21:23
@zenhack
Copy link
Collaborator Author

zenhack commented Apr 3, 2022

(Note that this includes the commits from #3609, since those are necessary for any of the tests to pass).

@ocdtrekkie
Copy link
Collaborator

Does ALLOW_UNINVITED permit users who sign in without an invite to create grains? I am not sure if the config option is self-explanatory enough. How does this interact with the permission to allow demo accounts, and what happens if one but not the other is set?

...Which is to say, I maybe might suggest a better config variable name, and definitely would like to see the docs updated in the PR.

@zenhack
Copy link
Collaborator Author

zenhack commented Apr 3, 2022

Yeah, I should definitely update the docs before this is merged, and I just copied the name from the meteor setting, so could probably be improved.

@ocdtrekkie ocdtrekkie added bug sandstorm-dev Issues hacking on Sandstorm labels Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug sandstorm-dev Issues hacking on Sandstorm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/account test is broken
2 participants