pi-hole / FTL

The Pi-hole FTL engine
https://pi-hole.net
Other
1.36k stars 194 forks source link

v6: documentation on PUT /clients/ is not complete #1943

Closed jojost1 closed 4 months ago

jojost1 commented 7 months ago

Versions

Platform

Expected behavior

The API states the only fields are 'comment' and 'groups'. However, it looks like the web interface also sends a field 'client' (unclear to me what this is) and 'enabled', like this:

{"client":"00360032003a00390043003a00310034003a00450033003a00310035003a00330045","groups":[1],"comment":"Test Device","enabled":false}

Actual behavior / bug

Without a correct client field, it looks like Pi-hole creates a new client instead of updating an existing one. It's also unclear what the 'enabled' field means that the web interface sends (could just be a bug?).

Furthermore, the GET /clients endpoint also returns a 'name' property for a client which is undocumented as well. I'm not sure if this has to be send for PUT requests as well.

Additional context

This issue is about the new v6 API.

jojost1 commented 7 months ago

Additionally I found out that a call to delete a client is also not working for me (while e.g. deleting a group works fine). It's throwing a 404.

The web interface seems to use /clients:batchDelete always, so I can't check what's going on.

github-actions[bot] commented 6 months ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

jojost1 commented 6 months ago

The issue is still there.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 30 days with no activity. Please comment or update this issue or it will be closed in 5 days.

jojost1 commented 5 months ago

I still have the issue

DL6ER commented 5 months ago

Sorry for this to have gone unnoticed for so long, I moved it into the correct repository now.

extra client field (PUT)

This is a left-over from API development, it isn't needed.

I'm not sure what you mean by

without the correct client field, it looks like Pi-hole creates a new client instead of updating an existing one.

The client to be modified has to be specified through the URI (and needs to be URI-encoded if that matters, e.g. for MAC addresses or IPv6 addresses : -> %3A )

A request may look like:

PUT https://pi.hole/api/clients/00%3A11%3A22%3A44%3A55%3A66

{"groups":[0],"comment":"ABC"}

which will edit the client 00:11:22:33:44:55 if it exists. I just confirmed this indeed works as expected and does not create a new client.

extra enabled field (PUT)

This is a copy-paste remnant from development as well and entirely ignored by the /clients endpoint

extra name field (GET)

This is a read-only field transmitted by the API in case it knows the host name connected too this IP/MAC address. It does not have to be sent in POST/PUT but is automatically populated by DNS data and contained in GET.

DELETE not working

Please check if you are correctly encoding the client (as above in no. 1), I can delete clients in a local test just fine.

Related changes

https://github.com/pi-hole/FTL/pull/1944

https://github.com/pi-hole/web/pull/3016

DL6ER commented 4 months ago

The point you've mentioned have been fixed in the code. Feel free to reopen/comment if you find something we've talked about here not having been fixed.

jojost1 commented 4 months ago

Got it working, thanks for the reply!