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
IDS: API 'settings/get' response incomplete #7094
Comments
It's more likely other When trying to build ACL's for partial applications returning all content on get() is also a (minor) security concern, although these situations currently don't exist (and should safeguard the setter as well). We could consider adding an endpoint specifically for integration purposes at some point. |
@AdSchellevis Thank you for the quick response and important information. Specific endpoints for 'external' integrations might make sense - at least for pulling all entries of one kind. Example:
This totally makes sense for the UI - as the row-view only needs very limited information and the 'detail' view needs them all. |
@ansibleguy Well, the endpoints are not being deprecated at all, but contents may change (as with all api's in all products). The intend of this specific endpoint might have been misunderstood, which I can understand, but realistically this issue isn't really solvable from the documentation. Often our ticket system helps in finding some longer standing issues which are not pressing, but we expect to change at some point. Please keep in mind our API's are developed for our As the code behind all of this is quite consistent, I don't mind adding one endpoint in the base class which does expose the "raw" model (which is what If your product actively uses the current |
@AdSchellevis Thank you for the response.
I understand. The current implementation of my 'product' uses the I get the point that 'access' to the Example:
Just brainstorming - I'm not sure what design would make more sense. |
The "context aware" ones will be difficult to implement in a generic way other than what's already available, in most cases you can just use the implemented When seeking a way to fetch all items from a model (with possibly more containers inside), the current |
The only problem I got with the Just wanted to inform you that this is not optimal from an 'outside perspective'. Could you tell me when it's planned that the |
nothing is planned yet, so we do have time to look for alternatives which works for both. Maybe it's also an option to change the default behavior of the |
@ansibleguy returning all fields for a search endpoint is likely easy to change for most of them, as they use https://x.x.x.x/api/ids/settings/searchPolicy To install using opnsense-patch, use:
If this helps you out, I don't mind changing the endpoints in core to return all fields available. |
Thank you - I'll try it out 👍🏼 |
Sorry for the delay.. Your patch looks promising. If it's that easy to implement this on your side, in my opinion - it would make sense to implement it. This could immensely simplify the API usage for external integrations. For the record: Before: After applying The only drawback I could think of, is that each client will have to fetch & process (a little) more data on the OPNSense WebUI table views. This could be noticeable if the table entry-count is set to a few hundred. I tested it with |
This should be in 24.1.6 already. Close? |
@ansibleguy in some cases returning all might not be the best idea, but in most cases the extra fields don't matter that much. Let's assess this per case, if you miss data on a controller, just open a ticket for us to remove the static field choices. @fichtner let's close indeed. |
Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
Describe the bug
I am currently working on implementing the IDS API as Ansible modules.
The issue I'm running into is:
In all other API endpoints the
get
(api/ids/settings/get
) returns the data of all sub-endpoints. This is very useful as external integrations need this information for linking config-objects.To Reproduce
Steps to reproduce the behavior:
https://<YOUR-FW>/api/ids/settings/get
https://<YOUR-FW>/api/monit/settings/get
Expected behavior
Examples:
As expected - monit
Response:
IDS
IDS expected
Describe alternatives you considered
I am aware that one COULD implement it the same way the WebUI does.
Example:
searchPolicy
to get a list of UUIDsgetPolicy
FOR EACH ITEM to get the details (maybe even in async)But this can get time-intensive as every HTTP request takes a few hundred MS. This may not sound like much, but it does not scale well.
Also: In my opinion an API should share the same behavior between endpoints.
Environment
OPNsense 23.7.10_1-amd64
FreeBSD 13.2-RELEASE-p7
OpenSSL 1.1.1w
Common KVM processor (6 cores, 6 threads) [PVE]
8GB RAM
The text was updated successfully, but these errors were encountered: