Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sessions): implementing permission revoking #699

Merged
merged 4 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions integration/sessions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ describe('Sessions/Permissions', () => {
onChainValidated: false
}
}
// New session PCI
let new_pci: string;
// New session signing (private) key
let signing_key: string;

it('create new session', async () => {
Expand Down Expand Up @@ -95,4 +97,60 @@ describe('Sessions/Permissions', () => {
)
expect(resp.status).toBe(200)
})

it('revoke PCI permission', async () => {
// Check PCI is exists
let resp = await httpClient.get(
`${baseUrl}/v1/sessions/${address}`
)
expect(resp.status).toBe(200)
expect(resp.data.pci.length).toBe(1)
expect(resp.data.pci[0]).toBe(new_pci)

// Create a signature
const privateKey = createPrivateKey({
key: Buffer.from(signing_key, 'base64'),
format: 'der',
type: 'sec1',
});

// Create a bad signature and try to revoke PCI
let bad_signature = createSign('SHA256');
bad_signature.update('bad_pci');
bad_signature.end();
let signature = bad_signature.sign(privateKey, 'base64');
let payload = {
pci: new_pci,
signature,
}
resp = await httpClient.post(
`${baseUrl}/v1/sessions/${address}/revoke`,
payload
)
expect(resp.status).toBe(401)

// Create a good signature and try to revoke PCI
const good_signature = createSign('SHA256');
good_signature.update(new_pci);
good_signature.end();
signature = good_signature.sign(privateKey, 'base64');
payload = {
pci: new_pci,
signature,
}

// Revoke PCI
resp = await httpClient.post(
`${baseUrl}/v1/sessions/${address}/revoke`,
payload
)
expect(resp.status).toBe(200)

// check PCI is revoked
resp = await httpClient.get(
`${baseUrl}/v1/sessions/${address}`
)
expect(resp.status).toBe(200)
expect(resp.data.pci.length).toBe(0)
})
})
9 changes: 7 additions & 2 deletions src/handlers/sessions/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use {
crate::{
error::RpcError,
state::AppState,
storage::irn::OperationType,
utils::crypto::{disassemble_caip10, json_canonicalize, verify_ecdsa_signature},
},
axum::{
Expand Down Expand Up @@ -41,7 +42,9 @@ async fn handler_internal(
.hget(address.clone(), request_payload.pci.clone())
.await?
.ok_or_else(|| RpcError::PermissionNotFound(request_payload.pci.clone()))?;
state.metrics.add_irn_latency(irn_call_start, "hget".into());
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hget.into());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to change to enum but I would go a step further and convert the enum into a string inside add_irn_latency()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 3ddc187. Thanks!

let mut storage_permissions_item =
serde_json::from_str::<StoragePermissionsItem>(&storage_permissions_item)?;

Expand All @@ -66,7 +69,9 @@ async fn handler_internal(
serde_json::to_string(&storage_permissions_item)?.into(),
)
.await?;
state.metrics.add_irn_latency(irn_call_start, "hset".into());
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hset.into());

Ok(Json(storage_permissions_item).into_response())
}
9 changes: 7 additions & 2 deletions src/handlers/sessions/create.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use {
super::{super::HANDLER_TASK_METRICS, NewPermissionPayload, StoragePermissionsItem},
crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10},
crate::{
error::RpcError, state::AppState, storage::irn::OperationType,
utils::crypto::disassemble_caip10,
},
axum::{
extract::{Path, State},
response::{IntoResponse, Response},
Expand Down Expand Up @@ -81,7 +84,9 @@ async fn handler_internal(
serde_json::to_string(&storage_permissions_item)?.into(),
)
.await?;
state.metrics.add_irn_latency(irn_call_start, "hset".into());
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hset.into());

let response = NewPermissionResponse {
pci,
Expand Down
9 changes: 7 additions & 2 deletions src/handlers/sessions/get.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use {
super::{super::HANDLER_TASK_METRICS, GetPermissionsRequest, StoragePermissionsItem},
crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10},
crate::{
error::RpcError, state::AppState, storage::irn::OperationType,
utils::crypto::disassemble_caip10,
},
axum::{
extract::{Path, State},
response::{IntoResponse, Response},
Expand Down Expand Up @@ -34,7 +37,9 @@ async fn handler_internal(
.hget(request.address.clone(), request.pci.clone())
.await?
.ok_or_else(|| RpcError::PermissionNotFound(request.pci))?;
state.metrics.add_irn_latency(irn_call_start, "hget".into());
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hget.into());
let storage_permissions_item =
serde_json::from_str::<StoragePermissionsItem>(&storage_permissions_item)?;

Expand Down
7 changes: 5 additions & 2 deletions src/handlers/sessions/list.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use {
super::super::HANDLER_TASK_METRICS,
crate::{error::RpcError, state::AppState, utils::crypto::disassemble_caip10},
crate::{
error::RpcError, state::AppState, storage::irn::OperationType,
utils::crypto::disassemble_caip10,
},
axum::{
extract::{Path, State},
response::{IntoResponse, Response},
Expand Down Expand Up @@ -40,7 +43,7 @@ async fn handler_internal(
let pci = irn_client.hfields(address).await?;
state
.metrics
.add_irn_latency(irn_call_start, "hfields".into());
.add_irn_latency(irn_call_start, OperationType::Hfields.into());
let response = ListPermissionResponse { pci };

Ok(Json(response).into_response())
Expand Down
9 changes: 9 additions & 0 deletions src/handlers/sessions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub mod context;
pub mod create;
pub mod get;
pub mod list;
pub mod revoke;

/// Payload to create a new permission
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -75,3 +76,11 @@ pub struct StoragePermissionsItem {
context: Option<PermissionContextItem>,
verification_key: String,
}

/// Permission revoke request schema
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct PermissionRevokeRequest {
pci: String,
signature: String,
}
66 changes: 66 additions & 0 deletions src/handlers/sessions/revoke.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use {
super::{super::HANDLER_TASK_METRICS, PermissionRevokeRequest, StoragePermissionsItem},
crate::{
error::RpcError,
state::AppState,
storage::irn::OperationType,
utils::crypto::{disassemble_caip10, verify_ecdsa_signature},
},
axum::{
extract::{Path, State},
response::Response,
Json,
},
std::{sync::Arc, time::SystemTime},
wc::future::FutureExt,
};

pub async fn handler(
state: State<Arc<AppState>>,
address: Path<String>,
Json(request_payload): Json<PermissionRevokeRequest>,
) -> Result<Response, RpcError> {
handler_internal(state, address, request_payload)
.with_metrics(HANDLER_TASK_METRICS.with_name("sessions_revoke"))
.await
}

#[tracing::instrument(skip(state), level = "debug")]
async fn handler_internal(
state: State<Arc<AppState>>,
Path(address): Path<String>,
request_payload: PermissionRevokeRequest,
) -> Result<Response, RpcError> {
let irn_client = state.irn.as_ref().ok_or(RpcError::IrnNotConfigured)?;

// Checking the CAIP-10 address format
disassemble_caip10(&address)?;

// Get the PCI object from the IRN
let irn_call_start = SystemTime::now();
let storage_permissions_item = irn_client
.hget(address.clone(), request_payload.pci.clone())
.await?
.ok_or_else(|| RpcError::PermissionNotFound(request_payload.pci.clone()))?;
geekbrother marked this conversation as resolved.
Show resolved Hide resolved
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hget.into());
let storage_permissions_item =
serde_json::from_str::<StoragePermissionsItem>(&storage_permissions_item)?;

// Check the signature
verify_ecdsa_signature(
&request_payload.pci,
&request_payload.signature,
&storage_permissions_item.verification_key,
)?;

// Remove the session/permission item from the IRN
let irn_call_start = SystemTime::now();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be using Instant::now() and instant.elapsed() instead here instead of SystemTime as it is panic/Result-free and possibly a little more accurate/less overhead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! This change should be the followup since we are using this everywhere in blockchain-api.

irn_client.hdel(address, request_payload.pci).await?;
state
.metrics
.add_irn_latency(irn_call_start, OperationType::Hdel.into());

Ok(Response::default())
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ pub async fn bootstrap(config: Config) -> RpcResult<()> {
.route("/v1/sessions/:address", get(handlers::sessions::list::handler))
.route("/v1/sessions/:address/:pci", get(handlers::sessions::get::handler))
.route("/v1/sessions/:address/context", post(handlers::sessions::context::handler))
.route("/v1/sessions/:address/revoke", post(handlers::sessions::revoke::handler))
// Health
.route("/health", get(handlers::health::handler))
.route_layer(tracing_and_metrics_layer)
Expand Down
26 changes: 26 additions & 0 deletions src/storage/irn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,32 @@ const CONNECTION_TIMEOUT: Duration = Duration::from_secs(3);
const RECORDS_TTL: Duration = Duration::from_secs(60 * 60 * 24 * 30); // 30 days
const UDP_SOCKET_COUNT: usize = 1;

/// IRN storage operation type
#[derive(Debug)]
pub enum OperationType {
Hset,
Hget,
Hfields,
Hdel,
}

impl OperationType {
fn as_str(&self) -> &'static str {
match self {
OperationType::Hset => "hset",
OperationType::Hget => "hget",
OperationType::Hfields => "hfields",
OperationType::Hdel => "hdel",
}
}
}

impl From<OperationType> for String {
fn from(value: OperationType) -> Self {
value.as_str().to_string()
}
}

#[derive(Debug, Clone, Deserialize, Eq, PartialEq)]
pub struct Config {
pub node: Option<String>,
Expand Down
Loading