Paths

Table of Contentst

-
keyserver/src/responders/
-
src/
-
responders/
1/3
user-responders.js
-
lib/types/
-
types/
1/3
account-types.js
1/3
crypto-types.js

[keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder`
ClosedPublic
Actions

Authored by atul on Feb 25 2023, 1:18 AM.

Details

Reviewers

ashoat
tomek

Commits

rCOMM76c64df43f92: [keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder`

Summary

Pass primaryIdentityPublicKeys.ed25519, signedIdentityKeysBlob.payload and signedIdentityKeysBlob.signature to olmUtil.ed25519_verify(...) to ensure that the signature is valid.

Depends on D6896

Test Plan

I tested with a valid signature and observed that ed25519_verify(...) succeeded. I tested with an invalid signature ("blah"), and observed that olmUtil.ed25519(...) threw an exception.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Feb 25 2023, 1:18 AM

atul added inline comments.Feb 25 2023, 1:31 AM

keyserver/src/responders/user-responders.js
345 ↗	(On Diff #23097)	Need to move this to when `keyserver` starts up (IIRC `olm.init()` loads the `wasm`?)
346 ↗	(On Diff #23097)	And we should instantiate this object once and make it accessible "globally."

Harbormaster completed remote builds in B16906: Diff 23097.Feb 25 2023, 1:34 AM

atul requested review of this revision.Feb 25 2023, 1:34 AM

Nice! After this verification step, we should then JSON.parse(signedIdentityKeysBlob.payload) and run the result through an input validator. Then we'll be ready to store them in the DB. (Assume that's for a later diff, though.)

Requesting changes for this:

The reason this diff is titled [DRAFT] is because of the way we're calling olm.init() and new olm.Utility() every time logInResponder encounters a request with signedIdentityKeysBlob. Instead we'll want to probably run olm.init() once when keyserver starts up, and we'll want to instantiate olm.Utility() once and make it accessible globally.

We could probably just have a function getOlmUtility that returns a Promise that does lines 345 and 346 here, and caches the result.

This revision now requires changes to proceed.Feb 26 2023, 5:49 AM

We could probably just have a function getOlmUtility that returns a Promise that does lines 345 and 346 here, and caches the result.

I'll need to make sure there that await olm.init(); is "idempotent" and there aren't any issues with calling it multiple times. Just want to make sure in the case where the keyserver gets multiple siwe_auth requests at the same time that calling await olm.init() simultaneously/multiple times doesn't break anything.

The reason this diff is titled [DRAFT] is because of the way we're calling olm.init() and new olm.Utility() every time logInResponder encounters a request with signedIdentityKeysBlob. Instead we'll want to probably run olm.init() once when keyserver starts up, and we'll want to instantiate olm.Utility() once and make it accessible globally.

Doing this in separate diff (think it is "separate unit of work" and should be reviewed separately). I'll link here, but this can be landed without issue.

In D6897#205149, @atul wrote:

We could probably just have a function getOlmUtility that returns a Promise that does lines 345 and 346 here, and caches the result.

I'll need to make sure there that await olm.init(); is "idempotent" and there aren't any issues with calling it multiple times. Just want to make sure in the case where the keyserver gets multiple siwe_auth requests at the same time that calling await olm.init() simultaneously/multiple times doesn't break anything.

Ah yeah I get what you mean. You could address this by storing the promise in some variable, and then checking if the variable is already set with the promise. If so you would return the existing promise, if not you could call olm.init() to get the promise and set it.

The core thing here is to avoid passing the promise to await before doing the caching... that's what introduces the possibility of a race.

this can be landed without issue

Did you test calling await olm.init() multiple times?

In D6897#205161, @ashoat wrote:

this can be landed without issue

Did you test calling await olm.init() multiple times?

Yup:

ashoat accepted this revision.Feb 27 2023, 1:41 PM

This revision is now accepted and ready to land.Feb 27 2023, 1:41 PM

Thought the approach here was cleaner: https://phab.comm.dev/D6901

rebase + land

This revision was landed with ongoing or failed builds.Feb 27 2023, 3:15 PM

Closed by commit rCOMM76c64df43f92: [keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder` (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM76c64df43f92: [keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder`.

Harbormaster completed remote builds in B16963: Diff 23184.Feb 27 2023, 3:26 PM

Revision Contents
Changeset List

Path

Size

keyserver/

src/

responders/

user-responders.js

21 lines

lib/

types/

account-types.js

1 line

crypto-types.js

5 lines

Diff 23185

View Options

keyserver/src/responders/user-responders.js

use actix_web::FromRequest;		use actix_web::FromRequest;
use chrono::Utc;		use chrono::Utc;
use comm_services_lib::{		use comm_services_lib::{
auth::UserIdentity,		auth::{AuthService, AuthorizationCredential},
blob::client::{BlobServiceClient, BlobServiceError},		blob::client::{BlobServiceClient, BlobServiceError},
crypto::aes256,		crypto::aes256,
database,		database,
};		};
use derive_more::{Display, Error, From};		use derive_more::{Display, Error, From};
use std::{		use std::{
collections::HashMap,		collections::HashMap,
future::{ready, Ready},		future::{ready, Future},
		pin::Pin,
sync::Arc,		sync::Arc,
};		};
use tracing::{error, trace};		use tracing::{error, trace, warn};

use crate::{		use crate::{
config::CONFIG,		config::CONFIG,
database::{		database::{
client::{DatabaseClient, ReportsPage},		client::{DatabaseClient, ReportsPage},
item::{ReportContent, ReportItem},		item::{ReportContent, ReportItem},
},		},
email::{config::EmailConfig, ReportEmail},		email::{config::EmailConfig, ReportEmail},
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines	) -> Self {
Self {		Self {
db,		db,
blob_client,		blob_client,
requesting_user_id: None,		requesting_user_id: None,
email_config: email_config.map(Arc::new),		email_config: email_config.map(Arc::new),
}		}
}		}

pub fn authenticated(&self, user: UserIdentity) -> Self {		/// Clones the service with a new auth identity. When the credential is
let user_id = user.user_id.to_string();		/// a service-to-service, the `user_id` is None.
		pub fn with_authentication(&self, token: AuthorizationCredential) -> Self {
		let requesting_user_id = match &token {
		AuthorizationCredential::ServicesToken(_) => None,
		AuthorizationCredential::UserToken(user) => {
		Some(user.user_id.to_string())
		}
		};
Self {		Self {
db: self.db.clone(),		db: self.db.clone(),
email_config: self.email_config.clone(),		email_config: self.email_config.clone(),
blob_client: self.blob_client.with_user_identity(user),		blob_client: self.blob_client.with_authentication(token),
requesting_user_id: Some(user_id),		requesting_user_id,
}		}
}		}

pub async fn save_reports(		pub async fn save_reports(
&self,		&self,
inputs: Vec<ReportInput>,		inputs: Vec<ReportInput>,
) -> ServiceResult<Vec<ReportID>> {		) -> ServiceResult<Vec<ReportID>> {
let mut reports = Vec::with_capacity(inputs.len());		let mut reports = Vec::with_capacity(inputs.len());
▲ Show 20 Lines • Show All 101 Lines • ▼ Show 20 Lines	impl ReportsService {
) -> ServiceResult<ReportsPage> {		) -> ServiceResult<ReportsPage> {
let page = self.db.scan_reports(cursor, page_size).await?;		let page = self.db.scan_reports(cursor, page_size).await?;
Ok(page)		Ok(page)
}		}
}		}

impl FromRequest for ReportsService {		impl FromRequest for ReportsService {
type Error = actix_web::Error;		type Error = actix_web::Error;
type Future = Ready<Result<Self, actix_web::Error>>;		type Future = Pin<Box<dyn Future<Output = Result<Self, actix_web::Error>>>>;

#[inline]		#[inline]
fn from_request(		fn from_request(
req: &actix_web::HttpRequest,		req: &actix_web::HttpRequest,
_payload: &mut actix_web::dev::Payload,		_payload: &mut actix_web::dev::Payload,
) -> Self::Future {		) -> Self::Future {
		use actix_web::error::{ErrorForbidden, ErrorInternalServerError};
use actix_web::HttpMessage;		use actix_web::HttpMessage;

let Some(service) = req.app_data::<ReportsService>() else {		let base_service =
		req.app_data::<ReportsService>().cloned().ok_or_else(\|\| {
tracing::error!(		tracing::error!(
"FATAL! Failed to extract ReportsService from actix app_data. \		"FATAL! Failed to extract ReportsService from actix app_data. \
Check HTTP server configuration"		Check HTTP server configuration"
);		);
return ready(Err(actix_web::error::ErrorInternalServerError("Internal server error")));		ErrorInternalServerError("Internal server error")
};		});

let auth_service =		let auth_service =
if let Some(user_identity) = req.extensions().get::<UserIdentity>() {		req.app_data::<AuthService>().cloned().ok_or_else(\|\| {
tracing::trace!("Found user identity. Creating authenticated service");		tracing::error!(
service.authenticated(user_identity.clone())		"FATAL! Failed to extract AuthService from actix app_data. \
} else {		Check HTTP server configuration"
tracing::trace!(
"No user identity found. Leaving unauthenticated service"
);		);
service.clone()		ErrorInternalServerError("Internal server error")
};		});

		let request_auth_value =
		req.extensions().get::<AuthorizationCredential>().cloned();

ready(Ok(auth_service))		Box::pin(async move {
		let auth_service = auth_service?;
		let base_service = base_service?;

		// This is Some for endpoints hidden behind auth validation middleware
		let auth_token = match request_auth_value {
		Some(token @ AuthorizationCredential::UserToken(_)) => token,
		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
		let services_token =
		auth_service.get_services_token().await.map_err(\|err\| {
		error!("Failed to get services token: {err}");
		ErrorInternalServerError("Internal server error")
		})?;
		AuthorizationCredential::ServicesToken(services_token)
		}
		};
		let service = base_service.with_authentication(auth_token);
		Ok(service)
		})
		michalUnsubmitted Not Done Inline Actions Do you think it would make sense to move this into authorization middleware? So you can specify something like .wrap(get_comm_authentication_middleware(AllowedAuth::user())) // or .wrap(get_comm_authentication_middleware(AllowedAuth::unauthorized())) // -> this would insert `AuthorizationCredential` with service token automatically // or .wrap(get_comm_authentication_middleware(AllowedAuth::services().user())) I don't remember exactly what the API for the reports service is, but right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. michal: Do you think it would make sense to move this into authorization middleware? So you can specify…
		bartekAuthorUnsubmitted Done Inline Actions I played with various solutions a bit (spent a little too long on this). Your suggestion makes sense in case of API, and looks like it is doable. However, regarding having protected and unprotected endpoints: right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. This is expected, we don't authenticate endpoints yet because we don't have client token. However, this code works well with your existing auth middleware - endpoints wrapped with it will return 401 even before this code is executed. so in the future, when we have client token, doing the following will work: let auth_middleware = get_comm_authentication_middleware(); // this will require client token and fail if not provided web::scope("/authenticated") .wrap(auth_middleware) .service(...) // this will accept both authenticated and anonymous requests. // for the latter, service-to-service token will be obtained web::scope("/anonymous") // .wrap(auth_middleware) // no wrap .service(...) To conclude, I might refactor to look more as you suggested, but it makes more sense to do it later when we have client token available (2nd part of my task that is currently blocked). I can create a follow up task for this if you wish bartek: I played with various solutions a bit (spent a little too long on this). Your suggestion makes…
		michalUnsubmitted Not Done Inline Actions Makes sense, I got confused a bit michal: Makes sense, I got confused a bit
}		}
}		}

struct ProcessedReport {		struct ProcessedReport {
id: ReportID,		id: ReportID,
db_item: ReportItem,		db_item: ReportItem,
email: ReportEmail,		email: ReportEmail,
}		}
▲ Show 20 Lines • Show All 78 Lines • Show Last 20 Lines

View Options

lib/types/account-types.js

use actix_web::FromRequest;		use actix_web::FromRequest;
use chrono::Utc;		use chrono::Utc;
use comm_services_lib::{		use comm_services_lib::{
auth::UserIdentity,		auth::{AuthService, AuthorizationCredential},
blob::client::{BlobServiceClient, BlobServiceError},		blob::client::{BlobServiceClient, BlobServiceError},
crypto::aes256,		crypto::aes256,
database,		database,
};		};
use derive_more::{Display, Error, From};		use derive_more::{Display, Error, From};
use std::{		use std::{
collections::HashMap,		collections::HashMap,
future::{ready, Ready},		future::{ready, Future},
		pin::Pin,
sync::Arc,		sync::Arc,
};		};
use tracing::{error, trace};		use tracing::{error, trace, warn};

use crate::{		use crate::{
config::CONFIG,		config::CONFIG,
database::{		database::{
client::{DatabaseClient, ReportsPage},		client::{DatabaseClient, ReportsPage},
item::{ReportContent, ReportItem},		item::{ReportContent, ReportItem},
},		},
email::{config::EmailConfig, ReportEmail},		email::{config::EmailConfig, ReportEmail},
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines	) -> Self {
Self {		Self {
db,		db,
blob_client,		blob_client,
requesting_user_id: None,		requesting_user_id: None,
email_config: email_config.map(Arc::new),		email_config: email_config.map(Arc::new),
}		}
}		}

pub fn authenticated(&self, user: UserIdentity) -> Self {		/// Clones the service with a new auth identity. When the credential is
let user_id = user.user_id.to_string();		/// a service-to-service, the `user_id` is None.
		pub fn with_authentication(&self, token: AuthorizationCredential) -> Self {
		let requesting_user_id = match &token {
		AuthorizationCredential::ServicesToken(_) => None,
		AuthorizationCredential::UserToken(user) => {
		Some(user.user_id.to_string())
		}
		};
Self {		Self {
db: self.db.clone(),		db: self.db.clone(),
email_config: self.email_config.clone(),		email_config: self.email_config.clone(),
blob_client: self.blob_client.with_user_identity(user),		blob_client: self.blob_client.with_authentication(token),
requesting_user_id: Some(user_id),		requesting_user_id,
}		}
}		}

pub async fn save_reports(		pub async fn save_reports(
&self,		&self,
inputs: Vec<ReportInput>,		inputs: Vec<ReportInput>,
) -> ServiceResult<Vec<ReportID>> {		) -> ServiceResult<Vec<ReportID>> {
let mut reports = Vec::with_capacity(inputs.len());		let mut reports = Vec::with_capacity(inputs.len());
▲ Show 20 Lines • Show All 101 Lines • ▼ Show 20 Lines	impl ReportsService {
) -> ServiceResult<ReportsPage> {		) -> ServiceResult<ReportsPage> {
let page = self.db.scan_reports(cursor, page_size).await?;		let page = self.db.scan_reports(cursor, page_size).await?;
Ok(page)		Ok(page)
}		}
}		}

