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

Fix stack access with advanced permissions #7029

Closed
wants to merge 7 commits into from

Conversation

mlocati
Copy link
Contributor

@mlocati mlocati commented Sep 5, 2018

concrete5 can handle permissions at the stack level.
BTW concrete5 doesn't handle these permissions correctly.
Let's fix this.

Fix #7006

PS: this PR is still a work in progress. I'll finish this ASAP.

{
return $this->validate('approve_area_versions');
}
public function canDeleteArea()
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlocati @aembler Would it be possible to add a release number to new methods? That would make it easier for package developers to use these methods, without risking backward compatibility issues. Similarly to e.g. https://github.com/WordPress/wordpress-develop/blob/811eef33a31aab13d38a13522d4a58419566225b/src/wp-includes/user.php#L25

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a great idea. Sure, we have https://documentation.concrete5.org/api , but it's annoying to look at those pages every time we use a method.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is a handy feature - just wondering how would that work for things that haven't been released yet? A -dev tag that would need to be updated?

Also, 8.4.2 isn't included in the API docs :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, 8.4.2 isn't included in the API docs :(

I guess that the core team didn't implement what I suggested at concretecms/api#5 and they prefer updating the API docs manually...

@mlocati
Copy link
Contributor Author

mlocati commented Sep 6, 2018

@aembler If you think this is the right approach, I'll finish this.

@mlocati
Copy link
Contributor Author

mlocati commented Sep 7, 2018

I tested this PR (and everything seems to work) with the following cases:

  • clean installation without advanced permissions
  • clean installation with advanced permissions
  • upgrade existing installation with advanced permissions

@mlocati mlocati changed the title Fix stack access Fix stack access with advanced permissions Sep 7, 2018
@mlocati
Copy link
Contributor Author

mlocati commented Sep 7, 2018

Just to show what this pull request fixes, let's assume we have these permissions for a stack:

immagine

Before this pull request, the user can't edit the stack:

  1. No "Manage Stack Contents" menu item
    immagine
  2. Errors when editing the stack:
    immagine

With this pull request, everything works fine:

immagine


immagine

@mlocati
Copy link
Contributor Author

mlocati commented Sep 21, 2018

I'm closing this because it has a wrong approach.

@mlocati mlocati closed this Sep 21, 2018
@mlocati mlocati deleted the fix-stack-permissions-v2 branch September 26, 2018 15:43
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.

Check stack and global area permissions
4 participants