Implemented in-memory TokenStore and added opportunity to save user credentials into file #129

YuryBandarchuk16 · 2017-09-14T11:03:05Z

Firstly, I implemented in-memory TokenStore by just using HashMap.

After that I added some methods that allows to save UserCredentials to file.

Everything, that might be affected by these changes have been tests in appropriate test files.

… save current credentials in UserCredentials.java

tswast · 2017-09-14T16:27:40Z

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

@@ -37,8 +37,7 @@
 import com.google.api.client.util.Preconditions;
 import com.google.auth.http.HttpTransportFactory;

-import java.io.IOException;
-import java.io.InputStream;
+import java.io.*;


oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

@@ -166,6 +165,24 @@ public static GoogleCredentials fromStream(InputStream credentialsStream,
  }

  /**
+   * Saves the end user credentials in specified file


oauth2_http/java/com/google/auth/oauth2/UserCredentials.java

@@ -58,6 +58,8 @@
 */
 public class UserCredentials extends GoogleCredentials {

+  public static final String USER_CREDENTIALS_FILE_NAME = "GOOGLE_AUTH_USER_CREDENTIALS_FILE_NAME";


lesv

The TokenStore is only used for testing. I don't see much advantage to this change.

YuryBandarchuk16 · 2017-09-14T17:12:32Z

@lesv Updated UserAuthorized, so that now it uses MemoryTokensStorage as default one if TokenStore is not specified.

lesv · 2017-09-14T17:16:02Z

Cool - Good to mention issues in the initial commit as then I can come up to speed on the PR.

I'm going to wait for @garrettjonesgoogle to comment before approving this. But I'm happy with the changes.

I did as a Q, in GoogleCredentials.java, what kind of credentials are you writing?

YuryBandarchuk16 · 2017-09-14T17:25:34Z

@lesv In GoogleCredentials.java I am saving the following credentials (updated comment above the method):

RefreshToken
ClientId
ClientSecret
ServerTokenUri

As in the issue, the main purpose is to store the refresh token.

Removed test, since there is always TokenStore exist by default.

YuryBandarchuk16 · 2017-09-15T09:45:30Z

@lesv In the latest test run there was a failure, because the test checked the following: in case TokenStore is not specified and you tried to get data from it, it will throw an exception. Right now, there is always a default implementation exists for TokenStore. It means that we don't need this test anymore. I removed it in this commit: 5fcddeb

lesv · 2017-09-15T15:30:00Z

@YuryBandarchuk16 - @garrettjonesgoogle is out until at least next week. Do you need this merged and released soon, or can hold it for a bit?

YuryBandarchuk16 · 2017-09-15T15:33:36Z

@lesv Sure, can hold, there is no rush.

YuryBandarchuk16 · 2017-09-25T17:01:28Z

@lesv are there any updates?

lesv · 2017-09-25T21:36:01Z

@garrettjonesgoogle Do you have an opinion? It seems like a reasonable change to me.

garrettjonesgoogle · 2017-09-27T16:48:49Z

I will take a look.

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

+   * @param credentialsFileName Name of file where to store the credentials
+   * @throws IOException An error saving the credentials.
+   */
+  public void saveCredentialsToFile(InputStream credentials, String credentialsFileName) throws IOException {


oauth2_http/javatests/com/google/auth/oauth2/UserCredentialsTest.java

-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
+import java.io.*;


lesv · 2017-11-02T19:40:33Z

@YuryBandarchuk16 Could you address Garrett's asks - we are trying to release this asap.

…veCredentialsToFile'.

…archuk16/google-auth-library-java into MemoryCrenetialsStorage

YuryBandarchuk16 · 2017-11-02T20:17:36Z

@lesv Sorry for the delay, thanks for letting me know. I think I have already fixed everything @garrettjonesgoogle asked me to fix.

garrettjonesgoogle

I have done a deeper review now.

I don't think I'll try to include this in the release I'm about to do - I'll try to include it in a following release.

oauth2_http/pom.xml

+          <groupId>com.google.guava</groupId>
+          <artifactId>guava</artifactId>
+      </dependency>
+    <dependency>


oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

@@ -36,9 +36,14 @@
 import com.google.api.client.json.JsonObjectParser;
 import com.google.api.client.util.Preconditions;
 import com.google.auth.http.HttpTransportFactory;
+import org.apache.commons.io.FileUtils;


oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

+   * @param credentialsFileName Name of file where to store the credentials
+   * @throws IOException An error saving the credentials.
+   */
+  public void saveCredentialsToFile(InputStream credentials, String credentialsFileName) throws IOException {


oauth2_http/java/com/google/auth/oauth2/storage/MemoryTokensStorage.java

+
+public class MemoryTokensStorage implements TokenStore {
+
+    private final Map<String, String> tokensStorage = new HashMap<>();


oauth2_http/java/com/google/auth/oauth2/storage/TokenStore.java

@@ -29,7 +29,7 @@
 * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

-package com.google.auth.oauth2;
+package com.google.auth.oauth2.storage;


oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

+   * @throws IOException An error saving the credentials.
+   */
+  public void saveCredentialsToFile(InputStream credentials, String credentialsFileName) throws IOException {
+    String userDirectory = System.getProperty("user.dir");


oauth2_http/java/com/google/auth/oauth2/UserCredentials.java

+   * @throws IOException An error storing the credentials.
+   */
+  public void saveUserCredentials() throws IOException {
+    super.saveCredentialsToFile(getUserCredentialsStream(), USER_CREDENTIALS_FILE_NAME);


… to copy the inputStream to fileOutputStream.

… packages and indentation.

YuryBandarchuk16 · 2017-11-02T21:04:07Z

@garrettjonesgoogle Could I ask you to review all the changes, please?

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

+   * @param filePath Path to file where to store the credentials
+   * @throws IOException An error saving the credentials.
+   */
+  public void saveCredentialsToFile(InputStream credentials, String filePath) throws IOException {


oauth2_http/java/com/google/auth/oauth2/UserCredentials.java

+  }
+
+  /**
+   * Saves the end user credentials in file


oauth2_http/javatests/com/google/auth/oauth2/UserCredentialsTest.java

+            .setClientSecret(CLIENT_SECRET)
+            .setRefreshToken(REFRESH_TOKEN)
+            .build();
+    String filePath = System.getProperty("user.dir") + File.separator + "GOOGLE_APPLICATION_CREDENTIALS";


oauth2_http/java/com/google/auth/oauth2/MemoryTokensStorage.java

+import java.util.HashMap;
+import java.util.Map;
+
+public class MemoryTokensStorage implements TokenStore {


…Utils',

YuryBandarchuk16 · 2017-11-02T21:48:23Z

@garrettjonesgoogle I think I made all the changes you asked for.

garrettjonesgoogle

Looks good to me! I'll merge when the tests pass.

YuryBandarchuk16 added 6 commits September 14, 2017 11:17

Created MemoryCredentialsStorage and implemented added opportunity to…

4fa1b0d

… save current credentials in UserCredentials.java

Moved TokenStore.java to another directory

9cfc14d

Comments added

48ff167

Tests for UserAuthorizer updated

dd03f9d

Renamed MemoryCredentialsStorage to MemoryTokensStorage

71adc52

Tests for UserCredentials updated

8208263

YuryBandarchuk16 mentioned this pull request Sep 14, 2017

Provide a method to write User Credentials to file #54

Closed

Java header added

b44d07d

lesv reviewed Sep 14, 2017

View reviewed changes

oauth2_http/java/com/google/auth/oauth2/UserCredentials.java Outdated

@@ -58,6 +58,8 @@

*/

public class UserCredentials extends GoogleCredentials {

public static final String USER_CREDENTIALS_FILE_NAME = "GOOGLE_AUTH_USER_CREDENTIALS_FILE_NAME";

This comment was marked as spam.

Sign in to view

lesv reviewed Sep 14, 2017

View reviewed changes

small fixes

8adc97a

Used MemoryTokenStorage as default for UserAuthorizer

431453d

Comments regrading credentials

c3feabc

Removed unnecessary test

5fcddeb

Removed test, since there is always TokenStore exist by default.

garrettjonesgoogle reviewed Sep 27, 2017

View reviewed changes

YuryBandarchuk16 added 2 commits November 2, 2017 22:57

Add apache.commons.io dependency and update the implementation of 'sa…

7bcfeab

…veCredentialsToFile'.

Merge branch 'MemoryCrenetialsStorage' of https://github.com/YuryBand…

b28a59b

…archuk16/google-auth-library-java into MemoryCrenetialsStorage

Merge branch 'master' into MemoryCrenetialsStorage

32e098d

garrettjonesgoogle reviewed Nov 2, 2017

View reviewed changes

YuryBandarchuk16 added 3 commits November 2, 2017 23:41

Returned pom.xml to its previous state, used Guava 'ByteStreams.copy'…

46ea6af

… to copy the inputStream to fileOutputStream.

Move storage implementation to its original package.

7845198

Update the implementation of saving user credentials, fix issues with…

de94bcc

… packages and indentation.

YuryBandarchuk16 added 2 commits November 3, 2017 00:04

Merge branch 'master' into MemoryCrenetialsStorage

366e718

Merge branch 'master' into MemoryCrenetialsStorage

8e59f30

garrettjonesgoogle reviewed Nov 2, 2017

View reviewed changes

YuryBandarchuk16 and others added 3 commits November 3, 2017 00:44

Use temporary files for tests, move generic implementation to 'OAuth2…

4026b3a

…Utils',

Update method name to 'writeInputStreamToFile'.

e3d8020

Merge branch 'master' into MemoryCrenetialsStorage

7f60405

garrettjonesgoogle approved these changes Nov 2, 2017

View reviewed changes

garrettjonesgoogle merged commit 5e699a0 into googleapis:master Nov 2, 2017


		public class MemoryTokensStorage implements TokenStore {

		private final Map<String, String> tokensStorage = new HashMap<>();

Implemented in-memory TokenStore and added opportunity to save user credentials into file #129

Implemented in-memory TokenStore and added opportunity to save user credentials into file #129

Conversation

YuryBandarchuk16 commented Sep 14, 2017

tswast commented Sep 14, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

lesv left a comment

Choose a reason for hiding this comment

YuryBandarchuk16 commented Sep 14, 2017 • edited

lesv commented Sep 14, 2017

YuryBandarchuk16 commented Sep 14, 2017

YuryBandarchuk16 commented Sep 15, 2017

lesv commented Sep 15, 2017

YuryBandarchuk16 commented Sep 15, 2017

YuryBandarchuk16 commented Sep 25, 2017

lesv commented Sep 25, 2017

garrettjonesgoogle commented Sep 27, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

lesv commented Nov 2, 2017

YuryBandarchuk16 commented Nov 2, 2017

garrettjonesgoogle left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

YuryBandarchuk16 commented Nov 2, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

YuryBandarchuk16 commented Nov 2, 2017

garrettjonesgoogle left a comment

Choose a reason for hiding this comment

YuryBandarchuk16 commented Sep 14, 2017 •

edited