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

Utilize PEX permissions matcher #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

muff1nman
Copy link

@muff1nman muff1nman commented Jul 14, 2016

Hi, excellent project. Found it very useful.

I made a couple modifications that were useful to me, and thought you might be interested.

  • Added maven dependency framework support. Makes it much easier for incoming folks to build a working jar
  • Modified permission checking logic to use PEX Regex matcher directly
    • Makes it so that wildcard expressions are supported
    • Makes the behavior more similar between BPB and PEX
  • Simplified group hierarchy calculation
    • Allows use of # character for non-inheritance as done in PEX

@guipenedo
Copy link
Owner

looks great! have you tested this?

@muff1nman
Copy link
Author

I am running this now on my simple setup which consists of a single Admin group with 4 wildcard expressions. Things that have yet to be tested:

  • # non-inheritance operator
  • Multiple level hierarchies
  • Permissions overriding inherited ones

@muff1nman
Copy link
Author

While testing, found one interesting edge case. I had a hierarchy of:

group1 --> group2 --> user1
group1: bungeecord.command.list
group2: 
user1:

Which did not work. Turns out that if a user is nested under an empty group, the parent non empty group permissions will not apply as group2 will never be found. Remedied this with the last commit.

Also tested the # operator, tested multi-level hierachies and tested permissions overriding in the hiearchy.

I'm satisfied with it, but wouldn't hurt to have another person take a look.

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.

2 participants