IndominusByte / fastapi-jwt-auth

FastAPI extension that provides JWT Auth support (secure, easy to use, and lightweight)
http://indominusbyte.github.io/fastapi-jwt-auth/
MIT License
630 stars 143 forks source link

support for scopes #36

Open SamiAlsubhi opened 3 years ago

SamiAlsubhi commented 3 years ago

I love this library, It would be great if the library support scopes, it already supports "additional claims" where scopes can be added to the token, it would be great if there is a check during the authorization if API endpoint scopes is met by token scopes maybe like this: Authorize.jwt_required(scopes=["users","stores"])

IndominusByte commented 3 years ago

Glad to hear that 😁, I love this idea ❤️ we can discuss it much more while I make this improvement later. I leave this link for an example of this idea. Thank you so much @SamiAlsubhi

SamiAlsubhi commented 3 years ago

Thank you @IndominusByte 😄 . I am excited for this. If I may suggest this feature as well: sometimes you want the user_id to override the scopes required for an API endpoint. so instead of having two APIs. one for the user to read his profile and another for admin to read others for example:

@app.get('users/me')
@app.get('users/{id}')

It can be combined this way:

@app.get('users/{id}')
async def read_user(id:int, Authorize: AuthJWT = Depends()):
    Authorize.jwt_required(scopes=["users"], override_by_subject=id)

meaning if the token subject matches the id of the requested user data then it is a pass, otherwise scopes have to be met. this can be done for other operations that will save a lot of time.

because having scopes only without this feature will force creating another API endpoint.

what do you think about this?

SamiAlsubhi commented 3 years ago

I have made this modification in order to check for permissions, it works both in http and ws:

class NotEnoughPermissions(AuthJWTException):
    def __init__(self,status_code: int, message: str):
        self.status_code = status_code
        self.message = message

def permissions_required(self, token: Optional[str] = None, scopes:list=[], required_sub:int=None, override_sub:int=None) -> Optional[Dict[str,Union[str,int,bool]]]:
    decoded_token = self.get_raw_jwt(encoded_token=token)
    token_scopes = decoded_token["scopes"]
    token_sub = decoded_token["sub"]

    if override_sub and str(override_sub) == token_sub:
        pass
    elif len(scopes)>0:
        for scope in scopes:
            if scope not in token_scopes:
                raise NotEnoughPermissions(status_code=401, message="Not enough permissions")

    if required_sub and str(required_sub) != token_sub:
        raise NotEnoughPermissions(status_code=401, message="Not enough permissions")

AuthJWT.permissions_required = permissions_required

arguments:

required_sub benefits sometimes you may want a user to read their data while also meeting scopes requirement if any.

override_sub benefits if you have an API link with required scopes, for example, an admin can access, but also you want to allow a user to read his own data. so required scopes check will be ignored if override_sub matches token sub.

example Here both the scope and id have to match what is in the token.

@app.get("drivers/{id}/status/", response_model=schemas.DriverStatus)
async def driver_status(id:int, Authorize: AuthJWT = Depends(), db: Session = Depends(get_db)):
    Authorize.jwt_required()
    Authorize.permissions_required(scopes=["deliver"], required_sub=id)
IndominusByte commented 3 years ago

Thank you @IndominusByte 😄 . I am excited for this. If I may suggest this feature as well: sometimes you want the user_id to override the scopes required for an API endpoint. so instead of having two APIs. one for the user to read his profile and another for admin to read others for example:

@app.get('users/me')
@app.get('users/{id}')

It can be combined this way:

@app.get('users/{id}')
async def read_user(id:int, Authorize: AuthJWT = Depends()):
    Authorize.jwt_required(scopes=["users"], override_by_subject=id)

meaning if the token subject matches the id of the requested user data then it is a pass, otherwise scopes have to be met. this can be done for other operations that will save a lot of time.

because having scopes only without this feature will force creating another API endpoint.

what do you think about this?

if the endpoint has a different operation like admin and user it's doesn't make sense if you use one endpoint, because for example user and admin has a different scope

# user scope
"write:users read:users"

# admin scope
"write:admin read:admin delete:admin"

and my opinion if you want to retrieve data based on the role of the user you can do this for example

@router.get('/my-user', response_model=typing.Union[UserData,AdminData])
async def my_user(authorize: AuthJWT = Depends()):
    authorize.jwt_required()

    user_id = authorize.get_jwt_subject()
    # get data from db
    if user := await UserFetch.filter_by_id(user_id):
        if user['role'] == 'admin':
            # operation for admin
        else:
            # operation for user
IndominusByte commented 3 years ago

I have made this modification in order to check for permissions, it works both in http and ws:

class NotEnoughPermissions(AuthJWTException):
    def __init__(self,status_code: int, message: str):
        self.status_code = status_code
        self.message = message

def permissions_required(self, token: Optional[str] = None, scopes:list=[], required_sub:int=None, override_sub:int=None) -> Optional[Dict[str,Union[str,int,bool]]]:
    decoded_token = self.get_raw_jwt(encoded_token=token)
    token_scopes = decoded_token["scopes"]
    token_sub = decoded_token["sub"]

    if override_sub and str(override_sub) == token_sub:
        pass
    elif len(scopes)>0:
        for scope in scopes:
            if scope not in token_scopes:
                raise NotEnoughPermissions(status_code=401, message="Not enough permissions")

    if required_sub and str(required_sub) != token_sub:
        raise NotEnoughPermissions(status_code=401, message="Not enough permissions")

AuthJWT.permissions_required = permissions_required

arguments:

  • scopes: the required scopes that token need to have
  • required_sub: meaning that along with the required scopes, the token sub has to match this required_sub
  • override_sub: meaning that if provided and matches token sub then that overrides the required scopes
  • token: encoded token has to be provided in case of websockets

required_sub benefits sometimes you may want a user to read their data while also meeting scopes requirement if any.

override_sub benefits if you have an API link with required scopes, for example, an admin can access, but also you want to allow a user to read his own data. so required scopes check will be ignored if override_sub matches token sub.

example Here both the scope and id have to match what is in the token.

@app.get("drivers/{id}/status/", response_model=schemas.DriverStatus)
async def driver_status(id:int, Authorize: AuthJWT = Depends(), db: Session = Depends(get_db)):
    Authorize.jwt_required()
    Authorize.permissions_required(scopes=["deliver"], required_sub=id)

we can get the id of the user from claim sub jwt instead again passed the id from the required_sub parameter, how about we validate scope in jwt_optional() function ?, I think we should keep scope parameter on function like jwt_required(), jwt_optional() so on. but thank you so much @SamiAlsubhi for sharing with me 😁

what do you think if you make PR to help me solve this issue? and should we make this scope system similar to this library auth0/express-jwt-authz

wetgi commented 3 years ago
@router.get('/my-user', response_model=typing.Union[UserData,AdminData])
async def my_user(authorize: AuthJWT = Depends(AuthJWT(scopes=["deliver"]))):

would by my suggestion

SamiAlsubhi commented 3 years ago

@IndominusByte I made a PR. It is an initial proposal. In the PR, all token scopes must meet required scopes by supplying scopes parameter optionally in jwt_required(), jwt_optional(), fresh_jwt_required() and jwt_refresh_token_required(). The library you suggested also make available of checking having at least one scope in the required scopes. We can implement this later if my proposal is initially OK.

basiltt commented 2 years ago

Is this already implemented ?

henadzit commented 1 year ago

Is this already implemented ?

This project is abandoned unfortunately.