[aerogear-dev] Android OAuth2 PR

[Corinne Krych]

On 05 May 2014, at 17:56, Bruno Oliveira <bruno at abstractj.org> wrote:

> On 2014-05-05, Summers Pittman wrote:
>> On Mon 05 May 2014 08:36:59 AM EDT, Corinne Krych wrote:
>>> 
>>> 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.
> 
> What's the real need behind store those tokens?

It’s useful (more UX friendly I guess) to ask only once if you want to grant access to this third party app. It’s a common pattern in application like GoogleDrive.
I’ve seen app that stores those token in KeyChain

> If is really necessary,
> that's ok, otherwise let's do not store it. Offline, OAuth2 tokens are
> useless.
> 
> Or add an option to choose between store or not store like mentioned
> here.

+1
Definitively have it optional 

> 
>>> - The storage for refresh token should be more secure either encrypted
>>> storage with ag-crypto or keychain/keystore. wdyt?
>> See above
> 
> Encryption won't fix this, worse, will add an extra level of complexity
> to the application. Let's think about it, what do we need for
> encryption?
> 
> 1. Generate a private key
> 2. If it doesn't have a password is worthless (for local storage)
> 3. Alright, is password based, let's encrypt everything now
> 4. Now at every inclusion of tokens, you prompt users for the password
> 
> They would remember your name forever sometimes attached to bad names.
> IMO my suggestion is:
> 
> 1. Leave it as is and never allow to store in any kind of external storage
> 2. Do not store (+1)
> 2. Cache and destroy it when the application is closed
> 4. Use encryption and make the whole application more complex


what about storing them in Keychain/Keystore?


> 
>>> 
>>> 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/GoogleDrive.md#google-setup-optional
>>> [3] https://issues.jboss.org/browse/AGIOS-188
>>> 
>>> On 27 Apr 2014, at 08:55, Corinne Krych <corinnekrych at 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 at 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 at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>> 
>>>> 
>>>> --
>>>> 
>>>> abstractj
>>>> _______________________________________________
>>>> aerogear-dev mailing list
>>>> aerogear-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> aerogear-dev mailing list
>>> aerogear-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> 
>> 
>> 
>> --
>> Summers Pittman
>>>> Phone:404 941 4698
>>>> Java is my crack.
>> 
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> 
> --
> 
> abstractj
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev