Skip to content

Add sanity check for server configuration#1136

Open
alexisbirling-boop wants to merge 6 commits intowger-project:masterfrom
alexisbirling-boop:add-sanity-check-server-config
Open

Add sanity check for server configuration#1136
alexisbirling-boop wants to merge 6 commits intowger-project:masterfrom
alexisbirling-boop:add-sanity-check-server-config

Conversation

@alexisbirling-boop
Copy link
Copy Markdown
Contributor

Proposed Changes

  • Add automatic server configuration check after user login
  • Detect reverse proxy misconfigurations by comparing pagination URLs with base URL
  • Display warning dialog to users when misconfiguration is detected (e.g., pagination links pointing to localhost instead of actual domain)
  • Implement graceful error handling to avoid blocking login on check failures
  • Add internationalization support for user-facing dialog messages
  • Add unit tests covering various misconfiguration scenarios

Related Issue(s)

Closes #1116

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Set a 100-character limit to avoid white space diffs (run dart format .)

Screenshot of the sanity check widget (while testing)

issue3

@rolandgeider
Copy link
Copy Markdown
Member

thanks for the PR!

I would keep the error message as short as possible and just point users to the online docs, we can update those more easily (and describe the problem better). So something like "Server misconfiguration detected, headers are not being passed correctly, please consult the docs".

I'll take a look at the rest later today.

@alexisbirling-boop
Copy link
Copy Markdown
Contributor Author

Alright, I'll start working on the changes


if (context.mounted && res == LoginActions.proceed) {
final authProvider = context.read<AuthProvider>();
if (authProvider.serverConfigWarning) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be more consistent to use something like AuthState.updateRequired here


/// Checks if the server's reverse proxy is configured correctly
/// by comparing pagination URLs with the base URL domain.
Future<SanityCheckResult> checkServerPaginationUrls({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would move this to the auth provider (while it's not strictly anything to do with authentication is related enough, plus we already have the min application version check there)

}

/// Result of a server sanity check
class SanityCheckResult {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a boolean might be enough 😄 This can be useful if we added more checks, but at the moment there's nothing planed

builder: (context) => AlertDialog(
title: Row(
children: [
const Icon(Icons.warning, color: Colors.orange, size: 28),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

better use something like Theme.of(context).colorScheme.error instead of hard coding the color, then this updates automatically if we change something there

@rolandgeider rolandgeider linked an issue Mar 17, 2026 that may be closed by this pull request
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.

Add sanity check for server configuration

2 participants