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

remoting/client/ios/facade/remoting_service.mm

Index: remoting/client/ios/facade/remoting_service.mm
diff --git a/remoting/client/ios/facade/remoting_service.mm b/remoting/client/ios/facade/remoting_service.mm
index f30a8234738b895b47bbb7944ce73aea059d00b8..01d9c5b7ca6514df34564d87f401c03ec8b43c64 100644
--- a/remoting/client/ios/facade/remoting_service.mm
+++ b/remoting/client/ios/facade/remoting_service.mm
@@ -9,78 +9,44 @@
 #import "remoting/client/ios/facade/remoting_service.h"
 
 #import <Foundation/Foundation.h>
+#import <Security/Security.h>
 
 #import "base/mac/bind_objc_block.h"
+#import "remoting/client/ios/domain/host_info.h"
+#import "remoting/client/ios/domain/user_info.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_authentication.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"
-#include "remoting/client/ios/facade/host_info.h"
-#include "remoting/client/ios/facade/host_list_fetcher.h"
-#include "remoting/client/ios/facade/ios_client_runtime_delegate.h"
-
-const char kOauthRedirectUrl[] =
-    "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;
+static NSString* const kCRDAuthenticatedUserEmailKey =
+    @"kCRDAuthenticatedUserEmailKey";
 
-  std::unique_ptr<remoting::OAuthTokenGetter> oauth_tokenGetter(
-      new remoting::OAuthTokenGetterImpl(
-          std::move(oauth_credentials), on_credentials_update,
-          [[RemotingService SharedInstance] runtime]->url_requester(),
-          /*auto_refresh=*/true));
-  return oauth_tokenGetter;
-}
+NSString* const kHostsDidUpdate = @"kHostsDidUpdate";
 
-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;
-}
+NSString* const kUserDidUpdate = @"kUserDidUpdate";
+NSString* const kUserInfo = @"kUserInfo";
 
-@interface RemotingService () {
+@interface RemotingService ()<RemotingAuthenticationDelegate> {
   std::unique_ptr<remoting::OAuthTokenGetter> _tokenGetter;
-  UserInfo* _user;
-  NSArray<HostInfo*>* _hosts;
-  id<RemotingAuthenticationDelegate> _authDelegate;
-  id<RemotingHostListDelegate> _hostListDelegate;
   remoting::HostListFetcher* _hostListFetcher;
   remoting::IosClientRuntimeDelegate* _clientRuntimeDelegate;
 }
-
 @end
 
-//
-// RemodingService will act as the facade to the C++ layer that has not been
-// implemented/integrated yet.
-// TODO(nicholss): Implement/Integrate this class. At the moment it is being
-// used to generate fake data to implement the UI of the app.
-// Update: Half implemented now. User is still fake, but now real hosts lists.
-//
 @implementation RemotingService
 
+@synthesize authentication = _authentication;
+@synthesize hosts = _hosts;
+
 // RemotingService is a singleton.
 + (RemotingService*)SharedInstance {
   static RemotingService* sharedInstance = nil;
@@ -94,10 +60,10 @@ std::unique_ptr<remoting::OAuthTokenGetter> CreateOAuthTokenWithRefreshToken(
 - (instancetype)init {
   self = [super init];
   if (self) {
-    _user = nil;
+    _authentication = [[RemotingAuthentication alloc] init];
+    _authentication.delegate = self;
     _hosts = nil;
-    _hostListFetcher = new remoting::HostListFetcher(
-        remoting::ChromotingClientRuntime::GetInstance()->url_requester());
+    _hostListFetcher = nil;
     // TODO(nicholss): This might need a pointer back to the service.
     _clientRuntimeDelegate =
         new remoting::IosClientRuntimeDelegate();
@@ -108,161 +74,110 @@ std::unique_ptr<remoting::OAuthTokenGetter> CreateOAuthTokenWithRefreshToken(
 
 #pragma mark - RemotingService Implementation
 
-// TODO(nicholss): isAuthenticated needs to just kick off a request to
-// authenticate a user. and more than one controller might want to be a delegate
-// for this info so need to change this to be more of the registration types.
-// The remoting_service might also want to be registered for authentication
-// changes and it can update it's cache as it needs.
-
-- (void)setAuthenticationDelegate:(id<RemotingAuthenticationDelegate>)delegate {
-  _authDelegate = delegate;
-  if (_authDelegate) {
-    [_authDelegate nowAuthenticated:[self isAuthenticated]];
+- (void)startHostListFetchWith:(NSString*)accessToken {
+  if (!_hostListFetcher) {
+    _hostListFetcher = new remoting::HostListFetcher(
+        remoting::ChromotingClientRuntime::GetInstance()->url_requester());
   }
-  if (!_user && _tokenGetter) {
-    _tokenGetter->CallWithToken(base::BindBlockArc(
-        ^(remoting::OAuthTokenGetter::Status status,
-          const std::string& user_email, const std::string& access_token) {
-          if (status == remoting::OAuthTokenGetter::Status::SUCCESS) {
-            _user = [[UserInfo alloc] init];
-            _user.userEmail =
-                [NSString stringWithCString:user_email.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-          } else {
-            _user = nil;
+  _hostListFetcher->RetrieveHostlist(
+      base::SysNSStringToUTF8(accessToken),
+      base::BindBlockArc(^(const std::vector<remoting::HostInfo>& hostlist) {
+        NSMutableArray<HostInfo*>* hosts =
+            [NSMutableArray arrayWithCapacity:hostlist.size()];
+        std::string status;
+        for (const remoting::HostInfo& host_info : hostlist) {
+          remoting::HostStatus host_status = host_info.status;
+          switch (host_status) {
+            case remoting::kHostStatusOnline:
+              status = "ONLINE";

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

Not related to this CL. Why is the status a string rather than an enum?

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

I was displaying this status in the UI for testing.

+              break;
+            case remoting::kHostStatusOffline:
+              status = "OFFLINE";
+              break;
+            default:
+              NOTREACHED();
           }
-          if (_authDelegate) {
-            [_authDelegate nowAuthenticated:[self isAuthenticated]];
-          }
-        }));
-  }
+          // TODO(nicholss): Not yet integrated: createdTime, hostVersion,
+          // kind, offlineReason. Add them as the app will need this info.
+          HostInfo* host = [[HostInfo alloc] init];
+          host.hostId =
+              [NSString stringWithCString:host_info.host_id.c_str()
+                                 encoding:[NSString defaultCStringEncoding]];
+          host.hostName =
+              [NSString stringWithCString:host_info.host_name.c_str()
+                                 encoding:[NSString defaultCStringEncoding]];
+          host.jabberId =
+              [NSString stringWithCString:host_info.host_jid.c_str()
+                                 encoding:[NSString defaultCStringEncoding]];
+          host.publicKey =
+              [NSString stringWithCString:host_info.public_key.c_str()
+                                 encoding:[NSString defaultCStringEncoding]];
+          host.status =
+              [NSString stringWithCString:status.c_str()
+                                 encoding:[NSString defaultCStringEncoding]];
+          [hosts addObject:host];
+        }
+        _hosts = hosts;
+        [self hostListUpdated];

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

_hostListFetcher doesn't seem to reset the callback in case of failure, which
may form retain cycle. Pass a weakSelf here?

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

the callback is cleared with base::ResetAndReturn. This should get called
regardless of error:

https://cs.chromium.org/chromium/src/net/url_request/url_fetcher_delegate.h?t...

+      }));
 }
 
-- (BOOL)isAuthenticated {
-  if (_user) {
-    return YES;
-  }
-  return NO;
+#pragma mark - Notifications
+
+- (void)hostListUpdated {
+  [[NSNotificationCenter defaultCenter] postNotificationName:kHostsDidUpdate
+                                                      object:self
+                                                    userInfo:nil];
 }
 
-- (void)startHostListFetchWith:(NSString*)accessToken {
-  NSLog(@"startHostListFetchWith : %@ %@", accessToken, _authDelegate);
-  if (_authDelegate) {
-    [_authDelegate nowAuthenticated:YES];
+#pragma mark - RemotingAuthenticationDelegate
 
-    _hostListFetcher->RetrieveHostlist(
-        base::SysNSStringToUTF8(accessToken),
-        base::BindBlockArc(^(const std::vector<remoting::HostInfo>& hostlist) {
-          NSMutableArray<HostInfo*>* hosts =
-              [NSMutableArray arrayWithCapacity:hostlist.size()];
-          std::string status;
-          for (const remoting::HostInfo& host_info : hostlist) {
-            remoting::HostStatus host_status = host_info.status;
-            switch (host_status) {
-              case remoting::kHostStatusOnline:
-                status = "ONLINE";
-                break;
-              case remoting::kHostStatusOffline:
-                status = "OFFLINE";
-                break;
-              default:
-                NOTREACHED();
-            }
-            // TODO(nicholss): Not yet integrated: createdTime, hostVersion,
-            // kind, offlineReason. Add them as the app will need this info.
-            HostInfo* host = [[HostInfo alloc] init];
-            host.hostId =
-                [NSString stringWithCString:host_info.host_id.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-            host.hostName =
-                [NSString stringWithCString:host_info.host_name.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-            host.jabberId =
-                [NSString stringWithCString:host_info.host_jid.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-            host.publicKey =
-                [NSString stringWithCString:host_info.public_key.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-            host.status =
-                [NSString stringWithCString:status.c_str()
-                                   encoding:[NSString defaultCStringEncoding]];
-            [hosts addObject:host];
-          }
-          _hosts = hosts;
-          [_hostListDelegate hostListUpdated];
-        }));
+- (void)userDidUpdate:(UserInfo*)user {
+  NSDictionary* userInfo = nil;
+  if (user) {
+    userInfo = [NSDictionary dictionaryWithObject:user forKey:kUserInfo];
   }
+  [[NSNotificationCenter defaultCenter] postNotificationName:kUserDidUpdate
+                                                      object:self
+                                                    userInfo:userInfo];
 }
 
-- (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;
-          }));
-}
+#pragma mark - Properties
 
-- (void)authenticateWithRefreshToken:(NSString*)refreshToken
-                               email:(NSString*)email {
-  _tokenGetter = CreateOAuthTokenWithRefreshToken(
-      std::string(base::SysNSStringToUTF8(refreshToken)),
-      base::SysNSStringToUTF8(email));
+- (UserInfo*)user {
+  return _authentication.user;
 }
 
-- (UserInfo*)getUser {
-  if (![self isAuthenticated]) {
-    return nil;
+- (NSArray<HostInfo*>*)hosts {
+  if ([_authentication.user isAuthenticated]) {
+    return _hosts;
   }
-
-  NSMutableString* json = [[NSMutableString alloc] init];
-  [json appendString:@"{"];
-  [json appendString:@"\"userId\":\"AABBCC123\","];
-  [json appendString:@"\"userFullName\":\"John Smith\","];
-  [json appendString:@"\"userEmail\":\"john@example.com\","];
-  [json appendString:@"}"];
-
-  NSMutableData* data = [NSMutableData
-      dataWithData:[[json copy] dataUsingEncoding:NSUTF8StringEncoding]];
-
-  UserInfo* user = [UserInfo parseListFromJSON:data];
-  return user;
+  return nil;
 }
 
-- (void)setHostListDelegate:(id<RemotingHostListDelegate>)delegate {
-  bool attemptUpdate = (_hostListDelegate != delegate);
-  _hostListDelegate = delegate;
-  if (attemptUpdate && _hostListDelegate && _tokenGetter) {
-    // TODO(nicholss): It might be cleaner to set the delegate and then have
-    // them ask to refresh the host list rather than start this get hosts call.
-    _tokenGetter->CallWithToken(base::BindBlockArc(
-        ^(remoting::OAuthTokenGetter::Status status,
-          const std::string& user_email, const std::string& access_token) {
-          NSString* accessToken =
-              [NSString stringWithCString:access_token.c_str()
-                                 encoding:[NSString defaultCStringEncoding]];
-          [self startHostListFetchWith:accessToken];
-        }));
-  }
+- (remoting::ChromotingClientRuntime*)runtime {
+  return remoting::ChromotingClientRuntime::GetInstance();
 }
 
-- (NSArray<HostInfo*>*)getHosts {
-  if (![self isAuthenticated]) {
-    return nil;
-  }
-  return _hosts;
-}
+#pragma mark - Implementation
 
-- (remoting::ChromotingClientRuntime*)runtime {
-  return remoting::ChromotingClientRuntime::GetInstance();
+- (void)logout {
+  [_authentication logout];
+  _hosts = nil;
+  [self hostListUpdated];
 }
 
-- (void)callbackWithAccessToken:
-    (const remoting::OAuthTokenGetter::TokenCallback&)onAccessToken {
-  if (_tokenGetter) {
-    _tokenGetter->CallWithToken(onAccessToken);
-  }
+- (void)requestHostListFetch {
+  [_authentication
+      callbackWithAccessToken:base::BindBlockArc(^(
+                                  remoting::OAuthTokenGetter::Status status,
+                                  const std::string& user_email,
+                                  const std::string& access_token) {
+        NSString* accessToken =
+            [NSString stringWithCString:access_token.c_str()
+                               encoding:[NSString defaultCStringEncoding]];
+        [self startHostListFetchWith:accessToken];
+      })];
 }
 
 @end