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

[Question] Initialize SingleTenantAzureAuthorizationCodeBearer at non-top level #111

Closed
dabljues opened this issue Jan 28, 2023 · 6 comments
Labels
question Further information is requested

Comments

@dabljues
Copy link

Related SO question: https://stackoverflow.com/questions/75262398/fastapi-read-configuration-before-specifying-dependencies.

Hi. Let me just say that I'm very happy to use this library because it made the Azure authentication for me a piece of cake!

I have a problem with initializing SingleTenantAzureAuthorizationCodeBearer with values from a config file.

After I've succesfully added authentication to my API following the guide on the docs page, I started to prepare a version that would be production ready. And that would mean: values read from a config file. That's because we use Kubernetes so we have an easy way to provide a ConfigMap for a container.

Let's say this is my API code (I can split this into multiple files, but in this case it doesn't matter):

from fastapi import FastAPI
from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer
from pydantic import BaseSettings
import json

class Settings(BaseSettings):
    client_id: str
    tenant_id: str

with open(os.getenv("CONFIG_FILE", "config.json")) as f:
    config_data = json.load(f)

settings = Settings(client_id=config_data["client_id"], tenant_id=config_data["tenant_id"])

azure_scheme = SingleTenantAzureAuthorizationCodeBearer(
    app_client_id=settings.APP_CLIENT_ID,
    tenant_id=settings.TENANT_ID,
    scopes={
        f'api://{settings.APP_CLIENT_ID}/user_impersonation': 'user_impersonation',
    }
)

api = FastAPI(
   title="foo"
)

def init_api() -> FastAPI:
   # I want to read configuration here
   api.swagger_ui.init_oauth = {"clientID": config.CLIENT_ID}
   return api

@api.on_event('startup')
async def load_config() -> None:
    """
    Load OpenID config on startup.
    """
    await azure_scheme.openid_config.load_config()

@api.get("/", dependencies=[Depends(azure_scheme)])
def test():
   return {"hello": "world"}

This obviously works because both azure_scheme and settings get evaluated before load_config and test route definition.

However, this makes testing ugly, because now there's a config file to be read. Even pytest fixtures cannot be evaluated before imports in tests, so I end up with errors - "no config.json" etc. Plus, even if I'm able to "hack" this, every time I do an import from api.py, this code gets run - it's just not good.

I provided an init_api function. This is a perfect example - in my code I do other stuff there, initialize logging, do some other stuff. That way, in tests, I don't have to call init_api, I just import from <app_name>.api import api and do test on it - therefore I dodge all the stuff that I'd otherwise have to mock.

My only hack was to do something like this:

# [...]
settings = None
azure_scheme = None

api = FastAPI(
   title="foo"
)

def init_api():
    with open(os.getenv("CONFIG_FILE", "config.json")) as f:
        config_data = json.load(f)
    global settings
    settings = Settings(client_id=config_data["client_id"], tenant_id=config_data["tenant_id"])

    global azure_scheme
    azure_scheme = SingleTenantAzureAuthorizationCodeBearer(
        app_client_id=settings.APP_CLIENT_ID,
        tenant_id=settings.TENANT_ID,
        scopes={
            f'api://{settings.APP_CLIENT_ID}/user_impersonation': 'user_impersonation',
        }
    )

    api.add_api_route("/", endpoint=test, dependencies=[Depends(azure_scheme)])
  
@api.on_event('startup')
async def load_config() -> None:
    """
    Load OpenID config on startup.
    """
    if azure_scheme:
        await azure_scheme.openid_config.load_config()

def test():
   return {"hello": "world"}

So I took out the @api decorator and used .add_api_route to init_api. It works, but it's not ideal. In some ways it creates more issues than it solves (typing, more complicated code, access to globals).

Sorry for this long monologue, my question is brief: How can I initialize SingleTenantAzureAuthorizationCodeBearer as a dependency without having to read the config globally? And maybe without so much hacking as I've shown above 😄

@dabljues dabljues added the question Further information is requested label Jan 28, 2023
@JonasKs
Copy link
Member

JonasKs commented Jan 28, 2023

Hi! Thanks for the kind feedback 😊
I'm on my way out, replying from my phone, so I'll answer a bit short, and follow up at a later point.

What we do is load all these things as environment variables/.env file:

class CustomBaseSettings(BaseSettings):
    """Configure .env settings for all our setting-classes"""
    class Config:
        env_file = '.env'
        env_file_encoding = 'utf-8'
        case_sensitive = True


class Env(CustomBaseSettings):
    ENVIRONMENT: Literal['dev', 'lab', 'prod', 'qa', 'test'] = Field(..., env='ENVIRONMENT')

Then our pytest.ini looks like this:

[pytest]
addopts = --allow-hosts=localhost,127.0.0.1,::1
asyncio_mode = auto
env =
    ENVIRONMENT=test
env_files =
     .env
env_override_existing_values = 0

Don't exactly remember the purest plug-in, but it's either pytest-env or pytest-dotenv.

@dabljues
Copy link
Author

dabljues commented Jan 28, 2023

This is what I ended up doing. I mean I still have this hack with the config file on a separate branch I shown above, but it's kinda messy. When I went for the env variables, it gets easier. I don't provide them via pytest because I just have defaults for them and in tests I also use api.dependency_overrides to temporarily get rid of the SingleTenantAzureAuthorizationCodeBearer dependency.

I badly wanted to go for a config file (and this would usually be placed elsewhere, not like .env file), but seeing as clientId and tenantId are not exactly secrets, they may as well be passed by env variables, even hardcoded if someone doesn't care/wont be changing it on a regular basis. So, yeah, env variables are the way to go here, much easier.

Thanks for the feedback!

@JonasKs
Copy link
Member

JonasKs commented Jan 28, 2023

You're welcome. Thanks for following up!

Have a good weekend😊

@JonasKs JonasKs closed this as completed Jan 28, 2023
@martimors
Copy link

This will be much cleaner once the dependency injection feature in FastAPI supports lifespan dependencies, tiangolo/fastapi#617 . For now, I see no way around this than configuring azure_scheme with env-variables like mentioned above.

@JonasKs
Copy link
Member

JonasKs commented Jun 13, 2023

Agree.

@martimors
Copy link

In case anyone lands here, I opted to solve this using a global value and omitting the lifespan event entirely. This allowed me to use the dependency injection system without validating the app settings for each and every test. The important caveat is that, just like all other dependencies, the configuration is not validated on startup, but deterred until the first request. I get around this by writing some custom health-checks that k8s wait for (readiness hooks).

from typing import Annotated

from fastapi import Depends
from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer

from myapp.settings import Settings

azure_scheme = None  # When tests import this module, azure_scheme is not instantiated


def get_settings() -> Settings:
    """Settings are injected, not global"""
    return Settings()

SettingsDep = Annotated[Settings, Depends(get_settings)]

async def get_azure_scheme(settings: SettingsDep) -> SingleTenantAzureAuthorizationCodeBearer:
    global azure_scheme  # noqa: PLW0603 Use global until lifespan dependencies are available
    if not azure_scheme:
        azure_scheme = SingleTenantAzureAuthorizationCodeBearer(
            app_client_id=settings.azuread_app_client_id,
            tenant_id=settings.azuread_tenant_id,
            scopes={
                f"{settings.azuread_app_identifier_uri}/{settings.azuread_scope}": settings.azuread_scope
            },
        )
    return azure_scheme


AzureSchemeDep = Annotated[SingleTenantAzureAuthorizationCodeBearer, Depends(get_azure_scheme)]

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

No branches or pull requests

3 participants