Page MenuHomePhabricator
Files F3353800

D12507.diff
No OneTemporary
Actions

D12507.diff
View Options

diff --git a/services/reports/src/http/handlers.rs b/services/reports/src/http/handlers.rs
--- a/services/reports/src/http/handlers.rs
+++ b/services/reports/src/http/handlers.rs
@@ -1,5 +1,6 @@
use actix_web::{get, post, web, HttpResponse};
use comm_lib::http::auth::get_comm_authentication_middleware as auth_middleware;
+use comm_lib::http::auth_service::Authenticated;
use http::header;
use serde::Deserialize;
@@ -28,7 +29,7 @@
#[post("")]
async fn post_reports(
payload: web::Json<PostReportsPayload>,
- service: ReportsService,
+ service: Authenticated<ReportsService>,
) -> actix_web::Result<HttpResponse> {
use serde_json::json;
@@ -49,7 +50,7 @@
#[get("", wrap = "auth_middleware()")]
async fn query_reports(
query: web::Query<QueryOptions>,
- service: ReportsService,
+ service: Authenticated<ReportsService>,
) -> actix_web::Result<HttpResponse> {
let QueryOptions { cursor, page_size } = query.into_inner();
let page = service.list_reports(cursor, page_size).await?;
@@ -60,7 +61,7 @@
#[get("/{report_id}", wrap = "auth_middleware()")]
async fn get_single_report(
path: web::Path<String>,
- service: ReportsService,
+ service: Authenticated<ReportsService>,
) -> actix_web::Result<HttpResponse> {
let report_id = path.into_inner();
let report = service
@@ -74,7 +75,7 @@
#[get("/{report_id}/redux-devtools.json", wrap = "auth_middleware()")]
async fn redux_devtools_import(
path: web::Path<String>,
- service: ReportsService,
+ service: Authenticated<ReportsService>,
) -> actix_web::Result<HttpResponse> {
let report_id = path.into_inner();
let devtools_json = service
diff --git a/services/reports/src/http/service.rs b/services/reports/src/http/service.rs
--- a/services/reports/src/http/service.rs
+++ b/services/reports/src/http/service.rs
@@ -1,82 +1,14 @@
-use actix_web::FromRequest;
-use comm_lib::auth::{
- is_csat_verification_disabled, AuthService, AuthorizationCredential,
+use comm_lib::{
+ auth::AuthorizationCredential, http::auth_service::HttpAuthenticatedService,
};
-use std::{future::Future, pin::Pin};
-use tracing::{error, warn};
use crate::service::ReportsService;
-impl FromRequest for ReportsService {
- type Error = actix_web::Error;
- type Future = Pin<Box<dyn Future<Output = Result<Self, actix_web::Error>>>>;
-
- #[inline]
- fn from_request(
- req: &actix_web::HttpRequest,
- payload: &mut actix_web::dev::Payload,
- ) -> Self::Future {
- use actix_web::error::{ErrorForbidden, ErrorInternalServerError};
-
- let base_service =
- req.app_data::<ReportsService>().cloned().ok_or_else(|| {
- tracing::error!(
- "FATAL! Failed to extract ReportsService from actix app_data. \
- Check HTTP server configuration"
- );
- ErrorInternalServerError("Internal server error")
- });
-
- let auth_service = AuthService::from_request(req, payload).into_inner();
- let request_auth_value =
- AuthorizationCredential::from_request(req, payload).into_inner();
-
- Box::pin(async move {
- let auth_service = auth_service?;
- let base_service = base_service?;
-
- let credential = request_auth_value.ok();
-
- // This is Some if the request contains valid Authorization header
- let auth_token = match credential {
- Some(token @ AuthorizationCredential::UserToken(_)) => {
- let token_valid = auth_service
- .verify_auth_credential(&token)
- .await
- .map_err(|err| {
- error!("Failed to verify access token: {err}");
- ErrorInternalServerError("Internal server error")
- })?;
- if token_valid || is_csat_verification_disabled() {
- token
- } else {
- warn!("Posting report with invalid credentials! Defaulting to ServicesToken...");
- get_services_token_credential(&auth_service).await?
- }
- }
- Some(_) => {
- // Reports service shouldn't be called by other services
- warn!("Reports service requires user authorization");
- return Err(ErrorForbidden("Forbidden"));
- }
- None => {
- // Unauthenticated requests get a service-to-service token
- get_services_token_credential(&auth_service).await?
- }
- };
- let service = base_service.with_authentication(auth_token);
- Ok(service)
- })
+impl HttpAuthenticatedService for ReportsService {
+ fn make_authenticated(
+ self,
+ auth_credential: AuthorizationCredential,
+ ) -> Self {
+ self.with_authentication(auth_credential)
}
}
-
-async fn get_services_token_credential(
- auth_service: &AuthService,
-) -> Result<AuthorizationCredential, actix_web::Error> {
- let services_token =
- auth_service.get_services_token().await.map_err(|err| {
- error!("Failed to get services token: {err}");
- actix_web::error::ErrorInternalServerError("Internal server error")
- })?;
- Ok(AuthorizationCredential::ServicesToken(services_token))
-}
diff --git a/shared/comm-lib/src/auth/service.rs b/shared/comm-lib/src/auth/service.rs
--- a/shared/comm-lib/src/auth/service.rs
+++ b/shared/comm-lib/src/auth/service.rs
@@ -2,7 +2,10 @@
use chrono::{DateTime, Duration, Utc};
use grpc_clients::identity::unauthenticated::client as identity_client;
-use super::{AuthorizationCredential, ServicesAuthToken, UserIdentity};
+use super::{
+ is_csat_verification_disabled, AuthorizationCredential, ServicesAuthToken,
+ UserIdentity,
+};
const SECRET_NAME: &str = "servicesToken";
/// duration for which we consider previous token valid
@@ -70,11 +73,14 @@
}
/// Verifies the provided [`AuthorizationCredential`]. Returns `true` if
- /// authentication was successful.
+ /// authentication was successful or CSAT verification is disabled.
pub async fn verify_auth_credential(
&self,
credential: &AuthorizationCredential,
) -> AuthServiceResult<bool> {
+ if is_csat_verification_disabled() {
+ return Ok(true);
+ }
match credential {
AuthorizationCredential::UserToken(user) => {
let UserIdentity {
diff --git a/shared/comm-lib/src/http.rs b/shared/comm-lib/src/http.rs
--- a/shared/comm-lib/src/http.rs
+++ b/shared/comm-lib/src/http.rs
@@ -1,4 +1,5 @@
pub mod auth;
+pub mod auth_service;
pub mod multipart;
use crate::tools::BoxedError;
diff --git a/shared/comm-lib/src/http/auth_service.rs b/shared/comm-lib/src/http/auth_service.rs
new file mode 100644
--- /dev/null
+++ b/shared/comm-lib/src/http/auth_service.rs
@@ -0,0 +1,182 @@
+use std::any::type_name;
+use std::pin::Pin;
+use std::{future::Future, ops::Deref};
+
+use actix_web::HttpRequest;
+use actix_web::{
+ error::{ErrorForbidden, ErrorInternalServerError},
+ web, FromRequest,
+};
+use tracing::{error, warn};
+
+use crate::auth::{AuthService, AuthorizationCredential};
+
+/// Service that can be stored in HTTP server app data: `App::app_data()`
+/// and requires request authentication to work. The app data
+/// should contain the default (unauthenticated) instance of the service.
+/// The service is cloned with each request and fed with authentication token.
+///
+/// Services of this type can be retrieved in request handlers
+/// with the `Authenticated<SomeService>` extractor. See ['Authenticated']
+/// for more details.
+pub trait HttpAuthenticatedService: Clone {
+ /// Supplies base (unauthenticated) service with [`AuthorizationCredential`]
+ /// and returns new authenticated instance.
+ fn make_authenticated(self, auth_credential: AuthorizationCredential)
+ -> Self;
+
+ /// Whether service should accept requests authenticated with
+ /// a service-to-service token (callable by other services).
+ /// If false, service can only be called with `UserIdentity` token.
+ fn accepts_services_token(&self, _req: &HttpRequest) -> bool {
+ false
+ }
+
+ /// If the service should fall back to service-to-service token,
+ /// when user credentials are invalid. Default is `true`.
+ /// If you want to fail requests, prefer hiding endpoints behind
+ /// [`super::auth::get_comm_authentication_middleware()`] instead.
+ fn fallback_to_services_token(&self, _req: &HttpRequest) -> bool {
+ true
+ }
+}
+
+/// Extractor for services that require HTTP authentication to work.
+/// If the endpoint is authenticated, given `UserIdentity` credential is
+/// passed to the service. For unauthenticated endpoints, a service-to-service
+/// token is retrieved.
+///
+/// Note that this does not require making the endpoint authenticated.
+/// It only supplies authorization credential to the wrapped service.
+///
+/// Wrapped service must be specified in HTTP app data (`App::app_data()`),
+/// either directly or via [`web::Data`]. See [`web::Data`] documentation
+/// to decide whether to use it or not.
+///
+/// # Example
+/// ```ignore
+/// pub async fn request_handler(
+/// some_service: Authenticated<SomeService>,
+/// ) -> Result<HttpResponse> {
+/// Ok(HttpResponse::Ok().finish())
+/// }
+/// ```
+#[derive(Clone)]
+pub struct Authenticated<S: HttpAuthenticatedService> {
+ inner: S,
+}
+
+impl<S: HttpAuthenticatedService> Authenticated<S> {
+ /// Retrieves inner authenticated service
+ pub fn into_inner(self) -> S {
+ self.inner
+ }
+}
+
+impl<S: HttpAuthenticatedService> Deref for Authenticated<S> {
+ type Target = S;
+ fn deref(&self) -> &Self::Target {
+ &self.inner
+ }
+}
+
+impl<S: HttpAuthenticatedService> AsRef<S> for Authenticated<S>
+where
+ <Authenticated<S> as Deref>::Target: AsRef<S>,
+{
+ fn as_ref(&self) -> &S {
+ self.deref()
+ }
+}
+
+impl<S: HttpAuthenticatedService + 'static> FromRequest for Authenticated<S> {
+ type Error = actix_web::Error;
+ type Future = Pin<Box<dyn Future<Output = Result<Self, Self::Error>>>>;
+
+ fn from_request(
+ req: &HttpRequest,
+ payload: &mut actix_web::dev::Payload,
+ ) -> Self::Future {
+ let base_service = req
+ .app_data::<S>()
+ .or_else(|| {
+ // fallback to web::Data for compatibility with existing code
+ req.app_data::<web::Data<S>>().map(|it| it.as_ref())
+ })
+ .cloned()
+ .ok_or_else(|| {
+ tracing::error!(
+ "FATAL! Failed to extract `{}` from actix `App::app_data()`. \
+ Check HTTP server configuration!",
+ type_name::<S>()
+ );
+ ErrorInternalServerError("Internal server error")
+ });
+
+ let auth_service = AuthService::from_request(req, payload).into_inner();
+ let request_auth_value =
+ AuthorizationCredential::from_request(req, payload).into_inner();
+
+ let req = req.clone();
+ Box::pin(async move {
+ let auth_service = auth_service?;
+ let base_service = base_service?;
+
+ let credential = request_auth_value.ok();
+
+ // This is Some if the request contains valid Authorization header
+ let auth_token = match credential {
+ Some(token @ AuthorizationCredential::UserToken(_)) => {
+ let token_valid = auth_service
+ .verify_auth_credential(&token)
+ .await
+ .map_err(|err| {
+ error!("Failed to verify access token: {err}");
+ ErrorInternalServerError("Internal server error")
+ })?;
+
+ match token_valid {
+ true => token,
+ false if S::fallback_to_services_token(&base_service, &req) => {
+ warn!(
+ "Got {1} request with invalid credentials! {0}",
+ "Defaulting to ServicesToken...",
+ type_name::<S>()
+ );
+ get_services_token_credential(&auth_service).await?
+ }
+ false => {
+ return Err(ErrorForbidden("invalid credentials"));
+ }
+ }
+ }
+ Some(token @ AuthorizationCredential::ServicesToken(_)) => {
+ if S::accepts_services_token(&base_service, &req) {
+ token
+ } else {
+ // This service shouldn't be called by other services
+ warn!("{} requests requires user authorization", type_name::<S>());
+ return Err(ErrorForbidden("Forbidden"));
+ }
+ }
+ None => {
+ // Unauthenticated requests get a service-to-service token
+ get_services_token_credential(&auth_service).await?
+ }
+ };
+ let service = base_service.make_authenticated(auth_token);
+ Ok(Authenticated { inner: service })
+ })
+ }
+}
+
+async fn get_services_token_credential(
+ auth_service: &AuthService,
+) -> Result<AuthorizationCredential, actix_web::Error> {
+ let services_token =
+ auth_service.get_services_token().await.map_err(|err| {
+ error!("Failed to get services token: {err}");
+ actix_web::error::ErrorInternalServerError("Internal server error")
+ })?;
+ Ok(AuthorizationCredential::ServicesToken(services_token))
+}

File Metadata

Mime Type
text/plain
Expires
Sun, Nov 24, 11:28 AM (21 h, 39 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2575180
Default Alt Text
D12507.diff (12 KB)

Event Timeline