Make the one-line install survive Defender, and approval leave a PC that works

All three found testing the bootstrap installer on Windows.

- The published command used the irm-pipe-iex form, which Microsoft Defender
  detects as Trojan:Win32/Commando.A!ml and kills before it runs, every time,
  on a default install. The command now downloads the script to a file, runs
  it, and removes it in a finally block, which Defender does not object to.
  The UI also offers the script as a plain download for a tech who would
  rather save it.
- Approving a request only linked the asset, so a PC whose pending reason was
  "unknown hostname" - which is every PC installed this way - kept getting 403
  on every call afterwards, because its computer record had no hostname to
  match the credential against. Approval now stamps the hostname when the
  record has none, refuses with a 409 naming both when the record has a
  different one, and overwrites only when the admin confirms. The Credentials
  tab offers that confirmation.
- A hostname that resolves to no record now says so instead of reading as an
  impostor, while still refusing the request.
- While waiting for approval the agent retried on the 15-minute telemetry
  cadence, so a PC approved immediately enrolled a quarter of an hour later
  and the installer always ended on "still pending". It now retries about
  every minute for the first ten minutes, then every five to thirty minutes,
  then every fifteen, with jitter. A rate limit or server error still uses the
  slower backoff.

tests/test_plugins/test_endpoints_bootstrap_install.py

@@ -27,6 +27,17 @@ TOKENS_URL = '/api/endpoints/siteagent/bootstrap/tokens'
 SCRIPT_URL = '/api/endpoints/siteagent/bootstrap.ps1'
 PACKAGE_URL = '/api/endpoints/siteagent/bootstrap/package'
 BASE_URL = 'https://shopdb.test/shopdb'
+TEMP_SCRIPT = r'C:\Windows\Temp\shopdb-bootstrap.ps1'
+
+
+def _expected_command(secret, base=BASE_URL):
+    """Download, run, delete. Never `irm | iex` - Defender kills that shape."""
+    return ('powershell -ExecutionPolicy Bypass -c "'
+            "try {{ Invoke-WebRequest -UseBasicParsing "
+            "'{0}/api/endpoints/siteagent/bootstrap.ps1?t={1}' "
+            "-OutFile '{2}'; & '{2}' }} finally {{ "
+            "Remove-Item '{2}' -Force -ErrorAction SilentlyContinue }}\""
+            ).format(base, secret, TEMP_SCRIPT)
 
 
 def _make_cert():
