Skip to content

Commit

Permalink
#220 Permission validation for requests and events (#221)
Browse files Browse the repository at this point in the history 
* Add permission checks for requests

* Add permission checks for event emissions

* Add tests for basic permission validation

* Fixed permission validation with test cases

* Improve session permission tests

Co-authored-by: André Vants
  • Loading branch information
André Vants committed May 24, 2022
1 parent bb010be commit 35c827e
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Sources/WalletConnectAuth/Auth/AuthClient.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public final class AuthClient {
/// - Parameters:
/// - params: Parameters defining request and related session
public func request(params: Request) async throws {
try await sessionEngine.request(params: params)
try await sessionEngine.request(params)
}

/// For the responder to respond on pending peer's session JSON-RPC Request
Expand Down
31 changes: 18 additions & 13 deletions Sources/WalletConnectAuth/Engine/Common/SessionEngine.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,18 @@ final class SessionEngine {
}
}

func request(params: Request) async throws {
print("will request on session topic: \(params.topic)")
guard sessionStore.hasSession(forTopic: params.topic) else {
logger.debug("Could not find session for topic \(params.topic)")
return
func request(_ request: Request) async throws {
print("will request on session topic: \(request.topic)")
guard let session = sessionStore.getSession(forTopic: request.topic), session.acknowledged else {
logger.debug("Could not find session for topic \(request.topic)")
return // TODO: Marked to review on developer facing error cases
}
guard session.hasPermission(forMethod: request.method, onChain: request.chainId) else {
throw WalletConnectError.invalidPermissions
}
let request = SessionType.RequestParams.Request(method: params.method, params: params.params)
let sessionRequestParams = SessionType.RequestParams(request: request, chainId: params.chainId)
try await networkingInteractor.request(.wcSessionRequest(sessionRequestParams), onTopic: params.topic)
let chainRequest = SessionType.RequestParams.Request(method: request.method, params: request.params)
let sessionRequestParams = SessionType.RequestParams(request: chainRequest, chainId: request.chainId)
try await networkingInteractor.request(.wcSessionRequest(sessionRequestParams), onTopic: request.topic)
}

func respondSessionRequest(topic: String, response: JsonRpcResult) {
Expand All @@ -138,10 +141,10 @@ final class SessionEngine {
logger.debug("Could not find session for topic \(topic)")
return
}
let params = SessionType.EventParams(event: event, chainId: chainId)
guard session.hasNamespace(for: chainId, event: event.name) else {
guard session.hasPermission(forEvent: event.name, onChain: chainId) else {
throw WalletConnectError.invalidEvent
}
let params = SessionType.EventParams(event: event, chainId: chainId)
try await networkingInteractor.request(.wcSessionEvent(params), onTopic: topic)
}

Expand Down Expand Up @@ -235,7 +238,7 @@ final class SessionEngine {
networkingInteractor.respondError(for: payload, reason: .unauthorizedTargetChain(chain.absoluteString))
return
}
guard session.hasNamespace(for: chain, method: request.method) else {
guard session.hasPermission(forMethod: request.method, onChain: chain) else {
networkingInteractor.respondError(for: payload, reason: .unauthorizedMethod(request.method))
return
}
Expand All @@ -253,8 +256,10 @@ final class SessionEngine {
networkingInteractor.respondError(for: payload, reason: .noContextWithTopic(context: .session, topic: payload.topic))
return
}
guard session.peerIsController,
session.hasNamespace(for: eventParams.chainId, event: event.name) else {
guard
session.peerIsController,
session.hasPermission(forEvent: event.name, onChain: eventParams.chainId)
else {
networkingInteractor.respondError(for: payload, reason: .unauthorizedEvent(event.name))
return
}
Expand Down
73 changes: 37 additions & 36 deletions Sources/WalletConnectAuth/Types/Session/WCSession.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,47 +62,48 @@ struct WCSession: ExpirableSequence {
return controller.publicKey == peerParticipant.publicKey
}

// FIXME
func hasNamespace(for chain: Blockchain) -> Bool {
// TODO
// namespaces.contains{$0.chains.contains(chain)}
return true
return namespaces[chain.namespace] != nil
}

// TODO: Remove optional for chain param, it's required now / protocol change
func hasNamespace(for chain: Blockchain?, method: String) -> Bool {
// TODO
// if let chain = chain {
// let namespacesIncludingChain = namespaces.filter{$0.chains.contains(chain)}
// let methods = namespacesIncludingChain.flatMap{$0.methods}
// return methods.contains(method)
// } else {
// return namespaces
// .filter { $0.chains.isEmpty }
// .flatMap { $0.methods }
// .contains(method)
// }
return true
func hasPermission(forMethod method: String, onChain chain: Blockchain) -> Bool {
if let namespace = namespaces[chain.namespace] {
if namespace.accounts.contains(where: { $0.blockchain == chain }) {
if namespace.methods.contains(method) {
return true
}
if let extensions = namespace.extensions {
for extended in extensions {
if extended.accounts.contains(where: { $0.blockchain == chain }) {
if extended.methods.contains(method) {
return true
}
}
}
}
}
}
return false
}

func hasNamespace(for chain: Blockchain?, event: String) -> Bool {
// TODO
// if let chain = chain {
// if let namespace = namespaces.first(where: {$0.chains.contains(chain)}),
// namespace.events.contains(event) {
// return true
// } else {
// return false
// }
// } else {
// if let namespace = namespaces.first(where: {$0.chains.isEmpty}),
// namespace.events.contains(event) {
// return true
// } else {
// return false
// }
// }
return true
func hasPermission(forEvent event: String, onChain chain: Blockchain) -> Bool {
if let namespace = namespaces[chain.namespace] {
if namespace.accounts.contains(where: { $0.blockchain == chain }) {
if namespace.events.contains(event) {
return true
}
if let extensions = namespace.extensions {
for extended in extensions {
if extended.accounts.contains(where: { $0.blockchain == chain }) {
if extended.events.contains(event) {
return true
}
}
}
}
}
}
return false
}

mutating func updateNamespaces(_ namespaces: [String: SessionNamespace]) {
Expand Down
3 changes: 3 additions & 0 deletions Sources/WalletConnectAuth/WalletConnectError.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum WalletConnectError: Error {
case pairingAlreadyExist
case topicGenerationFailed
case invalidNamespaceMatch // TODO: Refactor into actual cases
case invalidPermissions // TODO: Same

case `internal`(_ reason: InternalReason)

Expand Down Expand Up @@ -58,6 +59,8 @@ extension WalletConnectError {
return "Pairing already exist"
case .invalidNamespaceMatch:
return "Invalid namespace approval."
case .invalidPermissions:
return "Invalid permissions for call."
case .internal(_): // TODO: Remove internal case
return ""
}
Expand Down
139 changes: 139 additions & 0 deletions Tests/WalletConnectTests/WCSessionTests.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import XCTest
@testable import WalletConnectAuth

final class WCSessionTests: XCTestCase {

let ethAccount = Account("eip155:1:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
let polyAccount = Account("eip155:137:0xab16a96d359ec26a11e2c2b3d8f8b8942d5bfcdb")!
let cosmosAccount = Account("cosmos:cosmoshub-4:cosmos1t2uflqwqe0fsj0shcfkrvpukewcw40yjj6hdc0")!

func testHasPermissionForMethod() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: ["method"],
events: [],
extensions: nil)
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertTrue(session.hasPermission(forMethod: "method", onChain: ethAccount.blockchain))
}

func testHasPermissionForMethodInExtension() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: [],
events: [],
extensions: [
SessionNamespace.Extension(
accounts: [ethAccount],
methods: ["method"],
events: [])])
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertTrue(session.hasPermission(forMethod: "method", onChain: ethAccount.blockchain))
}

func testDenyPermissionForMethodInOtherChain() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: [],
events: [],
extensions: nil),
"cosmos": SessionNamespace(
accounts: [cosmosAccount],
methods: ["method"],
events: [],
extensions: nil),
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertFalse(session.hasPermission(forMethod: "method", onChain: ethAccount.blockchain))
}

func testDenyPermissionForMethodInOtherChainExtension() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount, polyAccount],
methods: [],
events: [],
extensions: [
SessionNamespace.Extension(
accounts: [polyAccount],
methods: ["method"],
events: [])])
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertFalse(session.hasPermission(forMethod: "method", onChain: ethAccount.blockchain))
}

func testHasPermissionForEvent() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: [],
events: ["event"],
extensions: nil)
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertTrue(session.hasPermission(forEvent: "event", onChain: ethAccount.blockchain))
}

func testHasPermissionForEventInExtension() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: [],
events: [],
extensions: [
SessionNamespace.Extension(
accounts: [ethAccount],
methods: [],
events: ["event"])])
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertTrue(session.hasPermission(forEvent: "event", onChain: ethAccount.blockchain))
}

func testDenyPermissionForEventInOtherChain() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount],
methods: [],
events: [],
extensions: nil),
"cosmos": SessionNamespace(
accounts: [cosmosAccount],
methods: [],
events: ["event"],
extensions: nil),
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertFalse(session.hasPermission(forEvent: "event", onChain: ethAccount.blockchain))
}

func testDenyPermissionForEventInOtherChainExtension() {
let namespace = [
"eip155": SessionNamespace(
accounts: [ethAccount, polyAccount],
methods: [],
events: [],
extensions: [
SessionNamespace.Extension(
accounts: [polyAccount],
methods: [],
events: ["event"])])
]
var session = WCSession.stub()
session.updateNamespaces(namespace)
XCTAssertFalse(session.hasPermission(forEvent: "event", onChain: ethAccount.blockchain))
}
}

0 comments on commit 35c827e

Please sign in to comment.