On 11 Jun 2014, at 08:44, Yagyesh <yagyesh.agrawal(a)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-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