From b26e283f7df3fbb80ccc364935d2395b0b33252a Mon Sep 17 00:00:00 2001 From: Pierre Hubert Date: Wed, 26 Apr 2023 14:11:44 +0200 Subject: [PATCH] Handle errors cases when retrieving login token & rate limiting --- src/controllers/providers_controller.rs | 49 ++++++++++++++++++++++--- src/data/action_logger.rs | 8 ++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/controllers/providers_controller.rs b/src/controllers/providers_controller.rs index 32302b0..74cd64b 100644 --- a/src/controllers/providers_controller.rs +++ b/src/controllers/providers_controller.rs @@ -4,9 +4,10 @@ use actix::Addr; use actix_web::{web, HttpResponse, Responder}; use askama::Template; -use crate::actors::providers_states_actor; +use crate::actors::bruteforce_actor::BruteForceActor; use crate::actors::providers_states_actor::{ProviderLoginState, ProvidersStatesActor}; -use crate::constants::APP_NAME; +use crate::actors::{bruteforce_actor, providers_states_actor}; +use crate::constants::{APP_NAME, MAX_FAILED_LOGIN_ATTEMPTS}; use crate::controllers::base_controller::{build_fatal_error_page, redirect_user}; use crate::controllers::login_controller::BaseLoginPage; use crate::data::action_logger::{Action, ActionLogger}; @@ -127,6 +128,7 @@ pub async fn finish_login( remote_ip: RemoteIP, providers: web::Data>, states: web::Data>, + bruteforce: web::Data>, query: web::Query, logger: ActionLogger, ) -> impl Responder { @@ -167,6 +169,21 @@ pub async fn finish_login( } }; + // We perform rate limiting before attempting to use authorization code + let failed_attempts = bruteforce + .send(bruteforce_actor::CountFailedAttempt { + ip: remote_ip.into(), + }) + .await + .unwrap(); + + if failed_attempts > MAX_FAILED_LOGIN_ATTEMPTS { + logger.log(Action::ProviderRateLimited); + return HttpResponse::TooManyRequests().body(build_fatal_error_page( + "Too many failed login attempts, please try again later!", + )); + } + // Retrieve provider information & configuration let provider = providers .find_by_id(&state.provider_id) @@ -182,11 +199,33 @@ pub async fn finish_login( } }; - // TODO : rate limiting - // Get access token & user information let token = provider_config.get_token(&provider, &query.code).await; - log::debug!("{:#?}", token); + let token = match token { + Ok(t) => t, + Err(e) => { + log::error!("Failed to retrieve login token! {:?}", e); + + bruteforce + .send(bruteforce_actor::RecordFailedAttempt { + ip: remote_ip.into(), + }) + .await + .unwrap(); + + logger.log(Action::ProviderFailedGetToken { + state: &state, + code: query.code.as_str(), + }); + + return ProviderLoginError::get( + "Failed to retrieve login token from identity provider!", + &state.redirect, + ); + } + }; + + println!("go on {:?}", token); // TODO : check token signature // TODO : check if user is authorized to access application diff --git a/src/data/action_logger.rs b/src/data/action_logger.rs index 2f0a6f6..be0c538 100644 --- a/src/data/action_logger.rs +++ b/src/data/action_logger.rs @@ -7,6 +7,7 @@ use actix_identity::Identity; use actix_web::dev::Payload; use actix_web::{web, Error, FromRequest, HttpRequest}; +use crate::actors::providers_states_actor::ProviderLoginState; use crate::actors::users_actor; use crate::actors::users_actor::{AuthorizedAuthenticationSources, UsersActor}; use crate::data::client::Client; @@ -38,6 +39,11 @@ pub enum Action<'a> { ProviderCBInvalidState { state: &'a str, }, + ProviderRateLimited, + ProviderFailedGetToken { + state: &'a ProviderLoginState, + code: &'a str, + }, Signout, UserNeed2FAOnLogin(&'a User), UserSuccessfullyAuthenticated(&'a User), @@ -108,6 +114,8 @@ impl<'a> Action<'a> { format!("failed provider authentication with message '{message}'"), Action::ProviderCBInvalidState { state } => format!("provided invalid callback state after provider authentication: '{state}'"), + Action::ProviderRateLimited => "could not complete OpenID login because it has reached failed attempts rate limit!".to_string(), + Action::ProviderFailedGetToken {state, code} => format!("could not complete login from provider because the id_token could not be retrieved! (state={:?} code = {code})",state), Action::Signout => "signed out".to_string(), Action::UserNeed2FAOnLogin(user) => { format!(