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

adds role based access control #98

Closed
gemerden opened this issue Apr 8, 2020 · 8 comments
Closed

adds role based access control #98

gemerden opened this issue Apr 8, 2020 · 8 comments
Assignees

Comments

@gemerden
Copy link
Contributor

gemerden commented Apr 8, 2020

For my own project i have added a simple way to add role based access control to BasicAuth:

class HTTPRoleAuth(flask_httpauth.HTTPBasicAuth):
    def __init__(self, scheme=None, realm=None):
        super().__init__(scheme, realm)
        self.get_user_roles_callback = None

    def get_user_roles(self, f):
        """ user roles are the roles the user has """
        self.get_user_roles_callback = f
        return f

    def roles_required(self, *endpoint_roles):
        """ endpoint roles are the roles (str) the user has to have to get access to the (decorated) endpoint """
        def decorator(f):
            @wraps(f)
            def decorated(*args, **kwargs):
                """ basically the login_required decorated but with a check of 'authorize' """
                auth = self.get_auth()
                if request.method != 'OPTIONS':  # pragma: no cover
                    password = self.get_auth_password(auth)

                    if not (self.authenticate(auth, password) and
                            self.authorize(auth, endpoint_roles)):
                        request.data  # empty the stream
                        return self.auth_error_callback()

                return f(*args, **kwargs)
            return decorated
        return decorator

    def authorize(self, auth, endpoint_roles):
        """ if any of the roles of the user correspond to the endpoint roles; the user gets access """
        if not auth.username:
            return False
        user_roles = self.get_user_roles_callback(auth.username)
        return any(role in endpoint_roles for role in user_roles)

used as in:

    @route('/try_roles/<data>', methods=['GET'])
    @auth.roles_required("super", "security")
    def try_login_with_roles(self, data):
        """ roles test url """
        return {"success": True, "data": data}

compared to:

    @route('/try_login/<data>', methods=['GET'])
    @auth.login_required
    def try_login(self, data):
        """ login test url """
        return {"success": True, "data": data}

with roles_required a replacement for login_required with the additional requirement that the user has one of the roles (obtained through the get_user_roles callback) set by the decorator.

Is this something to add to the repo?

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Apr 8, 2020

Interesting solution, I like its simplicity.

The areas that I think we'd need to elaborate are:

  • the solution should extend to the other authentication mechanisms (digest, token, multi-auth)
  • I would try to generalize roles, so that they don't necessarily have to be strings. Many people use numeric constants for roles.
  • You have implemented access when any one of the roles listed belongs to the user. It may be good to also have a way to indicate that a list of roles must all be present in the user to gain access to the route.
  • I wonder if this functionality can be incorporated into the login_required decorator instead of building a very similar one for roles. Maybe something like @login_required(roles=[...]) would work.

@gemerden
Copy link
Contributor Author

gemerden commented Apr 9, 2020

Hi Miguel,

Thanks for the interest. I tried to implement your points this morning and i think i covered them all except multi-auth (bit more complex due to the nested login_required call):

  • Implemented @login_required(roles=[...]) , instead of roles_required. @login_required should still work as expected,
  • I changed my class to a mixin to use with basic, digest and token to add role based,
  • added the 'all roles' option (not sure of the use-case, but easy to add),
  • i think the original code already allowed non-string roles (nothing string specific in the code)

I also wrote some minimal tests for basic, digest and token.

You can find my fork/branch at: https://github.com/gemerden/Flask-HTTPAuth/tree/role-based-access-control

@gemerden
Copy link
Contributor Author

gemerden commented Apr 9, 2020

I also added a role based version of MultiAuth: MultiRoleAuth(MultiAuth); to simplify that i factored the selected_auth logic into a separate property of MultiAuth.

Also added basic tests ...

@sovankiry
Copy link

Good feature suggestion! Waiting to implement.

@gemerden
Copy link
Contributor Author

Hi Miguel,

Do you have any ideas of how I should progress to this? I added your initial suggestions and created a pull request.

Cheers, Lars

@miguelgrinberg
Copy link
Owner

Your PR is now merged (with some changes from myself). Once I get around to update the docs I'll publish it to PyPI. Thanks!

@gemerden
Copy link
Contributor Author

gemerden commented Jun 15, 2020

Missed your message. Great, hope to see it soon, so i can use it instead of my own version ;-)

@miguelgrinberg
Copy link
Owner

@gemerden it's been out for a while now. Enjoy it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants