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

MBL-1157: Create PKCE code for code verifier and code challenge #1921

Merged
merged 3 commits into from
Jan 31, 2024
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
9 changes: 9 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,7 @@
E10BE8E72B151D2700F73DC9 /* BlockUserInputTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E10BE8E52B151CC800F73DC9 /* BlockUserInputTests.swift */; };
E10D06632ACF385E00470B5C /* FetchBackerProjectsQuery.json in Resources */ = {isa = PBXBuildFile; fileRef = E10D06622ACF385E00470B5C /* FetchBackerProjectsQuery.json */; };
E10D06652AD48C9C00470B5C /* FetchBackerProjectsQueryRequestForTests.graphql_test in Resources */ = {isa = PBXBuildFile; fileRef = E10D06642AD48C9C00470B5C /* FetchBackerProjectsQueryRequestForTests.graphql_test */; };
E10F75E82B6937FA00024AD1 /* PKCETest.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1EEED2A2B686829009976D9 /* PKCETest.swift */; };
E170B9112B20E83B001BEDD7 /* MockGraphQLClient+CombineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E170B9102B20E83B001BEDD7 /* MockGraphQLClient+CombineTests.swift */; };
E1A1491E2ACDD76800F49709 /* FetchBackerProjectsQuery.graphql in Resources */ = {isa = PBXBuildFile; fileRef = E1A1491D2ACDD76700F49709 /* FetchBackerProjectsQuery.graphql */; };
E1A149202ACDD7BF00F49709 /* FetchProjectsEnvelope+FetchBackerProjectsQueryData.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1A1491F2ACDD7BF00F49709 /* FetchProjectsEnvelope+FetchBackerProjectsQueryData.swift */; };
Expand All @@ -1495,6 +1496,8 @@
E1A149272ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1A149262ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift */; };
E1AA8ABF2AEABBB100AC98BF /* Signal+Combine.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1EA34EE2AE1B28400942A04 /* Signal+Combine.swift */; };
E1BB25642B1E81AA000BD2D6 /* Publisher+Service.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */; };
E1EEED292B684AA7009976D9 /* PKCE.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1EEED282B684AA7009976D9 /* PKCE.swift */; };
E1FDB1E82AEAAC6100285F93 /* CombineTestObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1FDB1E72AEAAC6100285F93 /* CombineTestObserver.swift */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -3063,6 +3066,8 @@
E1A149262ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FetchBackerProjectsQueryDataTemplate.swift; sourceTree = "<group>"; };
E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Publisher+Service.swift"; sourceTree = "<group>"; };
E1EA34EE2AE1B28400942A04 /* Signal+Combine.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Signal+Combine.swift"; sourceTree = "<group>"; };
E1EEED282B684AA7009976D9 /* PKCE.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PKCE.swift; sourceTree = "<group>"; };
E1EEED2A2B686829009976D9 /* PKCETest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = PKCETest.swift; path = KsApi/PKCETest.swift; sourceTree = "<group>"; };
E1FDB1E72AEAAC6100285F93 /* CombineTestObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CombineTestObserver.swift; sourceTree = "<group>"; };
/* End PBXFileReference section */

