[aerogear-dev] Observations on iOS OAuth

[Corinne Krych]

On 11 Jun 2014, at 08:44, Yagyesh <yagyesh.agrawal at itpeoplecorp.com> wrote:

> I have few observations & suggestions based on the time i spent with OAuth
> related code in Aerogear iOS library:
> 
> 1.
> Below is the code snippet from authz method of AGAuthorizer.m:
> ————————
> if(![authzConfig.type isEqualToString:@"AG_OAUTH2_FACEBOOK"] &&
> ![authzConfig.type isEqualToString:@"AG_OAUTH2"])
>        return nil;
> 
>    authzConfig.type = [self oauth2Type:authzConfig];
> ————————
> As ‘type’ property is exposed to developers & they may set it in their code,
> i feel changing it without giving any info/warning will not be appreciated
> by the developers. 
> 
> 2.
> Also, since ‘type’ is exposed, we can also completely eliminate the need for
> setting the correct type via oauth2type method. The list of supported types
> can be exposed as string constants or enum, & developers can choose to set
> the type based on their requirements.

Yep some thing to work a bit more. 
The idea is to have extensible type and let the user develop its own, similar to Android factory using class type. This is tracked by ticket https://issues.jboss.org/browse/AGIOS-154
Refactor will be done in next release.

> In authz method, we can simply assert
> with proper message if type is not set. In my opinion, oauth2type method is
> anyways not very robust in determining the correct type & may need to be
> changed constantly as we add support for more oauth providers. Also, for
> instance if facebook changes the authentication end point and access end
> point to use to a common base url, oauth2type method may fail in determining
> correct type. This is just an example and probably facebook may never change
> anything. But the point i'm trying to make is that allowing developers to
> set the type will avoid any issues like this.
> 
> 3.
> Similarly, setting default values in init method of AGAuthzConfiguration.m
> is also not very useful. As default values will not work if developers do
> not set all the relevant config properties. And since we are setting
> defaults, we could not assert to developers in case any mandatory property
> is not set. 
> 
> 4.
> If we do want to set defaults, we should have defaults specific to modules.
> For example, Facebook OAuth should have its own set of default values for
> authzEndPoint, accessTokenEndPoint, revokeEndPoint etc. As values for these
> properties may not change with applications, even if developers forget to
> set these properties things may just work for them out of the box in most
> cases.

Good point.
OAuth2 include a lot of configuration for each provider, writing my blog post, I thought it might just be easier to expose a Facebook Oauth configuration. Therefore the developer can only create an instance of fb config, set his app_id and secret. much shorter, and type will be inferred by the specific configuration.
Here is a JIRA to collect idea on Fb config:
https://issues.jboss.org/browse/AGIOS-205

> 
> 5. 
> We can have asserts for all mandatory properties that are not set.
> 

Let me create a JIRA for those configuration ideas:
https://issues.jboss.org/browse/AGIOS-204

This JIRA will be part of next release (1.7). 
Too short timing to include them in AeroGear iOS 1.6 is coming out end of this week early next week.

We welcome contributions, feel free to send us a PR, process on how to contribute are explained here:
http://aerogear.org/docs/guides/Contributing/

> Let me know what do you guys think about these.

Thanks for your feedback!
++
Corinne

> 
> 
> 
> --
> View this message in context: http://aerogear-dev.1069024.n5.nabble.com/Observations-on-iOS-OAuth-tp8135.html
> Sent from the aerogear-dev mailing list archive at Nabble.com.
> 
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev