Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Working with flask-oidc #660

Closed
ashic opened this issue Nov 22, 2017 · 16 comments
Closed

Working with flask-oidc #660

ashic opened this issue Nov 22, 2017 · 16 comments

Comments

@ashic
Copy link

ashic commented Nov 22, 2017

I'm trying to use flask-oidc with Hydra. I've got it to the point where the user is redirected back to .../oidc_callback on the client website, which then attempts to exchange the "code" for tokens. This results in the step2_exchange of google's oauth2client to send a POST to hydra/oauth2/token

https://github.com/google/oauth2client/blob/3071457064f3705bab1b041bd624a10d5a2d2619/oauth2client/client.py#L1992

This results in the following error in Hydra:

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"

I was wondering if there's a way around this problem.

@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2017

Looks like that library is specifically using Google's OAuth2 flow which uses client id and secret in the post body instead of in the header which is, according to spec, not the best approach:

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.

You would have to make changes to the library and include the client id and secret in the authorization header using basic authorization.

@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2017

It's actually funny that google does that, Golang has actually an extra variable for that: https://github.com/golang/oauth2/blob/f95fa95eaa936d9d87489b15d1d18b97c1ba9c28/internal/token.go#L92

@ashic
Copy link
Author

ashic commented Nov 22, 2017

Yeah... that was my thoughts too. The trouble is that I'm looking to use flask-oidc, which in turn uses google's oauth2client (which by the way, is deprecated!). Is there a setting in Hydra that would enable reading the code from the post body (since it's discouraged in the spec, but not forbidden)?

@ashic
Copy link
Author

ashic commented Nov 22, 2017

Christ the google library is confused! The ctor for OAuth2WebServerFlow has the option:

            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.

but factory methods like flow_from_clientsecrets have no such options.

@ashic
Copy link
Author

ashic commented Nov 22, 2017

The google library is encoding parameters into the body like so:

        post_data = {
            'client_id': self.client_id,
            'code': code,
            'scope': self.scope,
        }
        if self.client_secret is not None:
            post_data['client_secret'] = self.client_secret
        if self._pkce:
            post_data['code_verifier'] = self.code_verifier
        if device_flow_info is not None:
            post_data['grant_type'] = 'http://oauth.net/grant_type/device/1.0'
        else:
            post_data['grant_type'] = 'authorization_code'
            post_data['redirect_uri'] = self.redirect_uri
        body = urllib.parse.urlencode(post_data)

Could you please advise which of these is expected in the authorisation header? What would the authorisation header string look like?

@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2017

See https://en.wikipedia.org/wiki/Basic_access_authentication - the client id is the username and the client secret is the password

@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2017

Is there a setting in Hydra that would enable reading the code from the post body (since it's discouraged in the spec, but not forbidden)?

No, unfortunately not

@ashic
Copy link
Author

ashic commented Nov 22, 2017

Thanks... I was asking about what to put in the header as there seems to be support for both basic auth and bearer tokens. With basic, the code would go in the body then?

@aeneasr
Copy link
Member

aeneasr commented Nov 22, 2017

yes, only the client id and secret are incorrect, everything else stays the same

@aeneasr aeneasr closed this as completed Nov 23, 2017
@ashic
Copy link
Author

ashic commented Nov 23, 2017

Right... so I'm monkey patching flask-oidc, and I've got it to include the header:

    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)

Inspecting in wireshark shows https://imgur.com/a/YfEAk . However, the request still fails, and hydra logs show:

time="2017-11-23T10:54:38Z" level=info msg="started handling request" method=POST remote="172.30.0.1:48214" request=/oauth2/token
time="2017-11-23T10:54:38Z" 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-23T10:54:38Z" level=info msg="completed handling request" measure#http://localhost:4444.latency=220486 method=POST remote="172.30.0.1:48214" request=/oauth2/token status=400 text_status="Bad Request" took="220.486µs"

Do you have any pointers as to what I'm missing?

@aeneasr
Copy link
Member

aeneasr commented Nov 23, 2017

Try setting LOG_LEVEL=debug, it will give more information on what happened

@aeneasr
Copy link
Member

aeneasr commented Nov 23, 2017

Also, the header looks extremely malformed, there shouldn't be any quotes:

image

@ashic
Copy link
Author

ashic commented Nov 23, 2017

I'm guessing that's wireshark formatting. I'm trying the log level thing.

@ashic
Copy link
Author

ashic commented Nov 23, 2017

Nope... it's not wireshark.... it's in the string... debugging.

@ashic
Copy link
Author

ashic commented Nov 23, 2017

Right... the issue was with how python encoding works. I needed to change the code to:

    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')

and it worked like a charm. Thanks for your continued help on this. I'll submit a PR to flask-oidc.

@aeneasr
Copy link
Member

aeneasr commented Nov 23, 2017

Nice, congratulations! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants