Handle ErrNotFound separately in GetClient result

The GetClient method should return a not found error if no matching
client was found. However, callers of this method expect nonexistent
clients to be returned as nil without an error. This mismatch means that
an authorize request with an incorrect client ID will get a response
stating there was an unexpected server error. Separate error handling of
ErrNotFound is needed.

access.go

@@ -535,6 +535,10 @@ func (s *Server) FinishAccessRequest(w *Response, r *http.Request, ar *AccessReq
 // storage. Sets an error on the response if auth fails or a server error occurs.
 func getClient(auth *BasicAuth, storage Storage, w *Response) Client {
 	client, err := storage.GetClient(auth.Username)
+	if err == ErrNotFound {
+		w.SetError(E_UNAUTHORIZED_CLIENT, "")
+		return nil
+	}
 	if err != nil {
 		w.SetError(E_SERVER_ERROR, "")
 		w.InternalError = err

access_test.go

@@ -303,6 +303,35 @@ func TestGetClientWithoutMatcher(t *testing.T) {
 		if client != nil {
 			t.Errorf("Expected error, got client: %v", client)
 		}
+
+		if !w.IsError {
+			t.Error("No error in response")
+		}
+
+		if w.ErrorId != E_UNAUTHORIZED_CLIENT {
+			t.Errorf("Expected error %v, got %v", E_UNAUTHORIZED_CLIENT, w.ErrorId)
+		}
+	}
+
+	// Ensure nonexistent client fails
+	{
+		auth := &BasicAuth{
+			Username: "nonexistent",
+			Password: "nonexistent",
+		}
+		w := &Response{}
+		client := getClient(auth, storage, w)
+		if client != nil {
+			t.Errorf("Expected error, got client: %v", client)
+		}
+
+		if !w.IsError {
+			t.Error("No error in response")
+		}
+
+		if w.ErrorId != E_UNAUTHORIZED_CLIENT {
+			t.Errorf("Expected error %v, got %v", E_UNAUTHORIZED_CLIENT, w.ErrorId)
+		}
 	}
 
 	// Ensure good secret works

authorize.go

@@ -124,6 +124,10 @@ func (s *Server) HandleAuthorizeRequest(w *Response, r *http.Request) *Authorize
 
 	// must have a valid client
 	ret.Client, err = w.Storage.GetClient(r.Form.Get("client_id"))
+	if err == ErrNotFound {
+		w.SetErrorState(E_UNAUTHORIZED_CLIENT, "", ret.State)
+		return nil
+	}
 	if err != nil {
 		w.SetErrorState(E_SERVER_ERROR, "", ret.State)
 		w.InternalError = err

authorize_test.go

@@ -88,6 +88,45 @@ func TestAuthorizeToken(t *testing.T) {
 	}
 }
 
+func TestAuthorizeTokenWithInvalidClient(t *testing.T) {
+	sconfig := NewServerConfig()
+	sconfig.AllowedAuthorizeTypes = AllowedAuthorizeType{TOKEN}
+	server := NewServer(sconfig, NewTestingStorage())
+	server.AuthorizeTokenGen = &TestingAuthorizeTokenGen{}
+	server.AccessTokenGen = &TestingAccessTokenGen{}
+	resp := server.NewResponse()
+	redirectUri := "http://redirecturi.com"
+
+	req, err := http.NewRequest("GET", "http://localhost:14000/appauth", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Form = make(url.Values)
+	req.Form.Set("response_type", string(TOKEN))
+	req.Form.Set("client_id", "invalidclient")
+	req.Form.Set("state", "a")
+	req.Form.Set("redirect_uri", redirectUri)
+
+	if ar := server.HandleAuthorizeRequest(resp, req); ar != nil {
+		ar.Authorized = true
+		server.FinishAuthorizeRequest(resp, req, ar)
+	}
+
+	if !resp.IsError {
+		t.Fatalf("Response should be an error")
+	}
+
+	if resp.ErrorId != E_UNAUTHORIZED_CLIENT {
+		t.Fatalf("Incorrect error in response: %v", resp.ErrorId)
+	}
+
+	usedRedirectUrl, redirectErr := resp.GetRedirectUrl()
+
+	if redirectErr == nil && usedRedirectUrl == redirectUri {
+		t.Fatalf("Response must not redirect to the provided redirect URL for an invalid client")
+	}
+}
+
 func TestAuthorizeCodePKCERequired(t *testing.T) {
 	sconfig := NewServerConfig()
 	sconfig.RequirePKCEForPublicClients = true