Skip to content

Commit 00830b1

Browse files
atrakhConvex, Inc.
authored and
Convex, Inc.
committed
Add read_only bool to admin key and identity and enforce it (#27335)
GitOrigin-RevId: 2f5dc1eb566716c0cdde2bc41463ef7d254b9386
1 parent 23d3042 commit 00830b1

File tree

13 files changed

+152
-25
lines changed

13 files changed

+152
-25
lines changed

crates/application/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,7 @@ impl<RT: Runtime> Application<RT> {
937937
caller: FunctionCaller,
938938
pause_client: PauseClient,
939939
) -> anyhow::Result<Result<RedactedMutationReturn, RedactedMutationError>> {
940+
identity.ensure_can_run_function(UdfType::Mutation)?;
940941
let block_logging = self
941942
.log_visibility
942943
.should_redact_logs_and_error(
@@ -999,6 +1000,8 @@ impl<RT: Runtime> Application<RT> {
9991000
identity: Identity,
10001001
caller: FunctionCaller,
10011002
) -> anyhow::Result<Result<RedactedActionReturn, RedactedActionError>> {
1003+
identity.ensure_can_run_function(UdfType::Action)?;
1004+
10021005
let block_logging = self
10031006
.log_visibility
10041007
.should_redact_logs_and_error(
@@ -1064,6 +1067,7 @@ impl<RT: Runtime> Application<RT> {
10641067
caller: FunctionCaller,
10651068
mut response_streamer: HttpActionResponseStreamer,
10661069
) -> anyhow::Result<()> {
1070+
identity.ensure_can_run_function(UdfType::HttpAction)?;
10671071
let block_logging = self
10681072
.log_visibility
10691073
.should_redact_logs_and_error(
@@ -1165,6 +1169,8 @@ impl<RT: Runtime> Application<RT> {
11651169
}));
11661170
};
11671171

1172+
identity.ensure_can_run_function(analyzed_function.udf_type)?;
1173+
11681174
match analyzed_function.udf_type {
11691175
UdfType::Query => self
11701176
.read_only_udf(request_id, path, args, identity, caller)

crates/keybroker/src/broker.rs

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use common::{
3434
MemberId,
3535
PersistenceVersion,
3636
TeamId,
37+
UdfType,
3738
},
3839
};
3940
use errors::ErrorMetadata;
@@ -193,6 +194,25 @@ impl Identity {
193194
UncheckedIdentityProto::Unknown(()) => Ok(Identity::Unknown),
194195
}
195196
}
197+
198+
pub fn ensure_can_run_function(&self, udf_type: UdfType) -> anyhow::Result<()> {
199+
// Everyone can run queries.
200+
if udf_type == UdfType::Query {
201+
return Ok(());
202+
}
203+
match self {
204+
Identity::InstanceAdmin(admin_identity) | Identity::ActingUser(admin_identity, _) => {
205+
if admin_identity.is_read_only() {
206+
anyhow::bail!(ErrorMetadata::forbidden(
207+
"Unauthorized",
208+
format!("You do not have permission to run {udf_type} functions.")
209+
));
210+
}
211+
},
212+
_ => {},
213+
}
214+
Ok(())
215+
}
196216
}
197217

198218
impl From<Identity> for InertIdentity {
@@ -481,6 +501,11 @@ pub struct AdminIdentity {
481501
instance_name: String,
482502
principal: AdminIdentityPrincipal,
483503
key: String,
504+
// is_read_only being true implies that this identity should not be able to write data.
505+
// At the function level, read only admins are allowed to run queries but not mutations and
506+
// actions. At the database level, they are allowed to read data from user and system tables
507+
// but not write to them.
508+
is_read_only: bool,
484509
}
485510

486511
impl From<AdminIdentity> for pb::convex_identity::AdminIdentity {
@@ -489,6 +514,7 @@ impl From<AdminIdentity> for pb::convex_identity::AdminIdentity {
489514
instance_name,
490515
principal,
491516
key,
517+
is_read_only,
492518
}: AdminIdentity,
493519
) -> Self {
494520
Self {
@@ -502,6 +528,7 @@ impl From<AdminIdentity> for pb::convex_identity::AdminIdentity {
502528
),
503529
},
504530
key: Some(key),
531+
is_read_only,
505532
}
506533
}
507534
}
@@ -521,10 +548,12 @@ impl AdminIdentity {
521548
None => anyhow::bail!("Missing principal"),
522549
};
523550
let key = msg.key.ok_or_else(|| anyhow::anyhow!("Missing key"))?;
551+
let is_read_only: bool = msg.is_read_only;
524552
Ok(Self {
525553
instance_name,
526554
principal,
527555
key,
556+
is_read_only,
528557
})
529558
}
530559

@@ -537,12 +566,21 @@ impl AdminIdentity {
537566
instance_name,
538567
principal,
539568
key: access_token,
569+
is_read_only: false,
540570
}
541571
}
542572

543573
pub fn principal(&self) -> &AdminIdentityPrincipal {
544574
&self.principal
545575
}
576+
577+
// is_read_only being true implies that this identity should not be able to
578+
// write data. At the function level, read only admins are allowed to run
579+
// queries but not mutations and actions. At the database level, they are
580+
// allowed to read data from user and system tables but not write to them.
581+
pub fn is_read_only(&self) -> bool {
582+
self.is_read_only
583+
}
546584
}
547585

548586
#[cfg(any(test, feature = "testing"))]
@@ -557,6 +595,7 @@ impl Arbitrary for AdminIdentity {
557595
instance_name: "fake-instance-name".to_string(),
558596
principal,
559597
key,
598+
is_read_only: false,
560599
})
561600
}
562601
}
@@ -568,6 +607,7 @@ impl AdminIdentity {
568607
instance_name,
569608
principal: AdminIdentityPrincipal::Member(member_id),
570609
key: "chocolate-charlies-cupcake".to_string(),
610+
is_read_only: false,
571611
}
572612
}
573613

@@ -623,11 +663,15 @@ impl KeyBroker {
623663
}
624664

625665
pub fn issue_admin_key(&self, member_id: MemberId) -> AdminKey {
626-
AdminKey::new(self.issue_key(Some(member_id)))
666+
AdminKey::new(self.issue_key(Some(member_id), false))
667+
}
668+
669+
pub fn issue_read_only_admin_key(&self, member_id: MemberId) -> AdminKey {
670+
AdminKey::new(self.issue_key(Some(member_id), true))
627671
}
628672

629673
pub fn issue_system_key(&self) -> SystemKey {
630-
SystemKey::new(self.issue_key(None))
674+
SystemKey::new(self.issue_key(None, false))
631675
}
632676

633677
pub fn issue_store_file_authorization<RT: Runtime>(
@@ -652,7 +696,7 @@ impl KeyBroker {
652696
/// Private helper method to generate an admin key.
653697
/// If `member_id` is None, it generates a system key, otherwise
654698
/// an admin key for the given user.
655-
fn issue_key(&self, member_id: Option<MemberId>) -> String {
699+
fn issue_key(&self, member_id: Option<MemberId>, is_read_only: bool) -> String {
656700
let now = SystemTime::now();
657701
let since_epoch = now
658702
.duration_since(SystemTime::UNIX_EPOCH)
@@ -666,6 +710,7 @@ impl KeyBroker {
666710
instance_name: None,
667711
issued_s: since_epoch.as_secs(),
668712
identity: Some(identity),
713+
is_read_only,
669714
};
670715
format_admin_key(
671716
&self.instance_name,
@@ -691,6 +736,7 @@ impl KeyBroker {
691736
instance_name: instance_name_from_encrypted_part,
692737
issued_s,
693738
identity,
739+
is_read_only,
694740
} = self
695741
.encryptor
696742
.decode_proto(ADMIN_KEY_VERSION, encrypted_part)
@@ -712,6 +758,7 @@ impl KeyBroker {
712758
instance_name: self.instance_name.clone(),
713759
principal: AdminIdentityPrincipal::Member(MemberId(member_id)),
714760
key: key.to_string(),
761+
is_read_only,
715762
}),
716763
AdminIdentityProto::System(()) => Identity::system(),
717764
})
@@ -996,6 +1043,7 @@ mod tests {
9961043
instance_name: Some(kb.instance_name.clone()),
9971044
issued_s: since_epoch.as_secs(),
9981045
identity: Some(identity),
1046+
is_read_only: false,
9991047
};
10001048
kb.encryptor.encode_proto(ADMIN_KEY_VERSION, proto)
10011049
}

crates/local_backend/src/admin.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,29 +19,79 @@ pub fn must_be_admin_from_keybroker(
1919
Ok(identity)
2020
}
2121

22+
pub async fn must_be_admin_from_key_with_write_access(
23+
app_auth: &ApplicationAuth,
24+
instance_name: String,
25+
admin_key: String,
26+
) -> anyhow::Result<Identity> {
27+
must_be_admin_from_key_internal(app_auth, instance_name, admin_key, true).await
28+
}
29+
2230
pub async fn must_be_admin_from_key(
31+
app_auth: &ApplicationAuth,
32+
instance_name: String,
33+
admin_key: String,
34+
) -> anyhow::Result<Identity> {
35+
must_be_admin_from_key_internal(app_auth, instance_name, admin_key, false).await
36+
}
37+
38+
async fn must_be_admin_from_key_internal(
2339
app_auth: &ApplicationAuth,
2440
instance_name: String,
2541
admin_key_or_access_token: String,
42+
needs_write_access: bool,
2643
) -> anyhow::Result<Identity> {
2744
let identity = app_auth
2845
.check_key(admin_key_or_access_token, instance_name.clone())
2946
.await
3047
.context(bad_admin_key_error(Some(instance_name)))?;
48+
if needs_write_access {
49+
must_be_admin_with_write_access(&identity)?;
50+
}
3151
Ok(identity)
3252
}
3353

54+
pub fn must_be_admin_with_write_access(
55+
identity: &Identity,
56+
) -> anyhow::Result<AdminIdentityPrincipal> {
57+
must_be_admin_internal(identity, true)
58+
}
59+
3460
pub fn must_be_admin(identity: &Identity) -> anyhow::Result<AdminIdentityPrincipal> {
61+
must_be_admin_internal(identity, false)
62+
}
63+
64+
fn must_be_admin_internal(
65+
identity: &Identity,
66+
needs_write_access: bool,
67+
) -> anyhow::Result<AdminIdentityPrincipal> {
3568
if let Identity::InstanceAdmin(admin_identity) = identity {
69+
if needs_write_access && admin_identity.is_read_only() {
70+
return Err(read_only_admin_key_error().into());
71+
}
3672
Ok(admin_identity.principal().clone())
3773
} else {
3874
Err(bad_admin_key_error(identity.instance_name()).into())
3975
}
4076
}
4177

78+
pub fn must_be_admin_member_with_write_access(identity: &Identity) -> anyhow::Result<MemberId> {
79+
must_be_admin_member_internal(identity, true)
80+
}
81+
4282
pub fn must_be_admin_member(identity: &Identity) -> anyhow::Result<MemberId> {
83+
must_be_admin_member_internal(identity, false)
84+
}
85+
86+
fn must_be_admin_member_internal(
87+
identity: &Identity,
88+
needs_write_access: bool,
89+
) -> anyhow::Result<MemberId> {
4390
if let Identity::InstanceAdmin(admin_identity) = identity {
4491
if let AdminIdentityPrincipal::Member(member_id) = admin_identity.principal() {
92+
if needs_write_access && admin_identity.is_read_only() {
93+
return Err(read_only_admin_key_error().into());
94+
}
4595
Ok(*member_id)
4696
} else {
4797
Err(bad_admin_key_error(identity.instance_name()).into())
@@ -64,3 +114,10 @@ pub fn bad_admin_key_error(instance_name: Option<String>) -> ErrorMetadata {
64114
};
65115
ErrorMetadata::forbidden("BadDeployKey", msg)
66116
}
117+
118+
pub fn read_only_admin_key_error() -> ErrorMetadata {
119+
ErrorMetadata::forbidden(
120+
"ReadOnlyAdminKey",
121+
"You do not have permission to perform this operation.",
122+
)
123+
}

crates/local_backend/src/dashboard.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ use value::{
2929
};
3030

3131
use crate::{
32-
admin::must_be_admin_member,
32+
admin::{
33+
must_be_admin_member,
34+
must_be_admin_member_with_write_access,
35+
},
3336
authentication::ExtractIdentity,
3437
schema::IndexMetadataResponse,
3538
LocalAppState,
@@ -74,7 +77,7 @@ pub async fn delete_tables(
7477
ExtractIdentity(identity): ExtractIdentity,
7578
Json(DeleteTableArgs { table_names }): Json<DeleteTableArgs>,
7679
) -> Result<impl IntoResponse, HttpResponseError> {
77-
must_be_admin_member(&identity)?;
80+
must_be_admin_member_with_write_access(&identity)?;
7881
let table_names = table_names
7982
.into_iter()
8083
.map(|t| Ok(t.parse::<ValidIdentifier<TableName>>()?.0))

crates/local_backend/src/deploy_config.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ use sync_types::CanonicalizedModulePath;
6969
use value::ConvexObject;
7070

7171
use crate::{
72-
admin::must_be_admin_from_key,
72+
admin::{
73+
must_be_admin_from_key,
74+
must_be_admin_with_write_access,
75+
},
7376
parse::parse_module_path,
7477
EmptyResponse,
7578
LocalAppState,
@@ -381,6 +384,8 @@ pub async fn push_config_handler(
381384
.await
382385
.context("bad admin key error")?;
383386

387+
must_be_admin_with_write_access(&identity)?;
388+
384389
let udf_server_version = Version::parse(&config.udf_server_version).context(
385390
ErrorMetadata::bad_request("InvalidVersion", "The function version is invalid"),
386391
)?;

crates/local_backend/src/deploy_config2.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ use value::{
109109
};
110110

111111
use crate::{
112-
admin::must_be_admin_from_key,
112+
admin::{
113+
must_be_admin_from_key,
114+
must_be_admin_from_key_with_write_access,
115+
},
113116
deploy_config::{
114117
analyze_modules,
115118
ModuleJson,
@@ -356,7 +359,7 @@ pub async fn start_push_handler(
356359
st: &LocalAppState,
357360
request: StartPushRequest,
358361
) -> anyhow::Result<StartPushResponse> {
359-
let _identity = must_be_admin_from_key(
362+
let _identity = must_be_admin_from_key_with_write_access(
360363
st.application.app_auth(),
361364
st.instance_name.clone(),
362365
request.admin_key.clone(),
@@ -716,7 +719,7 @@ async fn finish_push_handler(
716719
req: FinishPushRequest,
717720
) -> anyhow::Result<FinishPushDiff> {
718721
let start_push = StartPushResponse::try_from(req.start_push)?;
719-
let _identity = must_be_admin_from_key(
722+
let _identity = must_be_admin_from_key_with_write_access(
720723
st.application.app_auth(),
721724
st.instance_name.clone(),
722725
req.admin_key.clone(),

crates/local_backend/src/environment_variables.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use model::environment_variables::types::{
1616
use serde::Deserialize;
1717

1818
use crate::{
19-
admin::must_be_admin,
19+
admin::must_be_admin_with_write_access,
2020
authentication::ExtractIdentity,
2121
LocalAppState,
2222
};
@@ -56,7 +56,7 @@ pub async fn update_environment_variables(
5656
ExtractIdentity(identity): ExtractIdentity,
5757
Json(UpdateEnvVarsRequest { changes }): Json<UpdateEnvVarsRequest>,
5858
) -> Result<impl IntoResponse, HttpResponseError> {
59-
must_be_admin(&identity)?;
59+
must_be_admin_with_write_access(&identity)?;
6060

6161
let mut env_var_changes = vec![];
6262
for change in changes {

0 commit comments

Comments
 (0)