8:01 a.m.
On Mon 05 May 2014 08:36:59 AM EDT, Corinne Krych wrote:
Hello Summers
First quick review, here are the feedback/questions:
1. add a cookbook recipe to help for demoing Android OAuth2. I've
successfully tested it using @secondsun 's demo [1]. It should be part
of android-cookbook with some readme instructions on how to fill
OAuth2 Google Drive config will help (See the one for iOS [2]). As
we're using public OAuth client, no client secret is required we could
used a pre-configured one like iOS and JS?
Probably true. I'll add a JIRA.
https://issues.jboss.org/browse/AGDROID-248 (Docs)
https://issues.jboss.org/browse/AGDROID-249 (App)
2. Android version is now definitively ahead of iOS one ;) as you’ve
implemented refresh token but configuration is very alike for the
naming etc… +1
3. I like AuthzSevice idea where we store the tokens for easier
automatic refresh. Most end-user app will ask for grant only once so
such a service that retieve and check validity of token is needed;
- But, what about making it configurable to leave the option to store
or not to store tokens?
Seems like it is somewhat related to this :
https://issues.jboss.org/browse/AGDROID-241
Perhaps the jira should be to make storage configurable. If we wanted
to explicitly NOT store then we could make a dummy Store which just
routed everything to /dev/null.
- The storage for refresh token should be more secure either
encrypted
storage with ag-crypto or keychain/keystore. wdyt?
See above
4. not sure about what is the purpose of AdditionalAuthorizationParams
in AuthzConfig?
So the OAuth2 spec isn't implemented very well. As an example
to get a
refresh token from google you have to pass the parameter "access_type"
with a value "offline". This is not part of the spec per se.
5. Obviously as you said more work need to be done for removing token,
Authorizer..
for iOs we have an epic AGIOS-188 [3] for all Oauth2 work. Checking
Android tickets, I was a bit surprised by AGDROID-244 and AGDROID-242,
does it mean support for OAuth?
I would like it. After the PR is merged passos and
I should have this
scheduled. I am sure these will not be a priority but it is a nice to have.
Good work! Need to look into AGIOS-145 refresh token and (newly
created) AGIOS-190 AuthzService to catch up with you guys.
++
Corinne
[1] https://github.com/secondsun/aerogear-android-oauth2-demo
[2]
https://github.com/aerogear/aerogear-ios-cookbook/blob/master/GoogleDrive...
[3] https://issues.jboss.org/browse/AGIOS-188
On 27 Apr 2014, at 08:55, Corinne Krych <corinnekrych(a)gmail.com> wrote:
>
> Yep same here i'd love to review it an compare with iOS version. I'll
> send feedback next week too.
> ++
> Corinne
>
> On Friday, April 25, 2014, Bruno Oliveira <bruno(a)abstractj.org> wrote:
> Hi Summers, not sure about the timing. But I would like to review on the
> next week.
>
> On 2014-04-24, Summers Pittman wrote:
>>
>>
>> https://github.com/aerogear/aerogear-android/pull/146
>>
>> This PR is 1) big and 2) incomplete
>> (https://issues.jboss.org/browse/AGDROID/component/12319553). However,
>> it represents a certain set of functionality and I want to get
>> feedback/cleanup/merge before I continue making it even bigger.
>>
>> I would be EXCITED if someone can review this monster. If it needs to
>> be cut up and submitted piecemeal to make it more digestible I will also
>> take feedback on how to do that.
>>
>> Summers
>>
>> --
>> Summers Pittman
>>>
>>>>
>>>> Phone:404 941 4698
>>>> Java is my crack.
>>>
>>
>>
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
> --
>
> abstractj
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev
_______________________________________________
aerogear-dev mailing list
aerogear-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev
--
Summers Pittman
>Phone:404 941 4698
>Java is my crack.