Petr Viktorin wrote:
On 03/10/2014 08:55 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/27/2014 10:18 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Updated patch based on feedback from Foreman team. I added a new URI,
/features, which Foreman uses to determine what capabilities a proxy
has.

rob

On my VMs, where the first request is handled properly but the server
hangs on the second one. I gave you access to the machines for
investigation.

Sent you something out-of-band but in short, I wasn't able to reproduce
on your reproducing VMs :-( Ping me tomorrow and we'll try it together.

It ended up a combination of my mistake and a bug in GSSProxy. At least
you found the bug. https://fedorahosted.org/gss-proxy/ticket/121

Please add the Python libraries (python-cherrypy, python-requests,
python-kerberos) to BuildRequires. Otherwise the build fails due to
pylint errors.

Fixed.


In the man page:

+Create a host or user whose credentials will be used by the server to
make requests and add it to the role:
+
+ $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy
+ $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management'

the first command should be
     ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy
since by default the username is 'sserversmartproxy'.

The problem was a missing space before smartproxy. I have the login name
last in this example. Fixed.


A nitpick regarding the systemd service: according to [0], Type=forking
should be avoided. Is there a reason against setting Type=simple, and
removing the PID file?

Because I wasn't able to get this working with cherrypy daemon mode.
AFAICT it forks itself and systemd wouldn't end up with the right pid so
can't stop the service.

And now the updated patch. The changes are super-minor.

rob


One last nitpick: the IPAErrors get encoded as JSON but the
Content-Encoding is set to text/html. It's a one-line change so I went
ahead and tested with it. ACK from me if you agree.

+1

Here are a couple more enhancements I'm considering, this seems simpler than inter-diff since it is so small. Here is why I made the changes, in order:

I doubled the calls to create the connection but one isn't in a try/except!? Remove the obvious one.

We currently completely eat GSSAPI errors, I figure we should log failures.

IPA stores the principal in the request context so using that will save a GSSAPI call (and as we learned, a lock in gssproxy).

I included your content-type change.

rob


diff --git a/smartproxy/ipa-smartproxy b/smartproxy/ipa-smartproxy
index d5e8f22..ba50cef 100755
--- a/smartproxy/ipa-smartproxy
+++ b/smartproxy/ipa-smartproxy
@@ -32,6 +32,7 @@ from cherrypy.process import plugins
 from ipalib import api
 from ipalib import errors
 from ipalib import util
+from ipalib.request import context
 from ipaserver.rpcserver import json_encode_binary
 from ipapython.version import VERSION

@@ -83,12 +84,12 @@ def Command(command, *args, **options):
             message="Not a local request"
         )

-    if not api.Backend.rpcclient.isconnected():
-        api.Backend.rpcclient.connect()
     try:
         if not api.Backend.rpcclient.isconnected():
             api.Backend.rpcclient.connect()
     except errors.CCacheError, e:
+        cherrypy.log(msg="Kerberos error: %s" % e,
+            context='IPA', traceback=False)
         raise IPAError(
             status=401,
             message=e
@@ -171,9 +172,11 @@ class IPAError(cherrypy.HTTPError):
             error = {'message':
'Unable to handle error message type %s' % type(self._mess
age)}

+        principal = getattr(context, 'principal', None)
+        response.headers["Content-Type"] = "application/json;charset=
utf-8"
         response.body = json.dumps({'error': error,
                                     'id': 0,
-                                    'principal': util.get_current_principal
(),
+                                    'principal': principal,
                                     'result': None,
                                     'version': VERSION},
                                     sort_keys=True, indent=2)

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to