silverstripe / silverstripe-session-manager

Allow users to manage and revoke access to multiple login sessions across devices.
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Allow canView for admins #178

Closed lekoala closed 5 months ago

lekoala commented 5 months ago

Description

I'd like to make a really simple change

I'm building an admin ui where i display user sessions for security purposes. Currently, the canView method only allow the member themselves

I know i can write an extension just for that but it feels like a waste when this could be provided natively by this module

So either, wouldn't we replace the "return false" at the end of handlePermission by a Permisison:check('CMS_ACCESS') or add a config flag that makes this opt in. If we go the config flag route, this would not have any side effect and only potential benefits for very limited changes.

Additional context or points of discussion

No response

Validations

GuySartorelli commented 5 months ago

So either, wouldn't we replace the "return false" at the end of handlePermission by a Permisison:check('CMS_ACCESS') or add a config flag that makes this opt in.

We should use a config flag - we don't allow admins to see this information by default for security reasons.

If developers opt into this using the config flag, we should use Permission:check('ADMIN'), not Permission:check('CMS_ACCESS') - we only want to grant access to administrators, not everyone who can access the CMS.

GuySartorelli commented 5 months ago

Actually - sorry, I've been thinking a bit more about this. I don't think this is a feature we'd support in core. As I mentioned we don't allow administrators to access this for security reasons. Having code there to circumvent that in core (especially when a simple extension can be applied in project code) doesn't seem prudent.

lekoala commented 5 months ago

no worries, extension it is then :)