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

Use URL and username for 2FA app titles #40

Assignees
Labels
enhancement New feature or request owner: dimpact

Comments

@joeribekker
Copy link
Member

joeribekker commented Jul 1, 2024

See: open-formulieren/open-forms#4455

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

In all components.

@joeribekker joeribekker added triage Triage means the team has not yet refined this issue. enhancement New feature or request owner: dimpact labels Jul 2, 2024
@joeribekker joeribekker removed the triage Triage means the team has not yet refined this issue. label Jul 2, 2024
@joeribekker joeribekker moved this from Triage to Todo in Data en API fundament Jul 2, 2024
@stevenbal stevenbal moved this from Todo to In Progress in Data en API fundament Aug 15, 2024
stevenbal added a commit to maykinmedia/open-klant that referenced this issue Aug 15, 2024
to ensure that we no longer rely on in when generating the label for 2FA
@stevenbal
Copy link
Contributor

stevenbal commented Aug 15, 2024

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

@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 stevenbal added the blocked Issue is blocked. There should be a comment that explains why. label Aug 19, 2024
@stevenbal
Copy link
Contributor

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

@alextreme
Copy link
Member

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

Created a separate issue for removing sites #59

@stevenbal stevenbal removed the blocked Issue is blocked. There should be a comment that explains why. label Aug 22, 2024
stevenbal added a commit to open-zaak/open-zaak that referenced this issue Aug 22, 2024
stevenbal added a commit to open-zaak/open-zaak that referenced this issue Aug 22, 2024
stevenbal added a commit to maykinmedia/open-klant that referenced this issue Aug 23, 2024
@github-project-automation github-project-automation bot moved this from Implemented to Done in Data en API fundament Aug 23, 2024
@stevenbal stevenbal reopened this Aug 23, 2024
@github-project-automation github-project-automation bot moved this from Done to In Progress in Data en API fundament Aug 23, 2024
@github-project-automation github-project-automation bot moved this from Implemented to Done in Data en API fundament Aug 29, 2024
@stevenbal stevenbal reopened this Aug 29, 2024
@github-project-automation github-project-automation bot moved this from Done to In Progress in Data en API fundament Aug 29, 2024
stevenbal added a commit to open-zaak/open-notificaties that referenced this issue Aug 29, 2024
stevenbal added a commit to maykinmedia/objects-api that referenced this issue Aug 29, 2024
stevenbal added a commit to maykinmedia/objecttypes-api that referenced this issue Aug 29, 2024
@stevenbal stevenbal moved this from In Progress to Implemented in Data en API fundament Aug 29, 2024
@github-project-automation github-project-automation bot moved this from Implemented to Done in Data en API fundament Sep 18, 2024
@annashamray annashamray reopened this Sep 18, 2024
@github-project-automation github-project-automation bot moved this from Done to In Progress in Data en API fundament Sep 18, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Data en API fundament Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment