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

Proposed use case: Readonly Store #1892

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

morningspace
Copy link
Contributor

Background of this use case, see: #1878.

Initial version of use case proposal: Readyonly Store

RELEASE_NOTES=[DOCUMENTATION]

Signed-off-by: morningspace <morningspace@yahoo.com>
Copy link
Member

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not yet sold on the specific implementation details, e.g. having a readonly option.
Maybe there is an easier way to support this use case.

$ gopass config readonly true
readonly: true
# To apply the config to a sub-store
$ gopass config readly true --store team-sharable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read gopass config readonly.

But I don't think I want to bring back nested configs. They were very painful.
There needs to be a better way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, let me fix it. Re: nested configs, do you mean the per-store config? I wonder why that's painful. Understand that can introduce complexity such as the possible conflict between per-store config and global config.


Ultimately it turns out that this scenario requires a feature such as a store in readonly mode, where people can configure their local store or a set of sub-stores in readonly mode, to disable the writes and the autosync-on-writes to the store, but they can still pull to sync the latest changes from the remote store. This is not a one-stop solution for the RBAC model of the team sharing store, because we still need GitHub to setup the store access at server-side, but it will provide better usage experience from gopass client side.

Configuration examples:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this is the best way to do it, yet.

Maybe we should drop the Configuration examples section and have a more open ended Usage examples one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm open to make any change as long as it can address the issue nicely.

@morningspace
Copy link
Contributor Author

Maybe there is an easier way to support this use case.

Yeah, I hope so too. Right now, I am not sure whether there's simpler option. If we consider this as a configurable thing, then I think no matter what name it is, readonly, offline, or whatever else, it is essentially a way to control the writes and autosync-on-writes behavior conditionally. Also, it seems that has to be a per-store config, because practically, to make the local store completely readonly is not a good idea as people may have both private and sharable stuff stored on local.

On the other side, if we can consider it as a non-configurable thing, maybe a better idea, since people can explicitly turn off the readonly config for a team sharing store which will bring back the painful things again. With that, it looks to declare a store to be readonly during the init phase would make much more sense. However, the store owners or privileged users still need write access to the store. This makes the idea slip to some kind of RBAC model which is also something I may want to avoid since it may also be complex or even more complex than a config option.

@dominikschulz
Copy link
Member

I'd still want to merge this and eventually support the use case but I don't like to proposed configuration section, yet.

So either @morningspace changes this or we merge it as is and update it afterwards.

@AnomalRoil
Copy link
Member

AnomalRoil commented Jan 3, 2022

@dominikschulz I think you can edit it by sending extra commits directly on the branch morningspace/gopass, and it will accept commits from the original repo maintainers.

@dominikschulz dominikschulz self-assigned this Jan 10, 2022
@dominikschulz dominikschulz added this to the 1.x.x milestone Mar 16, 2022
Copy link
Member

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dominikschulz dominikschulz merged commit 861d0c2 into gopasspw:master Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants