Skip to content

Commit ef6c79a

Browse files
committed
refactor: dedup team authz queries in permissions
Phase 2 (part 1) — behavior-preserving extraction, covered by the Phase 0 authz db_tests. - Extract effective_permissions(): the bit_or role-union query was copy-pasted in has_team_permission and require_all_team_permissions; both now call it. - Consolidate the team-membership EXISTS check: team_sync.rs had its own is_team_member duplicating permissions::require_team_member's query. Moved a pub is_team_member into permissions (require_team_member now builds on it); team_sync imports it, keeping its per-call-site warn! logs intact. Validated: 21/21 pass (real Postgres + no-DB), clippy -D warnings clean.
1 parent 93e2b17 commit ef6c79a

2 files changed

Lines changed: 29 additions & 45 deletions

File tree

src/permissions.rs

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,14 @@ pub const BUILTIN_ROLES: &[(&str, i64, i32)] = &[
5151
("connect-only", 28676, 4), // no edit perms today
5252
];
5353

54-
/// Returns the union of all role permission bits for (team_id, user_id).
55-
/// Returns false if user has no roles in the team (or is not a member).
56-
pub async fn has_team_permission(
54+
/// Union of all role permission bits granted to (team_id, user_id).
55+
/// Returns 0 if the user has no roles in the team (or is not a member).
56+
async fn effective_permissions(
5757
pool: &PgPool,
5858
team_id: Uuid,
5959
user_id: Uuid,
60-
permission: i64,
61-
) -> Result<bool, StatusCode> {
62-
let effective = sqlx::query_scalar::<_, i64>(
60+
) -> Result<i64, StatusCode> {
61+
sqlx::query_scalar::<_, i64>(
6362
r#"
6463
SELECT COALESCE(bit_or(tr.permissions), 0)
6564
FROM team_member_roles tmr
@@ -74,9 +73,17 @@ pub async fn has_team_permission(
7473
.map_err(|e| {
7574
error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team permission");
7675
StatusCode::INTERNAL_SERVER_ERROR
77-
})?;
76+
})
77+
}
7878

79-
Ok((effective & permission) != 0)
79+
/// Returns true if any of (team_id, user_id)'s roles grant `permission`.
80+
pub async fn has_team_permission(
81+
pool: &PgPool,
82+
team_id: Uuid,
83+
user_id: Uuid,
84+
permission: i64,
85+
) -> Result<bool, StatusCode> {
86+
Ok((effective_permissions(pool, team_id, user_id).await? & permission) != 0)
8087
}
8188

8289
pub async fn require_all_team_permissions(
@@ -85,36 +92,21 @@ pub async fn require_all_team_permissions(
8592
user_id: Uuid,
8693
permissions: &[i64],
8794
) -> Result<(), StatusCode> {
88-
let effective = sqlx::query_scalar::<_, i64>(
89-
r#"
90-
SELECT COALESCE(bit_or(tr.permissions), 0)
91-
FROM team_member_roles tmr
92-
JOIN team_roles tr ON tr.id = tmr.role_id
93-
WHERE tmr.team_id = $1 AND tmr.user_id = $2
94-
"#,
95-
)
96-
.bind(team_id)
97-
.bind(user_id)
98-
.fetch_one(pool)
99-
.await
100-
.map_err(|e| {
101-
error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team permissions");
102-
StatusCode::INTERNAL_SERVER_ERROR
103-
})?;
104-
95+
let effective = effective_permissions(pool, team_id, user_id).await?;
10596
if permissions.iter().all(|p| (effective & *p) != 0) {
10697
Ok(())
10798
} else {
10899
Err(StatusCode::FORBIDDEN)
109100
}
110101
}
111102

112-
pub async fn require_team_member(
103+
/// Returns true if the user is a member of the team.
104+
pub async fn is_team_member(
113105
pool: &PgPool,
114106
team_id: Uuid,
115107
user_id: Uuid,
116-
) -> Result<(), StatusCode> {
117-
let is_member = sqlx::query_scalar::<_, bool>(
108+
) -> Result<bool, StatusCode> {
109+
sqlx::query_scalar::<_, bool>(
118110
"SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)",
119111
)
120112
.bind(team_id)
@@ -124,9 +116,15 @@ pub async fn require_team_member(
124116
.map_err(|e| {
125117
error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team membership");
126118
StatusCode::INTERNAL_SERVER_ERROR
127-
})?;
119+
})
120+
}
128121

129-
if is_member {
122+
pub async fn require_team_member(
123+
pool: &PgPool,
124+
team_id: Uuid,
125+
user_id: Uuid,
126+
) -> Result<(), StatusCode> {
127+
if is_team_member(pool, team_id, user_id).await? {
130128
Ok(())
131129
} else {
132130
Err(StatusCode::FORBIDDEN)

src/routes/team_sync.rs

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,28 +11,14 @@ use tracing::{error, info, warn};
1111
use uuid::Uuid;
1212

1313
use crate::auth::AuthUser;
14+
use crate::permissions::is_team_member;
1415
use crate::self_host;
1516
use crate::sync_notifier::{notify_team_vault_changed, SyncNotifier};
1617

1718
const MAX_TEAM_BLOB_SIZE: usize = 10 * 1024 * 1024; // 10 MB
1819

1920
// ─── Helpers ─────────────────────────────────────────────────────────────────
2021

21-
/// Returns true if the given user is a member of the given team.
22-
async fn is_team_member(pool: &PgPool, team_id: Uuid, user_id: Uuid) -> Result<bool, StatusCode> {
23-
sqlx::query_scalar::<_, bool>(
24-
"SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)",
25-
)
26-
.bind(team_id)
27-
.bind(user_id)
28-
.fetch_one(pool)
29-
.await
30-
.map_err(|e| {
31-
error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to check team membership");
32-
StatusCode::INTERNAL_SERVER_ERROR
33-
})
34-
}
35-
3622
/// Returns Ok if the vault owner has a Teams or Business subscription.
3723
/// In self-hosted mode this is a no-op — every tier is unlocked.
3824
async fn require_teams_tier_for_vault(pool: &PgPool, team_id: Uuid) -> Result<(), StatusCode> {

0 commit comments

Comments
 (0)