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

Support "OR" behavior for permissions. #429

Open
CarmJos opened this issue Sep 14, 2024 · 10 comments
Open

Support "OR" behavior for permissions. #429

CarmJos opened this issue Sep 14, 2024 · 10 comments
Labels
controversial enhancement New feature or request

Comments

@CarmJos
Copy link
Contributor

CarmJos commented Sep 14, 2024

I expect that one instruction supports multiple different permissions, and the user only needs to have one set of permissions to execute the corresponding instruction.

It seems like

return p.hasPermission("xxx.admin") || p.hasPermission("xxx.command.reload");

And I saw the @Permission is "Repeatable" annotation, but I just tested with 2 seperated @Permission for different permissions, also need to have all of them. It's that a mistake?

@Rollczi
Copy link
Owner

Rollczi commented Sep 14, 2024

You must need all required permissions.

@CarmJos
Copy link
Contributor Author

CarmJos commented Sep 14, 2024

Is there any way to achieve this requirement?

@Rollczi
Copy link
Owner

Rollczi commented Sep 14, 2024

No, but you can implement validator on your own.

@CarmJos
Copy link
Contributor Author

CarmJos commented Sep 14, 2024

I think this feature is quite common, and since this annotation is "Repeatable", it should support "or" judgment.

Otherwise, just write all permissions in a single annotation and it's over.

@Rollczi
Copy link
Owner

Rollczi commented Sep 15, 2024

I think this feature is quite common, and since this annotation is "Repeatable", it should support "or" judgment.

Otherwise, just write all permissions in a single annotation and it's over.

This change in the behavior of the @Permission annotation may affect the security of other projects.

@Rollczi Rollczi added enhancement New feature or request controversial labels Sep 15, 2024
@Rollczi Rollczi changed the title Support multi-permission for commands & sub commands. Support "OR" behavior for permissions. Sep 15, 2024
@KermanIsPretty
Copy link
Contributor

I think it would be to just let the user implement a OrPermission annotation if they require this functionality.

@CarmJos
Copy link
Contributor Author

CarmJos commented Sep 17, 2024

I think it would be to just let the user implement a OrPermission annotation if they require this functionality.

This is what I am thinking about, too. But since @Permission has been marked as "Repeatable", why not directly mark different permission sets with different @Permission to determine whether the user meets one of them.

@CarmJos
Copy link
Contributor Author

CarmJos commented Jan 14, 2025

I fount that it is impossible to support this function if not change the original codes.

So may you make it

  1. a "selectable" settings like builder.permissionPredicate(MODE), with "Strict"(Need all claimed permissions) and "Parallel"(Need one of the @Permission("") group claimed) ?

Or

  1. a functional interface like builder.hanlePermisisons(((MultiList<String>)perms)-> /**do something **/ ). With implemented STRICT and PARALLEL handlers to choose/recode.

Whatever the solution you choose, all means you may need to move "@Permission" meta from a single List to a MultiList or List<List>, to make this function possiable.

Anyway, many thanks for you hard-working!

@CarmJos
Copy link
Contributor Author

CarmJos commented Jan 14, 2025

Notice:

Whatevet the mode is, if a player want to execute one subcommand, he should have the command's parents' permissions, e.g.


@Permission("server.admin")
@Permission("cmd.super","cmd.execute")
class SuperCommand{
   
   @Permission("server.admin")
   @Permission("cmd.super.sub")
   void subCommand(){


   }

}

which means if i want to execute /super sub, i need to have "server.admin" or "cmd.super"+"cmd.execute", then i also need to have "cmd.super.sub" or "server.admin".

@CarmJos
Copy link
Contributor Author

CarmJos commented Jan 16, 2025

@huanmeng-qwq and I have finished this feature, but there're some problems need for discuss, please see the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial enhancement New feature or request
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

3 participants