[aerogear-dev] Observations on iOS OAuth

[Yagyesh]

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-tp8135.html
Sent from the aerogear-dev mailing list archive at Nabble.com.