-
Notifications
You must be signed in to change notification settings - Fork 135
Would be a lot better if you create a SimpleAuthUser #2
Comments
I thought about doing that but the responses from every possible provider are very different so I couldn't cover them all from SimpleAuth itself, given that all providers will be their own repositories. The decentralized design makes this difficult. |
I thought about it so. But then why not we have a result block of: typedef void (^SimpleAuthUserResultHandler)(SimpleAuthUser *user, NSError *error); and user will have 3-4 properties:
by using that result handler block you don't have to change the result handler blocks in future. you can just add remove properties of SimpleAuthUser, once most of the providers are implemented. I would advise this approach, because I see many repositories that can't implement the parameters, blocks in future releases, bcoz they were using different patterns in previous releases. |
The problem is that Facebook uses a token and an expiration date, Twitter uses a token and a secret, Instagram uses just a token, ... Most providers return completely different data. Some might have an email and password and no token at all. Trying to come up with a single concrete class for every provider to use would be messy I think. To put it another way, there isn't anything that a dictionary can't store that a class can, which I think is the best level of abstraction between a provider and an API consumer. |
ok understood. but at least there should be some similar pattern for dictionary. Take a look at omniauth-facebook hash at https://github.com/mkdynamic/omniauth-facebook#auth-hash. Because in current SimpleAuth-Facebook you can't get a oauth_token. I would say that return a dictionary but filling it manually based on a common pattern. |
Yeah I have an open issue on the Facebook provider where it currently doesn't return the token as it should. |
I merged that PR on the Facebook provider. You ok if I close this issue now? |
Well in my project I am building a dictionary based on OmniAuth pattern, if you think it's useful then you can go something along this way. or you can close the issue.
and in authorize
Well I am doing this becuase my app is in Ruby on Rails and I am using omniauth, so my Website and iOS auth share same pattern. It's totally upto you. |
BTW same goes for twitter, because in current SimpleAuth-Twitter, the only information it's returning is tokens, screen_name and user_id. I needed more information like Full Name, Picture etc. So made another request and fill the dictionary based on that. |
Ok I totally see what you're going for now. I'll start working on this now, beginning with Facebook. I'll also add profile fetching to the Twitter provider. I also created a google provider repo for you and gave you push access. Let me know if you need anything else there! |
@calebd Also it would be a lot better that you create a NSObject for SimpleAuthUser and fill the necessary properties in that. So in case someone requires a Facebook Access Token, expires at etc. it can be filled in the SimpleAuthUser class. What do you think?
The text was updated successfully, but these errors were encountered: