frankie567 / httpx-oauth

Async OAuth client using HTTPX
https://frankie567.github.io/httpx-oauth/
MIT License
142 stars 49 forks source link

NotImplementedError - get_id_email #103

Open martincolladodev opened 4 years ago

martincolladodev commented 4 years ago

First at all, thanks for the new release!! Currently for the new release, the OAuth route, calls an methods that's not implemented yet, causing an exception NotImplementedError(). Probably would be nice to not call it until the method is implemented?

@router.get("/callback", name=f"{oauth_client.name}-callback")
    async def callback(
        request: Request,
        response: Response,
        access_token_state=Depends(oauth2_authorize_callback),
    ):
        token, state = access_token_state
        account_id, account_email = await oauth_client.get_id_email(
            token["access_token"]
        )

fastapi-users: 1.0.0 fastapi: 0.54.2

frankie567 commented 4 years ago

It sounds like you use the generic OAuth2, right? If so, it's expected to have a NotImplementedError on this method : https://github.com/frankie567/httpx-oauth/blob/master/httpx_oauth/oauth2.py#L165-L166

Which service do you intend to use? If it's not one provided by httpx-oauth, you should subclass the OAuth2 client and implement this method. For example:

from typing import Any, Dict, Tuple, cast

import httpx
from httpx_oauth.errors import GetIdEmailError
from httpx_oauth.oauth2 import OAuth2

class RandomServiceOAuth2(OAuth2):
    async def get_id_email(self, token: str) -> Tuple[str, str]:
        async with httpx.AsyncClient() as client:
            response = await client.get(
                "https://api.randomservice.com/me",
                headers={"Authorization": f"token {token}"},
            )

            if response.status_code >= 400:
                raise GetIdEmailError(response.json())

            data = cast(Dict[str, Any], response.json())

            return data["id"], data["email"]
martincolladodev commented 4 years ago

Exactly! I'm trying to use the Gitlab OAuth2 Provider (https://docs.gitlab.com/ee/api/oauth2.html). I'll try with a subclass as you mention.

PS: btw, which oauth2 flow are you using por this library or the httpx-oauth library? Do you think that would be nice to include the optional user_info method for the openid provider as authlib does? (https://docs.authlib.org/en/latest/client/frameworks.html?highlight=info#openid-connect-userinfo)

frankie567 commented 4 years ago

In a nutshell, what fastapi-users do here is just to retrieve a valid access token from the service, and store it with an account_id and an email (to be able to authenticate the user later). That's the purpose of the get_id_email method.

The rest is yours to implement. If you wish to retrieve user data, you can do it by using the access token you now have in the database (in your own route, or, for example, in a worker process).

martincolladodev commented 4 years ago

Finally I used as you say an a subclass like this

class GitlabServiceOAuth2(OAuth2):
    async def get_id_email(self, token: str) -> Tuple[str, str]:
        async with httpx.AsyncClient() as client:
            response = await client.post(
                f"https://{GITLAB_INSTANCE}/oauth/userinfo?access_token={token}"
            )
            if response.status_code >= 400:
                raise GetIdEmailError(response.json())

            data = cast(Dict[str, Any], response.json())
            return data["sub"], data["email"]

I am receiving all the information, but the problem comes when tries to finalize de /callback method and it's trying to search for the token["expires_at"] https://github.com/frankie567/fastapi-users/blob/3fab5e5bbb3e799aab7c91108527f7703ea64792/fastapi_users/router/oauth.py#L102, and not all the services all giving that information. Would be easy to override the /callback method using that class?

frankie567 commented 4 years ago

Better to continue this conversation here :)

Hmm, I see that GitLab uses the expires_in property. This is something that should be handled here:

https://github.com/frankie567/httpx-oauth/blob/060372871af59aa9d6d14c623baca6571642024f/httpx_oauth/oauth2.py#L36-L47

But maybe there is something wrong with this. I'll investigate. I'll probably add a GitLab client to the library 👍

frankie567 commented 4 years ago

@martincolladofab Could you post the access token response of GitLab? Their documentation states that expires_in should be here, but I don't understand why we wouldn't compute expires_at in that case.

martincolladodev commented 4 years ago

Sure, Gitlab - Onpremise 13.0 (Omnibus)

Calling the /authorize with:

Response provided by Gitlab with the /authorize call:

