Skip to content

Permission handling additions #199

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

Closed
LBegnaud opened this issue Dec 4, 2019 · 6 comments
Closed

Permission handling additions #199

LBegnaud opened this issue Dec 4, 2019 · 6 comments
Labels
discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. pr There is a PR targeting this issue.
Milestone

Comments

@LBegnaud
Copy link
Contributor

LBegnaud commented Dec 4, 2019

Desired Behavior

Adding onto the discussion in #146 , I would like to include the ability to make the permission_codename search function customizable

Contrast to Current Behavior

currently you must explicitly set all permissions you'd like on a group/user in the yml file. Would like to include the option to change this via the config file (and keep the default as codename=

Changes Required

I can submit a PR with the changes, but essentially, add the option to check for a custom function for each permission entry. I.E in the groups.yml

NetBox - Add:
    permissions:
    - add_
      - codename_startswith

would cause the permissions loop to use Permission.objects.filter(codename_startswith='add_')

Discussion: Benefits and Drawbacks

Preserves the current functionality and introduces the benefit of being able to make a clean groups.yml file that defines some generic groups:

  NetBox - Add:
    permissions:
    - add_
      - codename_startswith

  NetBox - Change:
    permissions:
    - change_
      - codename_startswith

  NetBox - Delete:
    permissions:
    - delete_
      - codename_startswith

  NetBox - View:
    permissions:
    - view_
      - codename_startswith

I haven't started the process of building this out, so i'm unsure if this is the exact syntax, maybe it would just be

  NetBox - View:
    permissions:
    - view_: codename_startswith

but i believe this is possible regardless.

@LBegnaud
Copy link
Contributor Author

LBegnaud commented Dec 5, 2019

decided on using the codename filter function as a key in the permissions section of a group or user yml file. I'm finalizing a PR but basically instead of specifying an optional codename filter function for each codename, The original permissions yaml syntax will assume filter function codename and otherwise will loop through all codnames specified for a codename filter function.

Will submit the PR shortly

@cimnine cimnine added discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. pr There is a PR targeting this issue. labels Dec 10, 2019
@cimnine
Copy link
Collaborator

cimnine commented Dec 10, 2019

Thank you for this idea. I have some reservations about this though, as I explain below. Just to be clear, this is my personal opinion so far, it's not a strong opinion yet and I welcome other opinions.

What I like: This is a very flexible feature and could make handling permissions in the initializer scripts easier.
What I dislike: The proposal is a very technical feature that requires knowledge about the inner workings of Netbox, Django and/or Python. If codename_startswith is the only feature that is relevant anyway, I would rather see us implement an asterisk-style matching:

  • if there is an asterisk at the end of the permission's name (e.g. permission_*), then strip the asterisk and treat the permission name as codename_startswith=permission_
  • else do the regular matching.

On a more general level, I wonder how far we should go with those initializer scripts. They were initially thought to be used to ease local development, i.e. to bootstrap an empty Netbox instance quickly; an instance that does not have many users or other entities and that is eventually destroyed anyway.
And that's also the quality level these scripts have in my opinion. So I wonder how many features we should pack on top of them, or if we should rather try to get those scripts upstream somehow, where they could be properly maintained.

This is merely a brain-dump and as I said initially, I would love other opinions. Also on how and for what people use the initializer scripts.

@LBegnaud
Copy link
Contributor Author

I quite like the idea of simplifying it using asterisks. Should be trivial to determine the codename filter based on placement of the asterisk. I must admit, however, that this PR was stretching my python abilities as is...

Speaking to your general comments, I'm using these catchall permission definitions to keep my group permissions up-to-date through the changing netbox schema, so i suspect that would be useful for others as well. I agree that the rest of the initializers probably aren't terribly useful after an initial creation.

@LBegnaud
Copy link
Contributor Author

...that was much easier than expected. I've updated the PR to utilize codename if the permission doesn't have a wildcard (*) and utilize codename__iregex if it does include the wildcard. Let me know what you think!

@cimnine cimnine mentioned this issue Jan 31, 2020
3 tasks
@cimnine
Copy link
Collaborator

cimnine commented Jan 31, 2020

@LBegnaud can you have a look at #236 ? I've taken your work in #200 and made some minor changes to it, particularly to avoid the use of eval. (Because eval is evil 🙈.) If you agree, I would close your PR when I merge mine and proceed to include this enhancement with the next release.

This was referenced Feb 3, 2020
@cimnine cimnine added this to the 0.22.0 milestone Feb 8, 2020
@cimnine
Copy link
Collaborator

cimnine commented Feb 8, 2020

This feature is included in the just released version 0.22.0.

@cimnine cimnine closed this as completed Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. pr There is a PR targeting this issue.
Projects
None yet
Development

No branches or pull requests

2 participants