ACL design of dashboard

[jasonjoo2010]

Issue Description

Version 1.6.0 introduces authorization and it's an awesome feature. That helps dashboard to be more complete.

My discussion here is focused on the actual authorizing design.

Interface Design

First i think here is a little fuzzy on AuthUser. authTarget.

        /**
         * Query whether current user has the specific privilege to the target, the target
         * may be an app name or an ip address, or other destination.
         * <p>
         * This method will use return value to represent  whether user has the specific
         * privileges to the target, but to throw a RuntimeException to represent no auth
         * is also a good way.
         * </p>
         *
         * @param target        the target to check
         * @param privilegeType the privilege type to check
         * @return if current user has the specific privileges to the target, return true,
         * otherwise return false.
         */
        boolean authTarget(String target, PrivilegeType privilegeType);

If throwing an exception is an option it's better to include in declaration like:

 boolean authTarget(String target, PrivilegeType privilegeType) throws RuntimeException;

But i don't think it's a good idea throwing an exception because we have a boolean value returned already to mark it success or fail.

Integrating

For function integrating we can find following lines everywhere:

AuthUser authUser = authService.getAuthUser(request);
authUser.authTarget(app, PrivilegeType.READ_RULE);

It includes two intents:

1. Get the current logged user information
2. Check if he has the specific privilege.

But it's a little inconvenient. I have a proposal on it like:

@AuthAction(privilege = PrivilegeType.READ_RULE)
@GetMapping("example")
public String action() {
}

or

@AuthAction(app = app, privilege = PrivilegeType.READ_RULE)
@GetMapping("example")
public String action() {
}

or even a parent privilege like

@AuthAction(privilege = PrivilegeType.RULES)
@RequestMapping("/rules")
@Controller
public class RulesController() {
}

When you want user information we can inject it by Spring Argument Resolver like:

@AuthAction(app = app, privilege = PrivilegeType.READ_RULE)
@GetMapping("example")
public String action(AuthUser authUser) {
}

I think we can make more discussions.

[thiyagu06]

@jasonjoo2010 , I think creating custom annotation based security is the best approach and spring security provide an way include our annotation. You can refer my implementation here. https://github.com/thiyagu06/auth-manager/blob/master/src/main/java/com/izettle/authmanagement/controller/LoginAttemptsController.java#L44

I would like to contribute on this one.

[jasonjoo2010]

@thiyagu06 Right.

@sczyh30 And what do you think about this issue, should this design go further?

Brief:

1. Action/Controller level annotations to declare permission requirements.
2. More simple way to get the user authorized.

[sczyh30]

@thiyagu06 Right.

@sczyh30 And what do you think about this issue, should this design go further?

Brief:

1. Action/Controller level annotations to declare permission requirements.
2. More simple way to get the user authorized.

+1 for the idea of annotations of auth control. It's more elegant and convenient than function integrating. Contributions are welcomed.

For the design of authTarget, what do you think of it? @CarpenterLee

[jasonjoo2010]

So any updated?

What's the conclusion?

And kindly @thiyagu06 Thiyagugk are you working on it?

[sczyh30]

But i don't think it's a good idea throwing an exception because we have a boolean value returned already to mark it success or fail.

I agree with this. Using the return value is enough. And it's better to have controller/handler level annotations to declare permission restrictions.

@thiyagu06 Any progress on this?

[thiyagu06]

But i don't think it's a good idea throwing an exception because we have a boolean value returned already to mark it success or fail.

I agree with this. Using the return value is enough. And it's better to have controller/handler level annotations to declare permission restrictions.

@thiyagu06 Any progress on this?

@sczyh30 , I have started to work on it.. Will let you the progress in few days.

[thiyagu06]

But i don't think it's a good idea throwing an exception because we have a boolean value returned already to mark it success or fail.

I agree with this. Using the return value is enough. And it's better to have controller/handler level annotations to declare permission restrictions. @thiyagu06 Any progress on this?

@sczyh30 , I have started to work on it.. Will let you the progress in few days.

1. We will have custom Auth annotation on the method/class level

@AuthAuction(PrivilegeType.READ)

2. During each call to the service an filter will intercept the requests and create AuthUser. AuthUser Object will have the app the user has access i.e priviligeTypes.

3. Before Will executing the each method, will verify the access and thrown an error if the user has access to the method?

correct me if i'm wrong.

[jasonjoo2010]

But i don't think it's a good idea throwing an exception because we have a boolean value returned already to mark it success or fail.

I agree with this. Using the return value is enough. And it's better to have controller/handler level annotations to declare permission restrictions. @thiyagu06 Any progress on this?

@sczyh30 , I have started to work on it.. Will let you the progress in few days.

1. We will have custom Auth annotation on the method/class level

@AuthAuction(PrivilegeType.READ)

1. During each call to the service an filter will intercept the requests and create AuthUser. AuthUser Object will have the app the user has access i.e priviligeTypes.
2. Before Will executing the each method, will verify the access and thrown an error if the user has access to the method?

correct me if i'm wrong.

Things sounds correctly and i suggest we can mainly restructure logic by introducing new annotations based on current implementations. We mainly make it easy to use/read.

And more don't forget we should make it easy to understand for action level AuthUser fetching.

@sczyh30 Any suggestion?

[sczyh30]

@thiyagu06 Any progress on this? :)

[sczyh30]

@thiyagu06 Friendly ping :)

[lzq2357]

authTarget 返回值居然没有用作权限判断？那返回值的意义是啥？而且接口的文档上也不注明必须抛异常，或者抛哪个异常，前端页面接收到异常也只会显示【失败】，而不是我写的message。

[sczyh30]

This will be improved in #1042. Further contributions are welcomed!