search

maykinmedia / open-api-framework

The Open API framework powers the open source registration components like Open Zaak en Open Klant.
MIT License
0 stars 0 forks source link

Use URL and username for 2FA app titles #40

Closed joeribekker closed 1 week ago

joeribekker commented 3 months ago

See: https://github.com/open-formulieren/open-forms/issues/4455

Remove the dependency on Sites and just use the URL that shows the page (ie. requests.hostname)

In all components.

stevenbal commented 1 month ago

I was looking into this and notificed that Objecttypes and Open Notificaties rely (partially) on sites for setup_configuration (in case OPENNOTIFICATIES_DOMAIN/OBJECTTYPES_DOMAIN aren't configured. Do you think we can deprecate sites at this point? Or is it still too recent since these domain envvars were added?

The same applies for Open Zaak and Objects, though both of these also use sites under the hood for create_kanaal in notifications_api_common (https://github.com/maykinmedia/notifications-api-common/blob/7abd32eaa9a2d8a748fcb9dbd232cbe51105be92/notifications_api_common/management/commands/register_kanalen.py#L32), so I think that requires some changes in that library (perhaps we could specify a path to a function to get the domain and then import that path in create_kanaal)

As an intermediate fix for the 2FA app title without immediately deprecating sites, I could also override the function of the view that generates the app title: https://github.com/jazzband/django-two-factor-auth/blob/06547e23f9596e81bec3c585e8276fa98d00f999/two_factor/views/core.py#L711

@annashamray thoughts?

annashamray commented 1 month ago

@stevenbal we yeah we use Sites for notifications and management commands and I think we mark it as deprecated only for Open Zaak Objecttypes API doesn't create kanalen and doesn't sent notifications, so Sites can be removed there with minimal effort.

It's another story with Objects API. To be honest I don't know how it's configured in prod environments. Should we discuss it with Alex?

stevenbal commented 1 month ago

@annashamray I've marked it as blocked, we can discuss it next standup

alextreme commented 1 month ago

Discussed, and removing Sites from all components is to be split off into a separate issue, I suspect that this will cause more problems than we expect with other dependancies.

For the issue at hand go for overriding the function that generates the app title.

stevenbal commented 1 month ago

Created a separate issue for removing sites https://github.com/maykinmedia/open-api-framework/issues/59