Merge lp:~nataliabidart/ubuntuone-client/fix-u1sync into lp:ubuntuone-client

[Natalia Bidart]

* Made u1sync client work from the command line. No more "authorize" option, only --oauth (LP: #634337).

[Facundo Batista (facundo)]

Thanks!

review: Approve

[Lucio Torre (lucio.torre)]

review: Approve

[dobey (dobey)]

Please leave this as needs review for now. There are some issues with launchpad, and setting this to Approve will denote the wrong revision number for merging.

I've reported https://bugs.edge.launchpad.net/launchpad/+bug/634451 and bugged the LP hackers to get it fixed. Hopefully it will be fixed very shortly and we can go back about our normal lives.

=== modified file 'ubuntuone/u1sync/client.py'
--- ubuntuone/u1sync/client.py	2010-06-23 15:31:32 +0000
+++ ubuntuone/u1sync/client.py	2010-09-10 02:20:59 +0000
@@ -28,8 +28,6 @@
 from Queue import Queue
 from threading import Lock
 import zlib
-import urlparse
-import ConfigParser
 from cStringIO import StringIO
 
 from twisted.internet import reactor, defer
@@ -37,12 +35,11 @@
 from ubuntuone.logger import LOGFOLDER
 from ubuntuone.storageprotocol.content_hash import crc32
 from ubuntuone.storageprotocol.context import get_ssl_context
-from ubuntu_sso.config import get_config as get_oauth_config
-from ubuntu_sso.auth import AuthorisationClient
 from ubuntuone.u1sync.genericmerge import MergeNode
 from ubuntuone.u1sync.utils import should_sync
 
 CONSUMER_KEY = "ubuntuone"
+CONSUMER_SECRET = "hammertime"
 
 from oauth.oauth import OAuthConsumer
 from ubuntuone.storageprotocol.client import (
@@ -177,9 +174,12 @@
     """U1 storage client facade."""
     required_caps = frozenset(["no-content", "fix462230"])
 
-    def __init__(self, realm, reactor=reactor):
-        """Create the instance."""
-
+    def __init__(self, realm=None, reactor=reactor):
+        """Create the instance.
+
+        'realm' is no longer used, but is left as param for API compatibility.
+
+        """
         self.reactor = reactor
         self.factory = SyncClientFactory(self)
 
@@ -189,37 +189,8 @@
         self._status_waiting = []
         self._active_waiters = set()
 
-        self.realm = realm
-
-        oauth_config = get_oauth_config()
-        if oauth_config.has_section(realm):
-            config_section = realm
-        elif self.realm.startswith("http://localhost") and \
-             oauth_config.has_section("http://localhost"):
-            config_section = "http://localhost"
-        else:
-            config_section = "default"
-
-        @log_timing
-        def get_oauth_option(option):
-            """Retrieves an option from oauth config."""
-            try:
-                return oauth_config.get(config_section, option)
-            except ConfigParser.NoOptionError:
-                return oauth_config.get("default", option)
-
-        @log_timing
-        def get_oauth_url(option):
-            """Retrieves an absolutized URL from the OAuth config."""
-            suffix = get_oauth_option(option)
-            return urlparse.urljoin(realm, suffix)
-
         self.consumer_key = CONSUMER_KEY
-        self.consumer_secret = get_oauth_option("consumer_secret")
-
-        self.request_token_url = get_oauth_url("request_token_url")
-        self.user_authorisation_url = get_oauth_url("user_authorisation_url")
-        self.access_token_url = get_oauth_url("access_token_url")
+        self.consumer_secret = CONSUMER_SECRET
 
     def force_shutdown(self):
         """Forces the client to shut itself down."""
@@ -256,49 +227,6 @@
                     self._active_waiters.remove(waiter)
 
     @log_timing
-    def obtain_oauth_token(self, create_token):
-        """Obtains an oauth token, optionally creating one if requried."""
-        token_result = self._get_waiter()
-
-        @log_timing
-        def have_token(token):
-            """When a token is available."""
-            token_result.wake(token)
-
-        @log_timing
-        def no_token():
-            """When no token is available."""
-            token_result.wake(None)
-
-        oauth_client = AuthorisationClient(realm=self.realm,
-                                           request_token_url=
-                                           self.request_token_url,
-                                           user_authorisation_url=
-                                           self.user_authorisation_url,
-                                           access_token_url=
-                                           self.access_token_url,
-                                           consumer_key=self.consumer_key,
-                                           consumer_secret=
-                                           self.consumer_secret,
-                                           callback_parent=have_token,
-                                           callback_denied=no_token,
-                                           do_login=create_token)
-
-
-        @log_timing
-        def _obtain_token():
-            """Obtains or creates a token."""
-            if create_token:
-                oauth_client.clear_token()
-            oauth_client.ensure_access_token()
-
-        self.reactor.callFromThread(_obtain_token)
-        token = self._wait(token_result)
-        if token is None:
-            raise AuthenticationError("Unable to obtain OAuth token.")
-        return token
-
-    @log_timing
     def _change_status(self, status, reason=None):
         """Changes the client status.  Usually called from the reactor
         thread.
@@ -412,10 +340,11 @@
         self._await_status_not("connecting", "connected", "authenticated")
 
     @log_timing
-    def oauth_from_token(self, token):
+    def oauth_from_token(self, token, consumer=None):
         """Perform OAuth authorisation using an existing token."""
 
-        consumer = OAuthConsumer(self.consumer_key, self.consumer_secret)
+        if consumer is None:
+            consumer = OAuthConsumer(self.consumer_key, self.consumer_secret)
 
         def _auth_successful(value):
             """Callback for successful auth.  Changes status to
 
=== modified file 'ubuntuone/u1sync/main.py'
--- ubuntuone/u1sync/main.py	2010-08-16 22:14:19 +0000
+++ ubuntuone/u1sync/main.py	2010-09-10 02:20:59 +0000
@@ -282,7 +282,6 @@
 def do_main(argv):
     """The main user-facing portion of the script."""
     usage = "Usage: %prog [options] [DIRECTORY]\n" \
-            "       %prog --authorize [options]\n" \
             "       %prog --list-shares [options]\n" \
             "       %prog --init [--share=SHARE_UUID] [options] DIRECTORY\n" \
             "       %prog --diff [--share=SHARE_UUID] [options] DIRECTORY"
@@ -294,9 +293,7 @@
                       default='fs-1.one.ubuntu.com',
                       help="The server address")
     parser.add_option("--oauth", dest="oauth", metavar="KEY:SECRET",
-                      default=None,
-                      help="Explicitly provide OAuth credentials "
-                           "(default is to query keyring)")
+                      help="Explicitly provide OAuth credentials")
     action_list = ", ".join(sorted(MERGE_ACTIONS.keys()))
     parser.add_option("--action", dest="action", metavar="ACTION",
                       default=None,
@@ -307,9 +304,6 @@
                                           "making changes")
     parser.add_option("--quiet", action="store_true", dest="quiet",
                       default=False, help="Produces less output")
-    parser.add_option("--authorize", action="store_const", dest="mode",
-                      const="authorize",
-                      help="Authorize this machine")
     parser.add_option("--list-shares", action="store_const", dest="mode",
                       const="list-shares", default="sync",
                       help="List available shares")
@@ -351,17 +345,14 @@
         else:
             directory = args[0]
     if options.mode == "init" or options.mode == "list-shares" or \
-       options.mode == "diff" or options.mode == "authorize":
+       options.mode == "diff":
         if options.action is not None:
             parser.error("--%s does not take the --action parameter" % \
                          options.mode)
         if options.dry_run:
             parser.error("--%s does not take the --dry-run parameter" % \
                          options.mode)
-    if options.mode == "authorize":
-        if options.oauth is not None:
-            parser.error("--authorize does not take the --oauth parameter")
-    if options.mode == "list-shares" or options.mode == "authorize":
+    if options.mode == "list-shares":
         if len(args) != 0:
             parser.error("--list-shares does not take a directory")
     if options.mode != "init" and options.mode != "diff":
@@ -376,8 +367,9 @@
         options.action = DEFAULT_MERGE_ACTION
 
 
+    passed_token = None
     if options.oauth is None:
-        passed_token = None
+        parser.error("--oauth is required")
     else:
         try:
             (key, secret) = options.oauth.split(':', 2)
@@ -395,18 +387,14 @@
     else:
         share_spec = None
 
-    client = Client(realm=options.realm, reactor=reactor)
+    client = Client(reactor=reactor)
 
     signal.signal(signal.SIGINT, lambda s, f: client.force_shutdown())
     signal.signal(signal.SIGTERM, lambda s, f: client.force_shutdown())
 
     def run_client():
         """Run the blocking client."""
-        if passed_token is None:
-            should_create_token = (options.mode == "authorize")
-            token = client.obtain_oauth_token(create_token=should_create_token)
-        else:
-            token = passed_token
+        token = passed_token
 
         client.connect_ssl(options.host, int(options.port), options.no_ssl_verify)
 
@@ -429,9 +417,6 @@
                         directory=directory,
                         quiet=options.quiet, subtree_path=options.subtree,
                         ignore_symlinks=False)
-            elif options.mode == "authorize":
-                if not options.quiet:
-                    print "Authorized."
         finally:
             client.disconnect()