{
   "access_token":"fab35f152d12c5008b14e9ff7e6c1c6a9f2093c896489e7466d2a1f7429bdf47",
   "token_type":"Bearer",
   "refresh_token":"d2423db64df0db58087d4e869848bd2ab8fccb4cf1fcea1702afbe364f6893ee",
   "scope":"openid",
   "created_at":1589625144,
   "id_token":"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlNRZnpmSFFmYlhaaUhpd3RaQmZ6bXhIWTkxb0w1VFRLNGY3eVF2c0diWkEifQ.eyJpc3MiOiJodHRwczovL2dpdC5nZXN0aW9uaWRpYWNjaW9uYS5jb20iLCJzdWIiOiIyIiwiYXVkIjoiMzNhMzAxMWMwN2ZlMTU2OWNlN2RmN2RiMTM5MWRmYjFiZmZhNjg4ODBkMjZhMmFkMmRlMTczZTU5NzU1MjZjYyIsImV4cCI6MTU5MDU2MjcyMiwiaWF0IjoxNTkwNTYyNjAyLCJhdXRoX3RpbWUiOjE1OTA0ODg1MDMsInN1Yl9sZWdhY3kiOiIyYmJkYTI0ZTU1ZGNlZDhmYzc3NDBlYzExMzQ5NzZhMjg3YjQyOGZlYmRiOTEwMDg4ZDc2OGM4Y2YwODFjNGJlIn0.dtryKnAsiWD1opJXuYLQNej_gZ1v7KMjp0AagX21P2-hl_3rRRyQ_WIHSlyf1xVF3brNO8JFl_QtrjLD3SuCkejcGBb_3GI5Ndj25p3V5kPiyvsAQU8MvupC1kobpzuCqpeU02flTjhhf8P2EPEEh7Zbw2H24B91eToe4sOrcKwSgEdQsOmrjqVkezQyGubjpcK8aLHfLB5BB2WfuLRFaO_cZR74cblFrt5r7boPNjEyFRnQ3YH_H3fIq02aSO1jJ-TLDPZ_JEt500rpZnbgpQNtemsDFGpAbrIArIRe4ImtkUe2kWTjOhD22iumoqudHLWVnN0KMWXY585lOSclZ9C2UPbdJGJtl8i7Mu4HqNfSJqltnxZAt18b52WH6asl8gQl7s6y7c-VC2uTO1h50vxaFPohqrRyYA5E4TAgW6QyoAYZkHSArO6xrSYBsHTV6kmVUiJ_5MX4Wbdkxlyi8USPRYfQ_2OCqE_Q6xz27F-BMAMYm8YHfKT7nerPxY-2zNIJbXZXiwQkJFj9EgqzQOtTXH872SgrSp9xFoTWC3Oewh-faS8IQuataUjF4awFyjwBfRFmuWkzs07rdjxNt_WFikeuZowI7ovRcntZtPHLvwTuDRcmjtv7PxBfr5YiLpBrrt_gRoNwJblwkRQLWLjRfjuZprH0VdgfcOt7Ya0"
}

As you can see, when you pass the id_token in jwt.io, the information about expiration is provided there:

image

Also the email as personal information is provided by the /oauth/userinfo call:

{
   "sub":"2",
   "sub_legacy":"2bbda24e55dced8fc7740ec1134976a287b428febdb910088d768c8cf081c4be",
   "name":"<mascared>",
   "nickname":"<mascared>",
   "email":"<mascared>",
   "email_verified":True,
   "profile":"<mascared>",
   "picture":"https://secure.gravatar.com/avatar/f69db53a21e16e3395843f0c5fa3e141?s=80&d=identicon",
   "groups":[
      "<mascared>"
   ]
}

Tell me if you need more information about it.

frankie567 commented 4 years ago

There's something I don't understand with their implementation. What's the purpose of id_token? Is this the one you use to make API requests or you use access_token?

I can't find any mention of that in their documentation.

martincolladodev commented 4 years ago

For the requests you need to send the access_token, but for the user information you need to call the /userinfo endpoint (https://docs.gitlab.com/ee/integration/openid_connect_provider.html). The ìd_token is given for the OpenID scope. (Some discussion about it: #21560 #4443)

frankie567 commented 4 years ago

Ok thanks! It's very hard to find my way into the GitLab documentation 😅 I can't even find a list of available scopes! Out of curiosity, could you paste the response when you use the profile scope?

martincolladodev commented 4 years ago

Sure!! Backend: jwt Scope: profile

{
   "access_token":"e260271eaf04057dd31ed7450cc15f75356e6fbf7c2fbb9a00d6ef549d4cfe70",
   "token_type":"Bearer",
   "refresh_token":"133831ac254da522ee2519c627a7fe708dc7cebb8db6392c6311d492b972a632",
   "scope":"profile",
   "created_at":1590678706
}

Scopes on Gitlab: image

frankie567 commented 4 years ago

Thank you, very helpful :)

Although, GitLab behaviour doesn't make lot of sense to me here. How could we know when the token is expired in a standard way (not by parsing the OpenID JWT, which is not part of the OAuth2 standard)? They say in their doc that we should have an expires_in property...

If possible, it would be nice if you could do something like this in your OAuth2 class:

class GitlabServiceOAuth2(OAuth2):
    ## The code you already have [...]

    async def get_access_token(self, code: str, redirect_uri: str):
        async with httpx.AsyncClient() as client:
            response = await client.post(
                self.access_token_endpoint,
                data={
                    "grant_type": "authorization_code",
                    "code": code,
                    "redirect_uri": redirect_uri,
                    "client_id": self.client_id,
                    "client_secret": self.client_secret,
                },
            )

            data = cast(Dict[str, Any], response.json())

            print("RAW RESPONSE", data)

            if response.status_code == 400:
                raise GetAccessTokenError(data)

            return OAuth2Token(data)

I'm interested in the result of the print statement that should appear in the console.

