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

GH-3096 addOwner link update #3299

Merged
merged 5 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions Multisig/App/SceneDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
// - WalletConnect links from dapps to connect to the safe
// - 'connect' link to establish new connection
// - 'open' link to move the app to foreground so that it is able to process WalletConnect request or response.
// - Request To Add Owner
// - <web app url>/<network:safe_address>/addOwner?address=<owner_address>
// - Request To Add Owner
// - <web app url>/addOwner?safe=<safe_address>&address=<owner_address>
// - Web3auth
// - handled by CustomAuth.handle()
private func handleUserActivity(_ userActivity: NSUserActivity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,29 @@ struct AddOwnerRequestParameters {
var chain: Chain
var safeAddress: Address
var ownerAddress: Address

func link(base: URL = App.configuration.services.webAppURL) -> String {
var url = base
.appendingPathComponent("addOwner")

let result: String

let queryItems = [
URLQueryItem(name: "safe", value: "\(chain.shortName!):\(safeAddress.checksummed)"),
URLQueryItem(name: "address", value: ownerAddress.checksummed)
]

if #available(iOS 16, *) {
url.append(queryItems: queryItems)
result = url.absoluteString
} else {
var comps = URLComponents(url: url, resolvingAgainstBaseURL: false)!
comps.queryItems = queryItems
result = comps.url!.absoluteString
}

return result
}
}

struct AddOwnerRequestValidator {
Expand All @@ -23,11 +46,10 @@ struct AddOwnerRequestValidator {
}
set {
_webAppURL = newValue
pattern = "^\(newValue)([-a-zA-Z0-9]{1,20}):(0x[a-fA-F0-9]{40})/addOwner\\?address=(0x[a-fA-F0-9]{40})$"
pattern = "^\(newValue)addOwner\\?safe=([-a-zA-Z0-9]{1,20}):(0x[a-fA-F0-9]{40})&address=(0x[a-fA-F0-9]{40})$"
}
}

private static var pattern = "^\(webAppURL)([-a-zA-Z0-9]{1,20}):(0x[a-fA-F0-9]{40})/addOwner\\?address=(0x[a-fA-F0-9]{40})$"
private static var pattern = "^\(webAppURL)addOwner\\?safe=([-a-zA-Z0-9]{1,20}):(0x[a-fA-F0-9]{40})&address=(0x[a-fA-F0-9]{40})$"

