This is an automated email from the ASF dual-hosted git repository.
mssun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-teaclave.git
The following commit(s) were added to refs/heads/master by this push:
new f730fe9 refactor(authentication): polish authentication error (#618)
f730fe9 is described below
commit f730fe9c99c03c2879410f01ceaeca48e08c3ecc
Author: Mingshen Sun <[email protected]>
AuthorDate: Fri Feb 11 09:36:54 2022 -0800
refactor(authentication): polish authentication error (#618)
---
services/authentication/enclave/src/api_service.rs | 108 ++++++++++-----------
services/authentication/enclave/src/error.rs | 53 +++++++---
.../authentication/enclave/src/internal_service.rs | 15 +--
tests/scripts/functional_tests.py | 3 +-
4 files changed, 102 insertions(+), 77 deletions(-)
diff --git a/services/authentication/enclave/src/api_service.rs
b/services/authentication/enclave/src/api_service.rs
index 2a00b53..874edc1 100644
--- a/services/authentication/enclave/src/api_service.rs
+++ b/services/authentication/enclave/src/api_service.rs
@@ -15,10 +15,11 @@
// specific language governing permissions and limitations
// under the License.
-use crate::error::TeaclaveAuthenticationApiError;
-use crate::user_db::{DbClient, DbError};
+use crate::error::AuthenticationError;
+use crate::error::AuthenticationServiceError;
+use crate::user_db::DbClient;
use crate::user_info::UserInfo;
-use anyhow::anyhow;
+
use std::prelude::v1::*;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use std::untrusted::time::SystemTimeEx;
@@ -50,15 +51,19 @@ impl TeaclaveAuthenticationApiService {
&self,
id: &str,
token: &str,
- ) -> Result<UserRole, TeaclaveAuthenticationApiError> {
+ ) -> Result<UserRole, AuthenticationError> {
let user: UserInfo = match self.db_client.get_user(&id) {
Ok(value) => value,
- Err(_) => bail!(TeaclaveAuthenticationApiError::PermissionDenied),
+ Err(_) => bail!(AuthenticationError::InvalidUserId),
};
+ if token.is_empty() {
+ bail!(AuthenticationError::InvalidToken);
+ }
+
match user.validate_token(&self.jwt_secret, &token) {
Ok(claims) => Ok(claims.get_role()),
- Err(_) => bail!(TeaclaveAuthenticationApiError::PermissionDenied),
+ Err(_) => bail!(AuthenticationError::IncorrectToken),
}
}
}
@@ -71,12 +76,12 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
@@ -84,27 +89,26 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let request = request.message;
ensure!(
!request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
+ AuthenticationServiceError::InvalidUserId
);
if self.db_client.get_user(&request.id).is_ok() {
- bail!(TeaclaveAuthenticationApiError::InvalidUserId);
+ bail!(AuthenticationServiceError::UserIdExist);
}
let role = UserRole::new(&request.role, &request.attribute);
ensure!(
role != UserRole::Invalid,
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::InvalidRole
);
ensure!(
authorize_user_register(&requester_role, &request),
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::PermissionDenied
);
let new_user = UserInfo::new(&request.id, &request.password, role);
match self.db_client.create_user(&new_user) {
Ok(_) => Ok(UserRegisterResponse {}),
- Err(DbError::UserExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
@@ -115,39 +119,38 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
let request = request.message;
ensure!(
!request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
+ AuthenticationServiceError::InvalidUserId
);
if self.db_client.get_user(&request.id).is_err() {
- bail!(TeaclaveAuthenticationApiError::InvalidUserId);
+ bail!(AuthenticationServiceError::InvalidUserId);
}
let role = UserRole::new(&request.role, &request.attribute);
ensure!(
role != UserRole::Invalid,
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::InvalidRole
);
ensure!(
authorize_user_update(&requester_role, &request),
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::PermissionDenied
);
let updated_user = UserInfo::new(&request.id, &request.password, role);
match self.db_client.update_user(&updated_user) {
Ok(_) => Ok(UserUpdateResponse {}),
- Err(DbError::UserNotExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
@@ -156,28 +159,25 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
request: Request<UserLoginRequest>,
) -> TeaclaveServiceResponseResult<UserLoginResponse> {
let request = request.message;
- ensure!(
- !request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
- );
+ ensure!(!request.id.is_empty(), AuthenticationError::InvalidUserId);
ensure!(
!request.password.is_empty(),
- TeaclaveAuthenticationApiError::InvalidPassword
+ AuthenticationError::InvalidPassword
);
let user = self
.db_client
.get_user(&request.id)
- .map_err(|_| TeaclaveAuthenticationApiError::PermissionDenied)?;
+ .map_err(|_| AuthenticationError::UserIdNotFound)?;
if !user.verify_password(&request.password) {
- bail!(TeaclaveAuthenticationApiError::PermissionDenied)
+ bail!(AuthenticationError::IncorrectPassword)
} else {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
- .map_err(|_|
TeaclaveAuthenticationApiError::ServiceUnavailable)?;
+ .map_err(|e| AuthenticationServiceError::Service(e.into()))?;
let exp = (now + Duration::from_secs(24 * 60 * 60)).as_secs();
match user.get_token(exp, &self.jwt_secret) {
Ok(token) => Ok(UserLoginResponse { token }),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e)),
}
}
}
@@ -189,26 +189,25 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
let request = request.message;
ensure!(
!request.password.is_empty(),
- TeaclaveAuthenticationApiError::InvalidPassword
+ AuthenticationError::InvalidPassword
);
let updated_user = UserInfo::new(&id, &request.password,
requester_role);
match self.db_client.update_user(&updated_user) {
Ok(_) => Ok(UserChangePasswordResponse {}),
- Err(DbError::UserNotExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
@@ -219,28 +218,28 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
let request = request.message;
ensure!(
!request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
+ AuthenticationServiceError::InvalidUserId
);
let user = self
.db_client
.get_user(&request.id)
- .map_err(|_| TeaclaveAuthenticationApiError::InvalidUserId)?;
+ .map_err(|_| AuthenticationServiceError::PermissionDenied)?;
ensure!(
authorize_reset_user_password(&requester_role, &user),
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::PermissionDenied
);
let mut encode_buffer = uuid::Uuid::encode_buffer();
@@ -252,8 +251,7 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
Ok(_) => Ok(ResetUserPasswordResponse {
password: new_password.to_string(),
}),
- Err(DbError::UserNotExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
@@ -264,33 +262,32 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
let request = request.message;
ensure!(
!request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
+ AuthenticationServiceError::InvalidUserId
);
let user = self
.db_client
.get_user(&request.id)
- .map_err(|_| TeaclaveAuthenticationApiError::InvalidUserId)?;
+ .map_err(|_| AuthenticationServiceError::PermissionDenied)?;
ensure!(
authorize_delete_user(&requester_role, &user),
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::PermissionDenied
);
match self.db_client.delete_user(&request.id) {
Ok(_) => Ok(DeleteUserResponse {}),
- Err(DbError::UserNotExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
@@ -301,24 +298,24 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
let id: String = request
.metadata
.get("id")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingUserId)?
.into();
let token: String = request
.metadata
.get("token")
- .ok_or_else(|| anyhow!("Missing credential"))?
+ .ok_or(AuthenticationServiceError::MissingToken)?
.into();
let requester_role = self.validate_user_credential(&id, &token)?;
let request = request.message;
ensure!(
!request.id.is_empty(),
- TeaclaveAuthenticationApiError::InvalidUserId
+ AuthenticationServiceError::InvalidUserId
);
ensure!(
authorize_list_users(&requester_role, &request),
- TeaclaveAuthenticationApiError::InvalidRole
+ AuthenticationServiceError::PermissionDenied
);
let users = match requester_role {
@@ -328,8 +325,7 @@ impl TeaclaveAuthenticationApi for
TeaclaveAuthenticationApiService {
match users {
Ok(ids) => Ok(ListUsersResponse { ids }),
- Err(DbError::UserNotExist) =>
Err(TeaclaveAuthenticationApiError::InvalidUserId.into()),
- Err(_) =>
Err(TeaclaveAuthenticationApiError::ServiceUnavailable.into()),
+ Err(e) => bail!(AuthenticationServiceError::Service(e.into())),
}
}
}
diff --git a/services/authentication/enclave/src/error.rs
b/services/authentication/enclave/src/error.rs
index 27106c3..bb35629 100644
--- a/services/authentication/enclave/src/error.rs
+++ b/services/authentication/enclave/src/error.rs
@@ -21,33 +21,58 @@ use teaclave_types::TeaclaveServiceResponseError;
use thiserror::Error;
#[derive(Error, Debug)]
-pub(crate) enum TeaclaveAuthenticationApiError {
- #[error("permission denied")]
- PermissionDenied,
- #[error("invalid userid")]
+pub(crate) enum AuthenticationError {
+ #[error("invalid user id")]
InvalidUserId,
+ #[error("invalid token")]
+ InvalidToken,
#[error("invalid password")]
InvalidPassword,
- #[error("invalid role")]
- InvalidRole,
- #[error("service unavailable")]
- ServiceUnavailable,
+ #[error("user id not found")]
+ UserIdNotFound,
+ #[error("incorrect password")]
+ IncorrectPassword,
+ #[error("incorrect token")]
+ IncorrectToken,
}
-impl From<TeaclaveAuthenticationApiError> for TeaclaveServiceResponseError {
- fn from(error: TeaclaveAuthenticationApiError) -> Self {
- TeaclaveServiceResponseError::RequestError(error.to_string())
+impl From<AuthenticationError> for AuthenticationServiceError {
+ fn from(error: AuthenticationError) -> Self {
+ AuthenticationServiceError::Authentication(error)
+ }
+}
+
+impl From<AuthenticationError> for TeaclaveServiceResponseError {
+ fn from(error: AuthenticationError) -> Self {
+ TeaclaveServiceResponseError::RequestError(
+ AuthenticationServiceError::from(error).to_string(),
+ )
}
}
#[derive(Error, Debug)]
-pub(crate) enum TeaclaveAuthenticationInternalError {
+pub(crate) enum AuthenticationServiceError {
#[error("permission denied")]
PermissionDenied,
+ #[error("authentication failed")]
+ Authentication(AuthenticationError),
+ #[error("invalid user id")]
+ InvalidUserId,
+ #[error("invalid role")]
+ InvalidRole,
+ #[error("user id exist")]
+ UserIdExist,
+ #[error("service internal error")]
+ Service(#[from] anyhow::Error),
+ #[error("missing user id")]
+ MissingUserId,
+ #[error("missing token")]
+ MissingToken,
}
-impl From<TeaclaveAuthenticationInternalError> for
TeaclaveServiceResponseError {
- fn from(error: TeaclaveAuthenticationInternalError) -> Self {
+impl From<AuthenticationServiceError> for TeaclaveServiceResponseError {
+ fn from(error: AuthenticationServiceError) -> Self {
+ log::debug!("AuthenticationServiceError: {:?}", error);
TeaclaveServiceResponseError::RequestError(error.to_string())
}
}
diff --git a/services/authentication/enclave/src/internal_service.rs
b/services/authentication/enclave/src/internal_service.rs
index 876131a..1f07a10 100644
--- a/services/authentication/enclave/src/internal_service.rs
+++ b/services/authentication/enclave/src/internal_service.rs
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-use crate::error::TeaclaveAuthenticationInternalError;
+use crate::error::AuthenticationError;
use crate::user_db::DbClient;
use crate::user_info::UserInfo;
use std::prelude::v1::*;
@@ -23,7 +23,7 @@ use teaclave_proto::teaclave_authentication_service::{
TeaclaveAuthenticationInternal, UserAuthenticateRequest,
UserAuthenticateResponse,
};
use teaclave_rpc::Request;
-use teaclave_service_enclave_utils::teaclave_service;
+use teaclave_service_enclave_utils::{bail, teaclave_service};
use teaclave_types::TeaclaveServiceResponseResult;
#[teaclave_service(teaclave_authentication_service,
TeaclaveAuthenticationInternal)]
@@ -48,16 +48,19 @@ impl TeaclaveAuthenticationInternal for
TeaclaveAuthenticationInternalService {
request: Request<UserAuthenticateRequest>,
) -> TeaclaveServiceResponseResult<UserAuthenticateResponse> {
let request = request.message;
- if request.credential.id.is_empty() ||
request.credential.token.is_empty() {
- return
Err(TeaclaveAuthenticationInternalError::PermissionDenied.into());
+ if request.credential.id.is_empty() {
+ bail!(AuthenticationError::InvalidUserId);
+ }
+ if request.credential.token.is_empty() {
+ bail!(AuthenticationError::InvalidToken);
}
let user: UserInfo = match
self.db_client.get_user(&request.credential.id) {
Ok(value) => value,
- Err(_) => return
Err(TeaclaveAuthenticationInternalError::PermissionDenied.into()),
+ Err(_) => bail!(AuthenticationError::InvalidUserId),
};
let claims = user
.validate_token(&self.jwt_secret, &request.credential.token)
- .map_err(|_|
TeaclaveAuthenticationInternalError::PermissionDenied)?;
+ .map_err(|_| AuthenticationError::IncorrectToken)?;
Ok(UserAuthenticateResponse { claims })
}
}
diff --git a/tests/scripts/functional_tests.py
b/tests/scripts/functional_tests.py
index 7b06095..b7e248b 100755
--- a/tests/scripts/functional_tests.py
+++ b/tests/scripts/functional_tests.py
@@ -163,7 +163,8 @@ class TestAuthenticationService(unittest.TestCase):
response = read_message(self.socket)
self.assertEqual(
- response, b'{"result":"err","request_error":"permission denied"}')
+ response,
+ b'{"result":"err","request_error":"authentication failed"}')
if __name__ == '__main__':
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
