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

Issue 13972043: Issue 351: Reimplement OAuth2 (Step 3 - Tests, Flows and Credential) (Closed)

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

Description

Issue 351: Reimplement OAuth2 Step 3

Patch Set 1 #

Patch Set 2 : minor #

Patch Set 3 : minor #

Total comments: 55

Patch Set 4 : Jon Skeet review #

Patch Set 5 : minor #

Total comments: 3

Patch Set 6 : 2nd round #

Total comments: 23

Patch Set 7 : Gus review #

Patch Set 8 : minor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2173 lines, -46 lines) Patch
M Src/GoogleApis.Auth.DotNet4/GoogleApis.Auth.DotNet4.csproj View 1 1 chunk +1 line, -0 lines 0 comments Download
A Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs View 1 2 3 4 5 6 1 chunk +104 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/GoogleApis.Auth.Tests.csproj View 1 chunk +124 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/AuthorizationCodeFlowTests.cs View 1 2 3 4 5 6 1 chunk +425 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/BearerTokenTests.cs View 1 chunk +102 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Requests/AuthorizationCodeRequestUrlTests.cs View 1 2 3 4 5 6 7 1 chunk +82 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Requests/AuthorizationCodeTokenRequestTests.cs View 1 2 3 4 5 6 1 chunk +44 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Requests/GoogleAuthorizationCodeRequestUrlTest.cs View 1 2 3 4 5 6 1 chunk +49 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Requests/RefreshTokenRequestTests.cs View 1 2 3 4 5 6 1 chunk +44 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Responses/AuthorizationCodeResponseUrlTests.cs View 1 2 3 4 5 6 1 chunk +77 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/OAuth2/Responses/TokenResponseTests.cs View 1 2 3 4 5 6 1 chunk +109 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/Properties/AssemblyInfo.cs View 1 2 3 4 5 6 1 chunk +40 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth.Tests/packages.config View 1 chunk +10 lines, -0 lines 0 comments Download
M Src/GoogleApis.Auth/GoogleApis.Auth.csproj View 1 2 3 4 5 6 1 chunk +6 lines, -1 line 0 comments Download
A Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs View 1 2 3 4 5 6 1 chunk +314 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/AuthorizationCodeInstalledApp.cs View 1 2 3 4 5 6 7 1 chunk +95 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/AuthorizationCodeWebApp.cs View 1 2 3 4 5 6 7 1 chunk +127 lines, -0 lines 0 comments Download
M Src/GoogleApis.Auth/OAuth2/BearerToken.cs View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M Src/GoogleApis.Auth/OAuth2/Credential.cs View 1 2 3 4 5 1 chunk +116 lines, -4 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/GoogleAuthorizationCodeFlow.cs View 1 1 chunk +58 lines, -0 lines 0 comments Download
M Src/GoogleApis.Auth/OAuth2/IAuthorizationCodeFlow.cs View 1 2 3 4 5 6 1 chunk +9 lines, -8 lines 0 comments Download
M Src/GoogleApis.Auth/OAuth2/IAuthorizationCodeInstalledApp.cs View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Src/GoogleApis.Auth/OAuth2/Requests/GoogleAssertionTokenRequest.cs View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Src/GoogleApis.Auth/OAuth2/Responses/TokenResponse.cs View 1 2 3 4 5 1 chunk +1 line, -18 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/TokenRequestExtenstions.cs View 1 2 3 4 5 6 7 1 chunk +66 lines, -0 lines 0 comments Download
A Src/GoogleApis.Auth/OAuth2/UserCredential.cs View 1 2 3 4 5 6 1 chunk +140 lines, -0 lines 0 comments Download
M Src/GoogleApis.Auth/Properties/AssemblyInfo.cs View 1 2 3 4 5 6 2 chunks +2 lines, -1 line 0 comments Download
M Src/GoogleApis/Apis/Util/IClock.cs View 1 2 3 4 2 chunks +11 lines, -0 lines 0 comments Download
M Src/GoogleApis/Apis/Util/Store/IDataStore.cs View 1 2 3 1 chunk +9 lines, -9 lines 0 comments Download
M Src/GoogleApis/Apis/Util/Utilities.cs View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M Src/GoogleApis/Properties/AssemblyInfo.cs View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 9
peleyal
This CL is based on the two latest CLs which were already committed https://codereview.appspot.com/13412046/ https://codereview.appspot.com/13632059/ ...
5 years, 9 months ago (2013-09-26 20:51:00 UTC) #1
jonskeet
I haven't got my head round the whole flow yet, so these are more stylistic ...
5 years, 9 months ago (2013-09-27 06:15:54 UTC) #2
peleyal
Thanks Jon! Looking forward your second round. Eyal https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs File Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs (right): https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs#newcode51 Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs:51: public ...
5 years, 9 months ago (2013-09-27 14:32:33 UTC) #3
jonskeet
A few more comments. https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs File Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs (right): https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs#newcode53 Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs:53: /// <summary> Have you considered ...
5 years, 9 months ago (2013-09-27 15:15:25 UTC) #4
peleyal
Thanks again :) Read for last (hopefully) review https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs File Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs (right): https://codereview.appspot.com/13972043/diff/23001/Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs#newcode53 Src/GoogleApis.Auth/OAuth2/AuthorizationCodeFlow.cs:53: /// ...
5 years, 9 months ago (2013-09-27 18:41:48 UTC) #5
peleyal
+shai.raiten@gmail.com
5 years, 9 months ago (2013-10-01 18:42:00 UTC) #6
class
Minor comments, mostly optional. https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs File Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs (right): https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode59 Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:59: /// The client secrets stream. ...
5 years, 9 months ago (2013-10-02 17:37:17 UTC) #7
peleyal
https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs File Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs (right): https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs#newcode59 Src/GoogleApis.Auth.DotNet4/OAuth2/GoogleWebAuthenticationBroker.cs:59: /// The client secrets stream. The authorization code flow ...
5 years, 9 months ago (2013-10-02 18:24:45 UTC) #8
class
5 years, 9 months ago (2013-10-02 20:17:03 UTC) #9
LGTM

https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth/OAuth2...
File Src/GoogleApis.Auth/OAuth2/AuthorizationCodeInstalledApp.cs (right):

https://codereview.appspot.com/13972043/diff/60001/Src/GoogleApis.Auth/OAuth2...
Src/GoogleApis.Auth/OAuth2/AuthorizationCodeInstalledApp.cs:65: // to retrieve a
new access token.
SGTM.
On 2013/10/02 18:24:45, peleyal wrote:
> You mean what happen if the token itself is revoke (access and refresh).
> In that case the first call will fail, and as a result the datastore will
clean
> the current token.
> Next call we will start the authorization code flow again.
> On 2013/10/02 17:37:18, class wrote:
> > Should this be "Retrieve a refresh token and/or access token from the code
> > exchange flow."? Also, what happens if the client is revoked? E.g. we have a
> > refresh token but the access token is expired.
>
Sign in to reply to this message.

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