Code review - Issue 13632059: Issue 351: Reimplement OAuth2 - Step 2 (Auth PCL - only data types)https://codereview.appspot.com/2013-09-30T12:33:12+00:00rietveld
Message from unknown
2013-09-23T20:22:33+00:00peleyalurn:md5:e424ef74b570235551fbabe012322bbf
Message from unknown
2013-09-23T20:27:50+00:00peleyalurn:md5:9ca62e31e3705cbbb77d8175011d7978
Message from peleyal@google.com
2013-09-23T20:29:55+00:00peleyalurn:md5:994676ac3a89865b69e81a20fced74c6
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
Message from class@google.com
2013-09-24T17:25:58+00:00classurn:md5:9f76ae68ed0a0d608b9be86b9dd2ec07
Nothing major here, I added a few comments on places where it might be helpful to have JWT certificate validation (http://tools.ietf.org/html/draft-ietf-oauth-json-web-token-08#section-7) or helper functions to make this easier.
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#newcode54
Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:54: /// <summary>Authenticates asynchronously the specified user.</summary>
Reverse the starting words:
Asynchronously authenticates the specified user.
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode60
Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:60: /// The client secrets stream. The AuthorizationCodeFlow constructor is responsible for disposing the stream
The client secrets stream. The AuthorizationCodeFlow constructor is responsible for disposing the stream.
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 code initializer</param>
Can you add periods to the param names? It's particular, but for comments, I try to just make them sentences or relatively close. It also can help to make the comments more clear.
<param name="initializer">The authorization code initializer object.</param>
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/GoogleJsonWebSignature.cs
File Src/GoogleApis.Auth/GoogleJsonWebSignature.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/GoogleJsonWebSignature.cs#newcode9
Src/GoogleApis.Auth/GoogleJsonWebSignature.cs:9: /// Google JSON Web Signature as specified in https://developers.google.com/accounts/docs/OAuth2ServiceAccount.
Just as an FYI, this is where we need to have support for JWT certificate verification as done in this sample:
https://github.com/googleplus/gplus-verifytoken-csharp/blob/master/verifytoken.ashx.cs
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/JsonWebSignature.cs
File Src/GoogleApis.Auth/JsonWebSignature.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/JsonWebSignature.cs#newcode28
Src/GoogleApis.Auth/JsonWebSignature.cs:28: public class JsonWebSignature
Is validation of the JsonWebSignature included somewhere?
http://tools.ietf.org/html/draft-ietf-oauth-json-web-token-08#section-7
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs
File Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs#newcode26
Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs:26: /// OAuth 2.0 client secrets model as specified in "https://code.google.com/apis/console/".
Just a heads up, this is moving to https://cloud.google.com/console
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs
File Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs#newcode20
Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs:20: public static class GoogleAuthConsts
This would be a good place to store "postmessage" once we have a means of supporting it as a URI
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs
File Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs#newcode70
Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs:70: /// <summary>Constructs a new authorization request with the specified URI.</summary>
Can you add a <param> for the URL and add information on the encoding?
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis/Apis/Util/Utilities.cs
File Src/GoogleApis/Apis/Util/Utilities.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis/Apis/Util/Utilities.cs#newcode46
Src/GoogleApis/Apis/Util/Utilities.cs:46: internal static string ThrowIfNullOrEmpty(this string str, string paramName)
Why is string specified with the this declaration? Also, might be helpful to include the <returns> block.
Message from unknown
2013-09-24T20:50:19+00:00peleyalurn:md5:f464854e3c78d530b786befc2f543519
Message from unknown
2013-09-24T20:55:10+00:00peleyalurn:md5:ecba01cae6d2cd2c630bd60d29a738cc
Message from peleyal@google.com
2013-09-24T20:55:34+00:00peleyalurn:md5:5399df9bea02821c67b04f5a3e79ac54
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 a datatype.
I added it by mistake to this CL.
I'll apply your comments, but you will see them take affect in Step3.
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode54
Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:54: /// <summary>Authenticates asynchronously the specified user.</summary>
On 2013/09/24 17:25:58, class wrote:
> Reverse the starting words:
> Asynchronously authenticates the specified user.
Done.
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode60
Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:60: /// The client secrets stream. The AuthorizationCodeFlow constructor is responsible for disposing the stream
If you mean that I should add a '.' at the end, we don't do that for parameters.
Do you think that we should do it here?
On 2013/09/24 17:25:58, class wrote:
> The client secrets stream. The AuthorizationCodeFlow constructor is responsible
> for disposing the stream.
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 code initializer</param>
I don't know. Should we start doing so?
We will have a lot of code to change.
Maybe a different CL - https://code.google.com/p/google-api-dotnet-client/issues/detail?id=393
On 2013/09/24 17:25:58, class wrote:
> Can you add periods to the param names? It's particular, but for comments, I try
> to just make them sentences or relatively close. It also can help to make the
> comments more clear.
>
> <param name="initializer">The authorization code initializer object.</param>
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/GoogleJsonWebSignature.cs
File Src/GoogleApis.Auth/GoogleJsonWebSignature.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/GoogleJsonWebSignature.cs#newcode9
Src/GoogleApis.Auth/GoogleJsonWebSignature.cs:9: /// Google JSON Web Signature as specified in https://developers.google.com/accounts/docs/OAuth2ServiceAccount.
We definitely should do that, but it's not part of the data type.
That's step3 or even later on.
But we will have to implement that!
On 2013/09/24 17:25:58, class wrote:
> Just as an FYI, this is where we need to have support for JWT certificate
> verification as done in this sample:
>
> https://github.com/googleplus/gplus-verifytoken-csharp/blob/master/verifytoken.ashx.cs
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/JsonWebSignature.cs
File Src/GoogleApis.Auth/JsonWebSignature.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/JsonWebSignature.cs#newcode28
Src/GoogleApis.Auth/JsonWebSignature.cs:28: public class JsonWebSignature
TODO
On 2013/09/24 17:25:58, class wrote:
> Is validation of the JsonWebSignature included somewhere?
>
> http://tools.ietf.org/html/draft-ietf-oauth-json-web-token-08#section-7
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs
File Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs#newcode26
Src/GoogleApis.Auth/OAuth2/GoogleClientSecrets.cs:26: /// OAuth 2.0 client secrets model as specified in "https://code.google.com/apis/console/".
On 2013/09/24 17:25:58, class wrote:
> Just a heads up, this is moving to https://cloud.google.com/console
Done.
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs
File Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs#newcode20
Src/GoogleApis.Auth/OAuth2/GoogleConsts.cs:20: public static class GoogleAuthConsts
ACK
On 2013/09/24 17:25:58, class wrote:
> This would be a good place to store "postmessage" once we have a means of
> supporting it as a URI
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs
File Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs#newcode70
Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs:70: /// <summary>Constructs a new authorization request with the specified URI.</summary>
On 2013/09/24 17:25:58, class wrote:
> Can you add a <param> for the URL and add information on the encoding?
Done. Let me know if you want to add something specific?
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis/Apis/Util/Utilities.cs
File Src/GoogleApis/Apis/Util/Utilities.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis/Apis/Util/Utilities.cs#newcode46
Src/GoogleApis/Apis/Util/Utilities.cs:46: internal static string ThrowIfNullOrEmpty(this string str, string paramName)
Take a look in http://msdn.microsoft.com/en-us/library/vstudio/bb383977.aspx
You will be amazed what you can do with .NET in general :)
On 2013/09/24 17:25:58, class wrote:
> Why is string specified with the this declaration? Also, might be helpful to
> include the <returns> block.
Message from class@google.com
2013-09-24T21:47:43+00:00classurn:md5:f5310622c91431dfdf22021fd3eb509b
LGTM, consider adding the suggested text to the parameter on AuthorizationRequestUri if there's anything special about the encoding type.
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#newcode60
Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:60: /// The client secrets stream. The AuthorizationCodeFlow constructor is responsible for disposing the stream
I'm fine with either approach, so long as it's consistent.
On 2013/09/24 20:55:35, peleyal wrote:
> If you mean that I should add a '.' at the end, we don't do that for parameters.
> Do you think that we should do it here?
> On 2013/09/24 17:25:58, class wrote:
> > The client secrets stream. The AuthorizationCodeFlow constructor is
> responsible
> > for disposing the stream.
>
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 code initializer</param>
ACK, I think it's fine either way, so long as it's done consistently.
On 2013/09/24 20:55:35, peleyal wrote:
> I don't know. Should we start doing so?
> We will have a lot of code to change.
>
> Maybe a different CL -
> https://code.google.com/p/google-api-dotnet-client/issues/detail?id=393
>
> On 2013/09/24 17:25:58, class wrote:
> > Can you add periods to the param names? It's particular, but for comments, I
> try
> > to just make them sentences or relatively close. It also can help to make the
> > comments more clear.
> >
> > <param name="initializer">The authorization code initializer object.</param>
>
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs
File Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs#newcode70
Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs:70: /// <summary>Constructs a new authorization request with the specified URI.</summary>
Just the encoding type, e.g. is this just a URI or is there something special the developer needs to do?
On 2013/09/24 20:55:35, peleyal wrote:
> On 2013/09/24 17:25:58, class wrote:
> > Can you add a <param> for the URL and add information on the encoding?
>
> Done. Let me know if you want to add something specific?
Message from peleyal@google.com
2013-09-25T12:36:57+00:00peleyalurn:md5:a196a1f1e1defc45374b8a41f07e2431
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 code initializer</param>
That's the case.
Thanks :)
On 2013/09/24 21:47:44, class wrote:
> ACK, I think it's fine either way, so long as it's done consistently.
>
> On 2013/09/24 20:55:35, peleyal wrote:
> > I don't know. Should we start doing so?
> > We will have a lot of code to change.
> >
> > Maybe a different CL -
> > https://code.google.com/p/google-api-dotnet-client/issues/detail?id=393
> >
> > On 2013/09/24 17:25:58, class wrote:
> > > Can you add periods to the param names? It's particular, but for comments, I
> > try
> > > to just make them sentences or relatively close. It also can help to make
> the
> > > comments more clear.
> > >
> > > <param name="initializer">The authorization code initializer object.</param>
> >
>
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs
File Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs (right):
https://codereview.appspot.com/13632059/diff/3001/Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs#newcode70
Src/GoogleApis.Auth/OAuth2/Requests/AuthorizationRequestUrl.cs:70: /// <summary>Constructs a new authorization request with the specified URI.</summary>
I removed the encoded word from the URL name (the name was copied from JAVA, and it look irrelevant here).
Thanks
On 2013/09/24 21:47:44, class wrote:
> Just the encoding type, e.g. is this just a URI or is there something special
> the developer needs to do?
>
> On 2013/09/24 20:55:35, peleyal wrote:
> > On 2013/09/24 17:25:58, class wrote:
> > > Can you add a <param> for the URL and add information on the encoding?
> >
> > Done. Let me know if you want to add something specific?
>
Message from shai.raiten@gmail.com
2013-09-29T05:37:44+00:00shai.raiten_gmail.comurn:md5:b28d7d4096162381c07a023b661b5f2a
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),
*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);
*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
});
[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/13632059/>
>
> 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
>
>
>
Message from peleyal@google.com
2013-09-30T12:33:12+00:00peleyalurn:md5:976e073f50d24c0a45c6c35febb4647f
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/13632059/>
>>
>> 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
>>
>>
>>
>