update: store API keys as a user type in store metadata (#1628)

* store API keys as a third user type in parseable.json

* skip apikey type user in listing

* remove key type and add roles

* fix status code

* update lock

Author: nikhilsinhaparseable

src/apikeys.rs

@@ -16,298 +16,26 @@
  *
  */
 
-use std::collections::HashMap;
+//! API key primitives.
+//!
+//! API keys are persisted as a third `UserType` variant (`UserType::ApiKey`)
+//! inside `parseable.json`, alongside native and OAuth users. The backing
+//! user's permissions are resolved from the `roles` assigned to it (same
+//! mechanism as native and OAuth users).
 
-use chrono::{DateTime, Utc};
-use once_cell::sync::Lazy;
-use serde::{Deserialize, Serialize};
-use tokio::sync::RwLock;
-use ulid::Ulid;
+use std::collections::HashSet;
 
-use crate::{
-    metastore::metastore_traits::MetastoreObject,
-    parseable::{DEFAULT_TENANT, PARSEABLE},
-    storage::object_storage::apikey_json_path,
-};
+use serde::Deserialize;
 
-pub static API_KEYS: Lazy<ApiKeyStore> = Lazy::new(|| ApiKeyStore {
-    keys: RwLock::new(HashMap::new()),
-});
-
-#[derive(Debug)]
-pub struct ApiKeyStore {
-    pub keys: RwLock<HashMap<String, HashMap<Ulid, ApiKey>>>,
-}
-
-/// Type of API key, determining how it can be used.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(rename_all = "lowercase")]
-pub enum KeyType {
-    /// Used as a substitute for basic auth on ingestion endpoints
-    Ingestion,
-    /// Used as a substitute for basic auth on query endpoints (global query access)
-    Query,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct ApiKey {
-    pub key_id: Ulid,
-    pub api_key: String,
-    pub key_name: String,
-    #[serde(default = "default_key_type")]
-    pub key_type: KeyType,
-    pub created_by: String,
-    pub created_at: DateTime<Utc>,
-    pub modified_at: DateTime<Utc>,
-    #[serde(default)]
-    pub tenant: Option<String>,
-}
-
-fn default_key_type() -> KeyType {
-    KeyType::Ingestion
-}
-
-/// Request body for creating a new API key
+/// Request body for creating a new API key. `roles` is a set of role names
+/// that must already exist in the tenant; permissions for the backing user
+/// are derived from these roles (same flow as native/OAuth users).
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CreateApiKeyRequest {
     pub key_name: String,
-    #[serde(default = "default_key_type")]
-    pub key_type: KeyType,
-}
-
-/// Response for list keys (api_key masked to last 4 chars)
-#[derive(Debug, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct ApiKeyListEntry {
-    pub key_id: Ulid,
-    pub api_key: String,
-    pub key_name: String,
-    pub key_type: KeyType,
-    pub created_by: String,
-    pub created_at: DateTime<Utc>,
-    pub modified_at: DateTime<Utc>,
-}
-
-impl ApiKey {
-    pub fn new(
-        key_name: String,
-        key_type: KeyType,
-        created_by: String,
-        tenant: Option<String>,
-    ) -> Self {
-        let now = Utc::now();
-        Self {
-            key_id: Ulid::new(),
-            api_key: uuid::Uuid::new_v4().to_string(),
-            key_name,
-            key_type,
-            created_by,
-            created_at: now,
-            modified_at: now,
-            tenant,
-        }
-    }
-
-    pub fn to_list_entry(&self) -> ApiKeyListEntry {
-        let masked = if self.api_key.len() >= 4 {
-            let last4 = &self.api_key[self.api_key.len() - 4..];
-            format!("****{last4}")
-        } else {
-            "****".to_string()
-        };
-        ApiKeyListEntry {
-            key_id: self.key_id,
-            api_key: masked,
-            key_name: self.key_name.clone(),
-            key_type: self.key_type,
-            created_by: self.created_by.clone(),
-            created_at: self.created_at,
-            modified_at: self.modified_at,
-        }
-    }
-}
-
-impl MetastoreObject for ApiKey {
-    fn get_object_path(&self) -> String {
-        apikey_json_path(&self.key_id, &self.tenant).to_string()
-    }
-
-    fn get_object_id(&self) -> String {
-        self.key_id.to_string()
-    }
-}
-
-impl ApiKeyStore {
-    /// Load API keys from object store into memory
-    pub async fn load(&self) -> anyhow::Result<()> {
-        let api_keys = PARSEABLE.metastore.get_api_keys().await?;
-        let mut map = self.keys.write().await;
-        for (tenant_id, keys) in api_keys {
-            let inner = keys
-                .into_iter()
-                .map(|mut k| {
-                    k.tenant = if tenant_id == DEFAULT_TENANT {
-                        None
-                    } else {
-                        Some(tenant_id.clone())
-                    };
-                    (k.key_id, k)
-                })
-                .collect();
-            map.insert(tenant_id, inner);
-        }
-        Ok(())
-    }
-
-    /// Create a new API key
-    pub async fn create(&self, api_key: ApiKey) -> Result<(), ApiKeyError> {
-        let tenant = api_key.tenant.as_deref().unwrap_or(DEFAULT_TENANT);
-        let key_id = api_key.key_id;
-
-        // Check duplicate name and reserve the slot under the write lock,
-        // then drop the lock before the async metastore call so we don't
-        // hold a global lock across an await.
-        {
-            let mut map = self.keys.write().await;
-            if let Some(tenant_keys) = map.get(tenant)
-                && tenant_keys.values().any(|k| k.key_name == api_key.key_name)
-            {
-                return Err(ApiKeyError::DuplicateKeyName(api_key.key_name));
-            }
-            map.entry(tenant.to_owned())
-                .or_default()
-                .insert(key_id, api_key.clone());
-        }
-
-        // Persist to storage without holding the lock. On failure, remove
-        // the reservation so stale entries don't linger in memory.
-        if let Err(e) = PARSEABLE
-            .metastore
-            .put_api_key(&api_key, &api_key.tenant)
-            .await
-        {
-            let mut map = self.keys.write().await;
-            if let Some(tenant_keys) = map.get_mut(tenant) {
-                tenant_keys.remove(&key_id);
-            }
-            return Err(e.into());
-        }
-
-        Ok(())
-    }
-
-    /// Delete an API key by key_id
-    pub async fn delete(
-        &self,
-        key_id: &Ulid,
-        tenant_id: &Option<String>,
-    ) -> Result<ApiKey, ApiKeyError> {
-        let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT);
-
-        // Read the key first without removing
-        let api_key = {
-            let map = self.keys.read().await;
-            let tenant_keys = map
-                .get(tenant)
-                .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))?;
-            tenant_keys
-                .get(key_id)
-                .cloned()
-                .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))?
-        };
-
-        // Delete from storage first
-        PARSEABLE
-            .metastore
-            .delete_api_key(&api_key, tenant_id)
-            .await?;
-
-        // Remove from memory only after successful storage deletion
-        {
-            let mut map = self.keys.write().await;
-            if let Some(tenant_keys) = map.get_mut(tenant) {
-                tenant_keys.remove(key_id);
-            }
-        }
-
-        Ok(api_key)
-    }
-
-    /// List all API keys for a tenant (returns masked entries)
-    pub async fn list(
-        &self,
-        tenant_id: &Option<String>,
-    ) -> Result<Vec<ApiKeyListEntry>, ApiKeyError> {
-        let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT);
-        let map = self.keys.read().await;
-        let entries = if let Some(tenant_keys) = map.get(tenant) {
-            tenant_keys.values().map(|k| k.to_list_entry()).collect()
-        } else {
-            vec![]
-        };
-        Ok(entries)
-    }
-
-    /// Get a specific API key by key_id (returns full key)
-    pub async fn get(
-        &self,
-        key_id: &Ulid,
-        tenant_id: &Option<String>,
-    ) -> Result<ApiKey, ApiKeyError> {
-        let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT);
-        let map = self.keys.read().await;
-        let tenant_keys = map
-            .get(tenant)
-            .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))?;
-        tenant_keys
-            .get(key_id)
-            .cloned()
-            .ok_or_else(|| ApiKeyError::KeyNotFound(key_id.to_string()))
-    }
-
-    /// Validate an API key against a required key type. Returns true if the
-    /// key is valid AND its type matches the required type.
-    /// For multi-tenant: checks the key belongs to the specified tenant.
-    /// For single-tenant: checks the key exists globally.
-    pub async fn validate_key(
-        &self,
-        api_key_value: &str,
-        tenant_id: &Option<String>,
-        required_type: KeyType,
-    ) -> bool {
-        let map = self.keys.read().await;
-        let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT);
-        if let Some(tenant_keys) = map.get(tenant) {
-            return tenant_keys
-                .values()
-                .any(|k| k.api_key == api_key_value && k.key_type == required_type);
-        }
-        false
-    }
-
-    /// Insert an API key directly into memory (used for sync from prism)
-    pub async fn sync_put(&self, api_key: ApiKey) {
-        let tenant = api_key
-            .tenant
-            .as_deref()
-            .unwrap_or(DEFAULT_TENANT)
-            .to_owned();
-        let mut map = self.keys.write().await;
-        map.entry(tenant)
-            .or_default()
-            .insert(api_key.key_id, api_key);
-    }
-
-    /// Remove an API key from memory (used for sync from prism)
-    pub async fn sync_delete(&self, key_id: &Ulid, tenant_id: &Option<String>) {
-        let tenant = tenant_id.as_deref().unwrap_or(DEFAULT_TENANT);
-        let mut map = self.keys.write().await;
-        if let Some(tenant_keys) = map.get_mut(tenant) {
-            tenant_keys.remove(key_id);
-        }
-    }
+    #[serde(default)]
+    pub roles: HashSet<String>,
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -322,10 +50,13 @@ pub enum ApiKeyError {
     Unauthorized(String),
 
     #[error("{0}")]
-    MetastoreError(#[from] crate::metastore::MetastoreError),
+    Storage(#[from] crate::storage::ObjectStorageError),
+
+    #[error("{0}")]
+    Rbac(#[from] crate::handlers::http::rbac::RBACError),
 
     #[error("{0}")]
-    AnyhowError(#[from] anyhow::Error),
+    Anyhow(#[from] anyhow::Error),
 }
 
 impl actix_web::ResponseError for ApiKeyError {
@@ -334,7 +65,8 @@ impl actix_web::ResponseError for ApiKeyError {
             ApiKeyError::KeyNotFound(_) => actix_web::http::StatusCode::NOT_FOUND,
             ApiKeyError::DuplicateKeyName(_) => actix_web::http::StatusCode::CONFLICT,
             ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN,
-            ApiKeyError::MetastoreError(_) | ApiKeyError::AnyhowError(_) => {
+            ApiKeyError::Rbac(err) => actix_web::ResponseError::status_code(err),
+            ApiKeyError::Storage(_) | ApiKeyError::Anyhow(_) => {
                 actix_web::http::StatusCode::INTERNAL_SERVER_ERROR
             }
         }