Implement interactive chat mode with user authentication#33
Implement interactive chat mode with user authentication#33chojuninengu wants to merge 16 commits intoRust-Cameroon:mainfrom
Conversation
…uthentication responses
…n and session management
|
@chojuninengu great job but a quick question; Does this code compile locally? |
|
@Dericko681 , it does compile locally |
|
@chojuninengu Oh great, I just figured out, update the start server command in the description of this PR to |
|
@Dericko681 , thanks for that update |
|
Hello @chojuninengu, great work! However, please add the Cargo.lock file to the .gitignore — it shouldn't be committed to the repository. |
|
@ndefokou , thanks for the remark, i just resolved it |
|
@chojuninengu just to leave a remark that if the pipeline is failing, you want to fix it before requiring a review.
|
509db5f to
c4a9fdc
Compare
c4a9fdc to
8f91f07
Compare
…g to be asynchronous
|
@Christiantyemele , please can you see into this issue, cuz this is the only thing that is holding my for now |
Dericko681
left a comment
There was a problem hiding this comment.
hey @chojuninengu you are doing an amazing job here but you can't modify unrelated files .github/workflows/ci.yml in this same PR. There is an open issue #34 which is debating how to fix the issue with failing CI/CD
| Chat { | ||
| /// Name of the group to chat in | ||
| #[arg(short, long)] | ||
| group: String, | ||
| /// Username to use | ||
| #[arg(short, long)] | ||
| username: String, | ||
| }, |
There was a problem hiding this comment.
i don't understand the function of this field, elaborate. cause from what i try to understand you want creating a join a group to be seperate task that is i create a group with a password then share the password to those i want them to join the group in that case then this your Chat field here is not clear in its naming convenction and the values it holds too. cause i don't see the different between Post and Chat when reading their names
| // First register/login the user | ||
| let register_command = FromClient::Register { | ||
| username: Arc::new(username.clone()), | ||
| password: Arc::new("password".to_string()), // TODO: Add proper password handling |
There was a problem hiding this comment.
if this is too big for you to handle plz just create a ticket in other not to forget
| let join_command = FromClient::Join { | ||
| group_name: Arc::new(group.clone()), |
There was a problem hiding this comment.
what is the essence of creating a group with a password if i don't join using that password, unless it's for future implementation ?
| let command = FromClient::Post { | ||
| group_name: Arc::new(group.clone()), | ||
| message: Arc::new(input.to_string()), | ||
| }; | ||
|
|
||
| utils::send_as_json(&mut socket, &command).await?; | ||
| socket.flush().await?; | ||
| } | ||
|
|
There was a problem hiding this comment.
You see what i was saying here you call Post in Chat but still have a seperate handler for posting so what is the difference in the two ?
| } | ||
| } | ||
|
|
||
| pub async fn register(&self, username: Arc<String>) -> anyhow::Result<User> { |
There was a problem hiding this comment.
i don't see the use of putting username here in an Arc
| let user = User { | ||
| username: username.clone(), | ||
| id: Arc::new(Uuid::new_v4().to_string()), | ||
| }; | ||
|
|
||
| users.insert(username.clone(), user.clone()); | ||
| Ok(user) | ||
| } |
There was a problem hiding this comment.
seriously, you have a hashmap storing key username and value username, id this does makes sense for the use case, what i mean here is there is a better way of doing this, and i have not even seen where you making use of this id
| pub async fn login(&self, username: Arc<String>) -> anyhow::Result<User> { | ||
| let users = self.users.lock().await; |
There was a problem hiding this comment.
For now i understand password verification will be done later
| pub async fn is_active(&self, username: &Arc<String>) -> bool { | ||
| let active_users = self.active_users.lock().await; | ||
| active_users.contains_key(username) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is not used and i can figure out where its even gonna be used, why do want to have users and active_users ?
| user_manager: Arc<UserManager>, | ||
| ) -> anyhow::Result<()> { | ||
| let outbound = Arc::new(Outbound::new(socket.clone())); | ||
| let mut connection = Connection::new(); |
There was a problem hiding this comment.
why are you doing this like why do want to keep track of the user in this way, mean while the user manager does that already
|
@chojuninengu indicate if you need help |



This PR implements an interactive chat mode with user authentication, addressing issue #1.
Changes include:
To test:
cargo run --bin server 127.0.0.1:8080cargo run --bin client 127.0.0.1:8080 chat -g general -u your_username