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

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

Merged
merged 22 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0549f4f
initial commit of users service
jreidinger Mar 26, 2024
5902ba9
use tuple of streams instead of StreamMap
jreidinger Mar 26, 2024
a9222b7
implement routes for first user
jreidinger Mar 26, 2024
547dfb2
add root password routes
jreidinger Mar 26, 2024
20b72e7
add route for ssh key
jreidinger Mar 27, 2024
9987a22
add root route for users to get info
jreidinger Mar 27, 2024
1316384
Add validation router and use it in users
jreidinger Mar 27, 2024
7abc0b9
adapt users js code (WIP)
jreidinger Mar 30, 2024
3689fd4
fix UI and also backend
jreidinger Apr 2, 2024
89cb757
Merge remote-tracking branch 'origin/architecture_2024' into users_2024
jreidinger Apr 2, 2024
52dc81d
another bunch of fixes
jreidinger Apr 3, 2024
205b287
add hints for developing with two machines and debugging hints
jreidinger Apr 3, 2024
7149061
format rust code
jreidinger Apr 3, 2024
56d735e
Apply suggestions from code review
jreidinger Apr 3, 2024
dd4af42
changes from review
jreidinger Apr 3, 2024
a5e4c02
reduce number of events for root user change
jreidinger Apr 4, 2024
f5f95bf
modify routing as agreed. Client part is WIP
jreidinger Apr 4, 2024
50e2073
adapt UI code to new http api
jreidinger Apr 5, 2024
67fdec0
Merge remote-tracking branch 'origin/architecture_2024' into users_2024
jreidinger Apr 5, 2024
eed4f8f
Merge remote-tracking branch 'origin/architecture_2024' into users_2024
jreidinger Apr 5, 2024
5bfad04
fixes from testing
jreidinger Apr 7, 2024
c561a33
Apply suggestions from code review
jreidinger Apr 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions rust/agama-lib/src/proxies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,14 @@ trait Issues {
#[dbus_proxy(property)]
fn all(&self) -> zbus::Result<Vec<(String, String, u32, u32)>>;
}

#[dbus_proxy(interface = "org.opensuse.Agama1.Validation", assume_defaults = true)]
trait Validation {
/// Errors property
#[dbus_proxy(property)]
fn errors(&self) -> zbus::Result<Vec<String>>;

/// Valid property
#[dbus_proxy(property)]
fn valid(&self) -> zbus::Result<bool>;
}
2 changes: 1 addition & 1 deletion rust/agama-lib/src/users.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Implements support for handling the users settings

mod client;
mod proxies;
pub mod proxies;
mod settings;
mod store;

Expand Down
14 changes: 12 additions & 2 deletions rust/agama-lib/src/users/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
use super::proxies::{FirstUser as FirstUserFromDBus, Users1Proxy};
use crate::error::ServiceError;
use agama_settings::{settings::Settings, SettingValue, SettingsError};
use serde::Serialize;
use serde::{Deserialize, Serialize};
use zbus::Connection;

/// Represents the settings for the first user
#[derive(Serialize, Debug, Default)]
#[derive(Serialize, Deserialize, Clone, Debug, Default, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct FirstUser {
/// First user's full name
pub full_name: String,
Expand Down Expand Up @@ -66,6 +67,7 @@ impl Settings for FirstUser {
}

/// D-Bus client for the users service
#[derive(Clone)]
pub struct UsersClient<'a> {
users_proxy: Users1Proxy<'a>,
}
Expand All @@ -91,6 +93,10 @@ impl<'a> UsersClient<'a> {
Ok(self.users_proxy.set_root_password(value, encrypted).await?)
}

pub async fn remove_root_password(&self) -> Result<u32, ServiceError> {
Ok(self.users_proxy.remove_root_password().await?)
}

/// Whether the root password is set or not
pub async fn is_root_password(&self) -> Result<bool, ServiceError> {
Ok(self.users_proxy.root_password_set().await?)
Expand Down Expand Up @@ -121,4 +127,8 @@ impl<'a> UsersClient<'a> {
)
.await
}

pub async fn remove_first_user(&self) -> zbus::Result<bool> {
Ok(self.users_proxy.remove_first_user().await? == 0)
}
}
2 changes: 1 addition & 1 deletion rust/agama-lib/src/users/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct UserSettings {
/// First user settings
///
/// Holds the settings for the first user.
#[derive(Debug, Default, Settings, Serialize, Deserialize)]
#[derive(Clone, Debug, Default, Settings, Serialize, Deserialize, utoipa::ToSchema)]
#[serde(rename_all = "camelCase")]
pub struct FirstUserSettings {
/// First user's full name
Expand Down
1 change: 1 addition & 0 deletions rust/agama-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ pub mod manager;
pub mod network;
pub mod questions;
pub mod software;
pub mod users;
pub mod web;
pub use web::service;
2 changes: 2 additions & 0 deletions rust/agama-server/src/users.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod web;
pub use web::{users_service, users_streams};
253 changes: 253 additions & 0 deletions rust/agama-server/src/users/web.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
//!
//! The module offers two public functions:
//!
//! * `users_service` which returns the Axum service.
//! * `users_stream` which offers an stream that emits the users events coming from D-Bus.

use crate::{
error::Error,
web::{
common::{service_status_router, validation_router},
Event,
},
};
use agama_lib::{
error::ServiceError,
users::{proxies::Users1Proxy, FirstUser, UsersClient},
};
use axum::{
extract::State,
routing::{get, put},
Json, Router,
};
use serde::{Deserialize, Serialize};
use std::pin::Pin;
use tokio_stream::{Stream, StreamExt};

#[derive(Clone)]
struct UsersState<'a> {
users: UsersClient<'a>,
}

/// Returns streams that emits users related events coming from D-Bus.
///
/// It emits the Event::RootPasswordChange, Event::RootSSHKeyChanged and Event::FirstUserChanged events.
///
/// * `connection`: D-Bus connection to listen for events.
pub async fn users_streams(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap, it would be consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

dbus: zbus::Connection,
) -> Result<Vec<(&'static str, Pin<Box<dyn Stream<Item = Event> + Send>>)>, Error> {
const FIRST_USER_ID: &str = "first_user";
const ROOT_PASSWORD_ID: &str = "root_password";
const ROOT_SSHKEY_ID: &str = "root_sshkey";
let result: Vec<(&str, Pin<Box<dyn Stream<Item = Event> + Send>>)> = vec![
(
FIRST_USER_ID,
Box::pin(first_user_changed_stream(dbus.clone()).await?),
),
(
ROOT_PASSWORD_ID,
Box::pin(root_password_changed_stream(dbus.clone()).await?),
),
(
ROOT_SSHKEY_ID,
Box::pin(root_ssh_key_changed_stream(dbus.clone()).await?),
),
];

Ok(result)
}

async fn first_user_changed_stream(
dbus: zbus::Connection,
) -> Result<impl Stream<Item = Event> + Send, Error> {
let proxy = Users1Proxy::new(&dbus).await?;
let stream = proxy
.receive_first_user_changed()
.await
.then(|change| async move {
if let Ok(user) = change.get().await {
let user_struct = FirstUser {
full_name: user.0,
user_name: user.1,
password: user.2,
autologin: user.3,
data: user.4,
};
return Some(Event::FirstUserChanged(user_struct));
}
None
})
.filter_map(|e| e);
Ok(stream)
}

async fn root_password_changed_stream(
dbus: zbus::Connection,
) -> Result<impl Stream<Item = Event> + Send, Error> {
let proxy = Users1Proxy::new(&dbus).await?;
let stream = proxy
.receive_root_password_set_changed()
.await
.then(|change| async move {
if let Ok(is_set) = change.get().await {
return Some(Event::RootPasswordChanged {
password_is_set: is_set,
});
}
None
})
.filter_map(|e| e);
Ok(stream)
}

async fn root_ssh_key_changed_stream(
dbus: zbus::Connection,
) -> Result<impl Stream<Item = Event> + Send, Error> {
let proxy = Users1Proxy::new(&dbus).await?;
let stream = proxy
.receive_root_sshkey_changed()
.await
.then(|change| async move {
if let Ok(key) = change.get().await {
let value = if key.is_empty() { None } else { Some(key) };
return Some(Event::RootSSHKeyChanged { key: value });
}
None
})
.filter_map(|e| e);
Ok(stream)
}

/// Sets up and returns the axum service for the software module.
jreidinger marked this conversation as resolved.
Show resolved Hide resolved
pub async fn users_service(dbus: zbus::Connection) -> Result<Router, ServiceError> {
const DBUS_SERVICE: &str = "org.opensuse.Agama.Manager1";
const DBUS_PATH: &str = "/org/opensuse/Agama/Users1";

let users = UsersClient::new(dbus.clone()).await?;
let state = UsersState { users };
let validation_router = validation_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
let status_router = service_status_router(&dbus, DBUS_SERVICE, DBUS_PATH).await?;
let router = Router::new()
.route("/config", get(get_info))
jreidinger marked this conversation as resolved.
Show resolved Hide resolved
.route("/user", put(set_first_user).delete(remove_first_user))
.route(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ).

"/root_password",
put(set_root_password).delete(remove_root_password),
)
.route(
"/root_sshkey",
put(set_root_sshkey).delete(remove_root_sshkey),
)
.merge(validation_router)
.merge(status_router)
.with_state(state);
Ok(router)
}

/// Removes the first user settings
#[utoipa::path(delete, path = "/users/user", responses(
(status = 200, description = "Removes the first user"),
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn remove_first_user(State(state): State<UsersState<'_>>) -> Result<(), Error> {
state.users.remove_first_user().await?;
Ok(())
}

#[utoipa::path(put, path = "/users/user", responses(
(status = 200, description = "User values are set"),
jreidinger marked this conversation as resolved.
Show resolved Hide resolved
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn set_first_user(
State(state): State<UsersState<'_>>,
Json(config): Json<FirstUser>,
) -> Result<(), Error> {
state.users.set_first_user(&config).await?;
Ok(())
}

#[derive(Clone, Debug, Default, Serialize, Deserialize, utoipa::ToSchema)]
pub struct RootPasswordSettings {
pub value: String,
pub encrypted: bool,
}

#[utoipa::path(delete, path = "/users/root_password", responses(
(status = 200, description = "Removes the root password"),
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn remove_root_password(State(state): State<UsersState<'_>>) -> Result<(), Error> {
state.users.remove_root_password().await?;
Ok(())
}

#[utoipa::path(put, path = "/users/root_password", responses(
(status = 200, description = "Root password is set"),
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn set_root_password(
State(state): State<UsersState<'_>>,
Json(config): Json<RootPasswordSettings>,
) -> Result<(), Error> {
state
.users
.set_root_password(&config.value, config.encrypted)
.await?;
Ok(())
}

#[utoipa::path(delete, path = "/users/root_sshkey", responses(
(status = 200, description = "Removes the root SSH key"),
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn remove_root_sshkey(State(state): State<UsersState<'_>>) -> Result<(), Error> {
state.users.set_root_sshkey("").await?;
Ok(())
}

#[utoipa::path(put, path = "/users/root_sshkey", responses(
(status = 200, description = "Root SSH Key is set"),
(status = 400, description = "The D-Bus service could not perform the action"),
))]
async fn set_root_sshkey(
State(state): State<UsersState<'_>>,
Json(key): Json<String>,
) -> Result<(), Error> {
state.users.set_root_sshkey(key.as_str()).await?;
Ok(())
}

#[derive(Clone, Debug, Default, Serialize, Deserialize, utoipa::ToSchema)]
pub struct RootInfo {
password: bool,
sshkey: Option<String>,
}

#[derive(Clone, Debug, Default, Serialize, Deserialize, utoipa::ToSchema)]
pub struct UsersInfo {
user: Option<FirstUser>,
root: RootInfo,
}

#[utoipa::path(put, path = "/users/config", responses(
(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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let mut result = UsersInfo::default();
let first_user = state.users.first_user().await?;
if first_user.user_name.is_empty() {
result.user = None;
} else {
result.user = Some(first_user);
}
result.root.password = state.users.is_root_password().await?;
let ssh_key = state.users.root_ssh_key().await?;
if ssh_key.is_empty() {
result.root.sshkey = None;
} else {
result.root.sshkey = Some(ssh_key);
}
Ok(Json(result))
}
8 changes: 6 additions & 2 deletions rust/agama-server/src/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::{
network::{web::network_service, NetworkManagerAdapter},
questions::web::{questions_service, questions_stream},
software::web::{software_service, software_stream},
users::web::{users_service, users_streams},
web::common::{issues_stream, progress_stream, service_status_stream},
};
use axum::Router;
Expand Down Expand Up @@ -61,7 +62,8 @@ where
"/network",
network_service(dbus.clone(), network_adapter).await?,
)
.add_service("/questions", questions_service(dbus).await?)
.add_service("/questions", questions_service(dbus.clone()).await?)
.add_service("/users", users_service(dbus.clone()).await?)
.with_config(config)
.build();
Ok(router)
Expand Down Expand Up @@ -105,7 +107,9 @@ async fn run_events_monitor(dbus: zbus::Connection, events: EventsSender) -> Res
)
.await?,
);

for (id, user_stream) in users_streams(dbus.clone()).await? {
stream.insert(id, user_stream);
}
stream.insert("software", software_stream(dbus.clone()).await?);
stream.insert(
"software-status",
Expand Down
Loading
Loading