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

[feature] Adding 'groups' to simplify auths #143

Closed
suspistew opened this issue Nov 9, 2019 · 3 comments
Closed

[feature] Adding 'groups' to simplify auths #143

suspistew opened this issue Nov 9, 2019 · 3 comments
Labels
enhancement New feature or request

Comments

@suspistew
Copy link
Contributor

Hey,

While thinking on #142, I thought we could make a change on the way that security configuration is set. It could help to be cleaner I guess even for the basic auth. If you agree, it has to be before starting #142.

What I suggest here is to be able to do this :

kafkahq:
  security:
    groups:
      admin: *
      topic_reader: ['topic/read', 'topic/insert', 'topic/delete', 'topic/config/update'
                          'group/read', 'group/delete']

    basic-auth:
      oldWayUser: # Old way user that must still work 
        password: pass
        roles: # Role for current users
          - topic/read
          - topic/insert
          - topic/delete
          - topic/config/update
          - group/read
          - group/delete
        attributes:
          # Regexp to filter topic available for current user
          topics-filter-regexp: "test.*"
       newWayUser: # new user that will use group instead of listing roles
         password: pass 
         group: topic_reader
         attributes:
           topics-filter-regexp: "test.*"
       newWayUserWithAnExtraRole: # new user that will use group instead of listing roles
         password: pass 
         group: topic_reader
         roles:
           - group/read
         attributes:
           topics-filter-regexp: "test.*"
       newAdminUser: # new user that will use group instead of listing roles
         password: pass 
         group: admin
         attributes:
           topics-filter-regexp: "test.*"

The goal is to simplify the management of the users in the configuration file. Instead of repeating roles every time, we create the possibility to have group of roles and use them.

When creating user details, we will merge roles from selected group if there is one, and those from roles list, if they exists.

What do you think ?

@tchiotludo tchiotludo added the enhancement New feature or request label Nov 9, 2019
@tchiotludo
Copy link
Owner

Totally agree, but to be totally transparent, peoples working on ldap may have started working on this part, since it more than necessary on ldap auth (group are here in ldap).

here is the brief I send us :

kafkahq:
  security:
    basic-auth:
      user:
        password: d74ff0ee8da3b9806b18c877dbf29bbde50b5bd8e4dad7a3a725000feb82e8f1
        # Change basic auth with roles instead of permission (like now)
        groups:
          - topic-reader
          - admin

    # list of groups in kafkaHQ
    groups:
      topic-reader:
        attributes:
           topics-filter-regexp: "test.*"
        roles:       
        - topic/read
      admin:
        roles:
        - topic/read
        - topic/insert
        - topic/delete
        - topic/config/gupdate
        - node/read
        - node/config/update
        - topic/data/read
        - topic/data/insert
        - topic/data/delete
        - group/read
        - group/delete
        - group/offsets/update
        - registry/read
        - registry/insert
        - registry/update
        - registry/delete
        - registry/version/delete
        - user/read
        - connect/read
        - connect/insert
        - connect/update
        - connect/delete
        - connect/state/update

Your version is retro compatible, that be can be a good thing for some people (will have less issue on githib 😆).
But for now, I don't hesitate to do BC, (I'm not 1.0.0 yeah !! 😄), and I really think that the roles per user is always a wrong thing. So go break the compatibility.

One concern is about topic prefix that I don't think before, so I cheat the example below to add this, thanks for pointing me this one.

As I know, the work is already started from other guy, so for #142, you will see who would be the first and I will handle the merge myself I think.

@suspistew
Copy link
Contributor Author

suspistew commented Nov 12, 2019

Hey,

I think we should clarify a bit about topics-filter-regexp.
Can it be at the group level but also at the user level ? I think it makes sense

Example : My group could be set to be a "topic-reader", but each user could have its own business, like one for the "car-" and another user for the "truck-". Each of them will have the same group but a different topics-filter-regexp.

Another thing is that we should also add (I think), an other field to limitate the available clusters.
Ex: My user A could have access to Dev cluster A, but has no interest in any access to cluster B.

For now, if kafkaHQ has only one instance, the user will have access to both of them.

What do you think about this ?

@tchiotludo
Copy link
Owner

For the topic regexp, I'll really prefer to create 2 groups with 2 different topic regexp.
If the user have 2 group, he will have 2 topic regexp (with an OR for the right).
IMO, Centralized the ACL to one entity is always simplest.

For the cluster limitation, the need is here (also the consumer group ⏩ ) 😄
But we can separate in different PR issue, since it was a 2 different use case.
The best will be to implement group first, and add this features after to avoid to many rework.

@ghost ghost mentioned this issue Nov 18, 2019
ghost pushed a commit that referenced this issue Jun 19, 2020
* implementing calendar

* implementing calendar

* finished update and implementation of calendar

* added an import

* removed console.logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants