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

Permissions for Who's Online admin/mod centre #4976

Merged
merged 1 commit into from Feb 19, 2019

Conversation

Sorck
Copy link
Contributor

@Sorck Sorck commented Aug 29, 2018

Do not reveal admin and mod centre usage unless allowed to see it as per issue #4972

  • Adds in all admin centre areas (except index) properly to the permissions checking array in Who.php
  • Removes fallbacks that say something along the lines of 'using the admin panel' - only those with access to the admin panel should be able to see this.
  • Does not add in the mod centre areas as they are harder to calculate permission for and have names that are likely to conflict with other actions.

Signed-off-by: James Robson git@sorck.net

elseif (in_array('moderate_forum', $allowedActions[$actions['action']]))
$data[$k] = $txt['who_moderate'];
elseif (in_array('admin_forum', $allowedActions[$actions['action']]))
$data[$k] = $txt['who_admin'];
else
Copy link
Member

Choose a reason for hiding this comment

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

We should leave this here, so it still shows the generic admin/mod messages for those without access.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we consider it a privacy/security issue to show an admin working in the admin centre if that user has no way to view the admin centre?

Assuming that doesn't matter, they could be expanded quite a bit for completeness as admin_forum is not a permission needed for many admin centre areas. We may also need another integration hook to cover any other actions that may want to declare themselves as a mod centre/admin panel page.

We could then either do it as an array with a list of permissions that defines the admin panel, or a list of areas that are within the admin panel.

// If a page has one of these permissions then it is in the approapriate major action, like the admin or moderation centre.
// the admin and moderate items can initially be populated from the 'admin' and 'moderate' keys in $allowedActions
$majorActionPermissions = array(
    'admin' => $allowedActions['admin'],
    'moderate' => $allowedActions['moderate'],
);
// Allow mods to add to the major action areas
call_integration_hook('integrate_who_major_actions', &$majorActionPermissions);

// Check if we should show a major action message
foreach ($majorActionPermissions as $action => $permissions)
    if (!empty(array_dif($permissions, $allowedActions[$actions['action']]))
        $data[$k] = $txt['who_' . $action];

Or if it was area based.

// Used to find out if the current area is part of the mod centre/admin centre
$majorActionAreas = array(
    'admin' => array(
        'admin',
        'packages',
        'etc',
    ),
    'moderate' => array(
        'moderate',
    ),
);
// Allow mods to add to the major actions
call_integration_hook('integrate_who_major_actions', &$majorActionAreas);

// Only check against major action messages if we are in a large area
if (isset($actions['area']))
    foreach ($majorActionAreas as $action => $areas)
        if (in_array($action, $areas))
            $data[$k] = $txt['who_' . $action];

Benefits:

  • mods can add themselves as major actions with sub areas
  • mods can declare their areas as being in the admin centre or mod centre

Copy link
Member

Choose a reason for hiding this comment

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

We have had the setup that way for a long time, even possibly in YabbSE. So I wouldn't consider it a risk. Not much can be identified by what they are doing. Just that they are in a mod or admin area.

The hook sounds great, but need others to provide input on how specific we get. I would go with the first and let the mod authors handle that.

Copy link
Contributor

Choose a reason for hiding this comment

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

So $allowedActions would be for normal non-admin/mod actions only?
If it's like that it sounds great, I wouldn't mind either of the suggestions both are nice.

@Sesquipedalian Sesquipedalian added this to the RC2 milestone Sep 18, 2018
@Sesquipedalian Sesquipedalian merged commit d5ba628 into SimpleMachines:release-2.1 Feb 19, 2019
Sesquipedalian added a commit that referenced this pull request Feb 19, 2019
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants