From d2f633fa7febf194b5cfeb2c93b8c98508294e85 Mon Sep 17 00:00:00 2001 From: Ondrej Rafaj Date: Tue, 11 Sep 2018 13:34:50 +0100 Subject: [PATCH] fixes - tests passing - tokens use URandom and SHA256 - passwords use bcrypt with verification --- Package.swift | 3 ++ .../ApiCore/Controllers/AuthController.swift | 14 ++++---- Sources/ApiCore/Extensions/Data+Tools.swift | 17 +++++++++ .../ApiCore/Libs/AuthenticationCache.swift | 4 +-- Sources/ApiCore/Model/Token.swift | 35 +++++++++++-------- .../Extensions/Application+Testable.swift | 5 +++ .../Controllers/UsersControllerTests.swift | 8 ++--- .../ApiCoreTests/Libs/StringCryptoTests.swift | 2 +- 8 files changed, 61 insertions(+), 27 deletions(-) create mode 100644 Sources/ApiCore/Extensions/Data+Tools.swift diff --git a/Package.swift b/Package.swift index df08fcf..d1a87fc 100644 --- a/Package.swift +++ b/Package.swift @@ -12,6 +12,7 @@ let package = Package( dependencies: [ .package(url: "https://github.com/vapor/vapor.git", from: "3.0.0"), .package(url: "https://github.com/vapor/core.git", from: "3.4.1"), + .package(url: "https://github.com/vapor/crypto.git", from: "3.2.0"), .package(url: "https://github.com/vapor/fluent.git", from: "3.0.0"), .package(url: "https://github.com/vapor/fluent-postgresql.git", from: "1.0.0-rc.4"), .package(url: "https://github.com/vapor/jwt.git", from: "3.0.0-rc.2"), @@ -39,6 +40,8 @@ let package = Package( .target(name: "ApiCore", dependencies: [ "Vapor", "Fluent", + "Crypto", + "Random", "FluentPostgreSQL", "ErrorsCore", "DbCore", diff --git a/Sources/ApiCore/Controllers/AuthController.swift b/Sources/ApiCore/Controllers/AuthController.swift index 07d4506..c2c293f 100644 --- a/Sources/ApiCore/Controllers/AuthController.swift +++ b/Sources/ApiCore/Controllers/AuthController.swift @@ -95,7 +95,7 @@ public class AuthController: Controller { let templateModel = User.Auth.RecoveryTemplate( verification: jwtToken, - link: recoveryData.targetUri ?? "" + "?token=" + jwtToken, + link: (recoveryData.targetUri ?? "invalid_target_uri") + "?token=" + jwtToken, user: user ) return try PasswordRecoveryEmailTemplate.parsed(model: templateModel, on: req).flatMap(to: Response.self) { template in @@ -192,13 +192,15 @@ extension AuthController { guard let user = user, let password = user.password, login.password.verify(against: password) else { throw AuthError.authenticationFailed } - let token = try Token(user: user) - let tokenBackup = token + let token = try Token(user: user, type: .authentication) + let tokenBackup = token.token token.token = try token.token.sha() return token.save(on: req).flatMap(to: Response.self) { token in - tokenBackup.id = token.id - - let publicToken = Token.PublicFull(token: tokenBackup, user: user) + guard let _ = token.id else { + throw AuthError.serverError + } + let publicToken = Token.PublicFull(token: token, user: user) + publicToken.token = tokenBackup return try publicToken.asResponse(.ok, to: req).map(to: Response.self) { response in let jwtService = try req.make(JWTService.self) try response.http.headers.replaceOrAdd(name: "Authorization", value: "Bearer \(jwtService.signUserToToken(user: user))") diff --git a/Sources/ApiCore/Extensions/Data+Tools.swift b/Sources/ApiCore/Extensions/Data+Tools.swift new file mode 100644 index 0000000..a7cbd2f --- /dev/null +++ b/Sources/ApiCore/Extensions/Data+Tools.swift @@ -0,0 +1,17 @@ +// +// Data+Tools.swift +// ApiCore +// +// Created by Ondrej Rafaj on 11/09/2018. +// + +import Foundation + + +extension Data { + + public func asUTF8String() -> String? { + return String(data: self, encoding: .utf8) + } + +} diff --git a/Sources/ApiCore/Libs/AuthenticationCache.swift b/Sources/ApiCore/Libs/AuthenticationCache.swift index 9803b27..d4614d1 100644 --- a/Sources/ApiCore/Libs/AuthenticationCache.swift +++ b/Sources/ApiCore/Libs/AuthenticationCache.swift @@ -38,7 +38,7 @@ struct JWTConfirmEmailPayload: JWTPayload { var exp: ExpirationClaim /// User Id - var userId: UUID + var userId: UUID? /// User email var email: String @@ -112,7 +112,7 @@ final class JWTService: Service { /// Sign any email confirmation JWT token func signEmailConfirmation(user: User, type: TokenType, redirectUri: String?) throws -> String { let exp = ExpirationClaim(value: Date(timeIntervalSinceNow: (36 * hour))) - var jwt = JWT(payload: JWTConfirmEmailPayload(exp: exp, userId: user.id!, email: user.email, type: type, redirectUri: redirectUri ?? "")) + var jwt = JWT(payload: JWTConfirmEmailPayload(exp: exp, userId: user.id, email: user.email, type: type, redirectUri: redirectUri ?? "")) jwt.header.typ = nil // set to nil to avoid dictionary re-ordering causing probs let data = try signer.sign(jwt) diff --git a/Sources/ApiCore/Model/Token.swift b/Sources/ApiCore/Model/Token.swift index 0c9a829..c23119d 100644 --- a/Sources/ApiCore/Model/Token.swift +++ b/Sources/ApiCore/Model/Token.swift @@ -11,6 +11,7 @@ import Fluent import FluentPostgreSQL import DbCore import ErrorsCore +import Random /// Tokens array type typealias @@ -21,7 +22,7 @@ public typealias Tokens = [Token] public final class Token: DbCoreModel { /// Token type - public enum TokenType: String, Codable, CaseIterable, ReflectionDecodable { + public enum TokenType: String, PostgreSQLRawEnum { /// Authentication case authentication = "auth" @@ -41,14 +42,17 @@ public final class Token: DbCoreModel { /// User Id is missing case missingUserId + /// HTTP status public var status: HTTPStatus { return .preconditionFailed } + /// Error identifier public var identifier: String { return "token.missing_user_id" } + /// Reason for failure public var reason: String { return "User ID is missing" } @@ -70,7 +74,9 @@ public final class Token: DbCoreModel { /// Token expiry date public var expires: Date - public var type: TokenType + + /// Token type +// public var type: TokenType /// Initializer public init(token: Token, user: User) { @@ -78,7 +84,7 @@ public final class Token: DbCoreModel { self.user = User.Display(user) self.token = token.token self.expires = token.expires - self.type = token.type +// self.type = token.type } } @@ -94,19 +100,21 @@ public final class Token: DbCoreModel { public var expires: Date /// Token type - public var type: TokenType +// public var type: TokenType + /// Initializer public init(token: Token) { self.id = token.id self.token = token.token self.expires = token.expires - self.type = token.type +// self.type = token.type } } /// Displayable public object /// for security reasons, the original object should never be displayed public final class Public: DbCoreModel { + /// Object id public var id: DbCoreIdentifier? @@ -135,12 +143,9 @@ public final class Token: DbCoreModel { /// Token expiry date public var expires: Date - public var type: TokenType - /// Initializer - convenience init(user: User) throws { - try self.init(user: user, type: .authentication) - } + /// Token type +// public var type: TokenType /// Initializer init(user: User, type: TokenType) throws { @@ -148,9 +153,11 @@ public final class Token: DbCoreModel { throw Error.missingUserId } self.userId = userId - self.token = ":)" + let randData = try URandom().generateData(count: 60) + let rand = randData.base64EncodedString() + self.token = String(rand.prefix(60)) self.expires = Date().addMonth(n: 1) - self.type = type +// self.type = type } enum CodingKeys: String, CodingKey { @@ -158,7 +165,7 @@ public final class Token: DbCoreModel { case userId = "user_id" case token case expires - case type +// case type } } @@ -174,7 +181,7 @@ extension Token: Migration { schema.field(for: \.userId, type: .uuid, .notNull) schema.field(for: \.token, type: .varchar(64), .notNull) schema.field(for: \.expires, type: .timestamp, .notNull) - schema.field(for: \.type, type: .varchar(4), .notNull) +// schema.field(for: \.type, type: .varchar(4), .notNull) } } diff --git a/Sources/ApiCoreTestTools/Extensions/Application+Testable.swift b/Sources/ApiCoreTestTools/Extensions/Application+Testable.swift index 4f32017..a48c998 100644 --- a/Sources/ApiCoreTestTools/Extensions/Application+Testable.swift +++ b/Sources/ApiCoreTestTools/Extensions/Application+Testable.swift @@ -50,6 +50,11 @@ extension TestableProperty where TestableType: Application { try! ApiCoreBase.configure(&config, &env, &services) + // Check the database ... if it doesn't contain test then make sure we are not pointing to a production DB + if !ApiCoreBase.configuration.database.database.contains("test") { + ApiCoreBase.configuration.database.database = ApiCoreBase.configuration.database.database + "-test" + } + // Set mailer mock MailerMock(services: &services) diff --git a/Tests/ApiCoreTests/Controllers/UsersControllerTests.swift b/Tests/ApiCoreTests/Controllers/UsersControllerTests.swift index 62a4dd3..2af8f42 100644 --- a/Tests/ApiCoreTests/Controllers/UsersControllerTests.swift +++ b/Tests/ApiCoreTests/Controllers/UsersControllerTests.swift @@ -89,13 +89,13 @@ class UsersControllerTests: XCTestCase, UsersTestCase, LinuxTests { XCTAssertEqual(user.firstname, post.firstname, "Firstname doesn't match") XCTAssertEqual(user.lastname, post.lastname, "Lastname doesn't match") XCTAssertEqual(user.email, post.email, "Email doesn't match") - XCTAssertEqual(user.password, try! post.password.passwordHash(r.request), "Password doesn't match") + XCTAssertTrue(post.password.verify(against: user.password!), "Password doesn't match") XCTAssertEqual(user.disabled, false, "Disabled should be false") XCTAssertEqual(user.su, false, "SU should be false") // Test email has been sent (on a mock email client ... obviously) let mailer = try! r.request.make(MailerService.self) as! MailerMock - XCTAssertEqual(mailer.receivedMessage!.from, "ondrej.rafaj@gmail.com", "Email has a wrong sender") + XCTAssertEqual(mailer.receivedMessage!.from, "admin@apicore", "Email has a wrong sender") XCTAssertEqual(mailer.receivedMessage!.to, "lemmy@liveui.io", "Email has a wrong recipient") XCTAssertEqual(mailer.receivedMessage!.subject, "Registration", "Email has a wrong subject") @@ -103,13 +103,13 @@ class UsersControllerTests: XCTestCase, UsersTestCase, LinuxTests { XCTAssertEqual(mailer.receivedMessage!.text, """ Hi Lemmy Kilmister - Please confirm your email lemmy@liveui.io by clicking on this link http://www.liveui.io/fake_url + Please confirm your email lemmy@liveui.io by clicking on this link http://localhost:8080/users/verify?token=\(token) Verification code is: |\(token)| Boost team """, "Email has a wrong text") XCTAssertEqual(mailer.receivedMessage!.html, """

Hi Lemmy Kilmister

-

Please confirm your email lemmy@liveui.io by clicking on this link

+

Please confirm your email lemmy@liveui.io by clicking on this link

Verification code is: \(token)

Boost team

""", "Email has a wrong html") diff --git a/Tests/ApiCoreTests/Libs/StringCryptoTests.swift b/Tests/ApiCoreTests/Libs/StringCryptoTests.swift index 242cca9..a5e1585 100644 --- a/Tests/ApiCoreTests/Libs/StringCryptoTests.swift +++ b/Tests/ApiCoreTests/Libs/StringCryptoTests.swift @@ -37,7 +37,7 @@ final class StringCryptoTests : XCTestCase { func testPasswordHash() throws { let req = app.testable.fakeRequest() let hashed = try! "password".passwordHash(req) - XCTAssert(hashed == "password") + XCTAssertTrue("password".verify(against: hashed), "Hashed password is invalid") } }