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

NotImplementedError - get_id_email #103

Open
martincolladodev opened this issue May 24, 2020 · 17 comments
Open

NotImplementedError - get_id_email #103

martincolladodev opened this issue May 24, 2020 · 17 comments

Comments

@martincolladodev
Copy link

martincolladodev commented May 24, 2020

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

martincolladodev commented May 26, 2020

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 frankie567 transferred this issue from fastapi-users/fastapi-users May 27, 2020
@frankie567
Copy link
Owner

frankie567 commented May 27, 2020

Better to continue this conversation here :)

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

class OAuth2Token(Dict[str, Any]):
def __init__(self, token_dict: Dict[str, Any]):
if "expires_at" in token_dict:
token_dict["expires_at"] = int(token_dict["expires_at"])
elif "expires_in" in token_dict:
token_dict["expires_at"] = int(time.time()) + int(token_dict["expires_in"])
super().__init__(token_dict)
def is_expired(self):
if "expires_at" not in self:
return False
return time.time() > self["expires_at"]

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

@frankie567
Copy link
Owner

@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
Copy link
Author

Sure,
Gitlab - Onpremise 13.0 (Omnibus)

Calling the /authorize with:

  • authentication_backend: jwt

  • scopes: openid

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

martincolladodev commented May 28, 2020

Sure!!
Backend: jwt
Scope: profile

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

Scopes on Gitlab:
image

@frankie567
Copy link
Owner

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
Copy link
Author

martincolladodev commented May 29, 2020

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
Copy link

lepture commented May 31, 2020

@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
Copy link
Author

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.

fastapi-users/fastapi-users#68 (comment)

@lepture
Copy link

lepture commented May 31, 2020

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
Copy link
Owner

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.

@polar-sh polar-sh bot added polar Issues that can be backed through Polar and removed polar Issues that can be backed through Polar labels Jul 6, 2023
@polar-sh polar-sh bot added polar Issues that can be backed through Polar and removed polar Issues that can be backed through Polar labels Oct 11, 2023
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

3 participants