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

V14/fix/cookie breaking installer #16993

Merged
merged 3 commits into from
Sep 3, 2024
Merged

Conversation

Migaroez
Copy link
Contributor

@Migaroez Migaroez commented Sep 2, 2024

Prerequisites

  • I have added steps to test this contribution in the description below

Description

During the boot process of a fresh install. The BackofficeDefaultController is hit and if a (timewise) valid cookie is found, the system will try to find the user bound to that cookie, but it can't since there are no users configured yet. So it errors out and breaks the install process

This PR uses the runtime state to figure out whether authentication is even possible in the current state. If not it fails the authentication without blowing up which results in the installer popping up as required.

Testing

To force the bug.

  • Start a new project
  • Run the installer.
  • Stop the server.
  • Clean the connectionstring and /umbraco folder
  • Try to run the installer again, it should not work. On this branch it should.

All other calls to the BackofficeDefaultController should be have as before.

@Migaroez Migaroez changed the base branch from contrib to v14/dev September 2, 2024 11:15
Copy link
Contributor

@kjac kjac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm!

Testing note: You must login after the first install in order to provoke the error on the second install 😄

@kjac kjac merged commit 087a01d into v14/dev Sep 3, 2024
16 checks passed
@kjac kjac deleted the v14/fix/cookie-breaking-installer branch September 3, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants