From 88aadedc59b17e399a84b9904d383f6006941391 Mon Sep 17 00:00:00 2001
From: Kegan Dougal <kegan@matrix.org>
Date: Tue, 16 Jun 2020 13:51:34 +0100
Subject: [PATCH] Review comments and test fix

---
 clientapi/auth/auth.go  |  8 ++++++++
 userapi/api/api.go      | 13 ++++++++++++-
 userapi/internal/api.go | 10 ++++------
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/clientapi/auth/auth.go b/clientapi/auth/auth.go
index ef1d733e4..b8e408538 100644
--- a/clientapi/auth/auth.go
+++ b/clientapi/auth/auth.go
@@ -71,6 +71,14 @@ func VerifyUserFromRequest(
 		jsonErr := jsonerror.InternalServerError()
 		return nil, &jsonErr
 	}
+	if res.Err != nil {
+		if forbidden, ok := res.Err.(*api.ErrorForbidden); ok {
+			return nil, &util.JSONResponse{
+				Code: http.StatusForbidden,
+				JSON: jsonerror.Forbidden(forbidden.Message),
+			}
+		}
+	}
 	if res.Device == nil {
 		return nil, &util.JSONResponse{
 			Code: http.StatusUnauthorized,
diff --git a/userapi/api/api.go b/userapi/api/api.go
index a73816346..57b5165a4 100644
--- a/userapi/api/api.go
+++ b/userapi/api/api.go
@@ -25,13 +25,15 @@ type UserInternalAPI interface {
 // QueryAccessTokenRequest is the request for QueryAccessToken
 type QueryAccessTokenRequest struct {
 	AccessToken string
-	// optional user ID, valid only if the token is an appservice
+	// optional user ID, valid only if the token is an appservice.
+	// https://matrix.org/docs/spec/application_service/r0.1.2#using-sync-and-events
 	AppServiceUserID string
 }
 
 // QueryAccessTokenResponse is the response for QueryAccessToken
 type QueryAccessTokenResponse struct {
 	Device *Device
+	Err    error // e.g ErrorForbidden
 }
 
 // QueryProfileRequest is the request for QueryProfile
@@ -64,3 +66,12 @@ type Device struct {
 	// TODO: display name, last used timestamp, keys, etc
 	DisplayName string
 }
+
+// ErrorForbidden is an error indicating that the supplied access token is forbidden
+type ErrorForbidden struct {
+	Message string
+}
+
+func (e *ErrorForbidden) Error() string {
+	return "Forbidden: " + e.Message
+}
diff --git a/userapi/internal/api.go b/userapi/internal/api.go
index 99ab560ee..1f0d5c94b 100644
--- a/userapi/internal/api.go
+++ b/userapi/internal/api.go
@@ -58,12 +58,10 @@ func (a *UserInternalAPI) QueryProfile(ctx context.Context, req *api.QueryProfil
 }
 
 func (a *UserInternalAPI) QueryAccessToken(ctx context.Context, req *api.QueryAccessTokenRequest, res *api.QueryAccessTokenResponse) error {
-	appServiceDevice, err := a.queryAppServiceToken(ctx, req.AccessToken, req.AppServiceUserID)
-	if err != nil {
-		return err
-	}
-	if appServiceDevice != nil {
+	if req.AppServiceUserID != "" {
+		appServiceDevice, err := a.queryAppServiceToken(ctx, req.AccessToken, req.AppServiceUserID)
 		res.Device = appServiceDevice
+		res.Err = err
 		return nil
 	}
 	device, err := a.DeviceDB.GetDeviceByAccessToken(ctx, req.AccessToken)
@@ -114,7 +112,7 @@ func (a *UserInternalAPI) queryAppServiceToken(ctx context.Context, token, appSe
 			dev.UserID = appServiceUserID
 			return &dev, nil
 		}
-		return nil, fmt.Errorf("appservice has not registered this user")
+		return nil, &api.ErrorForbidden{Message: "appservice has not registered this user"}
 	}
 
 	// AS is not masquerading as any user, so use AS's sender_localpart