Skip to content

Commit

Permalink
Do not limit token permissions based on uesr permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
augustuswm committed Jan 5, 2024
1 parent 060cebb commit 848f5ff
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 45 deletions.
9 changes: 4 additions & 5 deletions rfd-api/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,13 +975,13 @@ impl ApiContext {
&self,
caller: &ApiCaller,
token: NewApiKey<ApiPermission>,
api_user: &ApiUser<ApiPermission>,
api_user_id: &Uuid,
) -> ResourceResult<UserToken, StoreError> {
if caller.any(&[
&ApiPermission::CreateApiUserToken(api_user.id),
&ApiPermission::CreateApiUserToken(*api_user_id),
&ApiPermission::CreateApiUserTokenAll,
]) {
ApiKeyStore::upsert(&*self.storage, token, api_user)
ApiKeyStore::upsert(&*self.storage, token)
.await
.to_resource_result()
} else {
Expand Down Expand Up @@ -2097,12 +2097,11 @@ pub(crate) mod test_mocks {
async fn upsert(
&self,
token: NewApiKey<ApiPermission>,
api_user: &rfd_model::ApiUser<ApiPermission>,
) -> Result<ApiKey<ApiPermission>, rfd_model::storage::StoreError> {
self.api_user_token_store
.as_ref()
.unwrap()
.upsert(token, api_user)
.upsert(token)
.await
}

Expand Down
9 changes: 4 additions & 5 deletions rfd-api/src/endpoints/api_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ async fn create_api_user_token_op(
let api_user = ctx.get_api_user(caller, &path.identifier).await?;

let key_id = Uuid::new_v4();

let key = RawApiKey::generate::<24>(&key_id)
.sign(&*ctx.secrets.signer)
.await
Expand All @@ -299,7 +298,7 @@ async fn create_api_user_token_op(
permissions: into_permissions(body.permissions),
expires_at: body.expires_at,
},
&api_user,
&api_user.id,
)
.await?;

Expand Down Expand Up @@ -916,11 +915,11 @@ mod tests {
let mut token_store = MockApiKeyStore::new();
token_store
.expect_upsert()
.withf(move |_, user| user.id == api_user_id)
.returning(|key, user| {
// .withf(move |_, user| user.id == api_user_id)
.returning(move |key| {
Ok(ApiKey {
id: Uuid::new_v4(),
api_user_id: user.id,
api_user_id: api_user_id,
key_signature: key.key_signature,
permissions: key.permissions,
expires_at: key.expires_at,
Expand Down
6 changes: 1 addition & 5 deletions rfd-model/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,7 @@ pub trait ApiKeyStore<T: Permission + Ord> {
filter: ApiKeyFilter,
pagination: &ListPagination,
) -> Result<Vec<ApiKey<T>>, StoreError>;
async fn upsert(
&self,
token: NewApiKey<T>,
api_user: &ApiUser<T>,
) -> Result<ApiKey<T>, StoreError>;
async fn upsert(&self, token: NewApiKey<T>) -> Result<ApiKey<T>, StoreError>;
async fn delete(&self, id: &Uuid) -> Result<Option<ApiKey<T>>, StoreError>;
}

Expand Down
27 changes: 3 additions & 24 deletions rfd-model/src/storage/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{
OAuthClientRedirectUriModel, OAuthClientSecretModel, RfdModel, RfdPdfModel,
RfdRevisionModel,
},
permissions::{Permission, Permissions},
permissions::Permission,
schema::{
access_groups, api_key, api_user, api_user_access_token, api_user_provider, job,
link_request, login_attempt, mapper, oauth_client, oauth_client_redirect_uri,
Expand Down Expand Up @@ -726,35 +726,14 @@ where
.collect())
}

async fn upsert(
&self,
key: NewApiKey<T>,
api_user: &ApiUser<T>,
) -> Result<ApiKey<T>, StoreError> {
// Validate the the token permissions are a subset of the users permissions
let permissions: Permissions<T> = key
.permissions
.iter()
.filter(|permission| {
let can = api_user.permissions.can(permission);

if !can {
tracing::info!(user = ?api_user.id, ?permission, "Attempted to create API token with excess permissions");
}

can
})
.cloned()
.collect::<BTreeSet<T>>()
.into();

async fn upsert(&self, key: NewApiKey<T>) -> Result<ApiKey<T>, StoreError> {
let key_m: ApiKeyModel<T> = insert_into(api_key::dsl::api_key)
.values((
api_key::id.eq(key.id),
api_key::api_user_id.eq(key.api_user_id),
api_key::key_signature.eq(key.key_signature.clone()),
api_key::expires_at.eq(key.expires_at),
api_key::permissions.eq(permissions),
api_key::permissions.eq(key.permissions),
))
.get_result_async(&*self.pool.get().await?)
.await?;
Expand Down
9 changes: 3 additions & 6 deletions rfd-model/tests/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ async fn test_api_user() {
.can(&TestPermission::DeleteApiKey(api_user_id).into()));

// 5. Create an API token for the user
let token = ApiKeyStore::upsert(
let token = ApiKeyStore::<TestPermission>::upsert(
&store,
NewApiKey {
id: Uuid::new_v4(),
Expand All @@ -194,7 +194,6 @@ async fn test_api_user() {
permissions: vec![TestPermission::GetApiKey(api_user_id).into()].into(),
expires_at: Utc::now() + Duration::seconds(5 * 60),
},
&api_user,
)
.await
.unwrap();
Expand All @@ -213,17 +212,16 @@ async fn test_api_user() {
.into(),
expires_at: Utc::now() + Duration::seconds(5 * 60),
},
&api_user,
)
.await
.unwrap();

assert!(!excess_token
assert!(excess_token
.permissions
.can(&TestPermission::CreateApiUser.into()));

// 7. Create an API token with excess permissions for the user
let expired_token = ApiKeyStore::upsert(
let expired_token = ApiKeyStore::<TestPermission>::upsert(
&store,
NewApiKey {
id: Uuid::new_v4(),
Expand All @@ -236,7 +234,6 @@ async fn test_api_user() {
.into(),
expires_at: Utc::now() - Duration::seconds(5 * 60),
},
&api_user,
)
.await
.unwrap();
Expand Down

0 comments on commit 848f5ff

Please sign in to comment.