Expand Down Expand Up @@ -6138,6 +6143,7 @@
A7E06C701C5A6EB300EBDCC2 = {
isa = PBXGroup;
children = (
E1EEED2A2B686829009976D9 /* PKCETest.swift */,
802800561C88F62500141235 /* Configs */,
E10BE8CE2B02975C00F73DC9 /* RichPushNotifications */,
A7E06DBC1C5C027800EBDCC2 /* Frameworks */,
Expand Down Expand Up @@ -6506,6 +6512,7 @@
D67F29351F68333800E399A6 /* GraphSchema.swift */,
D6C9A20D1F755FE200981E64 /* GraphSchemaTests.swift */,
D01587761EEB2ED6006E7684 /* MockService.swift */,
E1EEED282B684AA7009976D9 /* PKCE.swift */,
D01588261EEB2ED7006E7684 /* ServerConfig.swift */,
D01588271EEB2ED7006E7684 /* Service.swift */,
E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */,
Expand Down Expand Up @@ -8479,6 +8486,7 @@
E10BE8E02B1517FB00F73DC9 /* GraphAPI.BlockUserInput+BlockUserInput.swift in Sources */,
D01588EF1EEB2ED7006E7684 /* MessageThread.swift in Sources */,
8A4E95392450FBB500A578CF /* CreditCardType.swift in Sources */,
E1EEED292B684AA7009976D9 /* PKCE.swift in Sources */,
D0158A2F1EEB30A2006E7684 /* User.MemberDataTemplates.swift in Sources */,
D015883D1EEB2ED7006E7684 /* Route.swift in Sources */,
D0158A2B1EEB30A2006E7684 /* UpdateDraftTemplates.swift in Sources */,
Expand Down Expand Up @@ -8787,6 +8795,7 @@
D01589A81EEB306D006E7684 /* EncodableTests.swift in Sources */,
D0D10C111EEB4550005EBAD0 /* MessageThreadTests.swift in Sources */,
066C0ADE26CAEAFD0048F4D8 /* CreatePaymentSourceMutationTemplate.swift in Sources */,
E10F75E82B6937FA00024AD1 /* PKCETest.swift in Sources */,
06032A9426CEF4810013AAB9 /* FetchUserBackingsQueryTemplate.swift in Sources */,
06B9AF4326AF465000735908 /* UserCreditCards+UserFragmentTests.swift in Sources */,
066C0AE326CC0E410048F4D8 /* CreateBackingMutationTemplate.swift in Sources */,
Expand Down
87 changes: 87 additions & 0 deletions KsApi/PKCE.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import CryptoKit
import Foundation
import Security

enum PKCEError: Error {
case UnexpectedRuntimeError
}

public extension Data {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func base64URLEncodedStringWithNoPadding() -> String {
var encodedString = self.base64EncodedString()

// Convert base64 to base64url
encodedString = encodedString.replacingOccurrences(of: "+", with: "-")
encodedString = encodedString.replacingOccurrences(of: "/", with: "_")
// Strip padding
encodedString = encodedString.replacingOccurrences(of: "=", with: "")

return encodedString
}

mutating func fillWithRandomSecureBytes() throws {
do {
let numBytes = self.count
try self.withUnsafeMutableBytes { rawPointer in
let pointer = rawPointer.bindMemory(to: UInt8.self)
guard let address = pointer.baseAddress else {
throw PKCEError.UnexpectedRuntimeError
}

let result = SecRandomCopyBytes(kSecRandomDefault, numBytes, address)

if result != errSecSuccess {
throw PKCEError.UnexpectedRuntimeError
}
}
} catch {
throw error
}
}

func sha256Hash() throws -> Data {
let hash = SHA256.hash(data: self)
var hashData: Data?

hash.withUnsafeBytes { pointer in
let dataPointer = pointer.bindMemory(to: UInt8.self)
hashData = Data(buffer: dataPointer)
}

guard let unwrappedData = hashData else {
throw PKCEError.UnexpectedRuntimeError
Copy link
Contributor Author

@amy-at-kickstarter amy-at-kickstarter Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, I could make these methods return a nullable Data? and handle it further down. Preferences?

}

return unwrappedData
}
}

/// PKCE stands for Proof Key for Code Exchange.
/// The code verifier, and its associated challenge, are used to ensure that an oAuth request is valid.
/// See this documentation for more details: https://www.oauth.com/oauth2-servers/mobile-and-native-apps/authorization/
public struct PKCE {
/// Creates a random alphanumeric string of the specified length
static func createCodeVerifier(byteLength length: Int) throws -> String {
do {
var buffer = Data(count: length)
try buffer.fillWithRandomSecureBytes()
return buffer.base64URLEncodedStringWithNoPadding()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the RFC for OAuth suggests doing it - you create a shorter, cryptographically random buffer, and use base64 URL encoding to turn it into the valid character set.

} catch _ {
throw PKCEError.UnexpectedRuntimeError
}
}

/// Creates a base-64 encoded SHA256 hash of the given string.
static func createCodeChallenge(fromVerifier verifier: String) throws -> String {
guard let stringData = verifier.data(using: .utf8) else {
throw PKCEError.UnexpectedRuntimeError
}

do {
let hash = try stringData.sha256Hash()
return hash.base64URLEncodedStringWithNoPadding()
} catch {
throw PKCEError.UnexpectedRuntimeError
}
}
}
72 changes: 72 additions & 0 deletions KsApi/PKCETest.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
@testable import KsApi
import XCTest

final class PKCETest: XCTestCase {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some rough tests - we'll coordinate more examples later, to make sure iOS + Android have the same behavior.

func testData_RandomSecureBytes_overwritesOriginalData() {
let buffer1 = Data(repeating: 0, count: 32)
var buffer2 = Data(repeating: 0, count: 32)
var buffer3 = Data(repeating: 0, count: 32)

XCTAssertEqual(buffer1, buffer2)
XCTAssertEqual(buffer2, buffer3)

try! buffer2.fillWithRandomSecureBytes()
try! buffer3.fillWithRandomSecureBytes()

XCTAssertNotEqual(
buffer1,
buffer2,
"Buffer filled with random data should not be equal to buffer filled with zeroes"
)
XCTAssertNotEqual(
buffer1,
buffer3,
"Buffer filled with random data should not be equal to buffer filled with zeroes"
)
XCTAssertNotEqual(buffer2, buffer3, "Two randomly filled buffers should not be equal")
}

func testData_SHA256Hash_isCorrectValue() {
let testString = "Hello, world. I am a string."
let stringData = testString.data(using: .utf8)

let hash = try! stringData!.sha256Hash()

let expectedHashString = "c1c6864039f380248d30a73525c8351427c7fb468d58b7b1005f3e3727a042a1"
let actualHashString = hash.map { String(format: "%02x", $0) }.joined()

XCTAssertEqual(expectedHashString, actualHashString)
}

func testData_Base64URLEncodedString_isCorrectValue() {
let testString = "Hello, world. I am a string."

let expectedEncodedString = "SGVsbG8sIHdvcmxkLiBJIGFtIGEgc3RyaW5nLg"
let actualEncodedString = testString.data(using: .utf8)!.base64URLEncodedStringWithNoPadding()

XCTAssertEqual(expectedEncodedString, actualEncodedString)
}

func testPKCECodeChallenge_isValid() {
// TODO: Add more examples.
let res = try! PKCE.createCodeChallenge(fromVerifier: "foo")
XCTAssertEqual(res, "LCa0a2j_xo_5m0U8HTBBNBNCLXBkg7-g-YpeiGJm564")
}

func testPKCECreateCodeVerifier_matchesRequirements() {
let verifier = try! PKCE.createCodeVerifier(byteLength: 32)
XCTAssertEqual(verifier.count, 43)

let uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
let lowercase = "abcdefghijklmnopqrstuvwxyz"
let numbers = "0123456789"
let specials = "-._~"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, how would you get . or ~ in the verifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question - the RFC mentioned that these were valid, but you're right, they're not part of base64url 🤷‍♀️


let validCharacters = CharacterSet(charactersIn: uppercase + lowercase + numbers + specials)

for character in verifier {
let unichar = character.unicodeScalars.first!
XCTAssertTrue(validCharacters.contains(unichar), "\(unichar) not in valid character set")
}
}
}