Re: [aerogear-dev] Observations on iOS OAuth

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.

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.

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

Let me know what do you guys think about these.

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