pmuller / django-crowd-auth

Atlassian Crowd SSO integration for Django applications
Other
10 stars 6 forks source link

Middleware initialisation breaks app re- load/start if Crowd is unreachable #13

Open elwinarens opened 6 years ago

elwinarens commented 6 years ago

client.get_cookie_config() does a request to Crowd for the cookie settings:

def sso(get_response):
    """Crowd SSO middleware.
    """
    client = Client.from_settings()
    cookie_config = client.get_cookie_config()
    cookie_name = cookie_config['name']
    cookie_domain = cookie_config['domain']
    cookie_secure = cookie_config['secure']
    LOGGER.debug('Crowd cookie config %r', cookie_config)

This request is retried N times and in case Crowd is unreachable, results in a max retries uncaught exception being thrown, causing the app re- load/start to fail.

Expected behaviour is that the middleware fails gracefully and allows the app to re- load/start, independent of Crowd being up.

I will put up a PR with a fix.

pmuller commented 6 years ago

Hi Elwin

The current implementation expects Crowd to be always available. In my usage, Crowd is the only authentication backend, so I expect my apps to fail if Crowd is not available. Are you using more than 1 authentication backend at the same time? If so, how do you integrate Crowd SSO with others ?

What do you mean by "re-load/start" ? How do you handle a WSGI app "reload" in production ? (I only do this with the Django development server). What issue did you have when restarting a WSGI app ? (never had one)

When working on a PR for this need, please make this change of behavior optional (I really want to keep my apps to fail when they cannot talk with Crowd).

Thanks!

elwinarens commented 6 years ago

I ran into the error below, resulting in the app no longer being available/loaded.

We deploy artifacts with a script that does a service uwsgi reload at the end. If during this reload crowd is unreachable, the timeout error will prevent the application from loading it seems.

Maybe it's something that could be overcome by configuring uWSGI better, as my knowledge there is somewhat lacking still.

Traceback (most recent call last):
  File "venv/lib/python3.5/site-packages/requests/adapters.py", line 440, in send
    timeout=timeout
  File "venv/lib/python3.5/site-packages/urllib3/connectionpool.py", line 639, in urlopen
    _stacktrace=sys.exc_info()[2])
  File "venv/lib/python3.5/site-packages/urllib3/util/retry.py", line 388, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='<modified for customer privacy reasons>', port=443): Max retries exceeded with url: /crowd/rest/usermanagement/1/config/cookie.json (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f6f9bc025c0>: Failed to establish a new connection: [Errno 101] Network is unreachable',))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "venv/wsgi.py", line 20, in <module>
    application = get_wsgi_application()
  File "venv/lib/python3.5/site-packages/django/core/wsgi.py", line 14, in get_wsgi_application
    return WSGIHandler()
  File "venv/lib/python3.5/site-packages/django/core/handlers/wsgi.py", line 151, in __init__
    self.load_middleware()
  File "venv/lib/python3.5/site-packages/django/core/handlers/base.py", line 82, in load_middleware
    mw_instance = middleware(handler)
  File "venv/lib/python3.5/site-packages/django_crowd_auth/middlewares.py", line 17, in sso
    cookie_config = client.get_cookie_config()
  File "venv/lib/python3.5/site-packages/django_crowd_auth/client.py", line 13, in get_cookie_config
    response = self._get(url)
  File "venv/lib/python3.5/site-packages/crowd.py", line 81, in _get
    req = self.session.get(*args, **kwargs)
  File "venv/lib/python3.5/site-packages/requests/sessions.py", line 521, in get
    return self.request('GET', url, **kwargs)
  File "venv/lib/python3.5/site-packages/requests/sessions.py", line 508, in request
    resp = self.send(prep, **send_kwargs)
  File "venv/lib/python3.5/site-packages/requests/sessions.py", line 618, in send
    r = adapter.send(request, **kwargs)
  File "venv/lib/python3.5/site-packages/requests/adapters.py", line 508, in send
    raise ConnectionError(e, request=request)
requests.exceptions.ConnectionError: HTTPSConnectionPool(host='<modified for customer privacy reasons>', port=443): Max retries exceeded with url: /crowd/rest/usermanagement/1/config/cookie.json (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7f6f9bc025c0>: Failed to establish a new connection: [Errno 101] Network is unreachable',))
Tue Jun  5 03:45:45 2018 - unable to load app 0 (mountpoint='') (callable not found or import error)
Tue Jun  5 03:45:45 2018 - *** no app loaded. going in full dynamic mode ***
bellarc commented 5 years ago

I have this issue as well. Even if you have this as the only auth backend, it should still fail gracefully. Using an uncaught exception that crashes the app as a safeguard is a bad practice. There are other/better ways to handle that. However, having a flag making it optional isn't really a huge deal if that makes people happy.

We have multiple backends, so being completely reliant on Crowd responding in order to run our app isn't ideal.