From 777490bd19ea3bcd3d495336d9a39b75704448ff Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Dec 19 2016 14:23:16 +0000
Subject: Better message when fail to authenticate via Kerberos


Fixes: #180

Both lookaside and Koji require Kerberos authentication now. This patch
prints more meaningful message to notify user why specific operation
cannot be performed.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 9ba0fd0..807d41c 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -4,6 +4,7 @@ ChangeLog
 v1.47 (2016-12-15)
 ------------------
 
+- Better message when fail to authenticate via Kerberos - #180 (cqi)
 - Refactor Commands._srpmdetails
 - Add missing import koji.ssl.SSLCommon - BZ#1404102 (cqi)
 - Fix upload with old PyCURL - BZ#1241059 (lsedlar)
diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 91e376e..61907a2 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -346,7 +346,19 @@ class Commands(object):
 
         # Or try kerberos
         elif authtype == 'kerberos' or self._has_krb_creds() and authtype is None:
-            session.krb_login(proxyuser=self.runas)
+            self.log.debug('Logging into {0} with Kerberos authentication.'.format(
+                koji_config['server']))
+
+            if self._load_krb_user():
+                try:
+                    session.krb_login(proxyuser=self.runas)
+                except koji.krbV.Krb5Error as e:
+                    self.log.error('Kerberos authentication fails: %s', e.args[1])
+            else:
+                self.log.warning('Kerberos authentication is used, but you do not have a '
+                                 'valid credential.')
+                self.log.warning('Please use kinit to get credential with a principal that has '
+                                 'realm {0}'.format(', '.join(list(self.realms))))
 
         if not session.logged_in:
             raise rpkgError('Could not login to %s' % koji_config['server'])
@@ -803,7 +815,8 @@ class Commands(object):
         """This property ensures the user attribute"""
 
         if not self._user:
-            if not self._load_krb_user():
+            self._user = self._load_krb_user()
+            if not self._user:
                 self.load_user()
         return self._user
 
@@ -811,7 +824,7 @@ class Commands(object):
         """This attempts to get the username from active tickets"""
 
         if not self.realms:
-            return False
+            return None
 
         if not isinstance(self.realms, list):
             self.realms = [self.realms]
@@ -819,10 +832,9 @@ class Commands(object):
         for realm in self.realms:
             username = cccolutils.get_user_for_realm(realm)
             if username:
-                self._user = username
-                return True
+                return username
         # We could not find a username for any of the realms, let's fall back
-        return False
+        return None
 
     def load_user(self):
         """This sets the user attribute"""
diff --git a/pyrpkg/errors.py b/pyrpkg/errors.py
index 5ea5f08..62924a9 100644
--- a/pyrpkg/errors.py
+++ b/pyrpkg/errors.py
@@ -50,4 +50,13 @@ class DownloadError(rpkgError):
 
 class UploadError(rpkgError):
     """Raised when something went wrong during an upload"""
-    pass
+
+    def __init__(self, message, http_status=None):
+        self.message = message
+        self.http_status = http_status
+
+    def __str__(self):
+        return str(self.message)
+
+    def __unicode__(self):
+        return unicode(self.message)
diff --git a/pyrpkg/lookaside.py b/pyrpkg/lookaside.py
index de97b91..18823b7 100644
--- a/pyrpkg/lookaside.py
+++ b/pyrpkg/lookaside.py
@@ -24,6 +24,7 @@ import pycurl
 import six
 
 from .errors import DownloadError, InvalidHashType, UploadError
+from six.moves import http_client
 
 
 class CGILookasideCache(object):
@@ -122,6 +123,15 @@ class CGILookasideCache(object):
         sum = self.hash_file(filename, hashtype)
         return sum == hash
 
+    def raise_upload_error(self, http_status):
+        messages = {
+            http_client.UNAUTHORIZED: 'Request is unauthorized.',
+            http_client.INTERNAL_SERVER_ERROR: 'Error occurs inside the server.',
+            }
+        default = 'Fail to upload files. Server returns status {0}'.format(http_status)
+        message = messages.get(http_status, default)
+        raise UploadError(message, http_status=http_status)
+
     def download(self, name, filename, hash, outfile, hashtype=None, **kwargs):
         """Download a source file
 
@@ -235,7 +245,7 @@ class CGILookasideCache(object):
             output = buf.getvalue().strip()
 
         if status != 200:
-            raise UploadError(output)
+            self.raise_upload_error(status)
 
         # Lookaside CGI script returns these strings depending on whether
         # or not the file exists:
@@ -316,7 +326,7 @@ class CGILookasideCache(object):
         sys.stdout.flush()
 
         if status != 200:
-            raise UploadError(output)
+            self.raise_upload_error(status)
 
         if output:
             self.log.debug(output)