Standardize rule endpoint client identification

angular-client/src/api/rules.api.ts

@@ -21,27 +21,24 @@ export interface RulesResponse {
   client_rules: ClientRule[];
 }
 
-const basicAuthHeader = (clientId: string): string => 'Basic ' + btoa(`${clientId}:`);
+const clientIdHeader = (clientId: string): Record<string, string> => ({ 'X-Client-Id': clientId });
 
 export const getRulesByClientId = (clientId: string): Promise<Response> => {
-  return fetch(urls.getRulesByClientId(clientId));
+  return fetch(urls.getRulesWithSubscriptionStatus(), { headers: clientIdHeader(clientId) });
 };
 
 export const addRule = (clientId: string, rule: RulePayload): Promise<Response> => {
   return fetch(urls.addRule(), {
     method: 'PUT',
-    headers: {
-      'Content-Type': 'application/json',
-      Authorization: basicAuthHeader(clientId)
-    },
+    headers: { 'Content-Type': 'application/json', ...clientIdHeader(clientId) },
     body: JSON.stringify(rule)
   });
 };
 
 export const deleteRule = (clientId: string, ruleId: string): Promise<Response> => {
   return fetch(urls.deleteRule(ruleId), {
     method: 'POST',
-    headers: { Authorization: basicAuthHeader(clientId) }
+    headers: clientIdHeader(clientId)
   });
 };
 
@@ -53,23 +50,18 @@ export const editRule = (ruleId: string, rule: object): Promise<Response> => {
   });
 };
 
-export interface RuleSubscriptionRequest {
-  rule_ids: string[];
-  client_id: string;
-}
-
-export const subscribeToRules = (request: RuleSubscriptionRequest): Promise<Response> => {
+export const subscribeToRules = (clientId: string, ruleIds: string[]): Promise<Response> => {
   return fetch(urls.subscribeToRule(), {
     method: 'POST',
-    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify(request)
+    headers: { 'Content-Type': 'application/json', ...clientIdHeader(clientId) },
+    body: JSON.stringify(ruleIds)
   });
 };
 
-export const unsubscribeFromRules = (request: RuleSubscriptionRequest): Promise<Response> => {
+export const unsubscribeFromRules = (clientId: string, ruleIds: string[]): Promise<Response> => {
   return fetch(urls.unsubscribeFromRule(), {
     method: 'POST',
-    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify(request)
+    headers: { 'Content-Type': 'application/json', ...clientIdHeader(clientId) },
+    body: JSON.stringify(ruleIds)
   });
 };

angular-client/src/api/urls.ts

@@ -35,8 +35,8 @@ const updateVideos = () => `${getAllVideos()}/update`;
 const carCommandConfig = (key: string, values: number[]) =>
   `${baseURL}/config/set/${key}?${values.map((value) => `data=${value}`).join('&')}`;
 
-/* Rules */
-const getRulesByClientId = (clientId: string) => `${baseURL}/rules/${clientId}`;
+/* Rules — client id travels in the X-Client-Id header, not the path */
+const getRulesWithSubscriptionStatus = () => `${baseURL}/rules/subscription-status`;
 const addRule = () => `${baseURL}/rules/add`;
 const deleteRule = (ruleId: string) => `${baseURL}/rules/delete/${ruleId}`;
 const editRule = (ruleId: string) => `${baseURL}/rules/edit/${ruleId}`;
@@ -76,7 +76,7 @@ export const urls = {
 
   carCommandConfig,
 
-  getRulesByClientId,
+  getRulesWithSubscriptionStatus,
   addRule,
   deleteRule,
   editRule,

angular-client/src/pages/notification-rules-page/notification-rules-page.component.ts

@@ -142,10 +142,10 @@ export default class NotificationRulesPageComponent implements OnInit {
     try {
       const promises: Promise<Response>[] = [];
       if (toSubscribe.length > 0) {
-        promises.push(subscribeToRules({ rule_ids: toSubscribe, client_id: this.clientId }));
+        promises.push(subscribeToRules(this.clientId, toSubscribe));
       }
       if (toUnsubscribe.length > 0) {
-        promises.push(unsubscribeFromRules({ rule_ids: toUnsubscribe, client_id: this.clientId }));
+        promises.push(unsubscribeFromRules(this.clientId, toUnsubscribe));
       }
 
       const results = await Promise.all(promises);

angular-client/src/pages/notification-rules-page/rules-table/rules-table.component.ts

@@ -73,9 +73,11 @@ export class RulesTableComponent implements OnInit {
   }
 
   async onToggleSubscription(rule: ClientRule, subscribed: boolean): Promise<void> {
-    const request = { rule_ids: [rule.id], client_id: this.clientId() };
+    const ruleIds = [rule.id];
     try {
-      const response = subscribed ? await subscribeToRules(request) : await unsubscribeFromRules(request);
+      const response = subscribed
+        ? await subscribeToRules(this.clientId(), ruleIds)
+        : await unsubscribeFromRules(this.clientId(), ruleIds);
       if (response.ok) {
         this.rules.update((rules) => rules.map((r) => (r.id === rule.id ? { ...r, is_subscribed: subscribed } : r)));
       } else {

scylla-server/src/controllers/rule_controller.rs

@@ -1,9 +1,9 @@
 use std::sync::Arc;
 
-use axum::{Extension, Json, debug_handler, extract::Path};
-use axum_extra::{
-    TypedHeader,
-    headers::{Authorization, authorization::Basic},
+use axum::{
+    Extension, Json, debug_handler,
+    extract::{FromRequestParts, Path},
+    http::{StatusCode, request::Parts},
 };
 use serde::Deserialize;
 use serde_with::DurationSeconds;
@@ -16,47 +16,53 @@ use crate::{
     rule_structs::{ClientId, Rule, RuleId, RuleManager, RulesResponse},
 };
 
-#[derive(Deserialize)]
-pub struct RuleSubscriptionRequest {
-    rule_ids: Vec<String>,
-    client_id: String,
+const CLIENT_ID_HEADER: &str = "x-client-id";
+
+/// client id comes from the x-client-id header, keeping it out of conflict-prone route paths
+/// extractor pattern: <https://docs.rs/axum/latest/axum/extract/index.html#defining-custom-extractors>
+impl<S> FromRequestParts<S> for ClientId
+where
+    S: Send + Sync,
+{
+    type Rejection = ScyllaError;
+
+    async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> {
+        parts
+            .headers
+            .get(CLIENT_ID_HEADER)
+            .and_then(|value| value.to_str().ok())
+            .filter(|value| !value.is_empty())
+            .map(|value| ClientId(value.to_owned()))
+            .ok_or_else(|| {
+                ScyllaError::HttpError(
+                    StatusCode::BAD_REQUEST,
+                    format!("Missing or empty {CLIENT_ID_HEADER} header"),
+                )
+            })
+    }
 }
 
 #[debug_handler]
 pub async fn add_rule(
-    TypedHeader(auth): TypedHeader<Authorization<Basic>>,
+    client_id: ClientId,
     Extension(rules_manager): Extension<Arc<RuleManager>>,
     Json(rule): Json<Rule>,
 ) -> Result<Json<String>, ScyllaError> {
-    info!(
-        "Incoming rules reg: {}, from {}",
-        rule.topic,
-        auth.username().to_string()
-    );
-    match rules_manager
-        .add_rule(ClientId(auth.username().to_string()), rule)
-        .await
-    {
+    info!("Incoming rules reg: {}, from {}", rule.topic, client_id);
+    match rules_manager.add_rule(client_id, rule).await {
         Ok(_) => Ok(Json::from("Rule added!".to_owned())),
         Err(err) => Err(ScyllaError::RuleError(err)),
     }
 }
 
 #[debug_handler]
 pub async fn delete_rule(
-    TypedHeader(auth): TypedHeader<Authorization<Basic>>,
-    Extension(rules_manager): Extension<Arc<RuleManager>>,
+    client_id: ClientId,
     Path(rule_id): Path<String>,
+    Extension(rules_manager): Extension<Arc<RuleManager>>,
 ) -> Result<(), ScyllaError> {
-    info!(
-        "Incoming rules del: {}, from {}",
-        rule_id,
-        auth.username().to_string()
-    );
-    match rules_manager
-        .delete_rule(ClientId(auth.username().to_string()), RuleId(rule_id))
-        .await
-    {
+    info!("Incoming rules del: {}, from {}", rule_id, client_id);
+    match rules_manager.delete_rule(client_id, RuleId(rule_id)).await {
         Ok(_) => Ok(()),
         Err(err) => Err(ScyllaError::RuleError(err)),
     }
@@ -72,22 +78,22 @@ pub async fn get_all_rules(
 
 #[debug_handler]
 pub async fn get_client_subscribed_rules(
-    Path(client_id): Path<String>,
+    client_id: ClientId,
     Extension(rules_manager): Extension<Arc<RuleManager>>,
 ) -> Json<Vec<Rule>> {
     debug!("Fetching subscribed rules for client {}", client_id);
-    Json(rules_manager.get_client_rules(ClientId(client_id)).await)
+    Json(rules_manager.get_client_rules(client_id).await)
 }
 
 #[debug_handler]
 pub async fn get_all_rules_with_client_info(
-    Path(client_id): Path<String>,
+    client_id: ClientId,
     Extension(rules_manager): Extension<Arc<RuleManager>>,
 ) -> Result<Json<RulesResponse>, ScyllaError> {
     debug!("Fetching all rules");
     Ok(Json(
         rules_manager
-            .get_all_rules_with_subscription_status(ClientId(client_id))
+            .get_all_rules_with_subscription_status(client_id)
             .await,
     ))
 }
@@ -126,21 +132,19 @@ pub async fn edit_rule(
 
 #[debug_handler]
 pub async fn unsubscribe_rules(
+    client_id: ClientId,
     Extension(rules_manager): Extension<Arc<RuleManager>>,
-    Json(request): Json<RuleSubscriptionRequest>,
+    Json(rule_ids): Json<Vec<String>>,
 ) -> Result<Json<String>, ScyllaError> {
     info!(
         "Unsubscribing client {} from {} rules",
-        request.client_id,
-        request.rule_ids.len()
+        client_id,
+        rule_ids.len()
     );
 
-    let rule_ids: Vec<RuleId> = request.rule_ids.into_iter().map(RuleId).collect();
+    let rule_ids: Vec<RuleId> = rule_ids.into_iter().map(RuleId).collect();
 
-    match rules_manager
-        .unsubscribe_rules(ClientId(request.client_id), rule_ids)
-        .await
-    {
+    match rules_manager.unsubscribe_rules(client_id, rule_ids).await {
         Ok(_) => Ok(Json::from(
             "Successfully unsubscribed from rules".to_owned(),
         )),
@@ -150,21 +154,19 @@ pub async fn unsubscribe_rules(
 
 #[debug_handler]
 pub async fn subscribe_rules(
+    client_id: ClientId,
     Extension(rules_manager): Extension<Arc<RuleManager>>,
-    Json(request): Json<RuleSubscriptionRequest>,
+    Json(rule_ids): Json<Vec<String>>,
 ) -> Result<Json<String>, ScyllaError> {
     info!(
         "Subscribing client {} to {} rules",
-        request.client_id,
-        request.rule_ids.len()
+        client_id,
+        rule_ids.len()
     );
 
-    let rule_ids: Vec<RuleId> = request.rule_ids.into_iter().map(RuleId).collect();
+    let rule_ids: Vec<RuleId> = rule_ids.into_iter().map(RuleId).collect();
 
-    match rules_manager
-        .subscribe_rules(ClientId(request.client_id), rule_ids)
-        .await
-    {
+    match rules_manager.subscribe_rules(client_id, rule_ids).await {
         Ok(_) => Ok(Json::from("Successfully subscribed to rules".to_owned())),
         Err(err) => Err(ScyllaError::RuleError(err)),
     }

scylla-server/src/main.rs

@@ -440,18 +440,18 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         )
         .merge(
             Router::new()
-                .route("/rules/add", put(add_rule))
-                .route("/rules/delete/{rule_id}", post(delete_rule))
+                // client id arrives in the x-client-id header, never the path
                 .route("/rules", get(get_all_rules))
-                .route("/rules/{client_id}", get(get_all_rules_with_client_info))
                 .route(
-                    "/rules/subscribed/{client_id}",
-                    get(get_client_subscribed_rules),
+                    "/rules/subscription-status",
+                    get(get_all_rules_with_client_info),
                 )
-                .route("/rules/unsubscribe", post(unsubscribe_rules))
+                .route("/rules/subscribed", get(get_client_subscribed_rules))
+                .route("/rules/add", put(add_rule))
+                .route("/rules/delete/{rule_id}", post(delete_rule))
                 .route("/rules/edit/{rule_id}", put(edit_rule))
                 .route("/rules/subscribe", post(subscribe_rules))
-                //.route("/rules/delete/{rule_id}", post()).route("/rules/poll")
+                .route("/rules/unsubscribe", post(unsubscribe_rules))
                 .layer(Extension(rules_manager)),
         )
         // for CORS handling