validate response of federation APIs (#2408)

* validate json responses

* update deps

* tmp disable header check

* log all the webfinger fails

refactor and filter more malformed requests

* don't set incorrect serverURL strings

* test failing through admin api

* fix server url in fedi tests

* check response.text

* validate json/xml response of all apis

test Content-Type of api response and cleanup

* improve logs

* fix rebase

* cleanup json parser in api tests

* mark the api tests performed by admin

* Separate check for reading and format of serverURL

* test /federation/user/ with wrong username in ci

activitypub/controllers/webfinger.go

@@ -15,45 +15,53 @@ import (
 func WebfingerHandler(w http.ResponseWriter, r *http.Request) {
 	if !data.GetFederationEnabled() {
 		w.WriteHeader(http.StatusMethodNotAllowed)
+		log.Debugln("webfinger request rejected! Federation is not enabled")
+		return
+	}
+
+	instanceHostURL := data.GetServerURL()
+	if instanceHostURL == "" {
+		w.WriteHeader(http.StatusNotFound)
+		log.Warnln("webfinger request rejected! Federation is enabled but server URL is empty.")
+		return
+	}
+
+	instanceHostString := utils.GetHostnameFromURLString(instanceHostURL)
+	if instanceHostString == "" {
+		w.WriteHeader(http.StatusNotFound)
+		log.Warnln("webfinger request rejected! Federation is enabled but server URL is not set properly. data.GetServerURL(): " + data.GetServerURL())
 		return
 	}
 
 	resource := r.URL.Query().Get("resource")
-	resourceComponents := strings.Split(resource, ":")
+	preAcct, account, foundAcct := strings.Cut(resource, "acct:")
 
-	var account string
-	if len(resourceComponents) == 2 {
-		account = resourceComponents[1]
-	} else {
-		account = resourceComponents[0]
+	if !foundAcct || preAcct != "" {
+		w.WriteHeader(http.StatusBadRequest)
+		log.Debugln("webfinger request rejected! Malformed resource in query: " + resource)
+		return
 	}
 
 	userComponents := strings.Split(account, "@")
-	if len(userComponents) < 2 {
+	if len(userComponents) != 2 {
 		w.WriteHeader(http.StatusBadRequest)
+		log.Debugln("webfinger request rejected! Malformed account in query: " + account)
 		return
 	}
 	host := userComponents[1]
 	user := userComponents[0]
 
 	if _, valid := data.GetFederatedInboxMap()[user]; !valid {
-		// User is not valid
 		w.WriteHeader(http.StatusNotFound)
-		log.Debugln("webfinger request rejected")
+		log.Debugln("webfinger request rejected! Invalid user: " + user)
 		return
 	}
 
 	// If the webfinger request doesn't match our server then it
 	// should be rejected.
-	instanceHostString := data.GetServerURL()
-	if instanceHostString == "" {
-		w.WriteHeader(http.StatusNotFound)
-		return
-	}
-
-	instanceHostString = utils.GetHostnameFromURLString(instanceHostString)
-	if instanceHostString == "" || instanceHostString != host {
+	if instanceHostString != host {
 		w.WriteHeader(http.StatusNotImplemented)
+		log.Debugln("webfinger request rejected! Invalid query host: " + host + " instanceHostString: " + instanceHostString)
 		return
 	}
 

controllers/admin/config.go

@@ -417,6 +417,12 @@ func SetServerURL(w http.ResponseWriter, r *http.Request) {
 
 	rawValue, ok := configValue.Value.(string)
 	if !ok {
+		controllers.WriteSimpleResponse(w, false, "could not read server url")
+		return
+	}
+
+	serverHostString := utils.GetHostnameFromURLString(rawValue)
+	if serverHostString == "" {
 		controllers.WriteSimpleResponse(w, false, "server url value invalid")
 		return
 	}

test/automated/api/chatusers.test.js

@@ -76,7 +76,7 @@ test('verify user list is populated', async (done) => {
   });
 });
 
-test('disable a user', async (done) => {
+test('disable a user by admin', async (done) => {
   // To allow for visually being able to see the test hiding the
   // message add a short delay.
   await new Promise((r) => setTimeout(r, 1500));
@@ -110,7 +110,7 @@ test('verify messages from user are hidden', async (done) => {
   done();
 });
 
-test('re-enable a user', async (done) => {
+test('re-enable a user by admin', async (done) => {
   const res = await sendAdminPayload('chat/users/setenabled', { userId: userId, enabled: true });
   done();
 });
@@ -123,7 +123,7 @@ test('verify user is enabled', async (done) => {
   done();
 });
 
-test('ban an ip address', async (done) => {
+test('ban an ip address by admin', async (done) => {
   const resIPv4 = await sendAdminRequest('chat/users/ipbans/create', localIPAddressV4);
   const resIPv6 = await sendAdminRequest('chat/users/ipbans/create', localIPAddressV6);
   done();
@@ -142,7 +142,7 @@ test('verify access is denied', async (done) => {
   done();
 });
 
-test('remove an ip address ban', async (done) => {
+test('remove an ip address ban by admin', async (done) => {
   const resIPv4 = await sendAdminRequest('chat/users/ipbans/remove', localIPAddressV4);
   const resIPv6 = await sendAdminRequest('chat/users/ipbans/remove', localIPAddressV6);
   done();

test/automated/api/configmanagement.test.js

@@ -3,6 +3,7 @@ var request = require('supertest');
 const Random = require('crypto-random');
 
 const sendAdminRequest = require('./lib/admin').sendAdminRequest;
+const failAdminRequest = require('./lib/admin').failAdminRequest;
 const getAdminResponse = require('./lib/admin').getAdminResponse;
 
 request = request('http://127.0.0.1:8080');
@@ -245,6 +246,10 @@ test('set forbidden usernames', async (done) => {
 });
 
 test('set server url', async (done) => {
+	const resBadURL = await failAdminRequest(
+		'config/serverurl',
+		'not.valid.url'
+	);
 	const res = await sendAdminRequest(
 		'config/serverurl',
 		newYPConfig.instanceUrl

test/automated/api/federation.test.js

@@ -1,4 +1,5 @@
 var request = require('supertest');
+const parseJson = require('parse-json');
 const jsonfile = require('jsonfile');
 const Ajv = require('ajv-draft-04');
 const sendAdminRequest = require('./lib/admin').sendAdminRequest;
@@ -8,7 +9,8 @@ request = request('http://127.0.0.1:8080');
 var ajv = new Ajv();
 var nodeInfoSchema = jsonfile.readFileSync('schema/nodeinfo_2.0.json');
 
-const serverURL = 'owncast.server.test'
+const serverName = 'owncast.server.test'
+const serverURL = 'http://' + serverName
 const fediUsername = 'streamer'
 
 test('disable federation', async (done) => {
@@ -72,56 +74,108 @@ test('set required parameters and enable federation', async (done) => {
 test('verify responses of /.well-known/webfinger when federation is enabled', async (done) => {
 	const resNoResource = request.get('/.well-known/webfinger').expect(400);
 	const resBadResource = request.get(
-		'/.well-known/webfinger?resource=' + fediUsername + '@' + serverURL
+		'/.well-known/webfinger?resource=' + fediUsername + '@' + serverName
+	).expect(400);
+	const resBadResource2 = request.get(
+		'/.well-known/webfinger?resource=notacct:' + fediUsername + '@' + serverName
 	).expect(400);
 	const resBadServer = request.get(
-		'/.well-known/webfinger?resource=acct:' + fediUsername + '@not.my.server.lol'
+		'/.well-known/webfinger?resource=acct:' + fediUsername + '@not' + serverName
 	).expect(404);
 	const resBadUser = request.get(
-		'/.well-known/webfinger?resource=acct:not' + fediUsername + '@' + serverURL
+		'/.well-known/webfinger?resource=acct:not' + fediUsername + '@' + serverName
 	).expect(404);
-	const res = request.get(
-		'/.well-known/webfinger?resource=acct:' + fediUsername + '@' + serverURL
-	).expect(200);
+	const resNoAccept = request.get(
+		'/.well-known/webfinger?resource=acct:' + fediUsername + '@' + serverName
+	).expect(200)
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
+		});
+	const resWithAccept = request.get(
+		'/.well-known/webfinger?resource=acct:' + fediUsername + '@' + serverName
+	).expect(200)
+		.set('Accept', 'application/json')
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
 			done();
 		});
+});
 
 test('verify responses of /.well-known/host-meta when federation is enabled', async (done) => {
-	const res = request.get('/.well-known/host-meta').expect(200);
+	const res = request.get('/.well-known/host-meta')
+		.expect(200)
+		.expect('Content-Type', /xml/);
 	done();
 });
 
 test('verify responses of /.well-known/nodeinfo when federation is enabled', async (done) => {
-	const res = request.get('/.well-known/nodeinfo').expect(200);
+	const res = request.get('/.well-known/nodeinfo')
+		.expect(200)
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
 			done();
 		});
+});
 
 test('verify responses of /.well-known/x-nodeinfo2 when federation is enabled', async (done) => {
-	const res = request.get('/.well-known/x-nodeinfo2').expect(200);
+	const res = request.get('/.well-known/x-nodeinfo2')
+		.expect(200)
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
 			done();
 		});
+});
 
 test('verify responses of /nodeinfo/2.0 when federation is enabled', async (done) => {
 	const res = request
 		.get('/nodeinfo/2.0')
 		.expect(200)
+		.expect('Content-Type', /json/)
 		.then((res) => {
+			parseJson(res.text);
 			expect(ajv.validate(nodeInfoSchema, res.body)).toBe(true);
 			done();
 		});
 });
 
 test('verify responses of /api/v1/instance when federation is enabled', async (done) => {
-	const res = request.get('/api/v1/instance').expect(200);
+	const res = request.get('/api/v1/instance')
+		.expect(200)
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
 			done();
 		});
+});
 
 test('verify responses of /federation/user/ when federation is enabled', async (done) => {
-	const res = request.get('/federation/user/').expect(200);
+	const resNoAccept = request.get('/federation/user/')
+		.expect(307);
+	const resWithAccept = request.get('/federation/user/')
+		.set('Accept', 'application/json')
+		.expect(404);
+	const resWithAcceptWrongUsername = request.get('/federation/user/not' + fediUsername)
+		.set('Accept', 'application/json')
+		.expect(404);
+	const resWithAcceptUsername = request.get('/federation/user/' + fediUsername)
+		.set('Accept', 'application/json')
+		.expect(200)
+		.expect('Content-Type', /json/)
+		.then((res) => {
+			parseJson(res.text);
 			done();
 		});
+});
 
 test('verify responses of /federation/ when federation is enabled', async (done) => {
-	const res = request.get('/federation/').expect(200);
+	const resNoAccept = request.get('/federation/')
+		.expect(307);
+	const resWithAccept = request.get('/federation/')
+		.set('Accept', 'application/json')
+		.expect(404);
 	done();
 });

test/automated/api/lib/admin.js

@@ -46,6 +46,25 @@ async function sendAdminPayload(
 	return res;
 }
 
+async function failAdminRequest(
+	endpoint,
+	value,
+	adminPassword = defaultAdminPassword,
+	responseCode = 400
+) {
+	const url = '/api/admin/' + endpoint;
+	const res = await request
+		.post(url)
+		.auth('admin', adminPassword)
+		.send({ value: value })
+		.expect(responseCode);
+
+	expect(res.body.success).toBe(false);
+	return res;
+}
+
 module.exports.getAdminResponse = getAdminResponse;
 module.exports.sendAdminRequest = sendAdminRequest;
 module.exports.sendAdminPayload = sendAdminPayload;
+module.exports.failAdminRequest = failAdminRequest;
+

test/automated/api/package.json

@@ -9,7 +9,7 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "supertest": "^6.0.1",
+    "supertest": "^6.3.2",
     "websocket": "^1.0.32",
     "ajv": "^8.11.0",
     "ajv-draft-04" : "^1.0.0",