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

Feature Request: Admin management features #428

Open
exedore6 opened this issue Apr 30, 2021 · 17 comments
Open

Feature Request: Admin management features #428

exedore6 opened this issue Apr 30, 2021 · 17 comments

Comments

@exedore6
Copy link

It would be useful for an admin to be able to manage other user data.

As it stands today, an admin can see how many shares a user has created. Anything more would require a password change (which an admin can do) or digging in the database.

Examples of some things I'd think an admin should be able to do -

  • Examine what a user's aliases and shares
  • Manage their files.

One way to do this would be to allow an admin to impersonate a user in the API (to pass a userid in the request for example)

@eikek
Copy link
Owner

eikek commented Apr 30, 2021

Hi and thanks for this input! I have to admit, I never had the need to look into files of other users. What is your use-case? I know the admin can always look into the database or change a user's password.

@exedore6
Copy link
Author

exedore6 commented May 1, 2021

My use case is I'm using (and love) Sharry for my non-profit's development office to get large file submissions from constituents. It's awesome.
An individual at my organization creates an alias, and collects a couple of gigs of video.
Sends out the link/qr/code, and people (many who are non-technical) can submit the files with a minimum of fuss.

Their account is the only one that can access these resources - which means that if they wanted to delegate the collection of those files to another user (or a team of people), we've got to share passwords. A good example would be "Download all of the submissions to the video-montage project. As it stands, that must be done by the account that created the submission alias (I realize I could write a script that could query the database, grab the file chunks and metadata and reconstruct the file - I just feel like that should be part of the API.

Now this sort of thing might be out of scope for the project or a ton of work - in which case, I'd want to know. These are definitely more 'enterprise' features. I'm currently getting up to speed with Elm so I can better understand how tough some of these things would be to implement, but I wanted to start this conversation before you start getting pull requests for features that you Do Not Want or Do Not See the Value of. Also, I'd rather do it in a way that makes sense for everyone, not just me, there may be things that need to be added first, etc.

Essentially in my use case, shares and aliases don't necessarily belong to one person forever. Their ownership may change, they may be managed by a team of people.

@eikek
Copy link
Owner

eikek commented May 2, 2021

Thank you very much! It's always good for me to know some background. I understand what you need and can relate. Just recently I had a similar need: we got videos from the school of our child and so I wanted my partner to be also able to access it, too. It is true that this is a bit out of scope - never thought about this before. I'd like to have sharry as simple as possible only to be able to maintain it for a longer time (time is just so sparse unfortunately). But otoh it adds good value. I usually then think about whether the new feature's "complexity" can be maintained by me (a hard question to tell :)).

I like the idea and I like to discuss how we can implement it. I admit, I'm currently not very convinced about impersonating accounts. It would also mean in the UI to somewhere state as what account to act and users could not control it (every admin can see all their files without them being able to deny; Sure, an db-admin can always just look into the db, but still…:)). I think it would remain quite some tedious work for your use case - you might need to change to several accounts and memorize which account had which alias?

If I understand correctly, the idea is to have alias pages be shared among some users. What if a user can create an alias as today and then add other users by name to it. Then this alias will also show up in the alias list of all these users. Uploads done through this alias will show up in the list of all the users associated to the alias. To make it a bit simpler I'd say that any user can manage other users and the uploads once they are in the group and maybe also delete the alias etc; I'm not yet sure what makes sense here. This, of course, assumes that the users trust each other. Wdyt - would something like this help you?

@exedore6
Copy link
Author

exedore6 commented May 3, 2021

I'm less concerned about the trust (especially since the alternative is to share passwords). I'd add an association table to connect people to an alias, which the owner could manage. This would give the structure to have limits to non-owners (maybe only the owner can delete or deactivate the alias for example)

I think there should be some indication about what's mine and what's shared with me.

I don't think it would increase the complexity too much, the owner of the alias could ignore it if it didn't fit their use-case.

I get the concerns about the admin, but I think they could be addressed by logging any impersonation. The users aren't any more secure my making the management UI beyond account management be psql. I'd add an optional API argument for userid, that an admin can pass as an argument which would use the admins session for authentication, but any database calls would use the provided ID. It could stay at the API level in my opinion. Anything to keep admins from doing what they need to do at the database level.

Also, keep in mind that I'm down with doing the work - I just want the way I solve my problem to be useful for more than just me.

@eikek
Copy link
Owner

eikek commented May 4, 2021

I think there should be some indication about what's mine and what's shared with me.

Yes agreed, that would be good. Then it's necessary to make the distinction.

I don't think it would increase the complexity too much, the owner of the alias could ignore it if it didn't fit their use-case.

Yes, it's optional which is good. The complexity still is higher now IMHO since I need to test more scenarios. I agree it's not that much and I'm completely fine with it.

I get the concerns about the admin, but I think they could be addressed by logging any impersonation. The users aren't any more secure my making the management UI beyond account management be psql.

