puiterwijk / flask-oidc

OpenID Connect support for Flask
BSD 2-Clause "Simplified" License
154 stars 217 forks source link

Issue when working with Hydra #44

Open ashic opened 6 years ago

ashic commented 6 years ago

I'm looking to use this with Hydra. When handling oidc_callback, I get an error:

127.0.0.1 - - [22/Nov/2017 17:48:25] "GET /oidc_callback?code=EOG_WJio6_5E632BPlq8AMbV7z_gwIKTbYWxz5NIvgw.ULGZ9UBhO26ZtBds-wnMJ5rMN9wC35NQbV-CEIk7RYA&scope=openid&state=eyJjc3JmX3Rva2VuIjogIjJZR2JsbzR5TGlqc0lSZ2NvZXplU3dFcDhHMmFwUklBIiwgImRlc3RpbmF0aW9uIjogImV5SmhiR2NpT2lKSVV6STFOaUo5LkltaDBkSEE2THk5c2IyTmhiR2h2YzNRNk5UQXdNQzlpYjI4aS5xUU9OYmNyNlVuVGROY1ltLXQwa3ZUam9JcnFQbDAzbUxVU1hSQjNQTFhFIn0%3D HTTP/1.1" 500 -
Error on request:
Traceback (most recent call last):
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/werkzeug/serving.py", line 209, in run_wsgi
    execute(self.server.app)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/werkzeug/serving.py", line 197, in execute
    application_iter = app(environ, start_response)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1997, in __call__
    return self.wsgi_app(environ, start_response)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1985, in wsgi_app
    response = self.handle_exception(e)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1540, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1982, in wsgi_app
    response = self.full_dispatch_request()
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1614, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1517, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/flask_oidc/__init__.py", line 650, in _oidc_callback
    credentials = flow.step2_exchange(code)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/oauth2client/_helpers.py", line 133, in positional_wrapper
    return wrapped(*args, **kwargs)
  File "/mnt/e/ashicX/.conda/envs/flask/lib/python3.6/site-packages/oauth2client/client.py", line 2089, in step2_exchange
    raise FlowExchangeError(error_msg)
oauth2client.client.FlowExchangeError: invalid_requestThe request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed

On the hydra logs, I see this:

time="2017-11-22T17:48:25Z" level=info msg="started handling request" method=POST remote="172.30.0.1:42492" request=/oauth2/token
time="2017-11-22T17:48:25Z" level=error msg="An error occurred" error="HTTP authorization header missing or invalid: The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed"
time="2017-11-22T17:48:25Z" level=info msg="completed handling request" measure#http://localhost:4444.latency=126861 method=POST remote="172.30.0.1:42492" request=/oauth2/token status=400 text_status="Bad Request" took="126.861µs"

It appears hydra wants "code" to be in the Authorization Header. https://github.com/ory/hydra/issues/174 would suggest as much. Is there a way around this? It appears the (now deprecated) oauth client is putting the code in the POST body.

ashic commented 6 years ago

It appears Google are quite naughty here... https://tools.ietf.org/html/rfc6749#section-2.3.1

Including the client credentials in the request-body using the two
parameters is NOT RECOMMENDED and SHOULD be limited to clients unable
to directly utilize the HTTP Basic authentication scheme (or other
password-based HTTP authentication schemes). The parameters can only
be transmitted in the request-body and MUST NOT be included in the
request URI.
ashic commented 6 years ago

It appears google does have a setting to use headers: https://github.com/google/oauth2client/blob/3071457064f3705bab1b041bd624a10d5a2d2619/oauth2client/client.py#L1859

authorization_header: string, For use with OAuth 2.0 providers that
                                  require a client to authenticate using a
                                  header value instead of passing client_secret
                                  in the POST body.

That's in the constructor of OAuth2WebServerFlow.

ashic commented 6 years ago

While that's in the constructor, it looks like we're using flow_from_clientsecrets to create the instance, which doesn't provide an option to specify the code in the header.

ashic commented 6 years ago

I've managed to get it working by monkey patching _flow_for_request:

def flow_fr(self):
    """
    Build a flow with the correct absolute callback URL for this request.
    :return:
    """
    flow = copy(self.flow)
    redirect_uri = app.config['OVERWRITE_REDIRECT_URI']
    if not redirect_uri:
        flow.redirect_uri = url_for('_oidc_callback', _external=True)
    else:
        flow.redirect_uri = redirect_uri

    auth_method = app.config['OIDC_INTROSPECTION_AUTH_METHOD']
    print ("method is {}".format(auth_method))
    if auth_method == 'client_secret_basic':
        basic_auth_string = '%s:%s' % (self.client_secrets['client_id'], self.client_secrets['client_secret'])
        print ("Authorization header: {}".format(basic_auth_string))
        basic_auth_bytes = bytearray(basic_auth_string, 'utf-8')
        flow.authorization_header = 'Basic %s' % b64encode(basic_auth_bytes).decode('utf-8')
        print ("Header value: {}".format(flow.authorization_header))
    return flow

OpenIDConnect._flow_for_request = flow_fr 

The code is similar to _get_token_info. I did have to change the following line:

flow.authorization_header = 'Basic %s' % b64encode(basic_auth_bytes).decode('utf-8')

by adding a decode('utf-8') at the end. Otherwise the string was coming out as "Basic b'encoded value'". (e.g. with the b and single quotes around it). I'm wondering if the line in _get_token_info needs the decode('utf-8') bit as well.

I'll submit a PR with the change to _flow_for_request.