Thank you for your help :)

martincolladodev commented 4 years ago

What I pasted here was exactly that (I've had overwrite the entire class to investigate on the response from the server like you):

Backend: jwt Scope: profile

{
   "access_token":"e260271eaf04057dd31ed7450cc15f75356e6fbf7c2fbb9a00d6ef549d4cfe70",
   "token_type":"Bearer",
   "refresh_token":"133831ac254da522ee2519c627a7fe708dc7cebb8db6392c6311d492b972a632",
   "scope":"profile",
   "created_at":1590678706
}

Backend: jwt Scope: openid

{
   "access_token":"fab35f152d12c5008b14e9ff7e6c1c6a9f2093c896489e7466d2a1f7429bdf47",
   "token_type":"Bearer",
   "refresh_token":"d2423db64df0db58087d4e869848bd2ab8fccb4cf1fcea1702afbe364f6893ee",
   "scope":"openid",
   "created_at":1589625144,
   "id_token":"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlNRZnpmSFFmYlhaaUhpd3RaQmZ6bXhIWTkxb0w1VFRLNGY3eVF2c0diWkEifQ.eyJpc3MiOiJodHRwczovL2dpdC5nZXN0aW9uaWRpYWNjaW9uYS5jb20iLCJzdWIiOiIyIiwiYXVkIjoiMzNhMzAxMWMwN2ZlMTU2OWNlN2RmN2RiMTM5MWRmYjFiZmZhNjg4ODBkMjZhMmFkMmRlMTczZTU5NzU1MjZjYyIsImV4cCI6MTU5MDU2MjcyMiwiaWF0IjoxNTkwNTYyNjAyLCJhdXRoX3RpbWUiOjE1OTA0ODg1MDMsInN1Yl9sZWdhY3kiOiIyYmJkYTI0ZTU1ZGNlZDhmYzc3NDBlYzExMzQ5NzZhMjg3YjQyOGZlYmRiOTEwMDg4ZDc2OGM4Y2YwODFjNGJlIn0.dtryKnAsiWD1opJXuYLQNej_gZ1v7KMjp0AagX21P2-hl_3rRRyQ_WIHSlyf1xVF3brNO8JFl_QtrjLD3SuCkejcGBb_3GI5Ndj25p3V5kPiyvsAQU8MvupC1kobpzuCqpeU02flTjhhf8P2EPEEh7Zbw2H24B91eToe4sOrcKwSgEdQsOmrjqVkezQyGubjpcK8aLHfLB5BB2WfuLRFaO_cZR74cblFrt5r7boPNjEyFRnQ3YH_H3fIq02aSO1jJ-TLDPZ_JEt500rpZnbgpQNtemsDFGpAbrIArIRe4ImtkUe2kWTjOhD22iumoqudHLWVnN0KMWXY585lOSclZ9C2UPbdJGJtl8i7Mu4HqNfSJqltnxZAt18b52WH6asl8gQl7s6y7c-VC2uTO1h50vxaFPohqrRyYA5E4TAgW6QyoAYZkHSArO6xrSYBsHTV6kmVUiJ_5MX4Wbdkxlyi8USPRYfQ_2OCqE_Q6xz27F-BMAMYm8YHfKT7nerPxY-2zNIJbXZXiwQkJFj9EgqzQOtTXH872SgrSp9xFoTWC3Oewh-faS8IQuataUjF4awFyjwBfRFmuWkzs07rdjxNt_WFikeuZowI7ovRcntZtPHLvwTuDRcmjtv7PxBfr5YiLpBrrt_gRoNwJblwkRQLWLjRfjuZprH0VdgfcOt7Ya0"
}

I'll be digging on the documentation, because maybe it's something that I need to configure on the server side and the Gitlab Omnibus installation.

Thanks!

PS: I found some discussion about it here

lepture commented 4 years ago

@martincolladofab id_token is a JWT, you need to install another JWT library to parse id_token. I'm wondering what stops you from using Authlib, Authlib can handle OAuth 1.0, OAuth 2.0 and OpenID Connect for you.

martincolladodev commented 4 years ago

Actually, what stopped to me was this: (I started with authlib and later on I changed it by httpx-oauth)

Hi @martincolladofab! I deliberately put authlib aside because of its license : https://authlib.org/plans You cannot use it for free for commercial projects, which is a totally no-go for me.

That's why I created httpx-oauth which is a simple and pure async OAuth client.

https://github.com/frankie567/fastapi-users/issues/68#issuecomment-632643974

lepture commented 4 years ago

Authlib is licensed under BSD, which means you can use it for commercial projects. But if you want a commercial license instead of BSD, you can purchase a Plus plan. And a plus plan also has security mail-list & commercial support.

The plus plan is designed for creating OAuth servers.

frankie567 commented 4 years ago

Sorry @martincolladofab for not having replied earlier. Thank you for the detailed information. What bugs me is that GitLab doesn't seem to send the expires_in/expires_at property, which is not what their documentation states. And clearly, I don't want to go in the JWT parsing path.

When I have a bit more time, I'll make tests with hosted GitLab to see what's going on.