Skip to content

Commit

Permalink
Merge pull request #3299 from safe-global/GH-3096/add-owner-link
Browse files Browse the repository at this point in the history 
GH-3096 addOwner link update
  • Loading branch information
@DmitryBespalov
DmitryBespalov authored Aug 21, 2023
2 parents 5f2eb21 + e39563c commit 08b3b72
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 46 deletions.
4 changes: 2 additions & 2 deletions Multisig/App/SceneDelegate.swift
View file
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
28 changes: 25 additions & 3 deletions Multisig/UI/Settings/OwnerKeyManagement/AddKeyAsNewOwner/AddOwnerRequestUrlValidator.swift
View file
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
View file
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
6 changes: 5 additions & 1 deletion ...sig/UI/Settings/OwnerKeyManagement/GenerateOwnerKey/ShareAddOwnerLinkViewController.swift
View file
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
View file
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

0 comments on commit 08b3b72

Please sign in to comment.