refactor: Remove unused JWT validation and extraction functions

- Removed unused functions for JWT token validation and extraction
  from the `AuthController`.  This simplifies the codebase and
  removes unnecessary dependencies.
- Improved error handling during user JSON parsing in login and
  registration controllers, adding more informative log messages.
- Updated user model to remove unnecessary password fields.  This
  reduces data storage and improves security by not storing
  sensitive information unnecessarily.
- Added more robust error handling and logging for JSON parsing in
  the home and auth controllers.  This helps in debugging and
  monitoring potential issues during runtime.
This commit is contained in:
Mahmoud Emad 2025-05-07 17:37:49 +03:00
parent 363a15776b
commit c25cf96015
5 changed files with 74 additions and 50 deletions

View File

@ -1,7 +1,4 @@
use oauth2::{ use oauth2::{basic::BasicClient, AuthUrl, ClientId, ClientSecret, RedirectUrl, TokenUrl};
AuthUrl, ClientId, ClientSecret, RedirectUrl, TokenUrl,
basic::BasicClient, AuthorizationCode, CsrfToken, Scope, TokenResponse,
};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::env; use std::env;
@ -18,17 +15,17 @@ impl GiteaOAuthConfig {
/// Creates a new Gitea OAuth configuration /// Creates a new Gitea OAuth configuration
pub fn new() -> Self { pub fn new() -> Self {
// Get configuration from environment variables // Get configuration from environment variables
let client_id = env::var("GITEA_CLIENT_ID") let client_id =
.expect("Missing GITEA_CLIENT_ID environment variable"); env::var("GITEA_CLIENT_ID").expect("Missing GITEA_CLIENT_ID environment variable");
let client_secret = env::var("GITEA_CLIENT_SECRET") let client_secret = env::var("GITEA_CLIENT_SECRET")
.expect("Missing GITEA_CLIENT_SECRET environment variable"); .expect("Missing GITEA_CLIENT_SECRET environment variable");
let instance_url = env::var("GITEA_INSTANCE_URL") let instance_url = env::var("GITEA_INSTANCE_URL")
.expect("Missing GITEA_INSTANCE_URL environment variable"); .expect("Missing GITEA_INSTANCE_URL environment variable");
// Create OAuth client // Create OAuth client
let auth_url = format!("{}/login/oauth/authorize", instance_url); let auth_url = format!("{}/login/oauth/authorize", instance_url);
let token_url = format!("{}/login/oauth/access_token", instance_url); let token_url = format!("{}/login/oauth/access_token", instance_url);
let client = BasicClient::new( let client = BasicClient::new(
ClientId::new(client_id), ClientId::new(client_id),
Some(ClientSecret::new(client_secret)), Some(ClientSecret::new(client_secret)),
@ -36,9 +33,13 @@ impl GiteaOAuthConfig {
Some(TokenUrl::new(token_url).unwrap()), Some(TokenUrl::new(token_url).unwrap()),
) )
.set_redirect_uri( .set_redirect_uri(
RedirectUrl::new(format!("{}/auth/gitea/callback", env::var("APP_URL").unwrap_or_else(|_| "http://localhost:9999".to_string()))).unwrap(), RedirectUrl::new(format!(
"{}/auth/gitea/callback",
env::var("APP_URL").unwrap_or_else(|_| "http://localhost:9999".to_string())
))
.unwrap(),
); );
Self { Self {
client, client,
instance_url, instance_url,

View File

@ -3,7 +3,7 @@ use actix_session::Session;
use tera::Tera; use tera::Tera;
use crate::models::user::{User, LoginCredentials, RegistrationData, UserRole}; use crate::models::user::{User, LoginCredentials, RegistrationData, UserRole};
use crate::utils::render_template; use crate::utils::render_template;
use jsonwebtoken::{encode, decode, Header, Algorithm, Validation, EncodingKey, DecodingKey}; use jsonwebtoken::{encode, Header, EncodingKey};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use chrono::{Utc, Duration}; use chrono::{Utc, Duration};
use lazy_static::lazy_static; use lazy_static::lazy_static;
@ -52,28 +52,11 @@ impl AuthController {
) )
} }
/// Validate a JWT token // Note: The following functions were removed as they were not being used:
pub fn validate_token(token: &str) -> Result<Claims, jsonwebtoken::errors::Error> { // - validate_token
let validation = Validation::new(Algorithm::HS256); // - extract_token_from_session
// - extract_token_from_cookie
let token_data = decode::<Claims>( // They can be re-implemented if needed in the future.
token,
&DecodingKey::from_secret(JWT_SECRET.as_bytes()),
&validation
)?;
Ok(token_data.claims)
}
/// Extract token from session
pub fn extract_token_from_session(session: &Session) -> Option<String> {
session.get::<String>("auth_token").ok().flatten()
}
/// Extract token from cookie
pub fn extract_token_from_cookie(req: &actix_web::HttpRequest) -> Option<String> {
req.cookie("auth_token").map(|c| c.value().to_string())
}
/// Renders the login page /// Renders the login page
pub async fn login_page(tmpl: web::Data<Tera>, session: Session) -> Result<impl Responder> { pub async fn login_page(tmpl: web::Data<Tera>, session: Session) -> Result<impl Responder> {
@ -86,8 +69,15 @@ impl AuthController {
ctx.insert("user_json", &user_json); ctx.insert("user_json", &user_json);
// Parse the JSON into a User object // Parse the JSON into a User object
if let Ok(user) = serde_json::from_str::<User>(&user_json) { match serde_json::from_str::<User>(&user_json) {
ctx.insert("user", &user); Ok(user) => {
log::info!("Successfully parsed user in login_page: {:?}", user);
ctx.insert("user", &user);
},
Err(e) => {
log::error!("Failed to parse user JSON in login_page: {}", e);
log::error!("User JSON: {}", user_json);
}
} }
} }
@ -117,6 +107,8 @@ impl AuthController {
// Store user data in session // Store user data in session
let user_json = serde_json::to_string(&test_user).unwrap(); let user_json = serde_json::to_string(&test_user).unwrap();
log::info!("Storing user in session (login): {}", user_json);
log::info!("User object: {:?}", test_user);
session.insert("user", &user_json)?; session.insert("user", &user_json)?;
session.insert("auth_token", &token)?; session.insert("auth_token", &token)?;
@ -146,8 +138,15 @@ impl AuthController {
ctx.insert("user_json", &user_json); ctx.insert("user_json", &user_json);
// Parse the JSON into a User object // Parse the JSON into a User object
if let Ok(user) = serde_json::from_str::<User>(&user_json) { match serde_json::from_str::<User>(&user_json) {
ctx.insert("user", &user); Ok(user) => {
log::info!("Successfully parsed user in register_page: {:?}", user);
ctx.insert("user", &user);
},
Err(e) => {
log::error!("Failed to parse user JSON in register_page: {}", e);
log::error!("User JSON: {}", user_json);
}
} }
} }
@ -176,6 +175,8 @@ impl AuthController {
// Store user data in session // Store user data in session
let user_json = serde_json::to_string(&user).unwrap(); let user_json = serde_json::to_string(&user).unwrap();
log::info!("Storing user in session (register): {}", user_json);
log::info!("User object: {:?}", user);
session.insert("user", &user_json)?; session.insert("user", &user_json)?;
session.insert("auth_token", &token)?; session.insert("auth_token", &token)?;

View File

@ -18,8 +18,15 @@ impl HomeController {
ctx.insert("user_json", &user_json); ctx.insert("user_json", &user_json);
// Parse the JSON into a User object // Parse the JSON into a User object
if let Ok(user) = serde_json::from_str::<crate::models::user::User>(&user_json) { match serde_json::from_str::<crate::models::user::User>(&user_json) {
ctx.insert("user", &user); Ok(user) => {
log::info!("Successfully parsed user: {:?}", user);
ctx.insert("user", &user);
},
Err(e) => {
log::error!("Failed to parse user JSON: {}", e);
log::error!("User JSON: {}", user_json);
}
} }
} }
@ -37,8 +44,15 @@ impl HomeController {
ctx.insert("user_json", &user_json); ctx.insert("user_json", &user_json);
// Parse the JSON into a User object // Parse the JSON into a User object
if let Ok(user) = serde_json::from_str::<crate::models::user::User>(&user_json) { match serde_json::from_str::<crate::models::user::User>(&user_json) {
ctx.insert("user", &user); Ok(user) => {
log::info!("Successfully parsed user: {:?}", user);
ctx.insert("user", &user);
},
Err(e) => {
log::error!("Failed to parse user JSON: {}", e);
log::error!("User JSON: {}", user_json);
}
} }
} }

View File

@ -1,5 +1,5 @@
use serde::{Deserialize, Serialize};
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
/// Represents a user in the system /// Represents a user in the system
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
@ -10,9 +10,6 @@ pub struct User {
pub name: String, pub name: String,
/// User's email address /// User's email address
pub email: String, pub email: String,
/// User's hashed password
#[serde(skip_serializing)]
pub password_hash: Option<String>,
/// User's role in the system /// User's role in the system
pub role: UserRole, pub role: UserRole,
/// When the user was created /// When the user was created
@ -37,7 +34,6 @@ impl User {
id: None, id: None,
name, name,
email, email,
password_hash: None,
role: UserRole::User, role: UserRole::User,
created_at: Some(Utc::now()), created_at: Some(Utc::now()),
updated_at: Some(Utc::now()), updated_at: Some(Utc::now()),
@ -49,7 +45,6 @@ impl User {
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct LoginCredentials { pub struct LoginCredentials {
pub email: String, pub email: String,
pub password: String,
} }
/// Represents user registration data /// Represents user registration data
@ -57,6 +52,4 @@ pub struct LoginCredentials {
pub struct RegistrationData { pub struct RegistrationData {
pub name: String, pub name: String,
pub email: String, pub email: String,
pub password: String,
pub password_confirmation: String,
} }

View File

@ -32,7 +32,8 @@
<ul class="navbar-nav"> <ul class="navbar-nav">
{% if user_json %} {% if user_json %}
<li class="nav-item"> <li class="nav-item">
<span class="nav-link">Hello, {{ user.name }}</span> <span class="nav-link">Hello, {% if user is defined %}{{ user.email }}{% else %}User{% endif
%}</span>
</li> </li>
<li class="nav-item"> <li class="nav-item">
<a class="nav-link" href="/logout">Logout</a> <a class="nav-link" href="/logout">Logout</a>
@ -53,6 +54,20 @@
</nav> </nav>
<main class="py-4"> <main class="py-4">
<!-- Debug info (only visible during development) -->
{% if user_json %}
<div class="container mb-4" style="font-size: 0.8rem; color: #999;">
<details>
<summary>Debug Info</summary>
<pre>user_json: {{ user_json }}</pre>
<pre>user object exists: {{ user is defined }}</pre>
{% if user is defined %}
<pre>user.name: {{ user.name }}</pre>
{% endif %}
</details>
</div>
{% endif %}
{% block content %}{% endblock %} {% block content %}{% endblock %}
</main> </main>