Adapt the users service to the HTTP/JSON API #1117

jreidinger · 2024-03-26T08:42:18Z

Problem

Users section in architecture_2024 is disabled

Solution

Readd it using http API.

Testing

Tested manually

Only issue found during testing is that some put/delete request does not respond body in this PR and js part complain as it always expect json.

Screenshots

coveralls · 2024-03-27T22:16:06Z

coverage: 71.573% (-0.5%) from 72.103%
when pulling 7abc0b9 on users_2024
into 61853f4 on architecture_2024.

imobachgs

Implementation-wise, it looks good. However, I am not convinced with the API design. But I am not a big fan of users D-Bus API, so it is probably just a consequence.

imobachgs · 2024-04-03T17:54:06Z

rust/agama-server/src/users/web.rs

+/// It emits the Event::RootPasswordChange, Event::RootSSHKeyChanged and Event::FirstUserChanged events.
+///
+/// * `connection`: D-Bus connection to listen for events.
+pub async fn users_streams(


For consistency, we should do the same with other _stream functions. Perhaps we should have a list of small things to fix after the big merge.

yeap, it would be consistent

btw as all our streams return this quite complex result I think it makes sense to create type in crate::web for easier sharing.

rust/agama-server/src/users/web.rs

imobachgs · 2024-04-03T18:13:25Z

rust/agama-server/src/users/web.rs

+    let router = Router::new()
+        .route("/config", get(get_info))
+        .route("/user", put(set_first_user).delete(remove_first_user))
+        .route(


I think that having a single /root resource would be more straightforward (and extensible). Let's say that, tomorrow, we want to add another attribute.

We could have the following routes (just an idea):

GET /root to get the full user.

PUT (or PATCH) /root to update any of the attributes (just specify the one that changes).

DELETE /root/password to delete the password.

DELETE /root/sshkey to delete the SSH key.

Alternatively, we could use PUT to replace the whole resource and PATCH to update only one of the elements.

yeap, that is something I mention when I speak about need to have some guide to have consistent API. Both ways is possible. When talking about single resource I even think about having /users/config and methods get and patch. ( put is problematic as root password get is boolean, but patch needs value and encryption ).

imobachgs · 2024-04-03T18:18:47Z

rust/agama-server/src/users/web.rs

+    (status = 200, description = "Configuration for users including root and the first user", body = UsersInfo),
+    (status = 400, description = "The D-Bus service could not perform the action"),
+))]
+async fn get_info(State(state): State<UsersState<'_>>) -> Result<Json<UsersInfo>, Error> {


Here is where the API looks confusing to me. If I need to update the user, I do call PUT on the /users/user route. But, if I want to get the current user, I need to get the info from /users/config.

yeap, that is why I mention that we can have that /users/config and patch when need to change it. Question is how much it will be consistent with other APIs we have.

web/README.md

imobachgs · 2024-04-03T18:26:07Z

web/README.md

+The first location to check when something does not work is browser console which can give
+some hints. The second location to check for errors or warnings is output of `npm run server`
+where some issues when communicating with backend can be seen. And last but on least is
+journal on backend machine where is logged backend activity `journalctl -b`.


Thanks a lot for documenting this. I would add: busctl monitor --address /run/agama/bus as another source of information.

well, I think that this busctl monitor and other useful tools for dbus observing belongs more to backend documentation. Either service or rust dir. Here I try to focus mostly on debugging issues with web UI.

Yes, sure, but given that you mentioned journalctl too, I think it might be useful.

well, I mention journalctl mainly because you see there if request from web UI lands to backend. If it is correctly send there, then I would continue with backend debugging. But maybe we can link backend debugging document from this one.

Well, I would avoid having different documents about debugging. All the information about that matter should be in a single place.

ok, so lets add it.

imobachgs · 2024-04-03T18:26:57Z

web/src/client/http.js

+   * @param {string} url - Endpoint URL (e.g., "/l10n/config").
+   * @return {Promise<object>} Server response.
+   */
+  async delete(url) {


web/src/client/mixins.js

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

jreidinger · 2024-04-03T19:57:54Z

@imobachgs regarding API I think it would be useful to have call tomorrow. I check current http API we have to see if it is consistent or not.

So far it looks quite good and consistent ( sadly users has some specific methods especially for that root password which makes get/put with different data which looks bad ).
Inconsistent API I found:

in general returning empy json replies in methods that do not require to have response looks kind of strange.
network/system/apply uses put instead of post which I would expect
questions use for answer path put and probably post will fit better ( yes, I know I am author :)

jreidinger · 2024-04-04T11:11:58Z

Just do document result of call:
http API will look like:

/users/root -> get/patch method, where patch allows to set key, password or unset it
/users/initial -> get/put/delete method where put will put whole config for user

and events:

split to two events one when initial user change and one when root user config change

imobachgs

In general it looks good. Just a question: are UsersSection and UsersPage tests working again?

rust/agama-server/src/users/web.rs

web/README.md

imobachgs · 2024-04-08T08:15:21Z

web/README.md

+The first location to check when something does not work is browser console which can give
+some hints. The second location to check for errors or warnings is output of `npm run server`
+where some issues when communicating with backend can be seen. And last but on least is
+journal on backend machine where is logged backend activity `journalctl -b`.


Well, I would avoid having different documents about debugging. All the information about that matter should be in a single place.

imobachgs · 2024-04-08T08:17:04Z

web/src/client/http.js

+      method: "DELETE",
+    });
+
+    return response;


I am not sure whether we should return the at all. At some point we could wrap the standard responses objects with our own information.

In any case, if we want to, we should do that in all methods to be consistent.

well, response is important if we want to check if call succeed or failed ( method ok on response or if response is added as it is optional for non-get calls). What I do not like even on current get is that it returns true/false if response is not json, which is quite bad API. So providing whole response object is more clear for me.

imobachgs · 2024-04-08T08:20:52Z

web/src/client/users.js

-      if (changes.RootPasswordSet) {
+    return this.client.ws.onEvent((event) => {
+      if (event.type === "RootChanged") {
+        const res = {};


What res means?

well, I use it as result of data in event. Maybe I should use to something like changed_data?

Or just "changes"

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

jreidinger · 2024-04-08T09:05:53Z

Good question about tests. It is not enabled, so lets do it and do proper mocking in it

After a few months of work, it is time to merge the `architecture_2024` branch into `master`. It is still a work-in-progress, but all the efforts should be go now against that branch. ## Pull requests * #1061 * #1064 * #1073 * #1074 * #1080 * #1089 * #1091 * #1092 * #1094 * #1095 * #1099 * #1100 * #1102 * #1103 * #1112 * #1114 * #1116 * #1117 * #1119 * #1120 * #1123 * #1126 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1136 * #1139 * #1140 * #1143 * #1146 ## Other commits * 8efa41f * 9e2dec0

Prepare for releasing Agama 8. It includes the following pull requests: * #884 * #886 * #914 * #918 * #956 * #957 * #958 * #959 * #960 * #961 * #962 * #963 * #964 * #965 * #966 * #969 * #970 * #976 * #977 * #978 * #979 * #980 * #981 * #983 * #984 * #985 * #986 * #988 * #991 * #992 * #995 * #996 * #997 * #999 * #1003 * #1004 * #1006 * #1007 * #1008 * #1009 * #1010 * #1011 * #1012 * #1014 * #1015 * #1016 * #1017 * #1020 * #1022 * #1023 * #1024 * #1025 * #1027 * #1028 * #1029 * #1030 * #1031 * #1032 * #1033 * #1034 * #1035 * #1036 * #1038 * #1039 * #1041 * #1042 * #1043 * #1045 * #1046 * #1047 * #1048 * #1052 * #1054 * #1056 * #1057 * #1060 * #1061 * #1062 * #1063 * #1064 * #1066 * #1067 * #1068 * #1069 * #1071 * #1072 * #1073 * #1074 * #1075 * #1079 * #1080 * #1081 * #1082 * #1085 * #1086 * #1087 * #1088 * #1089 * #1090 * #1091 * #1092 * #1093 * #1094 * #1095 * #1096 * #1097 * #1098 * #1099 * #1100 * #1102 * #1103 * #1104 * #1105 * #1106 * #1109 * #1110 * #1111 * #1112 * #1114 * #1116 * #1117 * #1118 * #1119 * #1120 * #1121 * #1122 * #1123 * #1125 * #1126 * #1127 * #1128 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1135 * #1136 * #1138 * #1139 * #1140 * #1141 * #1142 * #1143 * #1144 * #1145 * #1146 * #1147 * #1148 * #1149 * #1151 * #1152 * #1153 * #1154 * #1155 * #1156 * #1157 * #1158 * #1160 * #1161 * #1162 * #1163 * #1164 * #1165 * #1166 * #1167 * #1168 * #1169 * #1170 * #1171 * #1172 * #1173 * #1174 * #1175 * #1177 * #1178 * #1180 * #1181 * #1182 * #1183 * #1184 * #1185 * #1187 * #1188 * #1189 * #1190 * #1191 * #1192 * #1193 * #1194 * #1195 * #1196 * #1198 * #1199 * #1200 * #1201 * #1203 * #1204 * #1205 * #1206 * #1207 * #1208 * #1209 * #1210 * #1211 * #1212 * #1213 * #1214 * #1215 * #1216 * #1217 * #1219 * #1220 * #1221 * #1222 * #1223 * #1224 * #1225 * #1226 * #1227 * #1229

jreidinger added 7 commits March 26, 2024 08:58

initial commit of users service

0549f4f

use tuple of streams instead of StreamMap

5902ba9

implement routes for first user

a9222b7

add root password routes

547dfb2

add route for ssh key

20b72e7

add root route for users to get info

9987a22

Add validation router and use it in users

1316384

jreidinger added 6 commits March 31, 2024 00:05

adapt users js code (WIP)

7abc0b9

fix UI and also backend

3689fd4

Merge remote-tracking branch 'origin/architecture_2024' into users_2024

89cb757

another bunch of fixes

52dc81d

add hints for developing with two machines and debugging hints

205b287

format rust code

7149061

jreidinger marked this pull request as ready for review April 3, 2024 12:37

imobachgs requested changes Apr 3, 2024

View reviewed changes

imobachgs changed the title ~~initial commit of users service~~ Adapt the users service to the HTTP/JSON API Apr 3, 2024

jreidinger and others added 2 commits April 3, 2024 21:32

Apply suggestions from code review

56d735e

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

changes from review

dd4af42

jreidinger added 6 commits April 4, 2024 15:09

reduce number of events for root user change

a5e4c02

modify routing as agreed. Client part is WIP

f5f95bf

adapt UI code to new http api

50e2073

Merge remote-tracking branch 'origin/architecture_2024' into users_2024

67fdec0

Merge remote-tracking branch 'origin/architecture_2024' into users_2024

eed4f8f

fixes from testing

5bfad04

imobachgs reviewed Apr 8, 2024

View reviewed changes

Apply suggestions from code review

c561a33

Co-authored-by: Imobach González Sosa <igonzalezsosa@suse.com>

imobachgs approved these changes Apr 8, 2024

View reviewed changes

jreidinger merged commit 5e1d4b2 into architecture_2024 Apr 8, 2024
3 checks passed

jreidinger deleted the users_2024 branch April 8, 2024 09:30

imobachgs mentioned this pull request May 3, 2024

Merge architecture_2024 branch into master #1177

Merged

imobachgs mentioned this pull request May 17, 2024

Update to release 8 #1232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt the users service to the HTTP/JSON API #1117

Adapt the users service to the HTTP/JSON API #1117

jreidinger commented Mar 26, 2024 •

edited

Loading

coveralls commented Mar 27, 2024 •

edited

Loading

imobachgs left a comment

imobachgs Apr 3, 2024

jreidinger Apr 3, 2024

jreidinger Apr 3, 2024

imobachgs Apr 3, 2024

jreidinger Apr 3, 2024

imobachgs Apr 3, 2024

jreidinger Apr 3, 2024

imobachgs Apr 3, 2024

jreidinger Apr 3, 2024

imobachgs Apr 3, 2024

jreidinger Apr 4, 2024

imobachgs Apr 8, 2024

jreidinger Apr 8, 2024

imobachgs Apr 3, 2024

jreidinger commented Apr 3, 2024

jreidinger commented Apr 4, 2024

imobachgs left a comment

imobachgs Apr 8, 2024

imobachgs Apr 8, 2024

jreidinger Apr 8, 2024

imobachgs Apr 8, 2024

jreidinger Apr 8, 2024

imobachgs Apr 8, 2024 •

edited

Loading

jreidinger commented Apr 8, 2024

Adapt the users service to the HTTP/JSON API #1117

Adapt the users service to the HTTP/JSON API #1117

Conversation

jreidinger commented Mar 26, 2024 • edited Loading

Problem

Solution

Testing

Screenshots

coveralls commented Mar 27, 2024 • edited Loading

imobachgs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreidinger commented Apr 3, 2024

jreidinger commented Apr 4, 2024

imobachgs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imobachgs Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

jreidinger commented Apr 8, 2024

jreidinger commented Mar 26, 2024 •

edited

Loading

coveralls commented Mar 27, 2024 •

edited

Loading

imobachgs Apr 8, 2024 •

edited

Loading