Merge pull request #170 from astridej/fix/handle-errnotfound

Handle ErrNotFound separately in GetClient result

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