impl FromRequest for ReportsService {		impl FromRequest for ReportsService {
type Error = actix_web::Error;		type Error = actix_web::Error;
type Future = Ready<Result<Self, actix_web::Error>>;		type Future = Pin<Box<dyn Future<Output = Result<Self, actix_web::Error>>>>;

#[inline]		#[inline]
fn from_request(		fn from_request(
req: &actix_web::HttpRequest,		req: &actix_web::HttpRequest,
_payload: &mut actix_web::dev::Payload,		_payload: &mut actix_web::dev::Payload,
) -> Self::Future {		) -> Self::Future {
		use actix_web::error::{ErrorForbidden, ErrorInternalServerError};
use actix_web::HttpMessage;		use actix_web::HttpMessage;

let Some(service) = req.app_data::<ReportsService>() else {		let base_service =
		req.app_data::<ReportsService>().cloned().ok_or_else(\|\| {
tracing::error!(		tracing::error!(
"FATAL! Failed to extract ReportsService from actix app_data. \		"FATAL! Failed to extract ReportsService from actix app_data. \
Check HTTP server configuration"		Check HTTP server configuration"
);		);
return ready(Err(actix_web::error::ErrorInternalServerError("Internal server error")));		ErrorInternalServerError("Internal server error")
};		});

let auth_service =		let auth_service =
if let Some(user_identity) = req.extensions().get::<UserIdentity>() {		req.app_data::<AuthService>().cloned().ok_or_else(\|\| {
tracing::trace!("Found user identity. Creating authenticated service");		tracing::error!(
service.authenticated(user_identity.clone())		"FATAL! Failed to extract AuthService from actix app_data. \
} else {		Check HTTP server configuration"
tracing::trace!(
"No user identity found. Leaving unauthenticated service"
);		);
service.clone()		ErrorInternalServerError("Internal server error")
};		});

		let request_auth_value =
		req.extensions().get::<AuthorizationCredential>().cloned();

ready(Ok(auth_service))		Box::pin(async move {
		let auth_service = auth_service?;
		let base_service = base_service?;

		// This is Some for endpoints hidden behind auth validation middleware
		let auth_token = match request_auth_value {
		Some(token @ AuthorizationCredential::UserToken(_)) => token,
		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
		let services_token =
		auth_service.get_services_token().await.map_err(\|err\| {
		error!("Failed to get services token: {err}");
		ErrorInternalServerError("Internal server error")
		})?;
		AuthorizationCredential::ServicesToken(services_token)
		}
		};
		let service = base_service.with_authentication(auth_token);
		Ok(service)
		})
		michalUnsubmitted Not Done Inline Actions Do you think it would make sense to move this into authorization middleware? So you can specify something like .wrap(get_comm_authentication_middleware(AllowedAuth::user())) // or .wrap(get_comm_authentication_middleware(AllowedAuth::unauthorized())) // -> this would insert `AuthorizationCredential` with service token automatically // or .wrap(get_comm_authentication_middleware(AllowedAuth::services().user())) I don't remember exactly what the API for the reports service is, but right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. michal: Do you think it would make sense to move this into authorization middleware? So you can specify…
		bartekAuthorUnsubmitted Done Inline Actions I played with various solutions a bit (spent a little too long on this). Your suggestion makes sense in case of API, and looks like it is doable. However, regarding having protected and unprotected endpoints: right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. This is expected, we don't authenticate endpoints yet because we don't have client token. However, this code works well with your existing auth middleware - endpoints wrapped with it will return 401 even before this code is executed. so in the future, when we have client token, doing the following will work: let auth_middleware = get_comm_authentication_middleware(); // this will require client token and fail if not provided web::scope("/authenticated") .wrap(auth_middleware) .service(...) // this will accept both authenticated and anonymous requests. // for the latter, service-to-service token will be obtained web::scope("/anonymous") // .wrap(auth_middleware) // no wrap .service(...) To conclude, I might refactor to look more as you suggested, but it makes more sense to do it later when we have client token available (2nd part of my task that is currently blocked). I can create a follow up task for this if you wish bartek: I played with various solutions a bit (spent a little too long on this). Your suggestion makes…
		michalUnsubmitted Not Done Inline Actions Makes sense, I got confused a bit michal: Makes sense, I got confused a bit
}		}
}		}

struct ProcessedReport {		struct ProcessedReport {
id: ReportID,		id: ReportID,
db_item: ReportItem,		db_item: ReportItem,
email: ReportEmail,		email: ReportEmail,
}		}
▲ Show 20 Lines • Show All 78 Lines • Show Last 20 Lines

View Options

lib/types/crypto-types.js

use actix_web::FromRequest;		use actix_web::FromRequest;
use chrono::Utc;		use chrono::Utc;
use comm_services_lib::{		use comm_services_lib::{
auth::UserIdentity,		auth::{AuthService, AuthorizationCredential},
blob::client::{BlobServiceClient, BlobServiceError},		blob::client::{BlobServiceClient, BlobServiceError},
crypto::aes256,		crypto::aes256,
database,		database,
};		};
use derive_more::{Display, Error, From};		use derive_more::{Display, Error, From};
use std::{		use std::{
collections::HashMap,		collections::HashMap,
future::{ready, Ready},		future::{ready, Future},
		pin::Pin,
sync::Arc,		sync::Arc,
};		};
use tracing::{error, trace};		use tracing::{error, trace, warn};

use crate::{		use crate::{
config::CONFIG,		config::CONFIG,
database::{		database::{
client::{DatabaseClient, ReportsPage},		client::{DatabaseClient, ReportsPage},
item::{ReportContent, ReportItem},		item::{ReportContent, ReportItem},
},		},
email::{config::EmailConfig, ReportEmail},		email::{config::EmailConfig, ReportEmail},
▲ Show 20 Lines • Show All 42 Lines • ▼ Show 20 Lines	) -> Self {
Self {		Self {
db,		db,
blob_client,		blob_client,
requesting_user_id: None,		requesting_user_id: None,
email_config: email_config.map(Arc::new),		email_config: email_config.map(Arc::new),
}		}
}		}

