Skip to content
New issue

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

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

Already on GitHub? # to your account

Pass the configuration options in init_app #171

Open
azmeuk opened this issue Feb 21, 2024 · 4 comments
Open

Pass the configuration options in init_app #171

azmeuk opened this issue Feb 21, 2024 · 4 comments

Comments

@azmeuk
Copy link

azmeuk commented Feb 21, 2024

I encounter issues when initializing flask-pyoidc and using url_for to define callbacks URLs.

bp = Blueprint("endpoints", __name__)

user_provider_configuration = ProviderConfiguration(
    ...
    post_logout_redirect_uris=url_for("endpoints.logout", _external=True),
)

auth = OIDCAuthentication({"default": user_provider_configuration}, current_app)

@bp.route("/logout")
@auth.oidc_logout
def logout():
    ...

There is a cycling dependency here, the logout endpoint needs a OIDCAuthentication object, that needs a ProviderConfiguration, that calls url_for, that needs the endpoint to be initialized.

I think this can be mitigated by delaying the ProviderConfiguration part, for instance with the help of init_app. I suggest to pass the configuration options to init_app so this would be valid:

bp = Blueprint("endpoints", __name__)

auth = OIDCAuthentication()

@bp.route("/logout")
@auth.oidc_logout
def logout():
    ...

user_provider_configuration = ProviderConfiguration(
    ...
    post_logout_redirect_uris=url_for("endpoints.logout", _external=True),
)

auth.init_app(app, {"default": user_provider_configuration})

What do you think?

@infohash
Copy link
Contributor

flask.current_app & flask.url_for cannot work outside of application context. They can only be called either inside a view function or if the application context is manually pushed otherwise you will get this error:

RuntimeError: Working outside of application context.

This typically means that you attempted to use functionality that needed
the current application. To solve this, set up an application context
with app.app_context(). See the documentation for more information.

So, this line 👇 won't work:

>>> current_app.init_app({"default": user_provider_configuration})

You need access to the app object here. You can either import app object in your module or import your module to the module where your app object lives and register the blueprint there.

...
post_logout_redirect_uris=url_for("endpoints.logout", _external=True),
...

This 👆 won't work and not even this 👇:

with app.app_context():
    ...
    post_logout_redirect_uris=url_for("endpoints.logout", _external=True)
    ...

This is because in order for the app to resolve URL of the view function, the view function must be registered on the app but it hasn't happened yet as the view function has not been initialized yet.

You need to restructure your code:

auth.py

from flask_pyoidc import OIDCAuthentication
from flask_pyoidc.provider_configuration import ClientMetadata, ProviderConfiguration

client_metadata = ClientMetadata(
    client_id="client123",
    client_secret="some_secret123",
    post_logout_redirect_uris=["https://client.example.com/logout"]
)

provider_config = ProviderConfiguration(issuer="https://idp.example.com", client_metadata=client_metadata)

auth = OIDCAuthentication({"default": provider_config})

views.py

from flask import Blueprint

from auth import auth

bp = Blueprint("endpoints", __name__)


@bp.get("/")
def index():
    return "hello flask"


@bp.get("/logout")
@auth.oidc_logout
def logout():
    return "You have been logged out!"

run.py

from flask import Flask

from auth import auth
from views import bp

app = Flask(__name__)
app.register_blueprint(blueprint=bp)
auth.init_app(app=app)

if __name__ == "__main__":
    app.run()

A single script will look like this

from flask import Blueprint, Flask
from flask_pyoidc import OIDCAuthentication
from flask_pyoidc.provider_configuration import ClientMetadata, ProviderConfiguration

app = Flask(__name__)
bp = Blueprint("endpoints", __name__)

client_metadata = ClientMetadata(
    client_id="client123",
    client_secret="some_secret123",
    post_logout_redirect_uris=["https://client.example.com/logout"]
)
provider_config = ProviderConfiguration(issuer="https://idp.example.com", client_metadata=client_metadata)
# If app is present within the same module, you can directly pass it here,
# instead of calling "auth.init_app(app=app)".
auth = OIDCAuthentication(provider_configurations={"default": provider_config}, app=app)


@bp.get("/")
@auth.oidc_auth("default")
def index():
    return "hello flask"


@bp.get("/logout")
@auth.oidc_logout
def logout():
    return "You have been logged out!"


app.register_blueprint(blueprint=bp)

if __name__ == "__main__":
    app.run()

Note

You wouldn't have to configure post_logout_redirect_uris if your logout view function was decorated directly under app and not under the blueprint routes. Flask-pyoidc registers logout routes if they are registered on app but weren't declared in post_logout_redirect_uris argument:

post_logout_redirect_uris = client._provider_configuration._client_registration_info.get(
'post_logout_redirect_uris')
if not post_logout_redirect_uris:
client._provider_configuration._client_registration_info[
'post_logout_redirect_uris'] = self._get_urls_for_logout_views()
logger.debug(
f'''registering with post_logout_redirect_uris = {
client._provider_configuration._client_registration_info[
'post_logout_redirect_uris']}''')
client.register()

But it is recommended to explicitly declare it so that someone else reading your code can know it upfront.

@azmeuk
Copy link
Author

azmeuk commented Feb 24, 2024

Thank you for your answer.

In my original post I actually oversimplified my actual code to get to the point. My real usecase has a setup closer to the run.py/auth.py/views.py you describe. The issue I have with your example is that the post_logout_redirect_uris URL is an absolute URL, and I would like to use url_for there.

This is because in order for the app to resolve URL of the view function, the view function must be registered on the app but it hasn't happened yet as the view function has not been initialized yet.

This is why I suggest to pass the OIDCAuthentication configuration as init_app arguments. This way you can call auth.init_app within an app context, , allowing you to make calls to url_for, after having registered the blueprints.

To be able to use url_for to build a post_logout_redirect_uris value, I currently do something hacky like this:

views.py

from flask import Blueprint

from auth import auth

bp = Blueprint("endpoints", __name__)


@bp.get("/")
def index():
    return "hello flask"


@bp.get("/logout")
@auth.oidc_logout
def logout():
    return "You have been logged out!"

run.py

from flask_pyoidc import OIDCAuthentication
from flask_pyoidc.provider_configuration import ClientMetadata, ProviderConfiguration

app = Flask(__name__)
auth = OIDCAuthentication({"default": None})

def setup_auth(app):
    with app.app_context():
        logout_url = url_for("myendpoint.logout", _external=True)

    client_metadata = ClientMetadata(
        client_id="client123",
        client_secret="some_secret123",
        post_logout_redirect_uris=[logout_url]
    )

    default_provider_config = ProviderConfiguration(issuer="https://idp.example.com", client_metadata=client_metadata)

    auth._provider_configurations = {
        "default": default_provider_configuration,
    }
    auth.init_app(app)


def create_app():
    app.register_blueprint(blueprint=bp)
    setup_auth(app)
    return app

if __name__ == "__main__":
    create_app().run()

The hacky part being auth._provider_configurations = {...}.

I would love to be able to initialize flask-pyoidc this way:

app = Flask(__name__)
auth = OIDCAuthentication()

def setup_auth(app):
    ...
    auth.init_app(app, {
        "default": default_provider_configuration,
    })

...

infohash added a commit to infohash/Flask-pyoidc that referenced this issue Feb 25, 2024
Addresses zamzterz#171

Instead of hardcoding complete post logout redirect URI, oidc_logout should be able to resolve URL from the endpoint name of the view function.

We are already doing this for routes that are directly created on app instance. This feature extends the functionality for routes created by Bueprints.
@infohash
Copy link
Contributor

I think the bigger problem was that there was no way to dynamically resolve post logout redirect URI when logout routes are created by blueprints. The linked PR resolves that problem so you won't have to specify post_logout_redirect_uris. You can also keep post_logout_redirect_uris in app config.

@azmeuk
Copy link
Author

azmeuk commented Feb 28, 2024

Thank you putting efforts in helping me to find a solution. I really appreciate this 🙇

There is another thing I forgot to mention: some of the values passed to ProviderConfiguration come from the flask configuration (app.config), so an application context is needed at the moment the ProviderConfiguration is initialized.

I really think the ideal solution would be to be able to initialize an OIDCAuthentication object at the top level with no configuration, so it can be imported in blueprint files, and allow it to be configured by passing arguments to the init_app method. This way init_app could be called separately inside an application context, that would allow usage of app.config, url_for and any other Flask utility one would want to.

This two-steps configuration is a common pattern among other libraries like flask-babel, flask-caching, flask-cors or authlib.

This is an example of implementation in a project with huge quantities of legacy code I have no control over, including configuring the OIDC settings by the configuration:

https://github.com/numerique-gouv/b3desk/blob/main/web/b3desk/__init__.py#L182-L220

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

No branches or pull requests

2 participants