getgrav / grav-plugin-login

Grav Login Plugin
http://getgrav.org
MIT License
44 stars 54 forks source link

Dynamic page visibility and user access not working #166

Closed HenroRitchie closed 4 years ago

HenroRitchie commented 6 years ago

I am busy with a site with a number of private areas. Users first need to register before accessing certain parts of the website. Registration and user access work fine and users need to login to access the content on the certain pages. However, these pages are still visible in the menu even with dynamic page visibility enabled while the user is logged out.

HenroRitchie commented 6 years ago

Furthermore, unless I am missing something obvious, user access to pages are not working. If a page requires specific group access to view the content, and the access is limited to that group, users not part of the group can still access the page.

rhukster commented 6 years ago

Before looking into this would be good to know how you set it up?

ie, what does your user permissions look like? please provide your account YAML file in a markdown code block (triple back ticks)

Also what does your page frontmatter look like that defines the permissions required?

mahagr commented 6 years ago

Also, which theme and menu are you using? Menu needs to support ACL, too, because all the pages are visible for Grav even if you cannot access them directly.

After reading the second message, I'm thinking that the access hasn't been properly set up, though...

HenroRitchie commented 6 years ago

Firstly apologies for the duplicates. Initially, I created one on the grav thread as well as on the login thread. The second one on the main thread was accidental as it seemed as if it did not submit to github.

Below the answers to your questions:

The intial grav instance is a docker instance with the base supplied by evns/grav. I then updated everything (including core and all plugins) to the latest versions.

User account file:

email: ****
fullname: 'Henro Ritchie'
level: Newbie
state: enabled
access:
  site:
    login: 'true'
  admin: {  }
  admin-addon-user-manager: {  }
hashed_password: ****
_json:
  avatar: 'null'
  level: '"Newbie"'
  state: '"enabled"'
  hashed_password: ****
language: en
twofa_enabled: false
twofa_secret: 4QQIDQTNIZHLY5TPZLBVG4PAI3527WEY
groups:
  - bosfontein

The group info is as follow:

bosfontein:
  groupname: bosfontein
  access:
    site:
      login: 'true'
    admin: {  }
    admin-addon-user-manager: {  }

Page header:

title: Bosfontein
access:
    site.login: true
    site.bosfontein: true

I am using Antimatter v2.1.2

I accept that the page permissions might not be setup correctly. I have been trying different combinations without success. I have succeeded before and implemented this in the same way. If I've made a mistake in my notes I don't know where.

What bothers me is that the page bosfontein is still listed in the menu with with access restricted to site.login:true in the formatter and all of its parent pages also restricted to site.login:true.

HenroRitchie commented 6 years ago

Hi

An update, after a while I've stumbled onto an issue logged on Github regarding the Antimatter theme which exposes the menu items even if access to them is restricted. This then provides insight into the one issue I have.

Any feedback on why the private areas and group permissions are not working correctly?

jmolivas commented 6 years ago

I reported on slack a similar issue. I detected while wiriting a plugin to improve UX making available on the GUI new fields to assign access groups to a page and enabling visibility_requires_access

This GUI add to the Frontmatter the following YAML values

login:
    visibility_requires_access: '1'
access:
    site:
        devops: '1'
        authenticated: '0'
        editor: '0'

After debugging login code I found class user/plugins/login/classes/Login.php contains a method isUserAuthorizedForPage

Not clear to me why is doing this validation if ($user->authorize($rule . '.' . $nested_rule) == $nested_value) {

Full code block including foreach

foreach ($value as $nested_rule => $nested_value) {
    if ($user->authorize($rule . '.' . $nested_rule) == $nested_value) {
        return true;
    }
}

I make this to work by validating $nested_value is equals to true

foreach ($value as $nested_rule => $nested_value) {
    if (!(bool)$nested_value) {
        continue;
    }
    if ($user->authorize($rule . '.' . $nested_rule) == $nested_value) {
        return true;
    }
}

On the other side (as a temporary fix) I added a block of code on the plugin I mentioned to add proper validation on pages with user-group access definition but is not taking out of the menu listing page but at least is forbidding access to the user not in the granted access group.

https://github.com/weknowinc/grav-plugin-permissions

NOTE: I could be totally wrong since my experience with grav is very limited and do not have the knowledge of project internals and core.

newmedicine commented 6 years ago

I was having the same issues. I fixed them with this code:

    public function pageVisibility(Event $e)
    {
        if ($this->config->get('plugins.login.dynamic_page_visibility')) {
            /** @var Pages $pages */
            $pages = $e['pages'];
            $user = $this->grav['user'];

            foreach ($pages->instances() as $page) {
                $header = $page->header();
                if (isset($header) && isset($header->access) && isset($header->visibility_requires_access) && $header->visibility_requires_access) {
                    $config = $this->mergeConfig($page);
                    $access = $this->login->isUserAuthorizedForPage($user, $page, $config);
                    if ($access === false) {
                        $page->visible(false);
                    }
                }
            }
        }
    }

If dynamic_page_visibility is not set, nothing happens. If there is a frontmatter header AND there is an access restriction specified (no point otherwise) AND visibility_required_access is present and true, visibility depends on visibility_required_access.

This should reduce the overhead of dynamic_page_visibility a bit as the if there is no header, or the header does not have an access restriction or a visibility_required_access term, visibility is assumed.

@rhukster what do you think? I don't think it disrupts existing logic (at least, the existing logic didn't work for me).

Nick

gojennie commented 5 years ago

I tried all those solutions above with no success.. Pages with 'access restriction set' still appears in the menus. Frontmatter is on and visibility_required_access is on.

Help! Privacy is such an integral part to a CMS. I will post my settings if you need it as well.

rhukster commented 5 years ago

Ok, just commited some fixes.. try with this: at line 200 in login.php

public function pageVisibility(Event $e)
    {
        if ($this->config->get('plugins.login.dynamic_page_visibility')) {
            /** @var Pages $pages */
            $pages = $e['pages'];
            $user = $this->grav['user'];

            foreach ($pages->instances() as $page) {
                $header = $page->header();
                if (isset($header) && isset($header->access) && isset($header->login['visibility_requires_access']) && $header->login['visibility_requires_access'] === true) {
                    $config = $this->mergeConfig($page);
                    $access = $this->login->isUserAuthorizedForPage($user, $page, $config);
                    if ($access === false) {
                        $page->visible(false);
                    }
                }
            }
        }
    }
rhukster commented 5 years ago

You do need this in your user/config/plugin/login.yaml:

dynamic_page_visibility: true

My test page:

---
title: Special

login:
    visibility_requires_access: true
access:
    site.special: true    

process:
    twig: true
---

# This is a special secure page you have access to... 
## welcome {{ grav.user.email }}
gojennie commented 5 years ago

Yes this works! Thank you for the quick response.

Note to those who want this as well: Double check the Frontmatter to make sure that there are 4 spaces before the line 'visibility_requires_access'. I noticed that when I create a new page, Grav inserts these login lines but for some strange reason it has more than 4 spaces and it ends up not understanding TRUE on visibility_requires_access.

Here is a screenshot of what Grav inserts into Frontmatter after creating new page: image

rhukster commented 4 years ago

Fixed in https://github.com/getgrav/grav-plugin-login/issues/228

Strixos commented 3 years ago

Hi there, I tried this solution. When I save the frontmatter, the menu is hidden after a first refresh and then after a second one, it's shown.

mahagr commented 3 years ago

Please create a new issue, this one has been closed.