Skip to content

Commit 464fb47

Browse files
mcalmermcalmer
authored
Merge pull request #802 from marv7000/nice-api-errors
Validate the presence of all API functions before calling them
2 parents 5bd1e5c + 51ef88e commit 464fb47

10 files changed

Lines changed: 283 additions & 12 deletions

File tree

‎mgradm/cmd/distro/cp.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func registerDistro(connection *api.ConnectionDetails, distro *types.Distributio
6161
"installType": distro.InstallType,
6262
}
6363

64-
_, err = client.Post("kickstart/tree/create", data)
64+
_, err = client.PostChecked("kickstart/tree/create", "api.kickstart.tree.create", data)
6565
if err != nil {
6666
return utils.Errorf(err, L("unable to register the distribution. Manual distro registration is required"))
6767
}

‎mgrctl/cmd/ssh/knownhost/remove.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func removeKnownHost(client *api.APIClient, hostname string, port string) error
2424
path := "admin/ssh/removeKnownHost?" + params.Encode()
2525

2626
var data map[string]interface{}
27-
res, err := api.Post[interface{}](client, path, data)
27+
res, err := api.PostChecked[interface{}](client, path, "api.admin.ssh.remove_known_host", data)
2828
if err != nil {
2929
return utils.Errorf(err, L("error in query '%s'"), path)
3030
}

‎shared/api/api.go‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2024 SUSE LLC
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

@@ -225,7 +225,7 @@ func (c *APIClient) ValidateCreds() bool {
225225
return err == nil
226226
}
227227

228-
// Post issues a POST HTTP request to the API target
228+
// Post issues a POST HTTP request to the API target, without validating the endpoint.
229229
//
230230
// `path` specifies an API endpoint
231231
// `data` contains a map of values to add to the POST query. `data` are serialized to the JSON
@@ -254,6 +254,26 @@ func (c *APIClient) Post(path string, data map[string]interface{}) (*http.Respon
254254
return res, nil
255255
}
256256

257+
// PostChecked validates the API endpoint before issuing a POST request.
258+
func (c *APIClient) PostChecked(path string, endpoint string, data map[string]interface{}) (*http.Response, error) {
259+
if err := c.validateEndpoint(endpoint); err != nil {
260+
return nil, err
261+
}
262+
return c.Post(path, data)
263+
}
264+
265+
// PostChecked issues a validated POST request to the API target using the client and decodes the response.
266+
func PostChecked[T interface{}](
267+
client *APIClient, path string, namespace string, data map[string]interface{},
268+
) (*APIResponse[T], error) {
269+
res, err := client.PostChecked(path, namespace, data)
270+
if err != nil {
271+
return nil, err
272+
}
273+
274+
return decodeAPIResponse[T](res)
275+
}
276+
257277
// Get issues GET HTTP request to the API target
258278
//
259279
// `path` specifies API endpoint together with query options
@@ -313,10 +333,32 @@ func Get[T interface{}](client *APIClient, path string) (*APIResponse[T], error)
313333
return nil, err
314334
}
315335

336+
return decodeAPIResponse[T](res)
337+
}
338+
339+
// GetChecked validates the API namespace before issuing a GET request.
340+
func (c *APIClient) GetChecked(path string, namespace string) (*http.Response, error) {
341+
if err := c.validateEndpoint(namespace); err != nil {
342+
return nil, err
343+
}
344+
return c.Get(path)
345+
}
346+
347+
// GetChecked issues a validated GET request to the API using the client and decodes the response.
348+
func GetChecked[T interface{}](client *APIClient, path string, endpoint string) (*APIResponse[T], error) {
349+
res, err := client.GetChecked(path, endpoint)
350+
if err != nil {
351+
return nil, err
352+
}
353+
354+
return decodeAPIResponse[T](res)
355+
}
356+
357+
func decodeAPIResponse[T interface{}](res *http.Response) (*APIResponse[T], error) {
316358
defer res.Body.Close()
317359

318360
var response APIResponse[T]
319-
if err = json.NewDecoder(res.Body).Decode(&response); err != nil {
361+
if err := json.NewDecoder(res.Body).Decode(&response); err != nil {
320362
return nil, err
321363
}
322364

‎shared/api/namespaces.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package api
6+
7+
import (
8+
"encoding/json"
9+
"errors"
10+
11+
. "github.com/uyuni-project/uyuni-tools/shared/l10n"
12+
)
13+
14+
const unsupportedFunctionError = "This function is currently not supported by the server."
15+
16+
// ValidateNamespace checks if the server advertises the requested API namespace.
17+
func ValidateNamespace(client *APIClient, namespace string) error {
18+
if namespace == "" {
19+
return nil
20+
}
21+
if err := client.loadValidNamespaces(); err != nil {
22+
return err
23+
}
24+
if _, ok := client.validNamespaces[namespace]; ok {
25+
return nil
26+
}
27+
return errors.New(L(unsupportedFunctionError))
28+
}
29+
30+
func (c *APIClient) validateEndpoint(endpoint string) error {
31+
if c.AuthCookie == nil || endpoint == "" {
32+
return nil
33+
}
34+
return ValidateNamespace(c, endpoint)
35+
}
36+
37+
func (c *APIClient) loadValidNamespaces() error {
38+
if c.validNamespaces != nil {
39+
return nil
40+
}
41+
42+
res, err := c.Get("access/listNamespaces")
43+
if err != nil {
44+
return err
45+
}
46+
defer res.Body.Close()
47+
48+
var response APIResponse[[]NamespaceAccess]
49+
if err = json.NewDecoder(res.Body).Decode(&response); err != nil {
50+
return err
51+
}
52+
if !response.Success {
53+
return errors.New(response.Message)
54+
}
55+
56+
c.validNamespaces = map[string]struct{}{}
57+
for _, namespace := range response.Result {
58+
if namespace.Namespace != "" {
59+
c.validNamespaces[namespace.Namespace] = struct{}{}
60+
}
61+
}
62+
return nil
63+
}

‎shared/api/namespaces_test.go‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package api
6+
7+
import (
8+
"net/http"
9+
"testing"
10+
11+
"github.com/uyuni-project/uyuni-tools/shared/api/mocks"
12+
"github.com/uyuni-project/uyuni-tools/shared/testutils"
13+
)
14+
15+
const removeKnownHostNamespace = "api.admin.ssh.remove_known_host"
16+
17+
func namespaceListResponse(namespace string) string {
18+
return `{"success":true,"result":[{"namespace":"` + namespace + `","access_mode":"W"}]}`
19+
}
20+
21+
func TestPostChecked(t *testing.T) {
22+
tests := []struct {
23+
name string
24+
path string
25+
endpoint string
26+
apiPath string
27+
expectedError string
28+
targetCalled bool
29+
}{
30+
{
31+
name: "Test advertised endpoint",
32+
path: "admin/ssh/removeKnownHost",
33+
endpoint: removeKnownHostNamespace,
34+
apiPath: "/rhn/manager/api/admin/ssh/removeKnownHost",
35+
targetCalled: true,
36+
},
37+
{
38+
name: "Test unsupported endpoint",
39+
path: "admin/ssh/removeUnknownHost",
40+
endpoint: "api.admin.ssh.remove_unknown_host",
41+
apiPath: "/rhn/manager/api/admin/ssh/removeUnknownHost",
42+
expectedError: unsupportedFunctionError,
43+
},
44+
}
45+
46+
for _, tt := range tests {
47+
t.Run(tt.name, func(t *testing.T) {
48+
client, err := Init(&ConnectionDetails{Server: "testServer", Cookie: "testCookie"})
49+
if err != nil {
50+
t.FailNow()
51+
}
52+
53+
targetCalled := false
54+
client.Client = &mocks.MockClient{DoFunc: func(req *http.Request) (*http.Response, error) {
55+
switch req.URL.Path {
56+
case "/rhn/manager/api/access/listNamespaces":
57+
return testutils.GetResponse(200, namespaceListResponse(removeKnownHostNamespace))
58+
case tt.apiPath:
59+
targetCalled = true
60+
return testutils.GetResponse(200, `{"success":true,"result":true}`)
61+
default:
62+
t.Errorf("Unexpected API path %s", req.URL.Path)
63+
return testutils.GetResponse(404, `{}`)
64+
}
65+
}}
66+
67+
errorMessage := ""
68+
if _, err := client.PostChecked(tt.path, tt.endpoint, nil); err != nil {
69+
errorMessage = err.Error()
70+
}
71+
72+
testutils.AssertStringContains(t, "Unexpected error message", errorMessage, tt.expectedError)
73+
testutils.AssertEquals(t, "Unexpected target call", tt.targetCalled, targetCalled)
74+
})
75+
}
76+
}
77+
78+
func TestGetChecked(t *testing.T) {
79+
tests := []struct {
80+
name string
81+
path string
82+
endpoint string
83+
apiPath string
84+
expectedError string
85+
targetCalled bool
86+
}{
87+
{
88+
name: "Test advertised endpoint",
89+
path: "org/getDetails?name=admin",
90+
endpoint: "api.org.get_details",
91+
apiPath: "/rhn/manager/api/org/getDetails",
92+
targetCalled: true,
93+
},
94+
{
95+
name: "Test unsupported endpoint",
96+
path: "org/getUnknownDetails?name=admin",
97+
endpoint: "api.org.get_unknown_details",
98+
apiPath: "/rhn/manager/api/org/getUnknownDetails",
99+
expectedError: unsupportedFunctionError,
100+
},
101+
}
102+
103+
for _, tt := range tests {
104+
t.Run(tt.name, func(t *testing.T) {
105+
client, err := Init(&ConnectionDetails{Server: "testServer", Cookie: "testCookie"})
106+
if err != nil {
107+
t.FailNow()
108+
}
109+
110+
targetCalled := false
111+
client.Client = &mocks.MockClient{DoFunc: func(req *http.Request) (*http.Response, error) {
112+
switch req.URL.Path {
113+
case "/rhn/manager/api/access/listNamespaces":
114+
return testutils.GetResponse(200, namespaceListResponse("api.org.get_details"))
115+
case tt.apiPath:
116+
targetCalled = true
117+
return testutils.GetResponse(200, `{"success":true,"result":true}`)
118+
default:
119+
t.Errorf("Unexpected API path %s", req.URL.Path)
120+
return testutils.GetResponse(404, `{}`)
121+
}
122+
}}
123+
124+
errorMessage := ""
125+
if _, err := client.GetChecked(tt.path, tt.endpoint); err != nil {
126+
errorMessage = err.Error()
127+
}
128+
129+
testutils.AssertStringContains(t, "Unexpected error message", errorMessage, tt.expectedError)
130+
testutils.AssertEquals(t, "Unexpected target call", tt.targetCalled, targetCalled)
131+
})
132+
}
133+
}
134+
135+
func TestRawPostDoesNotValidateEndpoint(t *testing.T) {
136+
client, err := Init(&ConnectionDetails{Server: "testServer", Cookie: "testCookie"})
137+
if err != nil {
138+
t.FailNow()
139+
}
140+
141+
targetCalled := false
142+
client.Client = &mocks.MockClient{DoFunc: func(req *http.Request) (*http.Response, error) {
143+
testutils.AssertEquals(t, "Wrong URL called", req.URL.Path, "/rhn/manager/api/org/createFirst")
144+
targetCalled = true
145+
return testutils.GetResponse(200, `{"success":true,"result":true}`)
146+
}}
147+
148+
if _, err := client.Post("org/createFirst", nil); err != nil {
149+
t.Fatalf("Expected raw POST request to bypass endpoint validation: %v", err)
150+
}
151+
testutils.AssertTrue(t, "Expected raw target endpoint to be called", targetCalled)
152+
}

‎shared/api/org/createFirst.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2024 SUSE LLC
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

@@ -31,7 +31,7 @@ func CreateFirst(cnxDetails *api.ConnectionDetails, orgName string, admin *types
3131
"email": admin.Email,
3232
}
3333

34-
res, err := api.Post[types.Organization](client, "org/createFirst", data)
34+
res, err := api.PostChecked[types.Organization](client, "org/createFirst", "api.org.create_first", data)
3535
if err != nil {
3636
return nil, utils.Errorf(err, L("failed to create first user and organization"))
3737
}

‎shared/api/org/getDetails.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2024 SUSE LLC
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

@@ -23,7 +23,9 @@ func GetOrganizationDetails(cnxDetails *api.ConnectionDetails, orgName string) (
2323
if err != nil {
2424
return nil, utils.Errorf(err, L("failed to connect to the server"))
2525
}
26-
res, err := api.Get[types.Organization](client, fmt.Sprintf("org/getDetails?name=%s", orgName))
26+
res, err := api.GetChecked[types.Organization](
27+
client, fmt.Sprintf("org/getDetails?name=%s", orgName), "api.org.get_details",
28+
)
2729
if err != nil {
2830
return nil, utils.Errorf(err, L("failed to get organization details"))
2931
}

‎shared/api/proxy/containerConfig.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2024 SUSE LLC
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

@@ -14,6 +14,7 @@ import (
1414
)
1515

1616
const containerConfigEndpoint = "proxy/containerConfig"
17+
const containerConfigAPIEndpoint = "api.proxy.container_config"
1718

1819
// ContainerConfig computes and downloads the configuration file for proxy containers with generated certificates.
1920
func ContainerConfig(client *api.APIClient, request ProxyConfigRequest) (*[]int8, error) {
@@ -28,7 +29,7 @@ func ContainerConfigGenerate(client *api.APIClient, request ProxyConfigGenerateR
2829
// common method to execute the request.
2930
func executeRequest(client *api.APIClient, data map[string]interface{}) (*[]int8, error) {
3031
log.Trace().Msgf("Creating proxy configuration file with data: %v...", data)
31-
res, err := api.Post[[]int8](client, containerConfigEndpoint, data)
32+
res, err := api.PostChecked[[]int8](client, containerConfigEndpoint, containerConfigAPIEndpoint, data)
3233
if err != nil {
3334
return nil, utils.Errorf(err, L("failed to create proxy configuration file"))
3435
}

‎shared/api/types.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: 2024 SUSE LLC
1+
// SPDX-FileCopyrightText: 2026 SUSE LLC
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

@@ -23,6 +23,16 @@ type APIClient struct {
2323

2424
// Connection details
2525
Details *ConnectionDetails
26+
27+
// Valid API namespaces advertised by the server
28+
validNamespaces map[string]struct{}
29+
}
30+
31+
// NamespaceAccess describes an API namespace advertised by the server.
32+
type NamespaceAccess struct {
33+
Namespace string `json:"namespace"`
34+
Description string `json:"description"`
35+
AccessMode string `json:"access_mode"`
2636
}
2737

2838
// HTTPClient is a minimal HTTPClient interface primarily for unit testing.

‎uyuni-tools.changes.mfriedrich.check-endpoints‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Validate the presence of all API functions before calling them

0 commit comments

Comments
 (0)