Yes this is true for db-admins. But in sharry you can create admin users who not necessarily have access to the database. So these could then also impersonate all other users; then the question is should they be allowed to impersonate other admins, too…. As you said, this must be logged prominently. Do you think impersonating is still needed for your use-case, if we have the "share alias" feature? I'd rather avoid this, I admit. I had hoped the share-alias thing would fit well?

@exedore6
Copy link
Author

exedore6 commented May 6, 2021

Being able to share ownership would do a lot of it. I'd want admins to be able to manage the permission list though.

One of the things to keep in mind in an instance like where mine (where the instance is managed by an organization, and every account is a member of said organization, is all of the content belongs to the organization - identity and access is managed outside of sharry - leaving the admin role pretty meaningless.

I don't love impersonation either - I just see it as the least complex way to allow Sharry administrators to manage the content side. You're right though, you'd want to be able to delegate these different roles - account administrator, content administrator (possibly associated with different user groups).

Which does add a bunch of complexity unfortunately.

@eikek
Copy link
Owner

eikek commented May 7, 2021

What do you mean by "permission lists"?

I think I see your point. I'm not sure if sharry is the tool for this…. It was always thought as a personal tool first. I don't really want to add permissions and different user roles and all that. I'm assuming that it's mostly not needed.

What I could imagine to add, though, is an additional endpoint (which must be enabled explicitly) for "real" admins (those who manage the config file). It wouldn't be available in the ui, but people could then write their scripts against it. This could provide a list of all files and download links to them. wdyt?

@exedore6
Copy link
Author

exedore6 commented May 8, 2021

That would work, though it seems like a lot of duplicated code to get the equivalent to adding impersonation as an option in the existing API.

Like, if the current API calls took an optional userid, which changes the context of the authenticated user is authorized, you'd get what you describe without duplicating any code.

@eikek
Copy link
Owner

eikek commented May 9, 2021

I don't think it means too much duplicated code, I have to see though. Impersonating means to conflate two things into one endpoint, and I currently don't like this. I would have to decide on each one based on the user admin flag. It's easy to get a bug into this which means a lot of harm. I'd prefer to not touch any existing code. Also, I want to completely remove this feature via a config setting. It's easier to simply not add a set of endpoints in this case than to push it into every existing endpoint. Another reason is that I don't want to add another meaning to the "admin" flag. A separate set of endpoints can be authenticated in a different way and then sharry admins can be used for account management as before and only people who manage the config file may use the separate endpoints.

But… I have to see when I'm looking at the code :-) It's good to know that it would also help you with your use case so I can still choose :-). Maybe it can be safe and easier with impersonating; I'm open to change my mind. Currently the above is my preferred way.

@exedore6
Copy link
Author

Would it be accurate to say that the "admin" flag means "can manage internal accounts?"

@eikek
Copy link
Owner

eikek commented May 12, 2021

Yes, it is only to manage accounts. External ones, too, wrt sharry; for example set the email, active/disabled and to nominate other admins.

@eikek
Copy link
Owner

eikek commented May 18, 2021

I'm going to implement sharing an alias page with other users for the next release. I think this is a nice addition and not too much work. You might then be able to add yourself to every alias (as per "convention" or db trigger maybe). Not ideal, but might help a bit, I hope? Then I'd like to do the "admin all files" stuff for a later release. We can still think about what should go into it; and whether to use impersonation or something else.

@eikek
Copy link
Owner

eikek commented Sep 5, 2021

Hi, I just realized that the impersonating feature can be achieved by external tools already. Keycloak can do this and you can configure sharry to use it via the oauth options. I think this is a better approach when there is more to managing user accounts. (caveat: I haven't tried it, but it looks like it could work :-)).

@otherjoel
Copy link

The only admin feature which I would really like to see is something to help me out with managing storage space on my small server: the ability to see all shares sorted by size and selectively delete them. I know I could do these things directly in the database. But it would be really nice to have it as part of the web UI.

To make things simpler for my users, my instance allows login only by OAuth (Azure AD), so ideally we could specify admin-allowed accounts by name in the config file. (Rather than having an admin "account" with its own username and password, which would preclude us from using the automatic login redirect.)

@eikek
Copy link
Owner

eikek commented Oct 6, 2022

sorry for the delay. Totally forgot about this issue - there is one thing left, if I recall and reread correctly: an additional endpoint to list all files in sharry (across all accounts). This should then also provide the size of each share.

@otherjoel
Copy link

Until this feature is implemented, assuming one is using Postgres, is there a simple query I can execute that will list all the files across all accounts, their size and owners?

@eikek
Copy link
Owner

eikek commented Aug 16, 2023

Hi @otherjoel , sorry for the delay! So I think this could be a starting point:

select a.login, s.name_ as share_name, f.filename, f.real_size
from share s
inner join account_ a on a.id = s.account_id
inner join share_file f on s.id = f.share_id
order by a.login, s.id;

It lists all files, the owning account, their size and share name. Of course, you can add more columns to the select (look into the tables via \d <table_name>).

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

No branches or pull requests

3 participants