Register the endpoints list without a trailing slash, and stop hiding its errors
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
|
||||||
|
|||||||
@@ -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