Pull request: home: improve mobileconfig http api

Merge in DNS/adguard-home from 2358-mobileconfig to master Updates #2358. Squashed commit of the following: commit ab3c7a7 Merge: fa002e4 b4a35fa Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Wed Nov 25 16:11:06 2020 +0300 Merge branch 'master' into 2358-mobileconfig commit fa002e4 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Wed Nov 25 15:19:00 2020 +0300 home: improve mobileconfig http api
AdguardTeam · Dec 10, 2020 · 10ad58d · 10ad58d
1 parent 5210a34
commit 10ad58d
Show file tree

Hide file tree

Showing 7 changed files with 217 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,8 @@ and this project adheres to
 
 ### Changed
 
+- Make the mobileconfig HTTP API more robust and predictable, add parameters and
+  improve error response ([#2358]).
 - Improved HTTP requests handling and timeouts. ([#2343]).
 - Our snap package now uses the `core20` image as its base [#2306].
 - Various internal improvements ([#2271], [#2297]).
@@ -31,6 +33,7 @@ and this project adheres to
 [#2297]: https://github.com/AdguardTeam/AdGuardHome/issues/2297
 [#2306]: https://github.com/AdguardTeam/AdGuardHome/issues/2306
 [#2343]: https://github.com/AdguardTeam/AdGuardHome/issues/2343
+[#2358]: https://github.com/AdguardTeam/AdGuardHome/issues/2358
 
 ### Fixed
 

diff --git a/internal/dhcpd/dhcphttp.go b/internal/dhcpd/dhcphttp.go
@@ -509,6 +509,9 @@ func (s *Server) registerHandlers() {
 }
 
 // jsonError is a generic JSON error response.
+//
+// TODO(a.garipov): Merge together with the implementations in .../home and
+// other packages after refactoring the web handler registering.
 type jsonError struct {
 	// Message is the error message, an opaque string.
 	Message string `json:"message"`

diff --git a/internal/home/control.go b/internal/home/control.go
@@ -112,8 +112,8 @@ func registerControlHandlers() {
 	httpRegister(http.MethodGet, "/control/profile", handleGetProfile)
 
 	// No auth is necessary for DOH/DOT configurations
-	Context.mux.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDoh))
-	Context.mux.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDot))
+	Context.mux.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDOH))
+	Context.mux.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDOT))
 	RegisterAuthHandlers()
 }
 

diff --git a/internal/home/home.go b/internal/home/home.go
@@ -683,3 +683,12 @@ func getHTTPProxy(req *http.Request) (*url.URL, error) {
 	}
 	return url.Parse(config.ProxyURL)
 }
+
+// jsonError is a generic JSON error response.
+//
+// TODO(a.garipov): Merge together with the implementations in .../dhcpd and
+// other packages after refactoring the web handler registering.
+type jsonError struct {
+	// Message is the error message, an opaque string.
+	Message string `json:"message"`
+}
diff --git a/internal/home/mobileconfig.go b/internal/home/mobileconfig.go
@@ -1,10 +1,11 @@
 package home
 
 import (
+	"encoding/json"
 	"fmt"
-	"net"
 	"net/http"
 
+	"github.com/AdguardTeam/golibs/log"
 	uuid "github.com/satori/go.uuid"
 	"howett.net/plist"
 )
@@ -51,6 +52,7 @@ func getMobileConfig(d DNSSettings) ([]byte, error) {
 	switch d.DNSProtocol {
 	case dnsProtoHTTPS:
 		name = fmt.Sprintf("%s DoH", d.ServerName)
+		d.ServerURL = fmt.Sprintf("https://%s/dns-query", d.ServerName)
 	case dnsProtoTLS:
 		name = fmt.Sprintf("%s DoT", d.ServerName)
 	default:
@@ -80,34 +82,46 @@ func getMobileConfig(d DNSSettings) ([]byte, error) {
 	return plist.MarshalIndent(data, plist.XMLFormat, "\t")
 }
 
-func handleMobileConfig(w http.ResponseWriter, d DNSSettings) {
+func handleMobileConfig(w http.ResponseWriter, r *http.Request, dnsp string) {
+	host := r.URL.Query().Get("host")
+	if host == "" {
+		host = Context.tls.conf.ServerName
+	}
+
+	if host == "" {
+		w.WriteHeader(http.StatusInternalServerError)
+
+		const msg = "no host in query parameters and no server_name"
+		err := json.NewEncoder(w).Encode(&jsonError{
+			Message: msg,
+		})
+		if err != nil {
+			log.Debug("writing 500 json response: %s", err)
+		}
+
+		return
+	}
+
+	d := DNSSettings{
+		DNSProtocol: dnsp,
+		ServerName:  host,
+	}
+
 	mobileconfig, err := getMobileConfig(d)
 	if err != nil {
 		httpError(w, http.StatusInternalServerError, "plist.MarshalIndent: %s", err)
+
+		return
 	}
 
 	w.Header().Set("Content-Type", "application/xml")
 	_, _ = w.Write(mobileconfig)
 }
 
-func handleMobileConfigDoh(w http.ResponseWriter, r *http.Request) {
-	handleMobileConfig(w, DNSSettings{
-		DNSProtocol: dnsProtoHTTPS,
-		ServerURL:   fmt.Sprintf("https://%s/dns-query", r.Host),
-	})
+func handleMobileConfigDOH(w http.ResponseWriter, r *http.Request) {
+	handleMobileConfig(w, r, dnsProtoHTTPS)
 }
 
-func handleMobileConfigDot(w http.ResponseWriter, r *http.Request) {
-	var err error
-
-	var host string
-	host, _, err = net.SplitHostPort(r.Host)
-	if err != nil {
-		httpError(w, http.StatusBadRequest, "getting host: %s", err)
-	}
-
-	handleMobileConfig(w, DNSSettings{
-		DNSProtocol: dnsProtoTLS,
-		ServerName:  host,
-	})
+func handleMobileConfigDOT(w http.ResponseWriter, r *http.Request) {
+	handleMobileConfig(w, r, dnsProtoTLS)
 }
diff --git a/internal/home/mobileconfig_test.go b/internal/home/mobileconfig_test.go
@@ -9,25 +9,132 @@ import (
 	"howett.net/plist"
 )
 
-func TestHandleMobileConfigDot(t *testing.T) {
-	var err error
+func TestHandleMobileConfigDOH(t *testing.T) {
+	t.Run("success", func(t *testing.T) {
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig?host=example.org", nil)
+		assert.Nil(t, err)
 
-	var r *http.Request
-	r, err = http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil)
-	assert.Nil(t, err)
+		w := httptest.NewRecorder()
 
-	w := httptest.NewRecorder()
+		handleMobileConfigDOH(w, r)
+		assert.Equal(t, http.StatusOK, w.Code)
 
-	handleMobileConfigDot(w, r)
-	assert.Equal(t, http.StatusOK, w.Code)
+		var mc MobileConfig
+		_, err = plist.Unmarshal(w.Body.Bytes(), &mc)
+		assert.Nil(t, err)
 
-	var mc MobileConfig
-	_, err = plist.Unmarshal(w.Body.Bytes(), &mc)
-	assert.Nil(t, err)
+		if assert.Equal(t, 1, len(mc.PayloadContent)) {
+			assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name)
+			assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName)
+			assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName)
+			assert.Equal(t, "https://example.org/dns-query", mc.PayloadContent[0].DNSSettings.ServerURL)
+		}
+	})
 
-	if assert.Equal(t, 1, len(mc.PayloadContent)) {
-		assert.Equal(t, "example.com DoT", mc.PayloadContent[0].Name)
-		assert.Equal(t, "example.com DoT", mc.PayloadContent[0].PayloadDisplayName)
-		assert.Equal(t, "example.com", mc.PayloadContent[0].DNSSettings.ServerName)
-	}
+	t.Run("success_no_host", func(t *testing.T) {
+		oldTLSConf := Context.tls
+		t.Cleanup(func() { Context.tls = oldTLSConf })
+
+		Context.tls = &TLSMod{
+			conf: tlsConfigSettings{ServerName: "example.org"},
+		}
+
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig", nil)
+		assert.Nil(t, err)
+
+		w := httptest.NewRecorder()
+
+		handleMobileConfigDOH(w, r)
+		assert.Equal(t, http.StatusOK, w.Code)
+
+		var mc MobileConfig
+		_, err = plist.Unmarshal(w.Body.Bytes(), &mc)
+		assert.Nil(t, err)
+
+		if assert.Equal(t, 1, len(mc.PayloadContent)) {
+			assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name)
+			assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName)
+			assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName)
+			assert.Equal(t, "https://example.org/dns-query", mc.PayloadContent[0].DNSSettings.ServerURL)
+		}
+	})
+
+	t.Run("error_no_host", func(t *testing.T) {
+		oldTLSConf := Context.tls
+		t.Cleanup(func() { Context.tls = oldTLSConf })
+
+		Context.tls = &TLSMod{conf: tlsConfigSettings{}}
+
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig", nil)
+		assert.Nil(t, err)
+
+		w := httptest.NewRecorder()
+
+		handleMobileConfigDOH(w, r)
+		assert.Equal(t, http.StatusInternalServerError, w.Code)
+	})
+}
+
+func TestHandleMobileConfigDOT(t *testing.T) {
+	t.Run("success", func(t *testing.T) {
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig?host=example.org", nil)
+		assert.Nil(t, err)
+
+		w := httptest.NewRecorder()
+
+		handleMobileConfigDOT(w, r)
+		assert.Equal(t, http.StatusOK, w.Code)
+
+		var mc MobileConfig
+		_, err = plist.Unmarshal(w.Body.Bytes(), &mc)
+		assert.Nil(t, err)
+
+		if assert.Equal(t, 1, len(mc.PayloadContent)) {
+			assert.Equal(t, "example.org DoT", mc.PayloadContent[0].Name)
+			assert.Equal(t, "example.org DoT", mc.PayloadContent[0].PayloadDisplayName)
+			assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName)
+		}
+	})
+
+	t.Run("success_no_host", func(t *testing.T) {
+		oldTLSConf := Context.tls
+		t.Cleanup(func() { Context.tls = oldTLSConf })
+
+		Context.tls = &TLSMod{
+			conf: tlsConfigSettings{ServerName: "example.org"},
+		}
+
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil)
+		assert.Nil(t, err)
+
+		w := httptest.NewRecorder()
+
+		handleMobileConfigDOT(w, r)
+		assert.Equal(t, http.StatusOK, w.Code)
+
+		var mc MobileConfig
+		_, err = plist.Unmarshal(w.Body.Bytes(), &mc)
+		assert.Nil(t, err)
+
+		if assert.Equal(t, 1, len(mc.PayloadContent)) {
+			assert.Equal(t, "example.org DoT", mc.PayloadContent[0].Name)
+			assert.Equal(t, "example.org DoT", mc.PayloadContent[0].PayloadDisplayName)
+			assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName)
+		}
+	})
+
+	t.Run("error_no_host", func(t *testing.T) {
+		oldTLSConf := Context.tls
+		t.Cleanup(func() { Context.tls = oldTLSConf })
+
+		Context.tls = &TLSMod{conf: tlsConfigSettings{}}
+
+		r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil)
+		assert.Nil(t, err)
+
+		w := httptest.NewRecorder()
+
+		handleMobileConfigDOT(w, r)
+		assert.Equal(t, http.StatusInternalServerError, w.Code)
+	})
 }
diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml
@@ -959,27 +959,61 @@
             'application/json':
               'schema':
                 '$ref': '#/components/schemas/ProfileInfo'
+
   '/apple/doh.mobileconfig':
     'get':
-      'tags':
-      - 'mobileconfig'
-      - 'global'
       'operationId': 'mobileConfigDoH'
-      'summary': 'Get DNS over HTTPS .mobileconfig'
+      'parameters':
+      - 'description': >
+          Host for which the config is generated.  If no host is provided,
+          `tls.server_name` from the configuration file is used.  If
+          `tls.server_name` is not set, the API returns an error with a 500
+          status.
+        'example': 'example.org'
+        'in': 'query'
+        'name': 'host'
+        'schema':
+          'type': 'string'
       'responses':
         '200':
-          'description': 'DNS over HTTPS plist file'
-
-  '/apple/dot.mobileconfig':
-    'get':
+          'description': 'DNS over HTTPS plist file.'
+        '500':
+          'content':
+            'application/json':
+              'schema':
+                '$ref': '#/components/schemas/Error'
+          'description': 'Server configuration error.'
+      'summary': 'Get DNS over HTTPS .mobileconfig.'
       'tags':
       - 'mobileconfig'
       - 'global'
+  '/apple/dot.mobileconfig':
+    'get':
       'operationId': 'mobileConfigDoT'
-      'summary': 'Get TLS over TLS .mobileconfig'
+      'parameters':
+      - 'description': >
+          Host for which the config is generated.  If no host is provided,
+          `tls.server_name` from the configuration file is used.  If
+          `tls.server_name` is not set, the API returns an error with a 500
+          status.
+        'example': 'example.org'
+        'in': 'query'
+        'name': 'host'
+        'schema':
+          'type': 'string'
       'responses':
         '200':
           'description': 'DNS over TLS plist file'
+        '500':
+          'content':
+            'application/json':
+              'schema':
+                '$ref': '#/components/schemas/Error'
+          'description': 'Server configuration error.'
+      'summary': 'Get DNS over TLS .mobileconfig.'
+      'tags':
+      - 'mobileconfig'
+      - 'global'
 
 'components':
   'requestBodies':