diff --git a/.gitignore b/.gitignore index 2f01b0f4d..eec83f377 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,11 @@ /Cargo.lock /out +# Rust Code Coverage +tarpaulin-report.html +cobertura.xml +*.profraw + # Ignore Gradle project-specific cache directory .gradle diff --git a/Makefile b/Makefile index 1d8e622f3..99fdfb4a7 100644 --- a/Makefile +++ b/Makefile @@ -194,6 +194,9 @@ test-server: docker-compose up -d docker-compose run wpcli +codecov-rust: + $(rust_docker_run) /bin/bash -c 'cargo install cargo-tarpaulin && cargo tarpaulin --out html --out xml' + stop-server: docker-compose down diff --git a/native/swift/Example/Example/LoginView.swift b/native/swift/Example/Example/LoginView.swift index 60d0d8c97..ad4be9d0c 100644 --- a/native/swift/Example/Example/LoginView.swift +++ b/native/swift/Example/Example/LoginView.swift @@ -90,12 +90,9 @@ struct LoginView: View { callbackURLScheme: "exampleauth" ) - guard let loginDetails = WordPressAPI.Helpers.extractLoginDetails(from: urlWithToken) else { - debugPrint("Unable to parse login details") - abort() - } - + let loginDetails = try urlWithToken.asOAuthResponseUrl().getPasswordDetails() try await loginManager.setLoginCredentials(to: loginDetails) + } catch let err { self.isLoggingIn = false self.loginError = err.localizedDescription diff --git a/native/swift/Sources/wordpress-api/Extensions/OAuthResponseUrl.swift b/native/swift/Sources/wordpress-api/Extensions/OAuthResponseUrl.swift new file mode 100644 index 000000000..2a024f5b6 --- /dev/null +++ b/native/swift/Sources/wordpress-api/Extensions/OAuthResponseUrl.swift @@ -0,0 +1,14 @@ +import Foundation +import wordpress_api_wrapper + +public extension URL { + func asOAuthResponseUrl() -> OAuthResponseUrl { + OAuthResponseUrl(stringValue: self.absoluteString) + } +} + +extension OAuthResponseUrl { + static func new(stringValue: String) -> OAuthResponseUrl { + OAuthResponseUrl(stringValue: stringValue) + } +} diff --git a/native/swift/Sources/wordpress-api/Extensions/WpRestApiurl.swift b/native/swift/Sources/wordpress-api/Extensions/WpRestApiurl.swift new file mode 100644 index 000000000..c8c52fd21 --- /dev/null +++ b/native/swift/Sources/wordpress-api/Extensions/WpRestApiurl.swift @@ -0,0 +1,20 @@ +import Foundation +import wordpress_api_wrapper + +enum WPRestAPIError: Error { + case invalidUrl +} + +public extension WpRestApiurl { + func asUrl() throws -> URL { + guard + let url = URL(string: stringValue), + url.scheme != nil, + url.host != nil + else { + throw WPRestAPIError.invalidUrl + } + + return url + } +} diff --git a/native/swift/Sources/wordpress-api/Login/API+Login.swift b/native/swift/Sources/wordpress-api/Login/API+Login.swift index f134ffe8c..f6c8c5244 100644 --- a/native/swift/Sources/wordpress-api/Login/API+Login.swift +++ b/native/swift/Sources/wordpress-api/Login/API+Login.swift @@ -11,7 +11,7 @@ public extension WordPressAPI { let ephemeralClient = WordPressAPI(urlSession: session, baseUrl: url, authenticationStategy: .none) let response = try await ephemeralClient.perform(request: request) - return wordpress_api_wrapper.getLinkHeader(response: response, name: "https://api.w.org/")?.asUrl() + return try wordpress_api_wrapper.getLinkHeader(response: response, name: "https://api.w.org/")?.asUrl() } func getRestAPICapabilities(forApiRoot url: URL, using session: URLSession) async throws -> WpapiDetails { diff --git a/native/swift/Sources/wordpress-api/WordPressAPI.swift b/native/swift/Sources/wordpress-api/WordPressAPI.swift index 9b32323c8..c325ff948 100644 --- a/native/swift/Sources/wordpress-api/WordPressAPI.swift +++ b/native/swift/Sources/wordpress-api/WordPressAPI.swift @@ -69,10 +69,6 @@ public struct WordPressAPI { throw ParseError.invalidUrl } - - public static func extractLoginDetails(from url: URL) -> WpapiApplicationPasswordDetails? { - return wordpress_api_wrapper.extractLoginDetailsFromUrl(url: url.asRestUrl()) - } } enum ParseError: Error { @@ -164,16 +160,6 @@ extension WpNetworkRequest { } } -extension WpRestApiurl { - func asUrl() -> URL { - guard let url = URL(string: stringValue) else { - preconditionFailure("Invalid URL: \(stringValue)") - } - - return url - } -} - extension URL { func asRestUrl() -> WpRestApiurl { WpRestApiurl(stringValue: self.absoluteString) diff --git a/native/swift/Tests/wordpress-api/Extensions/WPRestAPIUrlTests.swift b/native/swift/Tests/wordpress-api/Extensions/WPRestAPIUrlTests.swift new file mode 100644 index 000000000..37d0fb416 --- /dev/null +++ b/native/swift/Tests/wordpress-api/Extensions/WPRestAPIUrlTests.swift @@ -0,0 +1,14 @@ +import XCTest +import wordpress_api +import wordpress_api_wrapper // We need to construct internal types to test them properly + +final class WPRestAPIUrlTests: XCTestCase { + + func testThatValidUrlCanBeParsed() throws { + XCTAssertEqual(URL(string: "http://example.com"), try WpRestApiurl(stringValue: "http://example.com").asUrl()) + } + + func testThatInvalidUrlThrowsError() throws { + XCTAssertThrowsError(try WpRestApiurl(stringValue: "invalid").asUrl()) + } +} diff --git a/wp_api/src/lib.rs b/wp_api/src/lib.rs index 315fb06f9..3b600b9e3 100644 --- a/wp_api/src/lib.rs +++ b/wp_api/src/lib.rs @@ -186,7 +186,7 @@ pub fn parse_api_details_response(response: WPNetworkResponse) -> Result Option { if let Some(url) = response.get_link_header(name) { - return Some(url.into()) + return Some(url.into()); } None diff --git a/wp_api/src/login.rs b/wp_api/src/login.rs index 12472e200..2c7f99cb1 100644 --- a/wp_api/src/login.rs +++ b/wp_api/src/login.rs @@ -1,33 +1,52 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::str; -use crate::url::*; // After a successful login, the system will receive an OAuth callback with the login details // embedded as query params. This function parses that URL and extracts the login details as an object. +#[derive(Debug, PartialEq, uniffi::Object)] +pub struct OAuthResponseUrl { + string_value: String, +} + #[uniffi::export] -pub fn extract_login_details_from_url( - url: WPRestAPIURL, -) -> Option { - if let (Some(site_url), Some(user_login), Some(password)) = - url.as_url() - .query_pairs() - .fold((None, None, None), |accum, (k, v)| { - match k.to_string().as_str() { - "site_url" => (Some(v.to_string()), accum.1, accum.2), - "user_login" => (accum.0, Some(v.to_string()), accum.2), - "password" => (accum.0, accum.1, Some(v.to_string())), - _ => accum, +impl OAuthResponseUrl { + #[uniffi::constructor] + pub fn new(string_value: String) -> Self { + Self { string_value } + } + + pub fn get_password_details( + &self, + ) -> Result { + let mut builder = WPAPIApplicationPasswordDetails::builder(); + + let url = + url::Url::parse(&self.string_value).map_err(|err| OAuthResponseUrlError::InvalidUrl)?; + + for pair in url.query_pairs() { + match pair.0.to_string().as_str() { + "site_url" => builder = builder.site_url(pair.1.to_string()), + "user_login" => builder = builder.user_login(pair.1.to_string()), + "password" => builder = builder.password(pair.1.to_string()), + "success" => { + if pair.1 == "false" { + return Err(OAuthResponseUrlError::UnsuccessfulLogin); + } } - }) - { - Some(WPAPIApplicationPasswordDetails { - site_url, - user_login, - password, - }) - } else { - None + _ => (), + }; + } + + builder.build() //.map_err(|err| UrlParsingError::InvalidUrl) + } +} + +impl From<&str> for OAuthResponseUrl { + fn from(str: &str) -> Self { + OAuthResponseUrl { + string_value: str.to_string(), + } } } @@ -54,9 +73,173 @@ pub struct WPRestApiAuthenticationEndpoint { pub authorization: String, } -#[derive(Debug, Serialize, Deserialize, uniffi::Record)] +#[derive(Debug, PartialEq, Serialize, Deserialize, uniffi::Record)] pub struct WPAPIApplicationPasswordDetails { pub site_url: String, pub user_login: String, pub password: String, } + +impl WPAPIApplicationPasswordDetails { + fn builder() -> WPAPIApplicationPasswordDetailsBuilder { + WPAPIApplicationPasswordDetailsBuilder::default() + } +} + +#[derive(Default)] +struct WPAPIApplicationPasswordDetailsBuilder { + site_url: Option, + user_login: Option, + password: Option, +} + +impl WPAPIApplicationPasswordDetailsBuilder { + fn site_url(mut self, site_url: String) -> Self { + self.site_url = Some(site_url); + self + } + + fn user_login(mut self, user_login: String) -> Self { + self.user_login = Some(user_login); + self + } + + fn password(mut self, password: String) -> Self { + self.password = Some(password); + self + } + + fn build(self) -> Result { + let site_url = if let Some(site_url) = self.site_url { + site_url + } else { + return Err(OAuthResponseUrlError::MissingSiteUrl); + }; + + let user_login = if let Some(user_login) = self.user_login { + user_login + } else { + return Err(OAuthResponseUrlError::MissingUsername); + }; + + let password = if let Some(password) = self.password { + password + } else { + return Err(OAuthResponseUrlError::MissingPassword); + }; + + Ok(WPAPIApplicationPasswordDetails { + site_url, + user_login, + password, + }) + } +} + +#[derive(Debug, thiserror::Error, uniffi::Error)] +pub enum OAuthResponseUrlError { + #[error("Invalid URL")] + InvalidUrl, + + #[error("The given URL is missing the `site_url` query parameter")] + MissingSiteUrl, + + #[error("The given URL is missing the `username` query parameter")] + MissingUsername, + + #[error("The given URL is missing the `password` query parameter")] + MissingPassword, + + #[error("Unsuccessful Login")] + UnsuccessfulLogin, +} + +#[cfg(test)] +mod oauth_response_url_tests { + use super::*; + + #[test] + fn can_be_initialized() { + assert_eq!(OAuthResponseUrl::new("foo".to_string()), OAuthResponseUrl::from("foo")) + } + + #[test] + fn creates_password_details_for_valid_url() { + let url = OAuthResponseUrl::from( + "exampleauth://login?site_url=http://example.com&user_login=test&password=1234", + ); + + assert_eq!( + url.get_password_details().unwrap(), + default_password_details() + ); + } + + #[test] + fn ignores_extra_query_params_for_valid_url() { + let url = OAuthResponseUrl::from( + "exampleauth://login?site_url=http://example.com&user_login=test&password=1234&foo=bar", + ); + + assert_eq!( + url.get_password_details().unwrap(), + default_password_details() + ); + } + + #[test] + fn throws_error_for_missing_site_url() { + let result = OAuthResponseUrl::from("exampleauth://login?user_login=test&password=1234") + .get_password_details(); + assert!(matches!(result, Err(OAuthResponseUrlError::MissingSiteUrl))); + } + + #[test] + fn throws_error_for_missing_user_login() { + let result = + OAuthResponseUrl::from("exampleauth://login?site_url=http://example.com&password=1234") + .get_password_details(); + assert!(matches!( + result, + Err(OAuthResponseUrlError::MissingUsername) + )); + } + + #[test] + fn throws_error_for_missing_password() { + let result = OAuthResponseUrl::from( + "exampleauth://login?site_url=http://example.com&user_login=test", + ) + .get_password_details(); + assert!(matches!( + result, + Err(OAuthResponseUrlError::MissingPassword) + )); + } + + #[test] + fn throws_error_for_unsuccessful_login() { + let result = + OAuthResponseUrl::from("exampleauth://login?success=false").get_password_details(); + assert!(matches!( + result, + Err(OAuthResponseUrlError::UnsuccessfulLogin) + )); + } + + #[test] + fn throws_appropriate_error_for_malformed_response() { + let result = + OAuthResponseUrl::from("exampleauth://login?success=true").get_password_details(); + assert!(matches!(result, Err(OAuthResponseUrlError::MissingSiteUrl))); + } + + fn default_password_details() -> WPAPIApplicationPasswordDetails { + WPAPIApplicationPasswordDetails::builder() + .site_url("http://example.com".to_string()) + .user_login("test".to_string()) + .password("1234".to_string()) + .build() + .unwrap() + } +} diff --git a/wp_api/src/url.rs b/wp_api/src/url.rs index 645f53a03..d43292701 100644 --- a/wp_api/src/url.rs +++ b/wp_api/src/url.rs @@ -1,21 +1,18 @@ pub use url::Url; -// A type that's guaranteed to represent a valid URL -// -// It is a programmer error to instantiate this object with an invalid URL -#[derive(Debug, uniffi::Record)] +/// A type that's guaranteed to represent a valid URL +/// +/// It is a programmer error to instantiate this object with an invalid URL +#[derive(Debug, PartialEq, uniffi::Record)] pub struct WPRestAPIURL { + /// A string representation of the URL pub string_value: String, } impl WPRestAPIURL { - pub fn as_str(&self) -> &str { + fn as_str(&self) -> &str { self.string_value.as_str() } - - pub fn as_url(&self) -> url::Url { - Url::parse(self.string_value.as_str()).unwrap() - } } impl From for WPRestAPIURL { @@ -31,3 +28,67 @@ impl From for String { url.string_value } } + +impl TryFrom for url::Url { + type Error = UrlParsingError; + + fn try_from(url: WPRestAPIURL) -> Result { + Url::parse(url.as_str()).map_err(|err| UrlParsingError::InvalidUrl) + } +} + +impl From<&str> for WPRestAPIURL { + fn from(str: &str) -> Self { + WPRestAPIURL { + string_value: str.to_string(), + } + } +} + +#[derive(Debug, thiserror::Error, uniffi::Error)] +pub enum UrlParsingError { + #[error("The URL you've provided is invalid")] + InvalidUrl, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn can_be_converted_to_str() { + assert_eq!( + "http://example.com", + WPRestAPIURL::from("http://example.com").as_str() + ); + } + + #[test] + fn can_be_converted_to_string() { + assert_eq!( + String::from(WPRestAPIURL::from("http://example.com")), + "http://example.com".to_string() + ); + } + + #[test] + fn can_be_converted_to_url() { + assert_eq!( + url::Url::parse("http://example.com").unwrap(), + WPRestAPIURL::from("http://example.com").try_into().unwrap() + ); + } + + #[test] + fn can_be_created_from_url() { + assert_eq!( + WPRestAPIURL::from("http://example.com/"), + url::Url::parse("http://example.com").unwrap().into() + ); + } + + #[test] + fn panics_on_invalid_url() { + assert!(url::Url::try_from(WPRestAPIURL::from("invalid")).is_err()) + } +}