Register the endpoints list without a trailing slash, and stop hiding its errors
An endpoint reported telemetry correctly for hours while the Fleet page showed
"No endpoints found". The PC detail page displayed its panels the whole time, so
ingest was plainly working; only the list was empty.
The list route was the one route in 331 paths registered with a trailing slash:
@endpoints_bp.route('/', methods=['GET']) -> /api/endpoints/
The browser client calls it without one, so Flask answered a 308 redirect to an
ABSOLUTE url. On dev that resolves and nobody notices. Behind the IIS subpath it
cannot: with no ProxyFix, Werkzeug builds that Location from the scheme and host
it believes it has, so it emitted http:// and dropped the /shopdb prefix. A
browser will not follow an https page to http, the request failed, and the
page's catch block rendered the empty state.
Every other route in the plugin worked throughout, because none of them carries
a trailing slash - the detail panels call /endpoints/asset/<id>/status and were
never redirected. Detail works, list empty is the signature of this bug.
Registered as '' now, so the rule is /api/endpoints. That is what
CONTRIBUTING.md's API section spells and what every other list route in the
product already did. The inventory and the generated spec are updated to match.
Also here:
- tests/test_core/test_no_redirecting_routes.py fails the build if any /api
rule is registered with a trailing slash and strict slashes. Verified by
planting the violation: it named the offending route, then passed once
reverted. It takes the shared app fixture rather than building its own
application - doing that rebinds SQLAlchemy and leaves the NEXT module
without tables, which showed up as three unrelated "no such table: settings"
failures visible only in a full run.
- The Fleet page now renders "Could not load endpoints (HTTP nnn). This is a
failed request, not an empty fleet." A list that reports a 403 and an empty
fleet identically is what made this take as long as it did.
- TRUSTED_PROXY_COUNT (default 0) applies ProxyFix when set. Off by default on
purpose: it makes X-Forwarded-* trusted, and a client can send those itself,
while the login rate limit and account lockout are keyed on the client
address. Set it to 1 on an instance behind IIS.
Full suite 2124 passed, vitest 260 passed, naming and docs gates green.
This commit is contained in:
12
.env.example
12
.env.example
@@ -36,6 +36,18 @@ DATABASE_URL=mysql+pymysql://shopdb:CHANGE_ME@db:3306/shopdb_flask?charset=utf8m
|
|||||||
# Wildcard '*' is rejected by ProductionConfig.validate().
|
# Wildcard '*' is rejected by ProductionConfig.validate().
|
||||||
CORS_ORIGINS=http://localhost:5173
|
CORS_ORIGINS=http://localhost:5173
|
||||||
|
|
||||||
|
# ---- Reverse proxy (optional, default 0 = no proxy) ----
|
||||||
|
# Set to the number of proxies actually in front of this instance - 1 when IIS
|
||||||
|
# or nginx serves it under a subpath such as /shopdb. Without it the app sees
|
||||||
|
# the proxy's connection instead of the browser's (scheme http, the proxy's
|
||||||
|
# host and IP), so any absolute URL it builds is wrong at the browser. That is
|
||||||
|
# what made a redirect on the endpoints list unfollowable behind IIS.
|
||||||
|
#
|
||||||
|
# Leave it at 0 when nothing fronts the app. It makes X-Forwarded-* trusted,
|
||||||
|
# and a client can send those headers itself - the login rate limit and the
|
||||||
|
# account lockout are keyed on the client address.
|
||||||
|
TRUSTED_PROXY_COUNT=0
|
||||||
|
|
||||||
# ---- JWT lifecycle (optional, defaults shown) ----
|
# ---- JWT lifecycle (optional, defaults shown) ----
|
||||||
JWT_ACCESS_TOKEN_EXPIRES=3600
|
JWT_ACCESS_TOKEN_EXPIRES=3600
|
||||||
JWT_REFRESH_TOKEN_EXPIRES=2592000
|
JWT_REFRESH_TOKEN_EXPIRES=2592000
|
||||||
|
|||||||
@@ -3757,11 +3757,11 @@
|
|||||||
"endpoints": [
|
"endpoints": [
|
||||||
{
|
{
|
||||||
"method": "GET",
|
"method": "GET",
|
||||||
"path": "/api/endpoints/",
|
"path": "/api/endpoints",
|
||||||
"auth": "endpoints.view",
|
"auth": "endpoints.view",
|
||||||
"params": "query: search (hostname substring)",
|
"params": "query: search (hostname substring)",
|
||||||
"purpose": "List endpoints reporting through the ShopDB agent, with last-seen and online state",
|
"purpose": "List endpoints reporting through the ShopDB agent, with last-seen and online state",
|
||||||
"example": "curl -H \"Authorization: Bearer $TOKEN\" http://localhost:5001/api/endpoints/"
|
"example": "curl -H \"Authorization: Bearer $TOKEN\" http://localhost:5001/api/endpoints"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"method": "GET",
|
"method": "GET",
|
||||||
|
|||||||
@@ -21733,13 +21733,13 @@
|
|||||||
]
|
]
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"/api/endpoints/": {
|
"/api/endpoints": {
|
||||||
"get": {
|
"get": {
|
||||||
"tags": [
|
"tags": [
|
||||||
"plugin-endpoints"
|
"plugin-endpoints"
|
||||||
],
|
],
|
||||||
"summary": "List endpoints reporting through the ShopDB agent, with last-seen and online state",
|
"summary": "List endpoints reporting through the ShopDB agent, with last-seen and online state",
|
||||||
"description": "List endpoints reporting through the ShopDB agent, with last-seen and online state\n\n**Auth:** endpoints.view\n\n**Params:** query: search (hostname substring)\n\n**Example:**\n```\ncurl -H \"Authorization: Bearer $TOKEN\" http://localhost:5001/api/endpoints/\n```",
|
"description": "List endpoints reporting through the ShopDB agent, with last-seen and online state\n\n**Auth:** endpoints.view\n\n**Params:** query: search (hostname substring)\n\n**Example:**\n```\ncurl -H \"Authorization: Bearer $TOKEN\" http://localhost:5001/api/endpoints\n```",
|
||||||
"security": [
|
"security": [
|
||||||
{
|
{
|
||||||
"bearerAuth": []
|
"bearerAuth": []
|
||||||
|
|||||||
@@ -55,11 +55,25 @@ def _format_uptime(seconds):
|
|||||||
# Endpoint state
|
# Endpoint state
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
|
|
||||||
@endpoints_bp.route('/', methods=['GET'])
|
@endpoints_bp.route('', methods=['GET'])
|
||||||
@require_permission('endpoints.view')
|
@require_permission('endpoints.view')
|
||||||
def list_endpoints():
|
def list_endpoints():
|
||||||
"""List reported endpoint states.
|
"""List reported endpoint states.
|
||||||
|
|
||||||
|
Registered as '' so the rule is '/api/endpoints', with no trailing slash.
|
||||||
|
CONTRIBUTING.md's API section spells endpoints that way ('/api/machines'),
|
||||||
|
and every other plugin's list route follows it - this was the only route in
|
||||||
|
331 paths that did not.
|
||||||
|
|
||||||
|
It was not cosmetic. A rule ending in a slash makes Flask answer the
|
||||||
|
unslashed URL with a 308 to an ABSOLUTE url. On dev that resolves and
|
||||||
|
nobody notices. Behind the IIS subpath it does not: with no ProxyFix,
|
||||||
|
Werkzeug builds that Location from the scheme and host it believes it has,
|
||||||
|
so it emitted http:// and dropped the /shopdb prefix. The browser refuses
|
||||||
|
to follow an https page to http, the list request failed, and the page
|
||||||
|
rendered "No endpoints found" - indistinguishable from a fleet with nothing
|
||||||
|
in it. The agent had been reporting correctly the whole time.
|
||||||
|
|
||||||
Joins through Computer so each row carries computerid and can deep-link
|
Joins through Computer so each row carries computerid and can deep-link
|
||||||
into the existing PC detail page (/pcs/{computerid}). Endpoint state only
|
into the existing PC detail page (/pcs/{computerid}). Endpoint state only
|
||||||
exists for hostnames resolved to a Computer, so the join is not lossy.
|
exists for hostnames resolved to a Computer, so the join is not lossy.
|
||||||
|
|||||||
@@ -18,6 +18,10 @@
|
|||||||
<div class="card">
|
<div class="card">
|
||||||
<div v-if="loading" class="loading">Loading...</div>
|
<div v-if="loading" class="loading">Loading...</div>
|
||||||
|
|
||||||
|
<div v-else-if="loadError" class="load-error">
|
||||||
|
{{ loadError }}
|
||||||
|
</div>
|
||||||
|
|
||||||
<template v-else>
|
<template v-else>
|
||||||
<div class="table-container">
|
<div class="table-container">
|
||||||
<table>
|
<table>
|
||||||
@@ -78,6 +82,12 @@ import { useListQuery } from '@/composables/listQuery'
|
|||||||
const router = useRouter()
|
const router = useRouter()
|
||||||
const endpoints = ref([])
|
const endpoints = ref([])
|
||||||
const loading = ref(true)
|
const loading = ref(true)
|
||||||
|
// A failed request used to leave endpoints as [] and render "No endpoints
|
||||||
|
// found", which is the same thing a healthy empty fleet shows. A trailing-slash
|
||||||
|
// redirect broke this list in production while the agent reported correctly all
|
||||||
|
// along, and the page said only that there was nothing to see. An error must
|
||||||
|
// look like an error.
|
||||||
|
const loadError = ref('')
|
||||||
const { page, search, setPage, setSearch } = useListQuery({ onChange: loadEndpoints })
|
const { page, search, setPage, setSearch } = useListQuery({ onChange: loadEndpoints })
|
||||||
const totalPages = ref(1)
|
const totalPages = ref(1)
|
||||||
const perPage = ref(20)
|
const perPage = ref(20)
|
||||||
@@ -97,11 +107,20 @@ async function loadEndpoints() {
|
|||||||
}
|
}
|
||||||
if (search.value) params.search = search.value
|
if (search.value) params.search = search.value
|
||||||
|
|
||||||
|
loadError.value = ''
|
||||||
const response = await endpointsApi.list(params)
|
const response = await endpointsApi.list(params)
|
||||||
endpoints.value = response.data.data || []
|
endpoints.value = response.data.data || []
|
||||||
totalPages.value = response.data.meta?.pagination?.totalpages || response.data.meta?.pagination?.total_pages || 1
|
totalPages.value = response.data.meta?.pagination?.totalpages || response.data.meta?.pagination?.total_pages || 1
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('Error loading endpoints:', error)
|
console.error('Error loading endpoints:', error)
|
||||||
|
endpoints.value = []
|
||||||
|
// Name the status. 403 and 404 are the two that get misread as "no
|
||||||
|
// endpoints": the first means the role lacks endpoints.view, the second
|
||||||
|
// that the request never reached the route at all.
|
||||||
|
const status = error.response?.status
|
||||||
|
loadError.value = status
|
||||||
|
? `Could not load endpoints (HTTP ${status}). This is a failed request, not an empty fleet.`
|
||||||
|
: 'Could not reach the server to load endpoints.'
|
||||||
} finally {
|
} finally {
|
||||||
loading.value = false
|
loading.value = false
|
||||||
}
|
}
|
||||||
@@ -150,4 +169,10 @@ function changePerPage(newPerPage) {
|
|||||||
cursor: default;
|
cursor: default;
|
||||||
opacity: 0.6;
|
opacity: 0.6;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.load-error {
|
||||||
|
padding: 1.5rem;
|
||||||
|
text-align: center;
|
||||||
|
color: var(--danger);
|
||||||
|
}
|
||||||
</style>
|
</style>
|
||||||
|
|||||||
@@ -131,6 +131,28 @@ def create_app(config_name: str = None) -> Flask:
|
|||||||
if envname.startswith('COLLECTOR_API_KEY_') and envvalue:
|
if envname.startswith('COLLECTOR_API_KEY_') and envvalue:
|
||||||
app.config[envname] = envvalue
|
app.config[envname] = envvalue
|
||||||
|
|
||||||
|
# Behind a reverse proxy (IIS at /shopdb, nginx, a load balancer), the app
|
||||||
|
# sees the PROXY's connection, not the browser's: scheme http even though
|
||||||
|
# the user is on https, the proxy's host, the proxy's IP. Anything Flask
|
||||||
|
# builds from that is wrong at the browser - an absolute redirect Location
|
||||||
|
# above all, which is how a trailing-slash redirect on the endpoints list
|
||||||
|
# sent an https page to an http URL the browser then refused to follow.
|
||||||
|
#
|
||||||
|
# OFF BY DEFAULT AND DELIBERATELY. ProxyFix makes the app TRUST
|
||||||
|
# X-Forwarded-* headers, and a client can send those itself. Enabled with no
|
||||||
|
# proxy in front, any caller can claim a different scheme or source IP -
|
||||||
|
# which matters here because the login rate limit and the lockout are keyed
|
||||||
|
# on the client address. Set TRUSTED_PROXY_COUNT to the number of proxies
|
||||||
|
# actually in front of this instance (1 for a single IIS or nginx), and only
|
||||||
|
# when one really is.
|
||||||
|
proxycount = app.config.get('TRUSTED_PROXY_COUNT', 0)
|
||||||
|
if proxycount:
|
||||||
|
from werkzeug.middleware.proxy_fix import ProxyFix
|
||||||
|
app.wsgi_app = ProxyFix(
|
||||||
|
app.wsgi_app,
|
||||||
|
x_for=proxycount, x_proto=proxycount,
|
||||||
|
x_host=proxycount, x_prefix=proxycount)
|
||||||
|
|
||||||
# Ensure instance folder exists
|
# Ensure instance folder exists
|
||||||
os.makedirs(app.instance_path, exist_ok=True)
|
os.makedirs(app.instance_path, exist_ok=True)
|
||||||
|
|
||||||
|
|||||||
@@ -49,6 +49,15 @@ class Config:
|
|||||||
seconds=int(os.environ.get('JWT_REFRESH_TOKEN_EXPIRES', 2592000))
|
seconds=int(os.environ.get('JWT_REFRESH_TOKEN_EXPIRES', 2592000))
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Number of reverse proxies in front of this instance. 0 (the default)
|
||||||
|
# means none, and the app reads the connection it actually receives.
|
||||||
|
# Set to 1 when IIS or nginx fronts it, so X-Forwarded-Proto/Host/Prefix are
|
||||||
|
# honoured and a generated absolute URL matches what the browser asked for.
|
||||||
|
# Do NOT set it without a proxy: it makes the app believe forwarded headers
|
||||||
|
# a client can send itself, and the login rate limit and account lockout are
|
||||||
|
# keyed on the client address.
|
||||||
|
TRUSTED_PROXY_COUNT = int(os.environ.get('TRUSTED_PROXY_COUNT', 0))
|
||||||
|
|
||||||
CORS_ORIGINS = [
|
CORS_ORIGINS = [
|
||||||
origin.strip()
|
origin.strip()
|
||||||
for origin in os.environ.get('CORS_ORIGINS', 'http://localhost:5173').split(',')
|
for origin in os.environ.get('CORS_ORIGINS', 'http://localhost:5173').split(',')
|
||||||
|
|||||||
44
tests/test_core/test_no_redirecting_routes.py
Normal file
44
tests/test_core/test_no_redirecting_routes.py
Normal file
@@ -0,0 +1,44 @@
|
|||||||
|
"""No API route may answer a redirect just because of a trailing slash.
|
||||||
|
|
||||||
|
A rule registered as '/api/thing/' does not merely accept that URL - Werkzeug
|
||||||
|
answers '/api/thing' with a 308 to the slashed form, and the Location it builds
|
||||||
|
is ABSOLUTE. That is fine on the dev server, where scheme and host are what the
|
||||||
|
app thinks they are. It is not fine behind the IIS subpath: with no ProxyFix the
|
||||||
|
app emits http:// and without the /shopdb prefix, the browser refuses to follow
|
||||||
|
an https page to http, and the call fails.
|
||||||
|
|
||||||
|
What made it expensive is where the failure surfaced. The Fleet page caught the
|
||||||
|
error, logged it to the console and rendered "No endpoints found", which reads
|
||||||
|
exactly like a fleet with nothing in it. The agent had been reporting correctly
|
||||||
|
for hours. Every other route in the plugin worked, because every other route is
|
||||||
|
registered without a trailing slash - '/api/endpoints/' was the only one in 331
|
||||||
|
paths, and it was the only thing broken.
|
||||||
|
|
||||||
|
So the rule is: register API routes WITHOUT a trailing slash, or pass
|
||||||
|
strict_slashes=False so both spellings match directly. Either way no client can
|
||||||
|
be handed a redirect it cannot follow.
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_api_route_redirects_on_a_trailing_slash(app):
|
||||||
|
# Takes the shared `app` fixture rather than calling create_app itself.
|
||||||
|
# Building a second application inside a test rebinds SQLAlchemy and leaves
|
||||||
|
# the NEXT test module without tables - it passes alone and fails only in a
|
||||||
|
# full run, as "no such table: settings" three modules later. This file
|
||||||
|
# caused exactly that once already.
|
||||||
|
offenders = sorted(
|
||||||
|
rule.rule for rule in app.url_map.iter_rules()
|
||||||
|
if rule.rule.startswith('/api')
|
||||||
|
and rule.rule.endswith('/')
|
||||||
|
and rule.rule != '/api/'
|
||||||
|
and rule.strict_slashes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert not offenders, (
|
||||||
|
'%d API route(s) are registered with a trailing slash AND strict '
|
||||||
|
'slashes, so Flask answers the unslashed URL with a 308 to an absolute '
|
||||||
|
'URL. Behind a reverse proxy that redirect is unfollowable and the call '
|
||||||
|
'fails, usually looking like an empty list rather than an error.\n'
|
||||||
|
'Register the route without the trailing slash, or pass '
|
||||||
|
'strict_slashes=False:\n %s'
|
||||||
|
% (len(offenders), '\n '.join(offenders)))
|
||||||
Reference in New Issue
Block a user