feat: Add basic, google token and facebook token authentication#33
feat: Add basic, google token and facebook token authentication#33
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #33 +/- ##
===========================================
+ Coverage 80.15% 84.61% +4.46%
===========================================
Files 2 3 +1
Lines 131 169 +38
===========================================
+ Hits 105 143 +38
Misses 26 26
Continue to review full report at Codecov.
|
Sources/KituraKit/AuthHeaders.swift
Outdated
|
|
||
| /// Generate headers for a authenticating with a Facebook oauth token | ||
| /// - Parameter token: The Facebook oauth token | ||
| public static func facebookToken(_ token: String) -> [String: String] { |
There was a problem hiding this comment.
getFacebookAuthHeaders(token:)?
Sources/KituraKit/AuthHeaders.swift
Outdated
|
|
||
| /// Generate headers for a authenticating with a Google oauth token | ||
| /// - Parameter token: The Google oauth token | ||
| public static func googleToken(_ token: String) -> [String: String] { |
There was a problem hiding this comment.
getGoogleAuthHeaders(token:)?
Sources/KituraKit/AuthHeaders.swift
Outdated
| /// Generate headers for a authenticating with a username and password using HTTP basic | ||
| /// - Parameter username: The unique user id that identifies the user | ||
| /// - Parameter password: The password for the given username | ||
| public static func HTTPBasic(username: String, password: String) -> [String: String] { |
There was a problem hiding this comment.
getHTTPBasicAuthHeaders(username:password:)?
Sources/KituraKit/AuthHeaders.swift
Outdated
| /// Generate headers for a authenticating with a Facebook oauth token | ||
| /// - Parameter token: The Facebook oauth token | ||
| public static func facebookToken(_ token: String) -> [String: String] { | ||
| return(["X-token-type": "FacebookToken", "access_token": token]) |
Sources/KituraKit/AuthHeaders.swift
Outdated
| /// Generate headers for a authenticating with a Google oauth token | ||
| /// - Parameter token: The Google oauth token | ||
| public static func googleToken(_ token: String) -> [String: String] { | ||
| return(["X-token-type": "GoogleToken", "access_token": token]) |
Sources/KituraKit/AuthHeaders.swift
Outdated
| public static func HTTPBasic(username: String, password: String) -> [String: String] { | ||
| let authData = (username + ":" + password).data(using: .utf8)! | ||
| let authString = authData.base64EncodedString() | ||
| return(["Authorization": "Basic \(authString)"]) |
Sources/KituraKit/AuthHeaders.swift
Outdated
| import SwiftyRequest | ||
| import KituraContracts | ||
|
|
||
| public struct AuthHeaders { |
There was a problem hiding this comment.
I think this might be better called AuthHelpers
Sources/KituraKit/AuthHelpers.swift
Outdated
| * limitations under the License. | ||
| **/ | ||
|
|
||
| import Foundation |
There was a problem hiding this comment.
Remove this, it's a duplicate. And are all the others actually used?
djones6
left a comment
There was a problem hiding this comment.
I'd prefer us to have a protocol ClientAuthentication which defines something like func getHeaders() -> [String: String].
Then concrete implementations for each type (GoogleToken, FacebookToken, HTTPBasic) which init either from a token or a username and password. Then the user passes an instance of one of these types to an authentication: ClientAuthentication param on the client.
You could still have the default auth as an optional ClientAuthentication.
There was a problem hiding this comment.
The refactor looks much nicer, I like this. Two points:
- Rename to get rid of the redundant
Auth/ remove abbreviation as per comments above - All of the public entities should be documented (the
getHeaders()and especially theClientAuthenticationprotocol itself).
Actually, as discussed, let's go with ClientCredentials, defaultCredentials and credentials: as 'credentials' represents the data, 'authentication' represents the process.
| /// Generate headers for a authenticating with a username and password using HTTP basic | ||
| /// - Parameter username: The unique user id that identifies the user | ||
| /// - Parameter password: The password for the given username | ||
| public struct HTTPBasicAuth: ClientAuth { |
There was a problem hiding this comment.
Just HTTPBasic, I don't like the abbreviation. Since the type implements ClientAuthentication and the creation context is in an authentication: parameter or defaultAuthentication property, including 'Authentication' in the name is a bit redundant.
|
|
||
| /// Generate headers for a authenticating with a Facebook oauth token | ||
| /// - Parameter token: The Facebook oauth token | ||
| public struct FacebookTokenAuth: ClientAuth { |
|
|
||
| /// Generate headers for a authenticating with a Google oauth token | ||
| /// - Parameter token: The Google oauth token | ||
| public struct GoogleTokenAuth: ClientAuth { |
| } | ||
| } | ||
|
|
||
| public protocol ClientAuth { |
There was a problem hiding this comment.
ClientAuthentication please
Sources/KituraKit/Client.swift
Outdated
| public let baseURL: URL | ||
|
|
||
| /// Headers that will be used if none are provided to the route | ||
| public static var defaultAuth: ClientAuth? |
There was a problem hiding this comment.
defaultAuthentication (no need to abbreviate)
| */ | ||
| public protocol ClientCredentials { | ||
|
|
||
| /// Function to generate headers that will be added to the |
There was a problem hiding this comment.
Nit-pick: the function doesn't have to generate the headers, they could have been generated at initialization time, in which case this would be a simple getter. However, I think this is close enough.
djones6
left a comment
There was a problem hiding this comment.
Public initializers required, otherwise importers of KituraKit cannot create the credentials types.
| } | ||
| ``` | ||
| */ | ||
| public struct HTTPBasic: ClientCredentials { |
There was a problem hiding this comment.
This struct needs a public initializer. The one that the compiler generates is internal (so importers of KituraKit cannot instantiate an HTTPBasic).
| } | ||
| ``` | ||
| */ | ||
| public struct FacebookToken: ClientCredentials { |
| } | ||
| ``` | ||
| */ | ||
| public struct GoogleToken: ClientCredentials { |
Sources/KituraKit/Client.swift
Outdated
| /// let client = KituraKit.default | ||
| /// client.defaultCredentials = HTTPBasic(username: "John", password: "12345") | ||
| /// ``` | ||
| public static var defaultCredentials: ClientCredentials? |
There was a problem hiding this comment.
This (probably) shouldn't be static?
The example doesn't compile (has to say KituraKit.defaultCredentials), at which point all clients would share the same default credentials.
| } | ||
| ``` | ||
| */ | ||
| public struct NilCredentials: ClientCredentials { |
There was a problem hiding this comment.
Does this need a public initializer?
There was a problem hiding this comment.
Hang on... it has one already (line 181)
|
LGTM |
Pull request to add client side authentication to KituraKIt
The user adds their authentication to the client using either:
client.addBasicAuth(username: username, password: password)
client.addFacebookToken(token)
client.addGoogleToken(token)
The correct headers will then be added to all requests to allow authentication with the corresponding TypeSafeMiddlewares on Kitura.
This required internal fields for the request headers and Credentials be added to KituraKit and these are then added to the client request before it is sent.
Docs and tests have been added to cover these changes.