@@ -98,10 +109,10 @@ def test_generate_returns_the_secret_and_the_ready_made_command(
     secret = data['secret']
     assert secret.startswith('sdbb_')
     assert len(secret) == len('sdbb_') + 43
-    assert data['command'] == (
-        'powershell -ExecutionPolicy Bypass -c "irm '
-        '{0}/api/endpoints/siteagent/bootstrap.ps1?t={1} | iex"'.format(
-            BASE_URL, secret))
+    assert data['command'] == _expected_command(secret)
+    assert data['scripturl'] == (
+        '{0}/api/endpoints/siteagent/bootstrap.ps1?t={1}'.format(BASE_URL,
+                                                                 secret))
     token = data['token']
     assert token['tokenprefix'] == secret[:12]
     assert token['maxuses'] == 3
@@ -111,6 +122,29 @@ def test_generate_returns_the_secret_and_the_ready_made_command(
     assert token['createdby'] == 'testadmin'
 
 
+def test_the_command_downloads_and_runs_instead_of_piping_into_iex(
+        client, db, auth_headers):
+    """Defender detects `irm ... | iex` as Trojan:Win32/Commando.A!ml and kills
+    the process before a line of it runs, so the published command downloads to
+    a file, runs it and removes it. It also stays one pasteable line, and holds
+    no `$`: pasted into a PowerShell prompt, the parent would expand those
+    before the child process ever saw them."""
+    data = _secret(client, auth_headers)
+    command = data['command']
+
+    assert ' | iex' not in command
+    assert 'iex' not in command.lower()
+    assert '$' not in command
+    assert '\n' not in command
+    assert data['scripturl'] in command
+    assert data['secret'] in command
+    assert "Remove-Item '{0}' -Force".format(TEMP_SCRIPT) in command
+    assert 'finally' in command
+    # The link the UI offers instead is the route that really serves it.
+    assert client.get(SCRIPT_URL,
+                      query_string={'t': data['secret']}).status_code == 200
+
+
 def test_defaults_are_a_day_and_twenty_five_uses(client, db, auth_headers):
     token = _secret(client, auth_headers)['token']
     assert token['maxuses'] == 25
@@ -383,6 +417,7 @@ def test_the_command_falls_back_to_the_request_when_no_site_url_is_set(
         client, db, auth_headers):
     Setting.set('site_base_url', '', valuetype='string')
     data = _secret(client, auth_headers)
-    assert data['command'].startswith(
-        'powershell -ExecutionPolicy Bypass -c "irm https://localhost'
-        '/api/endpoints/siteagent/bootstrap.ps1?t=')
+    assert data['command'] == _expected_command(data['secret'],
+                                                base='https://localhost')
+    assert data['scripturl'].startswith(
+        'https://localhost/api/endpoints/siteagent/bootstrap.ps1?t=')

tests/test_plugins/test_endpoints_credentials.py

@@ -9,7 +9,7 @@ different PC is refused (ASSET_MISMATCH), the shared key is refused for that PC
 
 import pytest
 
-from shopdb.core.models import Asset, AssetType, Setting
+from shopdb.core.models import Asset, AssetType, AuditLog, Setting
 
 
 COLLECTOR_KEY = 'testcollectorkey'
@@ -732,6 +732,137 @@ def test_enrollment_pending_count_includes_unmatched_requests(
     assert states.count('pending') == data['counts']['pendingcomputers']
 
 
+# -- approve binds the hostname ----------------------------------------------
+#
+# A one-off PC installed by the bootstrap installer is pending for `unknown
+# hostname` by definition. Approving used to set matchedassetid only, so the
+# hostname the agent sends resolved to nothing, bind_hostname compared that with
+# the credential's asset and answered 403 ASSET_MISMATCH on every call - the PC
+# enrolled and then never worked until someone typed the hostname in by hand.
+
+def _forget_hostname(db, assetid):
+    from plugins.computers.models import Computer
+
+    Computer.query.filter_by(assetid=assetid).one().hostname = None
+    db.session.commit()
+
+
+def _hostname(assetid):
+    from plugins.computers.models import Computer
+
+    return Computer.query.filter_by(assetid=assetid).one().hostname
+
+
+def _approve(client, auth_headers, requestid, assetid, **body):
+    return client.post(
+        f'/api/endpoints/enrollment/requests/{requestid}/approve',
+        json=dict(assetid=assetid, **body), headers=auth_headers)
+
+
+def _computer_audits():
+    return AuditLog.query.filter_by(entitytype='Computer').all()
+
+
+def test_approve_records_the_hostname_on_a_computer_that_has_none(
+        client, db, app, auth_headers):
+    assetid = _computer(db, 'PLACEHOLDER', serialnumber='SER-9')
+    _forget_hostname(db, assetid)
+
+    pending = _enroll(client, {}, hostname='LOANER-07', serialnumber='SER-9')
+    assert pending.get_json()['data']['reason'] == 'unknown hostname'
+    requestid = pending.get_json()['data']['requestid']
+
+    approved = _approve(client, auth_headers, requestid, assetid)
+    assert approved.status_code == 200, approved.get_data(as_text=True)
+    assert approved.get_json()['data']['hostnameresult'] == 'set'
+    assert _hostname(assetid) == 'LOANER-07'
+    audit = _computer_audits()
+    assert len(audit) == 1
+    assert audit[0].changes['hostname'] == {'old': None, 'new': 'LOANER-07'}
+
+    # The whole point: the PC collects its credential and is then authorized.
+    secret = _credential(client, {}, hostname='LOANER-07',
+                         serialnumber='SER-9')
+    works = client.get('/api/endpoints/alerts?hostname=LOANER-07',
+                       headers={'X-Endpoint-Credential': secret})
+    assert works.status_code == 200, works.get_data(as_text=True)
+
+
+def test_approve_refuses_to_rename_a_computer_without_being_told_to(
+        client, db, app, auth_headers):
+    from plugins.endpoints.models import EndpointEnrollRequest
+
+    assetid = _computer(db, 'BAY-09', serialnumber='SER-9')
+    pending = _enroll(client, {}, hostname='LOANER-07', serialnumber='SER-9')
+    requestid = pending.get_json()['data']['requestid']
+
+    refused = _approve(client, auth_headers, requestid, assetid)
+    assert refused.status_code == 409
+    error = refused.get_json()['data']['error']
+    assert 'BAY-09' in error['message'] and 'LOANER-07' in error['message']
+    assert error['details']['hostnameconflict'] is True
+
+    # Nothing moved: not the hostname, not the request.
+    assert _hostname(assetid) == 'BAY-09'
+    assert _computer_audits() == []
+    row = db.session.get(EndpointEnrollRequest, requestid)
+    assert row.status == EndpointEnrollRequest.STATUS_PENDING
+    assert row.matchedassetid is None
+
+
+def test_approve_with_sethostname_overwrites_and_audits_the_rename(
+        client, db, app, auth_headers):
+    assetid = _computer(db, 'BAY-09', serialnumber='SER-9')
+    pending = _enroll(client, {}, hostname='LOANER-07', serialnumber='SER-9')
+    requestid = pending.get_json()['data']['requestid']
+
+    approved = _approve(client, auth_headers, requestid, assetid,
+                        sethostname=True)
+    assert approved.status_code == 200, approved.get_data(as_text=True)
+    assert approved.get_json()['data']['hostnameresult'] == 'overwritten'
+    assert _hostname(assetid) == 'LOANER-07'
+    audit = _computer_audits()
+    assert len(audit) == 1
+    assert audit[0].changes['hostname'] == {'old': 'BAY-09',
+                                            'new': 'LOANER-07'}
+    assert audit[0].username == 'testadmin'
+
+    assert _approve(client, auth_headers, requestid, assetid,
+                    sethostname='yes').status_code == 400
+
+
+def test_approve_onto_a_matching_hostname_changes_nothing(
+        client, db, app, keyheaders, auth_headers):
+    assetid = _computer(db, 'BAY-01')
+    _setting(db, 'plugin.endpoints.autoenroll', False)
+    pending = _enroll(client, keyheaders)
+    requestid = pending.get_json()['data']['requestid']
+
+    approved = _approve(client, auth_headers, requestid, assetid)
+    assert approved.status_code == 200, approved.get_data(as_text=True)
+    assert approved.get_json()['data']['hostnameresult'] == 'unchanged'
+    assert _hostname(assetid) == 'BAY-01'
+    assert _computer_audits() == []
+
+
+def test_a_hostname_on_no_computer_record_is_refused_but_named_as_such(
+        client, db, app, keyheaders):
+    """Same code and status, an honest message: the credential is good and the
+    hostname is simply on no record, which is a server-side data problem, not a
+    PC claiming to be another one."""
+    assetid = _computer(db, 'BAY-01')
+    secret = _credential(client, keyheaders)
+    _forget_hostname(db, assetid)
+
+    response = client.get('/api/endpoints/alerts?hostname=BAY-01',
+                          headers={'X-Endpoint-Credential': secret})
+    assert response.status_code == 403
+    assert _code(response) == 'ASSET_MISMATCH'
+    message = response.get_json()['data']['error']['message']
+    assert 'BAY-01' in message
+    assert 'no computer record' in message.lower()
+
+
 def test_approve_refuses_an_enrolled_computer(client, db, app, keyheaders,
                                               auth_headers):
     assetid = _computer(db, 'BAY-01')