[CRD iOS] Implementing save to keychain and user defaults for login info. Refactor remoting service.

remoting/client/ios/facade/remoting_authentication.mm

Index: remoting/client/ios/facade/remoting_authentication.mm
diff --git a/remoting/client/ios/facade/remoting_authentication.mm b/remoting/client/ios/facade/remoting_authentication.mm
new file mode 100644
index 0000000000000000000000000000000000000000..9b5cb92bce056b9228229d580f5429682e8ab8af
--- /dev/null
+++ b/remoting/client/ios/facade/remoting_authentication.mm
@@ -0,0 +1,189 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+#import "remoting/client/ios/facade/remoting_authentication.h"
+
+#import <Foundation/Foundation.h>
+#import <Security/Security.h>
+
+#import "base/mac/bind_objc_block.h"
+#import "remoting/client/ios/facade/host_info.h"
+#import "remoting/client/ios/facade/host_list_fetcher.h"
+#import "remoting/client/ios/facade/ios_client_runtime_delegate.h"
+#import "remoting/client/ios/facade/remoting_service.h"
+#import "remoting/client/ios/keychain_wrapper.h"
+
+#include "base/logging.h"
+#include "base/strings/sys_string_conversions.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "remoting/base/oauth_token_getter.h"
+#include "remoting/base/oauth_token_getter_impl.h"
+
+static NSString* const kCRDAuthenticatedUserEmailKey =
+    @"kCRDAuthenticatedUserEmailKey";
+
+const char kOauthRedirectUrl[] =

[Yuwei, 2017/05/04 05:10:14]

Put const and functions below inside an empty namespace?

[nicholss, 2017/05/08 17:08:10]

you get that for free inside a obj-c file.