pub fn authenticated(&self, user: UserIdentity) -> Self {		/// Clones the service with a new auth identity. When the credential is
let user_id = user.user_id.to_string();		/// a service-to-service, the `user_id` is None.
		pub fn with_authentication(&self, token: AuthorizationCredential) -> Self {
		let requesting_user_id = match &token {
		AuthorizationCredential::ServicesToken(_) => None,
		AuthorizationCredential::UserToken(user) => {
		Some(user.user_id.to_string())
		}
		};
Self {		Self {
db: self.db.clone(),		db: self.db.clone(),
email_config: self.email_config.clone(),		email_config: self.email_config.clone(),
blob_client: self.blob_client.with_user_identity(user),		blob_client: self.blob_client.with_authentication(token),
requesting_user_id: Some(user_id),		requesting_user_id,
}		}
}		}

pub async fn save_reports(		pub async fn save_reports(
&self,		&self,
inputs: Vec<ReportInput>,		inputs: Vec<ReportInput>,
) -> ServiceResult<Vec<ReportID>> {		) -> ServiceResult<Vec<ReportID>> {
let mut reports = Vec::with_capacity(inputs.len());		let mut reports = Vec::with_capacity(inputs.len());
▲ Show 20 Lines • Show All 101 Lines • ▼ Show 20 Lines	impl ReportsService {
) -> ServiceResult<ReportsPage> {		) -> ServiceResult<ReportsPage> {
let page = self.db.scan_reports(cursor, page_size).await?;		let page = self.db.scan_reports(cursor, page_size).await?;
Ok(page)		Ok(page)
}		}
}		}

