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

Allow restricting commands to arbitrary permission #6

Open
Saladoc opened this issue Jan 24, 2023 · 3 comments
Open

Allow restricting commands to arbitrary permission #6

Saladoc opened this issue Jan 24, 2023 · 3 comments

Comments

@Saladoc
Copy link

Saladoc commented Jan 24, 2023

What?

Allow restriction of a command to an arbitrary permission understood by discord (or rather javacord/jda) without having to create a custom Restriction implementation.

Why?

Use cases will most commonly be cases where a bot command reproduces a discord (moderation) functionality and should only be made available to users that have that capability, like:

  • Restricting a cleanup command to users with the MANAGE_MESSAGES permission.
  • Restricting a mass timeout/kick/ban command to users with the MODERATE_MEMBERS/KICK_MEMBERS/BAN_MEMBERS permission

Providing a functionality like this directly in command-framework will standardize handling. It will allow other code to better inspect the RestrictionChain and from that derive the actual permissions required - this will aid, for example, if slash commands are to be registered with the appropriate default permissions.

What it...

should be

  • Provide a class or interface for parameter based restrictions.
  • Allow to specify whether the ADMINISTRATOR permission being present should also be accepted (defaults to true)
  • Provide an annotation like RestrictedToPermission("MANAGE_MESSAGES") or RestrictedToPermission("MANAGE_MESSAGES", acceptAdmin=false)

might be

  • Implementation-specific with different annotations / classes / interfaces for Javacord and JDA accepting their respective permission types
  • Extensible for custom string-based permission systems (provided a resolver is supplied)

should not be

  • anything more than that

Problems / Thoughts

Such a system is not supported with the current annotation / restriction concept as far as I can see, so it will require some thought on how to properly integrate it. Maybe it could be implemented by adding a phase to hook into during creation of the commands where a user-provided bean could transform the command and for example check for custom annotations, parse them and add elements to the restriction chains - but those wouldn't play nicely with the @{All,Any,None}Of annotations.

@Vampire
Copy link
Owner

Vampire commented Jan 24, 2023

I'm not sure I understood why you cannot use the existing restriction system.
For example something like.

@Override
public boolean allowCommand(CommandContext<? extends Message> commandContext) {
    Message message = commandContext.getMessage();
    return message
            .getServer()
            .flatMap(server ->
                    message
                            .getAuthor()
                            .asUser()
                            .map(user -> server.hasAnyPermission(user, KICK_MEMBERS, BAN_MEMBERS, ADMINISTRATOR))
            )
            .orElse(FALSE);
}

@Saladoc
Copy link
Author

Saladoc commented Jan 24, 2023

I'd prefer a way to have an annotation on the command or a mechanism where I can parse custom annotations into restrictions. That way the permissions can be transparently viewed in the code.

Maybe this can also be used for populating the default permissions for slash commands.

@Vampire
Copy link
Owner

Vampire commented Jan 24, 2023

How about

@Retention(RUNTIME)
@Target(TYPE)
@Documented
public @interface RestrictedToPermission {
    PermissionType[] value();
    boolean acceptAdmin();
}

and

@Override
public boolean allowCommand(CommandContext<? extends Message> commandContext) {
    Message message = commandContext.getMessage();
    RestrictedToPermission permissionsAnnotation = commandContext
            .getCommand()
            .orElseThrow(AssertionError::new)
            .getClass()
            .getAnnotation(RestrictedToPermission.class);
    if (permissionsAnnotation == null) {
        return true;
    }
    return message
            .getServer()
            .flatMap(server ->
                    message
                            .getAuthor()
                            .asUser()
                            .map(user -> ((permissionsAnnotation.acceptAdmin() && server.hasPermission(user, ADMINISTRATOR))
                                          || server.hasAnyPermission(user, permissionsAnnotation.value())))
            )
            .orElse(FALSE);
}

?

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

No branches or pull requests

2 participants