JakeHillion/storj
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

satellite/admin: Fix API key delete by name

Browse Source 
We couldn't delete API keys by name whose name contained slashes because
Gorilla Mux router interpreted the as path separator and didn't resolve
to the right endpoint.

To fix the issue the name is sent as a query parameter rather than as a
path parameter.

Change-Id: Ica67d6b9f047d7c33a5350457afc822cb8d4c7a1
This commit is contained in:
Ivan Fraixedes 2023-11-30 11:43:05 +01:00 committed by Storj Robot
parent 8ec1a8de7d
commit e1c12674c5
6 changed files with 93 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
satellite/admin/README.md
View File

@ -41,7 +41,7 @@ Requires setting `Authorization` header for requests.
* [DELETE /api/projects/{project-id}](#delete-apiprojectsproject-id)
* [GET /api/projects/{project}/apikeys](#get-apiprojectsprojectapikeys)
* [POST /api/projects/{project}/apikeys](#post-apiprojectsprojectapikeys)
* [DELETE /api/projects/{project}/apikeys/{name}](#delete-apiprojectsprojectapikeysname)
* [DELETE /api/projects/{project}/apikeys?name={value}](#delete-apiprojectsprojectapikeysnamevalue)
* [GET /api/projects/{project-id}/usage](#get-apiprojectsproject-idusage)
* [GET /api/projects/{project-id}/limit](#get-apiprojectsproject-idlimit)
* [Update limits](#update-limits)
@ -368,7 +368,7 @@ A successful response body:
}
```
#### DELETE /api/projects/{project}/apikeys/{name}
#### DELETE /api/projects/{project}/apikeys?name={value}
Deletes the given apikey by its name.

4
satellite/admin/apikeys.go
View File

@ -265,8 +265,8 @@ func (server *Server) deleteAPIKeyByName(w http.ResponseWriter, r *http.Request)
return
}
apikeyName, ok := vars["name"]
if !ok {
apikeyName := r.URL.Query().Get("name")
if apikeyName == "" {
sendJSONError(w, "apikey name missing",
"", http.StatusBadRequest)
return

87
satellite/admin/apikeys_test.go
View File

@ -38,12 +38,19 @@ func TestApiKeyAdd(t *testing.T) {
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
projectID := planet.Uplinks[0].Projects[0].ID
keys, err := planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
keys, err := planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 1)
body := strings.NewReader(`{"name":"Default"}`)
req, err := http.NewRequestWithContext(ctx, http.MethodPost, fmt.Sprintf("http://"+address.String()+"/api/projects/%s/apikeys", projectID.String()), body)
req, err := http.NewRequestWithContext(
ctx,
http.MethodPost,
fmt.Sprintf("http://"+address.String()+"/api/projects/%s/apikeys", projectID.String()),
body,
)
require.NoError(t, err)
req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
@ -66,7 +73,9 @@ func TestApiKeyAdd(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, apikey)
keys, err = planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
keys, err = planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 2)
@ -90,7 +99,9 @@ func TestApiKeyDelete(t *testing.T) {
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
projectID := planet.Uplinks[0].Projects[0].ID
keys, err := planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
keys, err := planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 1)
@ -100,12 +111,23 @@ func TestApiKeyDelete(t *testing.T) {
body := assertReq(ctx, t, link, http.MethodDelete, "", http.StatusOK, "", planet.Satellites[0].Config.Console.AuthToken)
require.Len(t, body, 0)
keys, err = planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
keys, err = planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 0)
// Delete a deleted key returns Not Found.
body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusNotFound, "", planet.Satellites[0].Config.Console.AuthToken)
body = assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusNotFound,
"",
planet.Satellites[0].Config.Console.AuthToken,
)
require.Contains(t, string(body), "does not exist")
})
}
@ -124,20 +146,63 @@ func TestApiKeyDelete_ByName(t *testing.T) {
address := planet.Satellites[0].Admin.Admin.Listener.Addr()
projectID := planet.Uplinks[0].Projects[0].ID
keys, err := planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
keys, err := planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 1)
link := fmt.Sprintf("http://"+address.String()+"/api/projects/%s/apikeys/%s", projectID.String(), keys.APIKeys[0].Name)
body := assertReq(ctx, t, link, http.MethodDelete, "", http.StatusOK, "", planet.Satellites[0].Config.Console.AuthToken)
apiKeyName := keys.APIKeys[0].Name
link := fmt.Sprintf("http://"+address.String()+"/api/projects/%s/apikeys", projectID.String())
body := assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusOK,
"",
planet.Satellites[0].Config.Console.AuthToken,
[2]string{"name", apiKeyName},
)
require.Len(t, body, 0)
keys, err = planet.Satellites[0].DB.Console().APIKeys().GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
// Deleting a key which contains slashes and not exist returns 404. Gorilla Mux returns 405 if
// they key would be passed as path parameter regardless if it exists or not, so this tests that
// deleting a key whose name contains slashes works as expected and it isn't interpreted as a
// path separator.
body = assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusNotFound,
"",
planet.Satellites[0].Config.Console.AuthToken,
[2]string{"name", "this/is/my_key"},
)
require.Contains(t, string(body), "does not exist")
keys, err = planet.Satellites[0].DB.Console().
APIKeys().
GetPagedByProjectID(ctx, projectID, console.APIKeyCursor{Page: 1, Limit: 10})
require.NoError(t, err)
require.Len(t, keys.APIKeys, 0)
// Delete a deleted key returns Not Found.
body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusNotFound, "", planet.Satellites[0].Config.Console.AuthToken)
body = assertReq(
ctx,
t,
link,
http.MethodDelete,
"",
http.StatusNotFound,
"",
planet.Satellites[0].Config.Console.AuthToken,
[2]string{"name", apiKeyName},
)
require.Contains(t, string(body), "does not exist")
})
}

2
satellite/admin/server.go
View File

@ -153,7 +153,7 @@ func NewServer(
fullAccessAPI.HandleFunc("/projects/{project}", server.getProject).Methods("GET")
fullAccessAPI.HandleFunc("/projects/{project}/apikeys", server.addAPIKey).Methods("POST")
fullAccessAPI.HandleFunc("/projects/{project}/apikeys", server.listAPIKeys).Methods("GET")
fullAccessAPI.HandleFunc("/projects/{project}/apikeys/{name}", server.deleteAPIKeyByName).Methods("DELETE")
fullAccessAPI.HandleFunc("/projects/{project}/apikeys", server.deleteAPIKeyByName).Methods("DELETE").Queries("name", "")
fullAccessAPI.HandleFunc("/projects/{project}/buckets/{bucket}", server.getBucketInfo).Methods("GET")
fullAccessAPI.HandleFunc("/projects/{project}/buckets/{bucket}/geofence", server.createGeofenceForBucket).Methods("POST")
fullAccessAPI.HandleFunc("/projects/{project}/buckets/{bucket}/geofence", server.deleteGeofenceForBucket).Methods("DELETE")

9
satellite/admin/testutils_test.go
View File

@ -37,7 +37,7 @@ func assertGet(ctx context.Context, t *testing.T, link string, expected string,
// assertReq asserts the request and it's OK it returns the response body.
func assertReq(
ctx *testcontext.Context, t *testing.T, link string, method string, body string,
expectedStatus int, expectedBody string, authToken string,
expectedStatus int, expectedBody string, authToken string, queryParams ...[2]string,
) []byte {
t.Helper()
@ -55,6 +55,13 @@ func assertReq(
req.Header.Set("Authorization", authToken)
req.Header.Set("Content-Type", "application/json")
query := req.URL.Query()
for _, q := range queryParams {
query.Add(q[0], q[1])
}
req.URL.RawQuery = query.Encode()
res, err := http.DefaultClient.Do(req) //nolint:bodyclose
require.NoError(t, err)
defer ctx.Check(res.Body.Close)

8
satellite/admin/ui/src/lib/api.ts
View File

@ -238,10 +238,10 @@ export class Admin {
['API Key name', new InputText('text', true)]
],
func: async (projectId: string, apiKeyName: string): Promise<null> => {
return this.fetch(
'DELETE',
`projects/${projectId}/apikeys/${apiKeyName}`
) as Promise<null>;
const query = this.urlQueryFromObject({
name: apiKeyName
});
return this.fetch('DELETE', `projects/${projectId}/apikeys`, query) as Promise<null>;
}
},
{