miguelgrinberg / Flask-HTTPAuth

Simple extension that provides Basic, Digest and Token HTTP authentication for Flask routes
MIT License
1.27k stars 228 forks source link

add support for url resources #124

Closed skeetwu closed 3 years ago

skeetwu commented 3 years ago

Hi,

When I call the function "login_required", there is only one argument "role" that can be used, but in some cases, I need to allow users to add or customize the role name, so if we add a fixed role name to "role" when calling the function, we can meet the requirement. So, I plan to add a new argument "resource" as a string value, it should be an identifier string for a resource(a resource should be a path or a URL), then add a callback function "get_user_resources". When the user login, we can get the user's all roles, and get all resources list by these roles. for example: add a API:

@ns.route('/terminal/<string:ip>')
class Terminal(Resource):
    @auth.login_required(resource='C02')
    def get(self, ip):
        return terminal_dao.get_terminals_by_IP(ip)

add callback function

@auth.get_user_resources
def get_user_resources(username):
    # user resources values: "U01#U02#U03#C01#C02#C03"
    return User.query.filter_by(username=username).first().to_json()['resources'].split('#')

add a define for all menus and resources.

menus_list = [
    {'M0':
        {
            'name': 'Users,
            'path': '/menu/user',
            'icon': 'auth',
            'icon-color': '#cbcd32',
            'resources': {
                'U01': 'list all users',
                'U02': 'show one user',
                'U03': 'update one user'
            }
        }
    },
    {'M1':
        {
            'name': 'terminals',
            'path': '/menu/terminals',
            'icon': 'laptop',
            'icon-color': '#5d6ee4',
            'resources': {
                'C01': 'search all terminals',
                'C02': 'show one terminal',
                'C03': 'update all terminal'
            }
        }
    }
]

I have changed the code for this part, If you think this approach is acceptable, I will continue to finish the document and demo work. image

B&R, Skeet

miguelgrinberg commented 3 years ago

I don't understand what this feature does. The word "resource" has a very specific meaning in APIs, and I think you are using it for something else, which adds to the confusion. And what are these cryptic C01, C02, etc? That doesn't mean anything, I don't think that is a good practice to promote through this package.

You may want to think about what problem this change solves. Roles address a common need of API developers. What does this change do that cannot be implemented with roles?

skeetwu commented 3 years ago

Hi @miguelgrinberg

Thanks for your reply. If I understand correctly, roles address a common need of API developers, and we need to give it a fixed value, for example, we give a function's role as "admin",

@ns.route('/test')
class Test(Resource):
    @auth.login_required(role='admin')
    def get(self):
        return {'xx':'yy'}

But if we allow a user to create a role and customize the role name, for example, the user creates a role named "system", then add a URL resource "/test" to this role, if we want the new user who only has the role "system" to access the API resources "/test", how should we implement this feature?

In fact, I have completed the requirements in the current Flask-HTTPAuth, but not very graceful. Here is my step, I changed the purpose of "role" in the current Flask-HTTPAuth, I used it as the definition of a resource(API path), its value is a custom unique number for one resource, such as C01 or C02.

@ns.route('/test')
class Test(Resource):
    @auth.login_required(role='C01')
    def get(self):
        return {'xx':'yy'}

in function "get_user_roles", I just return a resources number list.

@auth.get_user_roles
def get_user_roles(username):
    roles = user_dao.get_role(username)
    resources =[]
    for role in roles;
        resources.apend(role_dao.get_resources(role ))
    return resources  #will return 'C01#C02'

After these steps, it works well, but this is really not graceful, so, I tried to create this PR.

Tell the truth, I think there may be some better ways, but I haven't found yet, if any, please let me know.

B&R, Skeet

miguelgrinberg commented 3 years ago

But if we allow a user to create a role and customize the role name

I think you have a misunderstanding of what a role is in an API. Users do not create or assign themselves roles, they are given to them. The roles are maintained by the administrator and they decide which parts of the system each user can access.

I think what you are trying to do is a different type of system that is not common in APIs, and you are trying to fit this into the role system of this package, which isn't designed for your purposes. My suggestion is that you create a separate decorator that can work in the way that you need, I don't think the role system in this package is what you want.

skeetwu commented 3 years ago

Hi, thx for your explanation, I'll try it as you suggest.