impl FromRequest for ReportsService {		impl FromRequest for ReportsService {
type Error = actix_web::Error;		type Error = actix_web::Error;
type Future = Ready<Result<Self, actix_web::Error>>;		type Future = Pin<Box<dyn Future<Output = Result<Self, actix_web::Error>>>>;

#[inline]		#[inline]
fn from_request(		fn from_request(
req: &actix_web::HttpRequest,		req: &actix_web::HttpRequest,
_payload: &mut actix_web::dev::Payload,		_payload: &mut actix_web::dev::Payload,
) -> Self::Future {		) -> Self::Future {
		use actix_web::error::{ErrorForbidden, ErrorInternalServerError};
use actix_web::HttpMessage;		use actix_web::HttpMessage;

let Some(service) = req.app_data::<ReportsService>() else {		let base_service =
		req.app_data::<ReportsService>().cloned().ok_or_else(\|\| {
tracing::error!(		tracing::error!(
"FATAL! Failed to extract ReportsService from actix app_data. \		"FATAL! Failed to extract ReportsService from actix app_data. \
Check HTTP server configuration"		Check HTTP server configuration"
);		);
return ready(Err(actix_web::error::ErrorInternalServerError("Internal server error")));		ErrorInternalServerError("Internal server error")
};		});

let auth_service =		let auth_service =
if let Some(user_identity) = req.extensions().get::<UserIdentity>() {		req.app_data::<AuthService>().cloned().ok_or_else(\|\| {
tracing::trace!("Found user identity. Creating authenticated service");		tracing::error!(
service.authenticated(user_identity.clone())		"FATAL! Failed to extract AuthService from actix app_data. \
} else {		Check HTTP server configuration"
tracing::trace!(
"No user identity found. Leaving unauthenticated service"
);		);
service.clone()		ErrorInternalServerError("Internal server error")
};		});

		let request_auth_value =
		req.extensions().get::<AuthorizationCredential>().cloned();

ready(Ok(auth_service))		Box::pin(async move {
		let auth_service = auth_service?;
		let base_service = base_service?;

		// This is Some for endpoints hidden behind auth validation middleware
		let auth_token = match request_auth_value {
		Some(token @ AuthorizationCredential::UserToken(_)) => token,
		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
		let services_token =
		auth_service.get_services_token().await.map_err(\|err\| {
		error!("Failed to get services token: {err}");
		ErrorInternalServerError("Internal server error")
		})?;
		AuthorizationCredential::ServicesToken(services_token)
		}
		};
		let service = base_service.with_authentication(auth_token);
		Ok(service)
		})
		michalUnsubmitted Not Done Inline Actions Do you think it would make sense to move this into authorization middleware? So you can specify something like .wrap(get_comm_authentication_middleware(AllowedAuth::user())) // or .wrap(get_comm_authentication_middleware(AllowedAuth::unauthorized())) // -> this would insert `AuthorizationCredential` with service token automatically // or .wrap(get_comm_authentication_middleware(AllowedAuth::services().user())) I don't remember exactly what the API for the reports service is, but right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. michal: Do you think it would make sense to move this into authorization middleware? So you can specify…
		bartekAuthorUnsubmitted Done Inline Actions I played with various solutions a bit (spent a little too long on this). Your suggestion makes sense in case of API, and looks like it is doable. However, regarding having protected and unprotected endpoints: right now this diff would allow everyone to "authenticate" to the reports service by just not providing credentials (and get the service token by default). This should probably be scoped per (group of) endpoints. This is expected, we don't authenticate endpoints yet because we don't have client token. However, this code works well with your existing auth middleware - endpoints wrapped with it will return 401 even before this code is executed. so in the future, when we have client token, doing the following will work: let auth_middleware = get_comm_authentication_middleware(); // this will require client token and fail if not provided web::scope("/authenticated") .wrap(auth_middleware) .service(...) // this will accept both authenticated and anonymous requests. // for the latter, service-to-service token will be obtained web::scope("/anonymous") // .wrap(auth_middleware) // no wrap .service(...) To conclude, I might refactor to look more as you suggested, but it makes more sense to do it later when we have client token available (2nd part of my task that is currently blocked). I can create a follow up task for this if you wish bartek: I played with various solutions a bit (spent a little too long on this). Your suggestion makes…
		michalUnsubmitted Not Done Inline Actions Makes sense, I got confused a bit michal: Makes sense, I got confused a bit
}		}
}		}

struct ProcessedReport {		struct ProcessedReport {
id: ReportID,		id: ReportID,
db_item: ReportItem,		db_item: ReportItem,
email: ReportEmail,		email: ReportEmail,
}		}
▲ Show 20 Lines • Show All 78 Lines • Show Last 20 Lines

[keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder`ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 23185

keyserver/src/responders/user-responders.js

lib/types/account-types.js

lib/types/crypto-types.js

[keyserver] Verify `signedIdentityKeysBlob` signature in `logInResponder`
ClosedPublic
Actions

Revision Contents
Changeset List