static func isValid(url: URL) -> Bool {
guard url.absoluteString.matches(pattern: pattern) else { return false }
Expand Down
10 changes: 4 additions & 6 deletions Multisig/UI/Settings/OwnerKeyManagement/Backup/UIFlow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ class UIFlow: NSObject {
if let presentedViewController = presenter.presentedViewController {
presentedViewController.dismiss(animated: true) { [weak self] in
self?.presenter.present(nav, animated: true)
// the actual presenting view controller might be different from the `presenter`
self?.presenter = nav.presentingViewController
}
} else {
presenter.present(nav, animated: true)
// the actual presenting view controller might be different from the `presenter`
presenter = nav.presentingViewController
}
}

Expand All @@ -75,12 +79,6 @@ class UIFlow: NSObject {
presenter.dismiss(animated: true) { [unowned self] in
completion(success)
}
} else if let presenter = navigationController.presentingViewController {
// in this case, the presenting view controller is not a direct ancestor of the
// currently opened view controller
presenter.dismiss(animated: true) { [unowned self] in
completion(success)
}
} else {
completion(success)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ class ShareAddOwnerLinkViewController: UIViewController {

tableView.rowHeight = UITableView.automaticDimension
tableView.estimatedRowHeight = 100
shareLinkView.set(text: App.configuration.services.webAppURL.appendingPathComponent("\(safe.chain!.shortName!):\(safe.displayAddress)/addOwner?address=\(owner.checksummed)").absoluteString)

let params = AddOwnerRequestParameters(chain: safe.chain!, safeAddress: safe.addressValue, ownerAddress: owner)
let text = params.link()

shareLinkView.set(text: text)
shareLinkView.onShare = { [weak self] text in
let vc = UIActivityViewController(activityItems: [text], applicationActivities: nil)
vc.completionWithItemsHandler = { _, success, _, _ in
Expand Down
81 changes: 47 additions & 34 deletions MultisigTests/Logic/AddOwnerRequestValidatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,61 +9,74 @@
import XCTest
@testable import Multisig

// Format change needs tests updated
class AddOwnerRequestValidatorTests: XCTestCase {
let domain = "app.safe.global"

override func setUp() async throws {
AddOwnerRequestValidator.webAppURL = URL(string: "https://app.safe.global/")!
AddOwnerRequestValidator.webAppURL = URL(string: "https://\(domain)/")!
}

func testAddOwnerRequestUrls() {
assertInvalid("https://safe.global", "missing path and query")
assertInvalid("ftp://url?query=https://safe.global/", "must start with the correct link")
assertInvalid("https://app.safe.global/", "missing rest of the path and query")
assertInvalid("https://app.safe.global/something/addOwner?address=else", "wrong format of path and query parameters")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner", "missing owner address query parameter")

assertInvalid("https://\(domain)", "missing path and query")
assertInvalid("ftp://url?query=https://\(domain)/", "must start with the correct link")
assertInvalid("https://\(domain)/", "missing rest of the path and query")
assertInvalid("https://\(domain)/addOwner?safe=something&address=else", "wrong format of path and query parameters")
assertInvalid("https://\(domain)/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner", "missing owner address query parameter")

// shortname with dash
assertValid("https://app.safe.global/eth-weth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support dash in shortname")
assertValid("https://\(domain)/addOwner?safe=eth-weth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support dash in shortname")

// shortname empty, 1, 20, 21
assertValid("https://app.safe.global/a:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support one-char shortname")
assertValid("https://app.safe.global/abcde12345abcde12345:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support 20-char shortname")
assertInvalid("https://app.safe.global/abcde12345abcde12345e:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "shortname too long")
assertInvalid("https://app.safe.global/:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "shortname empty")
assertValid("https://\(domain)/addOwner?safe=a:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support one-char shortname")
assertValid("https://\(domain)/addOwner?safe=abcde12345abcde12345:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support 20-char shortname")
assertInvalid("https://\(domain)/addOwner?safe=abcde12345abcde12345e:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "shortname too long")
assertInvalid("https://\(domain)/addOwner?safe=:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "shortname empty")

// safe address: empty, 39, 40, 41, hex, checksum wrong
assertInvalid("https://app.safe.global/eth:/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address empty")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address too short")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A1/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address too long")
assertValid("https://app.safe.global/eth:0x71592e6cbe7779d480c1d029e70904041f8f602a/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support hex safe address")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041f8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must detect wrong checksum in safe address")
assertInvalid("https://\(domain)/addOwner?safe=eth:&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address empty")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address too short")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A1&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "safe address too long")
assertValid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must support hex safe address")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041f8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "must detect wrong checksum in safe address")

// owner address: empty, 39, 40, 41, hex, checksum wrong
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=", "owner address empty")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b6", "owner address too short")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66a", "owner address too long")
assertValid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6a5adb2b88257a3dac7a76a7b4ecacda090b66", "must support hex owner address")
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090B66", "must detect wrong checksum in owner address")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=", "owner address empty")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b6", "owner address too short")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66a", "owner address too long")
assertValid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6a5adb2b88257a3dac7a76a7b4ecacda090b66", "must support hex owner address")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090B66", "must detect wrong checksum in owner address")

// additional params
assertInvalid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66&some=value", "happy case not working")
assertInvalid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66&some=value", "happy case not working")


// happy case
assertValid("https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "happy case not working")
assertValid("https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66", "happy case not working")
}

// func testExtractsParameters() {
// let url = URL(string: "https://app.safe.global/eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A/addOwner?address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66")!
// guard let parameters = AddOwnerRequestValidator.parameters(from: url) else {
// XCTFail("Parameters not found in correct link")
// return
// }
//
// XCTAssertEqual(parameters.chain.shortName, "eth")
// XCTAssertEqual(parameters.safeAddress, "0x71592E6Cbe7779D480C1D029e70904041F8f602A")
// XCTAssertEqual(parameters.ownerAddress, "0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66")
// }
func testExtractsParameters() {
let url = URL(string: "https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66")!
guard let parameters = AddOwnerRequestValidator.parameters(from: url) else {
XCTFail("Parameters not found in correct link")
return
}

XCTAssertEqual(parameters.chain.shortName, "eth")
XCTAssertEqual(parameters.safeAddress, "0x71592E6Cbe7779D480C1D029e70904041F8f602A")
XCTAssertEqual(parameters.ownerAddress, "0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66")
}

func testLink() {
let str = AddOwnerRequestParameters(
chain: .mainnetChain(),
safeAddress: "0x71592E6Cbe7779D480C1D029e70904041F8f602A",
ownerAddress: "0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66"
).link(base: AddOwnerRequestValidator.webAppURL)

XCTAssertEqual(str, "https://\(domain)/addOwner?safe=eth:0x71592E6Cbe7779D480C1D029e70904041F8f602A&address=0x8e6A5aDb2B88257A3DAc7A76A7B4EcaCdA090b66")
}

func assertInvalid(_ str: String, _ message: String, line: UInt = #line) {
guard let url = URL(string: str) else {
Expand Down