+    "https://chromoting-oauth.talkgadget."
+    "google.com/talkgadget/oauth/chrome-remote-desktop/dev";
+
+std::unique_ptr<remoting::OAuthTokenGetter>
+CreateOAuthTokenGetterWithAuthorizationCode(
+    const std::string& auth_code,
+    const remoting::OAuthTokenGetter::CredentialsUpdatedCallback&
+        on_credentials_update) {
+  std::unique_ptr<remoting::OAuthTokenGetter::OAuthIntermediateCredentials>
+      oauth_credentials(
+          new remoting::OAuthTokenGetter::OAuthIntermediateCredentials(
+              auth_code, /*is_service_account=*/false));
+  oauth_credentials->oauth_redirect_uri = kOauthRedirectUrl;
+
+  std::unique_ptr<remoting::OAuthTokenGetter> oauth_tokenGetter(
+      new remoting::OAuthTokenGetterImpl(
+          std::move(oauth_credentials), on_credentials_update,
+          [[RemotingService SharedInstance] runtime]->url_requester(),

[Yuwei, 2017/05/04 05:10:14]

Now that runtime is a property, use .runtime instead?

[nicholss, 2017/05/08 17:08:10]

Done.

+          /*auto_refresh=*/true));
+  return oauth_tokenGetter;
+}
+
+std::unique_ptr<remoting::OAuthTokenGetter> CreateOAuthTokenWithRefreshToken(
+    const std::string& refresh_token,
+    const std::string& email) {
+  std::unique_ptr<remoting::OAuthTokenGetter::OAuthAuthorizationCredentials>
+      oauth_credentials(
+          new remoting::OAuthTokenGetter::OAuthAuthorizationCredentials(
+              email, refresh_token, /*is_service_account=*/false));
+
+  std::unique_ptr<remoting::OAuthTokenGetter> oauth_tokenGetter(
+      new remoting::OAuthTokenGetterImpl(
+          std::move(oauth_credentials),
+          [[RemotingService SharedInstance] runtime]->url_requester(),
+          /*auto_refresh=*/true));
+  return oauth_tokenGetter;
+}
+
+@interface RemotingAuthentication () {
+  std::unique_ptr<remoting::OAuthTokenGetter> _tokenGetter;
+  UserInfo* _user;
+  KeychainWrapper* _keychainWrapper;
+  BOOL _firstLoadUserAttempt;
+}
+@end
+
+@implementation RemotingAuthentication
+
+@synthesize user = _user;
+@synthesize delegate = _delegate;
+
+- (instancetype)init {
+  self = [super init];
+  if (self) {
+    _keychainWrapper = [[KeychainWrapper alloc] init];
+    _user = nil;
+    _firstLoadUserAttempt = YES;
+  }
+  return self;
+}
+
+#pragma mark - Property Overrides
+
+- (UserInfo*)user {
+  if (_firstLoadUserAttempt && _user == nil) {
+    _firstLoadUserAttempt = NO;
+    [self setUser:[self loadUserInfo]];
+  }
+  return _user;
+}
+
+- (void)setUser:(UserInfo*)user {

[Yuwei, 2017/05/04 05:10:14]

Is this only privately used? If so maybe mark |user| readonly and provide a
private updateUser method?

Technically speaking if the caller calls setUsers then sounds like that UserInfo
should be stored in defaults/keychain..

[nicholss, 2017/05/08 17:08:10]

The private _user was cruft. Removed. also switched around calls to
storeUserInfo to only happen from setUser.

Good catch.

+  _user = user;
+  [_delegate userDidUpdate:_user];

[Yuwei, 2017/05/04 05:10:14]

This will be called with nil _user if loadUserInfo returns nil. Is this
expected?

[nicholss, 2017/05/08 17:08:10]

yes it is.

+}
+
+#pragma mark - Class Implementation
+
+- (void)authenticateWithAuthorizationCode:(NSString*)authorizationCode {
+  _tokenGetter = CreateOAuthTokenGetterWithAuthorizationCode(
+      std::string(base::SysNSStringToUTF8(authorizationCode)),
+      base::BindBlockArc(
+          ^(const std::string& user_email, const std::string& refresh_token) {
+            // TODO(nicholss): Do something with these new creds.
+            VLOG(1) << "New Creds: " << user_email << " " << refresh_token;
+            _user = [[UserInfo alloc] init];
+            _user.userEmail = base::SysUTF8ToNSString(user_email);
+            _user.refreshToken = base::SysUTF8ToNSString(refresh_token);
+            [self storeUserInfo:_user];

[Yuwei, 2017/05/04 05:10:14]

Use weakSelf? |credentials_updated_callback_| in the token getter never gets
reset.

See:
https://cs.chromium.org/chromium/src/ios/web/net/crw_ssl_status_updater.mm?l=...

[nicholss, 2017/05/08 17:08:10]

Done.

+          }));
+  // Stimulate the oAuth Token Getter to fetch and access token, this forces it
+  // to convert the authorization code into a refresh token, and saving the
+  // refresh token will happen automaticly in the above block.
+  [self callbackWithAccessToken:base::BindBlockArc(^(
+                                    remoting::OAuthTokenGetter::Status status,
+                                    const std::string& user_email,
+                                    const std::string& access_token) {
+          if (status == remoting::OAuthTokenGetter::Status::SUCCESS) {
+            VLOG(1) << "Success fetching access token from authorization code.";
+          } else {
+            LOG(ERROR)
+                << "Failed to fetch access token from authorization code. ("
+                << status << ")";
+          }
+        })];
+}
+
+- (void)authenticateWithRefreshToken:(NSString*)refreshToken
+                               email:(NSString*)email {
+  _tokenGetter = CreateOAuthTokenWithRefreshToken(

[Yuwei, 2017/05/04 05:10:14]

Update _user and store here?

[nicholss, 2017/05/08 17:08:10]

The refresh token comes from an active _user object. See the call from
loadUserInfo.

[Yuwei, 2017/05/08 19:00:58]

Shall we make this private or add comment about this?

[nicholss, 2017/05/08 21:16:04]

Yeah we can move it private. that works out ok

+      std::string(base::SysNSStringToUTF8(refreshToken)),
+      base::SysNSStringToUTF8(email));
+}
+
+- (void)callbackWithAccessToken:
+    (const remoting::OAuthTokenGetter::TokenCallback&)onAccessToken {
+  if (_tokenGetter) {
+    _tokenGetter->CallWithToken(onAccessToken);

[Yuwei, 2017/05/04 05:10:14]

Be careful here since a failure to reset onAccessToken will end up with retain
cycle and memory leakage.

[nicholss, 2017/05/08 17:08:10]

Added a TODO.

+  }
+}
+
+- (void)logout {
+  [self storeUserInfo:nil];
+  [self setUser:nil];
+}
+
+#pragma mark - Persistence
+
+- (void)storeUserInfo:(UserInfo*)user {
+  NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+  if (user) {
+    [defaults setObject:user.userEmail forKey:kCRDAuthenticatedUserEmailKey];
+    // TODO(nicholss): Need to match the token with the email.
+    [_keychainWrapper setRefreshToken:user.refreshToken];
+  } else {
+    [defaults removeObjectForKey:kCRDAuthenticatedUserEmailKey];
+    [_keychainWrapper resetKeychainItem];
+  }
+  [defaults synchronize];
+}
+
+- (UserInfo*)loadUserInfo {
+  UserInfo* user = [[UserInfo alloc] init];
+  NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+  user.userEmail = [defaults objectForKey:kCRDAuthenticatedUserEmailKey];
+  // TODO(nicholss): Need to match the token with the email.
+  user.refreshToken = [_keychainWrapper refreshToken];
+
+  if (!user || ![user isAuthenticated]) {
+    NSLog(@"Complete User not found in Keychain.");
+    user = nil;
+  } else {
+    NSLog(@"UserInfo: %@", user);
+    [self authenticateWithRefreshToken:user.refreshToken email:user.userEmail];
+  }
+  return user;
+}
+
+@end