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

Add privileges/authorization info to the API #438

Closed
wants to merge 1 commit into from

Conversation

wrobelda
Copy link

This adds information about the required privileges to the API pages. Uses the corresponding ACL.xml, if found in the source code.

@wrobelda
Copy link
Author

FYI, DEFAULT_BASE_METHODS need some fixing, they aren't getting their ACL generated. I'll ping once ready.

@wrobelda
Copy link
Author

Done.

@wrobelda wrobelda force-pushed the api_docs_authorization branch 2 times, most recently from a4b7c74 to 106de78 Compare November 22, 2022 15:28
@AdSchellevis
Copy link
Member

It's certainly a good idea to try to add the ACL information, but it's a bit more complicated to merge this into the scripts, ACL's could be part of the core repo as well (and plugins are allowed to have different models). I'll try to take a look as soon as I can find some time.

@wrobelda
Copy link
Author

wrobelda commented Nov 22, 2022

It's certainly a good idea to try to add the ACL information, but it's a bit more complicated to merge this into the scripts, ACL's could be part of the core repo as well (and plugins are allowed to have different models). I'll try to take a look as soon as I can find some time.

I suppose adding this, even if incomplete, can't harm for the time being? And while plugins can have different models, correct me if I am wrong, but the collect_api_endpoints.py assumes one specific structure, which I am building upon:

    controller = re.sub('(?<!^)(?=[A-Z])', '_', os.path.basename(base_filename.split('Controller.php')[0])).lower()
    module_name = src_filename.replace('\\', '/').split('/')[-3].lower()

(...)
        app_location = "/".join(src_filename.split('/')[:-5])
        model_xml = "%s/models/%s.xml" % (app_location, m[0].replace("\\", "/"))
        if os.path.isfile(model_xml):
            model_filename = model_xml.replace('//', '/')

Or am I missing something?

@AdSchellevis
Copy link
Member

The script certainly won't capture everything, but when it comes to ACL's and menu registrations the reality is a bit more complicated (see for example the ACL's in https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/models/OPNsense/Core/ACL/ACL.xml). The same endpoint may also exist in multiple ACL's. I haven't looked in depth to your code yet, but before we are adding this, we should at least make sure this works for 90% of core's MVC features to prevent future discussions.

So, as soon as I do have some time available, I'll take a look. By the way, the user/group manager also shows which endpoints a role (privilege) contains.
image

AdSchellevis added a commit to opnsense/core that referenced this pull request Dec 18, 2022
…e sure we can search them as well. (different solution for opnsense/docs#438)
fichtner pushed a commit to opnsense/core that referenced this pull request Dec 20, 2022
…e sure we can search them as well. (different solution for opnsense/docs#438)

(cherry picked from commit 6d6b52e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants