From 4923fb7d458ed28c0302244c54cb4df0acee7ee6 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Sat, 16 Jul 2022 13:16:14 +0200 Subject: [PATCH] Improved OAuth/signature verifier data types and error handling (#133) --- README.md | 17 +++++++++ docs/src/events-api.md | 48 ++++++++++++++++--------- src/client/src/api/oauth.rs | 29 +++++++++++---- src/client/src/errors.rs | 9 +++++ src/client/src/listener.rs | 26 ++++++++------ src/client/src/signature_verifier.rs | 20 +++++++---- src/client/src/token.rs | 5 +-- src/hyper/examples/events_api_server.rs | 22 ++++++------ src/hyper/src/listener/oauth.rs | 10 ++++-- src/models/src/common/mod.rs | 16 +++++++-- 10 files changed, 143 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index b5faf67..9f50931 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,23 @@ Routes for this example are available on http://:8080: - /interaction - for Slack Interaction Events - /command - for Slack Command Events +### Testing with ngrok +For development/testing purposes you can use [ngrok](https://ngrok.com/): +``` +ngrok http 8080 +``` +and copy the URL it gives for you to the example parameters for `SLACK_REDIRECT_HOST`. + +Example testing with ngrok: +``` +SLACK_CLIENT_ID= \ +SLACK_CLIENT_SECRET= \ +SLACK_BOT_SCOPE=app_mentions:read,incoming-webhook \ +SLACK_REDIRECT_HOST=https://.ngrok.io \ +SLACK_SIGNING_SECRET= \ +cargo run --example events_api_server +``` + ## Licence Apache Software License (ASL) diff --git a/docs/src/events-api.md b/docs/src/events-api.md index 607186a..4955f08 100644 --- a/docs/src/events-api.md +++ b/docs/src/events-api.md @@ -114,23 +114,23 @@ async fn create_slack_events_listener_server() -> Result<(), Box Result<(), Box \ +SLACK_CLIENT_SECRET= \ +SLACK_BOT_SCOPE=app_mentions:read,incoming-webhook \ +SLACK_REDIRECT_HOST=https://.ngrok.io \ +SLACK_SIGNING_SECRET= \ +cargo run --example events_api_server +``` + +## Slack Signature Verifier + The library provides Slack events signature verifier (`SlackEventSignatureVerifier`), + which is already integrated in the OAuth routes implementation for you, and you don't need to use it directly. + All you need is provide your client id and secret configuration to route implementation. Look at the [complete example here](https://github.com/abdolence/slack-morphism-rust/tree/master/src/hyper/examples/events_api_server.rs). - + In case you're embedding the library into your own Web/routes-framework, you can use it separately. diff --git a/src/client/src/api/oauth.rs b/src/client/src/api/oauth.rs index ace356f..cdd273f 100644 --- a/src/client/src/api/oauth.rs +++ b/src/client/src/api/oauth.rs @@ -3,8 +3,10 @@ //! use rsb_derive::Builder; +use rvstruct::*; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; +use std::fmt; use crate::client::*; use crate::token::*; @@ -25,8 +27,14 @@ where let full_uri: Url = SlackClientHttpApiUri::create_url_with_params( &SlackClientHttpApiUri::create_method_uri_path("oauth.v2.access"), &vec![ - ("code", Some(&req.code)), - ("redirect_uri", req.redirect_uri.as_ref()), + ("code", Some(req.code.value())), + ( + "redirect_uri", + req.redirect_uri + .as_ref() + .map(|url| url.as_str().to_string()) + .as_ref(), + ), ], ); @@ -42,14 +50,14 @@ where pub struct SlackOAuthV2AccessTokenRequest { pub client_id: SlackClientId, pub client_secret: SlackClientSecret, - pub code: String, - pub redirect_uri: Option, + pub code: SlackOAuthCode, + pub redirect_uri: Option, } #[skip_serializing_none] #[derive(Debug, PartialEq, Clone, Serialize, Deserialize, Builder)] pub struct SlackOAuthV2AccessTokenResponse { - pub access_token: String, + pub access_token: SlackApiTokenValue, pub token_type: SlackApiTokenType, pub scope: SlackApiTokenScope, pub bot_user_id: Option, @@ -64,7 +72,7 @@ pub struct SlackOAuthV2AccessTokenResponse { pub struct SlackOAuthV2AuthedUser { pub id: SlackUserId, pub scope: Option, - pub access_token: Option, + pub access_token: Option, pub token_type: Option, } @@ -76,3 +84,12 @@ pub struct SlackOAuthIncomingWebHook { pub configuration_url: Url, pub url: Url, } + +#[derive(Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] +pub struct SlackOAuthCode(pub String); + +impl fmt::Debug for SlackOAuthCode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "SlackOAuthCode(len:{})", self.value().len()) + } +} diff --git a/src/client/src/errors.rs b/src/client/src/errors.rs index cd932fb..7c3f1ac 100644 --- a/src/client/src/errors.rs +++ b/src/client/src/errors.rs @@ -3,6 +3,7 @@ use std::error::Error; use std::fmt::Display; use std::fmt::Formatter; use std::time::Duration; +use url::ParseError; #[derive(Debug)] pub enum SlackClientError { @@ -190,3 +191,11 @@ impl Display for SlackRateLimitError { } impl Error for SlackRateLimitError {} + +impl From for SlackClientError { + fn from(url_parse_error: ParseError) -> Self { + SlackClientError::HttpProtocolError( + SlackClientHttpProtocolError::new().with_cause(Box::new(url_parse_error)), + ) + } +} diff --git a/src/client/src/listener.rs b/src/client/src/listener.rs index 3a1597c..4d484b3 100644 --- a/src/client/src/listener.rs +++ b/src/client/src/listener.rs @@ -1,12 +1,14 @@ -use crate::{SlackClient, SlackClientHttpConnector}; +use crate::{ClientResult, SlackClient, SlackClientHttpConnector}; use futures::executor::block_on; use futures::FutureExt; use rsb_derive::Builder; +use slack_morphism_models::{SlackClientId, SlackClientSecret, SlackSigningSecret}; use std::any::{Any, TypeId}; use std::collections::HashMap; use std::fmt::Debug; use std::sync::Arc; use tracing::*; +use url::Url; type UserStatesMap = HashMap>; @@ -100,7 +102,7 @@ pub type ErrorHandler = fn( #[derive(Debug, PartialEq, Clone, Builder)] pub struct SlackCommandEventsListenerConfig { - pub events_signing_secret: String, + pub events_signing_secret: SlackSigningSecret, #[default = "SlackCommandEventsListenerConfig::DEFAULT_EVENTS_URL_VALUE.into()"] pub events_path: String, } @@ -111,7 +113,7 @@ impl SlackCommandEventsListenerConfig { #[derive(Debug, PartialEq, Clone, Builder)] pub struct SlackPushEventsListenerConfig { - pub events_signing_secret: String, + pub events_signing_secret: SlackSigningSecret, #[default = "SlackPushEventsListenerConfig::DEFAULT_EVENTS_URL_VALUE.into()"] pub events_path: String, } @@ -122,7 +124,7 @@ impl SlackPushEventsListenerConfig { #[derive(Debug, PartialEq, Clone, Builder)] pub struct SlackInteractionEventsListenerConfig { - pub events_signing_secret: String, + pub events_signing_secret: SlackSigningSecret, #[default = "SlackInteractionEventsListenerConfig::DEFAULT_EVENTS_URL_VALUE.into()"] pub events_path: String, } @@ -133,8 +135,8 @@ impl SlackInteractionEventsListenerConfig { #[derive(Debug, PartialEq, Clone, Builder)] pub struct SlackOAuthListenerConfig { - pub client_id: String, - pub client_secret: String, + pub client_id: SlackClientId, + pub client_secret: SlackClientSecret, pub bot_scope: String, pub redirect_callback_host: String, #[default = "SlackOAuthListenerConfig::DEFAULT_INSTALL_PATH_VALUE.into()"] @@ -158,11 +160,15 @@ impl SlackOAuthListenerConfig { pub const OAUTH_AUTHORIZE_URL_VALUE: &'static str = "https://slack.com/oauth/v2/authorize"; - pub fn to_redirect_url(&self) -> String { - format!( - "{}{}", - &self.redirect_callback_host, &self.redirect_callback_path + pub fn to_redirect_url(&self) -> ClientResult { + Url::parse( + format!( + "{}{}", + &self.redirect_callback_host, &self.redirect_callback_path + ) + .as_str(), ) + .map_err(|e| e.into()) } } diff --git a/src/client/src/signature_verifier.rs b/src/client/src/signature_verifier.rs index a9ebe5a..c33444e 100644 --- a/src/client/src/signature_verifier.rs +++ b/src/client/src/signature_verifier.rs @@ -1,9 +1,11 @@ use ring::hmac; use rsb_derive::Builder; +use rvstruct::*; +use slack_morphism_models::SlackSigningSecret; use std::error::Error; use std::fmt::{Display, Formatter}; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct SlackEventSignatureVerifier { secret_len: usize, key: hmac::Key, @@ -13,11 +15,11 @@ impl SlackEventSignatureVerifier { pub const SLACK_SIGNED_HASH_HEADER: &'static str = "x-slack-signature"; pub const SLACK_SIGNED_TIMESTAMP: &'static str = "x-slack-request-timestamp"; - pub fn new(secret: &str) -> Self { - let secret_bytes = secret.as_bytes(); + pub fn new(secret: &SlackSigningSecret) -> Self { + let secret_bytes = secret.value().as_bytes(); SlackEventSignatureVerifier { secret_len: secret_bytes.len(), - key: hmac::Key::new(hmac::HMAC_SHA256, secret.as_bytes()), + key: hmac::Key::new(hmac::HMAC_SHA256, secret_bytes), } } @@ -143,7 +145,7 @@ fn check_signature_success() { ring::rand::generate(&rng).unwrap().expose(); let key_str: String = hex::encode(key_value); - let verifier = SlackEventSignatureVerifier::new(&key_str); + let verifier = SlackEventSignatureVerifier::new(&key_str.to_string().into()); const TEST_BODY: &'static str = "test-body"; const TEST_TS: &'static str = "test-ts"; @@ -167,7 +169,7 @@ fn test_precoded_data() { const TEST_BODY: &'static str = "test-body"; const TEST_TS: &'static str = "test-ts"; - let verifier = SlackEventSignatureVerifier::new(TEST_SECRET); + let verifier = SlackEventSignatureVerifier::new(&TEST_SECRET.to_string().into()); match verifier.verify(TEST_HASH, TEST_BODY, TEST_TS) { Ok(_) => {} @@ -179,7 +181,11 @@ fn test_precoded_data() { #[test] fn check_empty_secret_error_test() { - match SlackEventSignatureVerifier::new("").verify("test-hash", "test-body", "test-ts") { + match SlackEventSignatureVerifier::new(&"".to_string().into()).verify( + "test-hash", + "test-body", + "test-ts", + ) { Err(SlackEventSignatureVerifierError::CryptoInitError(ref err)) => { assert!(!err.message.is_empty()) } diff --git a/src/client/src/token.rs b/src/client/src/token.rs index 8d3114f..974b48a 100644 --- a/src/client/src/token.rs +++ b/src/client/src/token.rs @@ -3,10 +3,7 @@ use rvstruct::ValueStruct; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; -use slack_morphism_models::SlackTeamId; - -// Re-exports for backward compatibility -pub use slack_morphism_models::SlackApiTokenScope; +use slack_morphism_models::{SlackApiTokenScope, SlackTeamId}; #[derive(Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct SlackApiTokenValue(pub String); diff --git a/src/hyper/examples/events_api_server.rs b/src/hyper/examples/events_api_server.rs index 7158435..f699732 100644 --- a/src/hyper/examples/events_api_server.rs +++ b/src/hyper/examples/events_api_server.rs @@ -97,23 +97,23 @@ async fn test_server() -> Result<(), Box> { } let oauth_listener_config = Arc::new(SlackOAuthListenerConfig::new( - config_env_var("SLACK_CLIENT_ID")?, - config_env_var("SLACK_CLIENT_SECRET")?, + config_env_var("SLACK_CLIENT_ID")?.into(), + config_env_var("SLACK_CLIENT_SECRET")?.into(), config_env_var("SLACK_BOT_SCOPE")?, config_env_var("SLACK_REDIRECT_HOST")?, )); - let push_events_config = Arc::new(SlackPushEventsListenerConfig::new(config_env_var( - "SLACK_SIGNING_SECRET", - )?)); - - let interactions_events_config = Arc::new(SlackInteractionEventsListenerConfig::new( - config_env_var("SLACK_SIGNING_SECRET")?, + let push_events_config = Arc::new(SlackPushEventsListenerConfig::new( + config_env_var("SLACK_SIGNING_SECRET")?.into(), )); - let command_events_config = Arc::new(SlackCommandEventsListenerConfig::new(config_env_var( - "SLACK_SIGNING_SECRET", - )?)); + let interactions_events_config = Arc::new(SlackInteractionEventsListenerConfig::new( + config_env_var("SLACK_SIGNING_SECRET")?.into(), + )); + + let command_events_config = Arc::new(SlackCommandEventsListenerConfig::new( + config_env_var("SLACK_SIGNING_SECRET")?.into(), + )); let listener_environment = Arc::new( SlackClientEventsListenerEnvironment::new(client.clone()) diff --git a/src/hyper/src/listener/oauth.rs b/src/hyper/src/listener/oauth.rs index 35ab320..77fd157 100644 --- a/src/hyper/src/listener/oauth.rs +++ b/src/hyper/src/listener/oauth.rs @@ -10,6 +10,7 @@ use futures::future::{BoxFuture, FutureExt}; use hyper::body::*; use hyper::client::connect::Connect; use hyper::{Method, Request, Response}; +use rvstruct::*; use std::future::Future; use std::sync::Arc; use tracing::*; @@ -22,9 +23,12 @@ impl SlackClientEventsHyperListener< let full_uri = SlackClientHttpApiUri::create_url_with_params( SlackOAuthListenerConfig::OAUTH_AUTHORIZE_URL_VALUE, &vec![ - ("client_id", Some(&config.client_id)), + ("client_id", Some(config.client_id.value())), ("scope", Some(&config.bot_scope)), - ("redirect_uri", Some(&config.to_redirect_url())), + ( + "redirect_uri", + Some(config.to_redirect_url()?.as_str().to_string()).as_ref(), + ), ], ); debug!("Redirecting to Slack OAuth authorize: {}", &full_uri); @@ -55,7 +59,7 @@ impl SlackClientEventsHyperListener< client_secret: config.client_secret.clone().into(), code: code.into(), }) - .with_redirect_uri(config.to_redirect_url()), + .with_redirect_uri(config.to_redirect_url()?), ) .await; diff --git a/src/models/src/common/mod.rs b/src/models/src/common/mod.rs index 048a216..2ff7153 100644 --- a/src/models/src/common/mod.rs +++ b/src/models/src/common/mod.rs @@ -109,9 +109,15 @@ pub struct SlackCommandId(pub String); #[derive(Debug, Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct SlackClientId(pub String); -#[derive(Debug, Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] +#[derive(Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct SlackClientSecret(pub String); +impl fmt::Debug for SlackClientSecret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "SlackClientSecret(len:{})", self.value().len()) + } +} + #[derive(Debug, Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct SlackApiTokenScope(pub String); @@ -124,9 +130,15 @@ impl fmt::Debug for SlackVerificationToken { } } -#[derive(Debug, Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] +#[derive(Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct SlackSigningSecret(pub String); +impl fmt::Debug for SlackSigningSecret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "SlackSigningSecret(len:{})", self.value().len()) + } +} + #[derive(Debug, Eq, PartialEq, Hash, Clone, Serialize, Deserialize, ValueStruct)] pub struct EmailAddress(pub String);