Add dialog for editing camera credentials - #3353
Conversation
There was a problem hiding this comment.
When I thought about implementing this, since most of these values have read-only input fields already, was to just complement the settings fields and in case make the existing ones editable. Of course they would need to be added to the data to transfer to the backend when saving settings, and some vice versa. For passwords, it could have been handled like the user passwords: skip sending it to frontend, instead prefill with ***** and filter that value out when sending back to backend, to leave the password untouched, in case.
Wouldn't this be overall simpler and more consistent than adding a new dialog and API endpoint?
EDIT: I guess, it would then also be generally possible to merge the "add" and "set" APIs, so that the only difference is in the frontend the dialog and connection checks.
|
Umm, I see what you mean. I actually tried that approach first as well, so if you prefer switching to that direction, no problem. It would require some more work to finish cleanly though. I ran into a few difficulties along the way, which is why I switched to this approach. The main reason I ended up with a separate dialog and endpoint is the failed remote motionEye case. When a remote motionEye camera is unavailable, the remote config cannot be obtained, and the UI treats that camera as disabled. Because of that, the normal settings fields are hidden and there is no full settings form available to edit and submit. AFAICS, there also currently is no way to update the local connection config of a remote motionEye camera, (before this PR). |
|
I mean now that you did the work already, I see no problem to merge this for now. I did not look deeper into it, and possible issues, just thought that, if there are inputboxes, everything can be generally send to the backend via common settings save and config
We could unhide the connection and credentials fields in this case, or treat it differently when the remote config cannot be obtained: not treat it as disabled, but an error state that hides all settings fields but those needed to fix the connection. ... but might be another task to handle the local config correctly in the backend, and skip remote sync attempts until connection succeeded. I see that is probably a reason why
Right, this is one of the cases where a new inputbox is needed, and new data field to transfer when saving settings. But I see there is more work to do, to go with the initial idea, and we should probably not unnecessarily delay the v0.44.0 release further (for which I saw this feature mandatory, for remote cameras at least). The new API and dialog look robust, I'll also do a quick test tomorrow. So let's go with this. After the release, we can have another look, make existing read-only fields editable, add new ones, gradually. It does not hurt to have things doubled in between. If/Once everything is in the general settings panel, and accessible whenever needed, and we see no other reason why keeping a dedicated dialog still has benefits, we can drop it again. |
|
Well, thanks for sharing your thoughts. Maybe in hindsight I could have communicated a bit better before starting the work on this, to align the approach with what was expected, although that's not always my strong suit. 😅 I'm just happy I could help bring the new release a little closer. |
|
No problem. It is not that I expected it like this or that, it was just what I had in mind when we found that it is needed to remove and readd remote cameras after the authentication rework, and when I remembered the same issue for netcams + the R/O fields. I still think it would be a more consistent approach, reducing/aligning API endpoints and dialogs, but sometimes things are not that simple, and a new API endpoint + dialog might be just the right pragmatic solution to get things done in reasonable/available time. Many thanks for your work! I'll review/test on live instance tomorrow, and in case also push a new beta right away + draft a release. So we can collect and review once more all changes since v0.43.1. There is still the venv + libcamera topics I want to deal with soon:
But this can be done as v0.44.1, so we get the security-relevant changes out faster. |
MichaIng
left a comment
There was a problem hiding this comment.
Works great!
One note: The dropdown for detected netcams is to select UDP vs TCP, stored in the netcam_params setting, resp. netcam_use_tcp until motion 4.3. Consequently this could be added to the credentials menu + API as well. Maybe the menu could just reuse the list API for UDP/TCP probing, resp. share code of the "add camera" menu as much as possible. Or we just add a toggle to the regular settings panel, following the long-term alignment idea.
But not a blocker/requirement. The main goal to avoid the remove/re-add on credentials change, especially remote motionEye instances with v0.44.0, is addressed.
There was a problem hiding this comment.
Pull request overview
Adds a UI + backend flow to update camera connection details (URL/credentials/remote secret) without going through the full camera settings form, including better handling for remote motionEye cameras that fail to load their configuration.
Changes:
- Adds an “Edit Credentials” action in the camera “Video Device” section and a new modal dialog to submit credential updates.
- Implements a new
POST /config/<camera_id>/credentials/operation to update connection settings for remote motionEye, simple MJPEG, and netcam cameras (with Motion restart for netcam). - Extends remote-camera error responses and camera list entries to surface “connection failed” metadata for the UI.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| motioneye/templates/main.html | Adds the “Edit Credentials” button row in the camera device settings table. |
| motioneye/static/js/main.js | Adds logic to show the credentials dialog, handle remote-connection-failed cameras, and post to the new credentials endpoint. |
| motioneye/static/css/main.css | Styles the new button and dialog table similarly to existing dialogs. |
| motioneye/server.py | Extends the config route regex to accept the new credentials operation. |
| motioneye/handlers/config.py | Adds set_credentials() handler logic and enriches remote failure responses/list entries with connection metadata. |
| motioneye/config.py | Extracts netcam user/pass encoding into a shared helper used by both add-camera and credential updates. |
Add a dedicated dialog and endpoint for updating camera connection information. This lets admins edit netcam URLs and credentials, simple MJPEG URLs, and remote motionEye URLs/secrets.
Co-authored-by: MichaIng <micha@dietpi.com>
When retrieving the URL for the dialog, strip `config/<cameraID>`, as this is re-added, which would cause a duplicate. It is not currently possible to change the remote camera ID with the credentials dialog. Signed-off-by: MichaIng <micha@dietpi.com> Co-authored-by: MichaIng <micha@dietpi.com>
4237568 to
2a70015
Compare
Adds a dedicated dialog for editing camera connection information and credentials, so users no longer need to manually edit configuration files or remove and re-add a camera when a password, URL, or remote secret changes.
Admins can now update:
This adds a new credentials update endpoint:
POST /config/<camera_id>/credentials/. The endpoint updates only camera connection data instead of going through the full camera settings form. It also allows updating the local configuration of a remote motionEye camera when the remote configuration cannot be loaded.For remote motionEye and simple MJPEG cameras, Motion does not need to be restarted for changes to take effect. For network cameras that go through the local Motion process, Motion is restarted after saving so the updated connection settings take effect.
For network cameras, the dialog can update the URL, username, and password. Leaving both username and password blank preserves the current username and password. Username and password encoding is shared with the add-camera flow through a new helper.
For simple MJPEG cameras, the dialog updates the stored URL and recreates the camera frames after saving so the stream immediately uses the new URL.
(Note: the add-camera dialog currently asks for an optional username and password for simple MJPEG cameras, but AFAICS those values are not currently stored for that camera type.)
For remote motionEye cameras, failed connections are shown in the camera selector as
Camera <id> - Connection failed. When a failed camera is selected from the dropdown, the edit dialog opens automatically so the connection details can be corrected. If there are no enabled cameras and the first camera in the list is a failed remote motionEye camera, the edit dialog is shown automatically (after attempting to load the camera configuration) together with the error message. After saving, the camera configuration is refreshed so the remote camera can appear normally again if the connection succeeds.The edit dialog needs the current remote motionEye URL because it is prefilled. When the connection fails, that URL of course cannot be read from the remote config, so failed camera-list entries and failed config responses now include the remote base URL from the local camera config together with the existing error message for the dialog to use.