stormpath / Turnstile

An authentication framework for Swift.
Apache License 2.0
165 stars 29 forks source link

Add docs for Digits #23

Closed kdawgwilk closed 7 years ago

kdawgwilk commented 7 years ago

I tried to keep things as loosely tied to Vapor as possible when explaining it but I wasn't even thinking outside of the Vapor framework as I implemented it so if there are things that need to change I would be happy to submit another PR

codecov-io commented 7 years ago

Codecov Report

Merging #23 into master will decrease coverage by -0.33%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
- Coverage   74.66%   74.34%   -0.33%     
==========================================
  Files          33       33              
  Lines        1129     1138       +9     
==========================================
+ Hits          843      846       +3     
- Misses        286      292       +6
Impacted Files Coverage Δ
Sources/TurnstileWeb/LoginProviders/Facebook.swift 56.41% <0%> (-1.49%) :x:
Sources/TurnstileWeb/LoginProviders/Google.swift 7.89% <0%> (+7.89%) :white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update da08879...0c13618. Read the comment docs.

edjiang commented 7 years ago

This looks great!

Two things we should investigate, though:

  1. You mention Twitter in the README. We should create a dedicated Twitter realm, then, or remove mention for now.
  2. Yes, the code is Vapor-specific =] Since the frameworks are all a bit different, I'd recommend outlining the general steps that the auth flow takes, and the developer action that is needed.

I can revise this too, if you'd like!

kdawgwilk commented 7 years ago

I am not sure what the future of Digits is going to be after the Google acquisition of Fabric tools. Is this still worth including into Turnstile?

edjiang commented 7 years ago

I'm fairly confident it will be rolled into Firebase Auth at worst; if that's the case it might switch to an OAuth 2 API with the OAuth 1.0a version deprecated. I'm not worried right now!

edjiang commented 7 years ago

Sorry this has taken so long; we should get the stuff merged. I'm busy for the next two weeks, but can start working on this again after then =]

kdawgwilk commented 7 years ago

I am going to have some time this weekend to get some of this moving forward. I haven't been working with my Vapor stuff much since I finished my iOS class last semester but I really want to get back and finish things up that I had started, including this.

kdawgwilk commented 7 years ago

I removed the mentions of Twitter since that integration has not been created yet and will add it back in once that happens. This should be good to merge

edjiang commented 7 years ago

Awesome. I still have merge power, but just FYI I am no longer working for Stormpath. There's one more mention of Twitter that I'll remove, but let me double check with @omgitstom that it's OK for me to touch this repo :)