-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: Multiple Type-Safe Authentication Methods #68
Conversation
Codecov Report
@@ Coverage Diff @@
## master #68 +/- ##
==========================================
+ Coverage 58.15% 66.89% +8.73%
==========================================
Files 6 7 +1
Lines 282 293 +11
==========================================
+ Hits 164 196 +32
+ Misses 118 97 -21
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I think we might want to tweak the example for init(successfulAuth:)
a bit (see comments below)
This initalizer creates an instance of the type conforming to `TypeSafeMultiCredentials` from a successfully authenticated `TypeSafeCredentials` instance. | ||
```swift | ||
### Usage Example: ### | ||
init(successfulAuth: TypeSafeCredentials) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be rewritten to initialize self.id
and self.provider
before the switch, and then initialize the more specific fields (in this case self.name
) within each case.
self.name = googleProfile.name | ||
default: | ||
self.id = successfulAuth.id | ||
self.provider = successfulAuth.provider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this example you are not initializing self.name
in the default case - I presume that means it's an optional which takes an implicit nil
value? It might be nicer for the example to explicitly set self.name = nil
in the default case.
README.md
Outdated
Within Codable routes, you implement a single credentials plugin by defining a Swift type that conforms to the plugins implementation of `TypeSafeCredentials`. This can then be applied to a codable route by defining it in the route signiture: | ||
|
||
```swift | ||
router.get("/authenticated") { (userProfile: ExampleAuth, respondWith: (ExampleAuth?, RequestError?) -> Void) in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be AuthedUser
(or SingleAuthedUser
) to be consistent with the multi-auth example below?
Description
Added a new protocol
TypeSafeMultiCredentials
. A user can create a final class or struct that conforms to this protocol to add authentication to a route with multiple authentication methods. The user defines an array ofTypeSafeCredentials
and then this class will iterate through the array trying to authenticate. When one the authentications is successful the users type will initialize an instance of itself which is passed to the route handler.Example of usage: