Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(431)

Issue 13632059: Issue 351: Reimplement OAuth2 - Step 2 (Auth PCL - only data types) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by peleyal
Modified:
11 years, 2 months ago
Reviewers:
class
CC:
shai.raiten_gmail.com, google-api-dotnet-client_googlegroups.com
Base URL:
https://google-api-dotnet-client.googlecode.com/hg/
Visibility:
Public.

Description

Issue 351: Reimplement OAuth2 - Step 2 (Auth PCL) Reimplement OAuth2 - Step 2 (Auth PCL) The new code of creating a service using OAuth2 will look like that: var credential = await GoogleWebAuthenticationBroker.AuthenticateAsync( new FileStream("client_secrets.json", FileMode.Open, FileAccess.Read), new[] { BooksService.Scopes.Books.GetStringValue() }, "user", CancellationToken.None).ConfigureAwait(false); // Create the service. var service = new BooksService(new BaseClientService.Initializer() { ApplicationName = "Books API Sample", HttpClientInitializer = credential });

Patch Set 1 #

Patch Set 2 : upload only data types and interfaces #

Total comments: 24

Patch Set 3 : Gus comments #

Patch Set 4 : minor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1843 lines, -2 lines) Patch
A Src/GoogleApis.Auth.DotNet4/GoogleApis.Auth.DotNet4.csproj View 1 chunk +115 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.DotNet4/OAuth2/LocalServerCodeReceiver.cs View 1 2 1 chunk +125 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.DotNet4/OAuth2/PromptCodeReceiver.cs View 1 chunk +63 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.DotNet4/Properties/AssemblyInfo.cs View 1 chunk +40 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.DotNet4/packages.config View 1 chunk +8 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/GoogleApis.Auth.csproj View 1 chunk +119 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/GoogleJsonWebSignature.cs View 1 2 1 chunk +42 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/JsonWebSignature.cs View 1 2 1 chunk +103 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/JsonWebToken.cs View 1 2 1 chunk +127 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/BearerToken.cs View 1 chunk +90 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/ClientSecrets.cs View 1 chunk +30 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs View 1 2 1 chunk +58 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs View 1 chunk +37 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/IAccessMethod.cs View 1 chunk +25 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/IAuthorizationCodeFlow.cs View 1 chunk +70 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/IAuthorizationCodeInstalledApp.cs View 1 chunk +39 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/ICodeReceiver.cs View 1 chunk +38 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationCodeRequestUrl.cs View 1 chunk +54 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationCodeTokenRequest.cs View 1 chunk +45 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs View 1 2 1 chunk +77 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/GoogleAssertionTokenRequest.cs View 3 1 chunk +39 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/GoogleAuthorizationCodeRequestUrl.cs View 1 chunk +61 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/RefreshTokenRequest.cs View 1 chunk +37 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Requests/TokenRequest.cs View 1 chunk +50 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Responses/AuthorizationCodeResponseUrl.cs View 1 chunk +107 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Responses/TokenErrorResponse.cs View 1 chunk +63 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Responses/TokenResponse.cs View 1 chunk +93 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/Responses/TokenResponseException.cs View 1 chunk +37 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/Properties/AssemblyInfo.cs View 1 chunk +38 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/packages.config View 1 chunk +8 lines, -0 lines 0 comments Download
M Src/GoogleApis.DotNet4/Apis/Util/Store/FileDataStore.cs View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Src/GoogleApis/Apis/Util/Utilities.cs View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M Src/GoogleApis/Properties/AssemblyInfo.cs View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7
peleyal
Hi Gus, I separate all the OAuth2 data types into one CL. The next CL ...
11 years, 2 months ago (2013-09-23 20:29:55 UTC) #1
class
Nothing major here, I added a few comments on places where it might be helpful ...
11 years, 2 months ago (2013-09-24 17:25:58 UTC) #2
peleyal
Thanks for the quick review! https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs File Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs (right): https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode3 Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:3: This file is not ...
11 years, 2 months ago (2013-09-24 20:55:34 UTC) #3
class
LGTM, consider adding the suggested text to the parameter on AuthorizationRequestUri if there's anything special ...
11 years, 2 months ago (2013-09-24 21:47:43 UTC) #4
peleyal
Thanks. I just pushed it https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs File Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs (right): https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode79 Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:79: /// <param name="initializer">The authorization ...
11 years, 2 months ago (2013-09-25 12:36:57 UTC) #5
shai.raiten_gmail.com
Hi Eyal, *client_secrets.json* ** Regarding the *client_secrets.json,* as we talked before I prefer hiding token, ...
11 years, 2 months ago (2013-09-29 05:37:44 UTC) #6
peleyal
11 years, 2 months ago (2013-09-30 12:33:12 UTC) #7
Thanks for the review!
See my comments inline.


On Sun, Sep 29, 2013 at 1:37 AM, Shai Raiten <shai.raiten@gmail.com> wrote:

> Hi Eyal,
>
> *client_secrets.json*
> **
> Regarding the *client_secrets.json,* as we talked before I prefer hiding
> token, secret as much as possible and this means not adding the *
> client_secrets.json* (Plain Text) but adding those values directly to the
> code. Yes, you can decompile but this require more then just opening the *
> client_secrets.json* file.
> Furthermore this is a "standard" in all integration apis (Facebook,
> Dropbox etc).
>
>  var credential = await GoogleWebAuthenticationBroker.AuthenticateAsync(
>                 new FileStream("client_secrets.json", FileMode.Open,
> FileAccess.Read),
>
> We have two option one of them is mentioned above the other one is the
following:
 var credential = await GoogleWebAuthenticationBroker.AuthenticateAsync(
  new ClientSecrets
  {
      ClientId = ...,
      ClientSecret = ...
  },

>
> *BooksService.Scopes.Books*
> **
> Why Scopes has another namespace called Books? Is there more than just
> Books?
>
>                 new[] { BooksService.Scopes.Books.GetStringValue() },
>                 "user", CancellationToken.None).ConfigureAwait(false);
>
Yes. Books is a scope. There can more than one scope per service. For
example, drive contains many scopes (
https://www.googleapis.com/discovery/v1/apis/drive/v2/rest?fields=auth).

> *Enforce Desire Usage*
>
> If HttpClientInitializer & ApplicationName aren't part of the service
> constructor developers might ignore (Application Name) those values. You do
> want the to monitor the different applications using GoogleAPI.
>
>             // Create the service.
>             var service = new BooksService(new
> BaseClientService.Initializer()
>             {
>                 ApplicationName = "Books API Sample",
>                 HttpClientInitializer = credential
>             });
>
I agree with you, BUT I don't want to break existing clients. That's the
only reason why I'm keeping this as an optional parameter.

Thanks again!
Eyal


>
>
> [image: logo]
> *Shai Raiten** VS ALM MVP & Senior Architect**, Sela International*
> | Mobile: 972 54 3050817
> http://blogs.microsoft.co.il/Blogs/shair/
> [image: Facebook] <http://www.facebook.com/shai.raiten> [image:
Twitter]<https://twitter.com/#!/shai_rai>
>  [image: LinkedIn] <http://il.linkedin.com/in/shairai>
> Contact me: [image: Skype] shai_rai
>
>
> On Mon, Sep 23, 2013 at 11:29 PM, <peleyal@google.com> wrote:
>
>> Reviewers: class,
>>
>> Message:
>> Hi Gus,
>>
>> I separate all the OAuth2 data types into one CL.
>> The next CL will include all the logic. This is only data types.. :)
>>
>> Let me know if you want me to upload the next chunk with this CL as
>> well.
>>
>> THANKS :)
>> Eyal
>>
>> Description:
>> Issue 351: Reimplement OAuth2 - Step 2 (Auth PCL)
>>
>> Reimplement OAuth2 - Step 2 (Auth PCL)
>>
>> The new code of creating a service using OAuth2 will look like that:
>>
>>  var credential = await GoogleWebAuthenticationBroker.**
>> AuthenticateAsync(
>>                 new FileStream("client_secrets.**json", FileMode.Open,
>> FileAccess.Read),
>>                 new[] { BooksService.Scopes.Books.**GetStringValue() },
>>                 "user", CancellationToken.None).**ConfigureAwait(false);
>>
>>             // Create the service.
>>             var service = new BooksService(new
>> BaseClientService.Initializer(**)
>>             {
>>                 ApplicationName = "Books API Sample",
>>                 HttpClientInitializer = credential
>>             });
>>
>> Please review this at
https://codereview.appspot.**com/13632059/<https://codereview.appspot.com/136...
>>
>> Affected files (+1931, -1 lines):
>>   A Src/GoogleApis.Auth.DotNet4/**GoogleApis.Auth.DotNet4.csproj
>>   A Src/GoogleApis.Auth.DotNet4/**OAuth2/**GoogleWebAuthenticationBroker.
>> **cs
>>   A Src/GoogleApis.Auth.DotNet4/**OAuth2/**LocalServerCodeReceiver.cs
>>   A Src/GoogleApis.Auth.DotNet4/**OAuth2/PromptCodeReceiver.cs
>>   A Src/GoogleApis.Auth.DotNet4/**Properties/AssemblyInfo.cs
>>   A Src/GoogleApis.Auth.DotNet4/**packages.config
>>   A Src/GoogleApis.Auth/**GoogleApis.Auth.csproj
>>   A Src/GoogleApis.Auth/**GoogleJsonWebSignature.cs
>>   A Src/GoogleApis.Auth/**JsonWebSignature.cs
>>   A Src/GoogleApis.Auth/**JsonWebToken.cs
>>   A Src/GoogleApis.Auth/OAuth2/**BearerToken.cs
>>   A Src/GoogleApis.Auth/OAuth2/**ClientSecrets.cs
>>   A Src/GoogleApis.Auth/OAuth2/**GoogleClientSecrets.cs
>>   A Src/GoogleApis.Auth/OAuth2/**GoogleConsts.cs
>>   A Src/GoogleApis.Auth/OAuth2/**IAccessMethod.cs
>>   A Src/GoogleApis.Auth/OAuth2/**IAuthorizationCodeFlow.cs
>>   A Src/GoogleApis.Auth/OAuth2/**IAuthorizationCodeInstalledApp**.cs
>>   A Src/GoogleApis.Auth/OAuth2/**ICodeReceiver.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/**
>> AuthorizationCodeRequestUrl.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/**
>> AuthorizationCodeTokenRequest.**cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/**AuthorizationRequestUrl.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/**
>> GoogleAssertionTokenRequest.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/**
>> GoogleAuthorizationCodeRequest**Url.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/RefreshTokenRequest.**cs
>>   A Src/GoogleApis.Auth/OAuth2/**Requests/TokenRequest.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Responses/**
>> AuthorizationCodeResponseUrl.**cs
>>   A Src/GoogleApis.Auth/OAuth2/**Responses/TokenErrorResponse.**cs
>>   A Src/GoogleApis.Auth/OAuth2/**Responses/TokenResponse.cs
>>   A Src/GoogleApis.Auth/OAuth2/**Responses/**TokenResponseException.cs
>>   A Src/GoogleApis.Auth/**Properties/AssemblyInfo.cs
>>   A Src/GoogleApis.Auth/packages.**config
>>   M Src/GoogleApis/Apis/Util/**Utilities.cs
>>   M Src/GoogleApis/Properties/**AssemblyInfo.cs
>>
>>
>>
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b