Refactors credentials API to include user credentials ID

Updates the credentials update and delete endpoints to require user credentials ID in the URL path, improving API consistency and enabling proper resource identification.

Modifies the PUT endpoint from `/credentials/group/{group_id}` to `/credentials/{user_credentials_id}/group/{group_id}` and updates the DELETE endpoint to include `/group/` segment for better REST API design.

Updates all related client code, tests, and API schema to handle the new endpoint structure with the additional user credentials ID parameter.

Co-authored-by: Arash Kadkhodaei <arash77.kad@gmail.com>
This commit is contained in:
Alireza Heidari
2025-10-07 16:07:58 +02:00
committed by mvdbeek
co-authored by Arash Kadkhodaei
parent 8ddd13eeaa
commit 25347eee7f
9 changed files with 140 additions and 101 deletions
+55 -70
View File
@@ -5194,23 +5194,6 @@ export interface paths {
patch?: never;
trace?: never;
};
"/api/users/{user_id}/credentials/group/{group_id}": {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
get?: never;
/** Updates user credentials */
put: operations["update_user_credentials_api_users__user_id__credentials_group__group_id__put"];
post?: never;
delete?: never;
options?: never;
head?: never;
patch?: never;
trace?: never;
};
"/api/users/{user_id}/credentials/{user_credentials_id}": {
parameters: {
query?: never;
@@ -5228,7 +5211,7 @@ export interface paths {
patch?: never;
trace?: never;
};
"/api/users/{user_id}/credentials/{user_credentials_id}/{group_id}": {
"/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}": {
parameters: {
query?: never;
header?: never;
@@ -5236,10 +5219,11 @@ export interface paths {
cookie?: never;
};
get?: never;
put?: never;
/** Updates user credentials */
put: operations["update_user_credentials_api_users__user_id__credentials__user_credentials_id__group__group_id__put"];
post?: never;
/** Deletes a specific credential */
delete: operations["delete_credentials_api_users__user_id__credentials__user_credentials_id___group_id__delete"];
/** Deletes a specific credential group */
delete: operations["delete_credentials_api_users__user_id__credentials__user_credentials_id__group__group_id__delete"];
options?: never;
head?: never;
patch?: never;
@@ -40472,54 +40456,6 @@ export interface operations {
};
};
};
update_user_credentials_api_users__user_id__credentials_group__group_id__put: {
parameters: {
query?: never;
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
user_id: string | "current";
group_id: string;
};
cookie?: never;
};
requestBody: {
content: {
"application/json": components["schemas"]["ServiceCredentialGroupPayload"];
};
};
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["ServiceCredentialGroupResponse"];
};
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
delete_service_credentials_api_users__user_id__credentials__user_credentials_id__delete: {
parameters: {
query?: never;
@@ -40562,7 +40498,56 @@ export interface operations {
};
};
};
delete_credentials_api_users__user_id__credentials__user_credentials_id___group_id__delete: {
update_user_credentials_api_users__user_id__credentials__user_credentials_id__group__group_id__put: {
parameters: {
query?: never;
header?: {
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
"run-as"?: string | null;
};
path: {
user_id: string | "current";
user_credentials_id: string;
group_id: string;
};
cookie?: never;
};
requestBody: {
content: {
"application/json": components["schemas"]["ServiceCredentialGroupPayload"];
};
};
responses: {
/** @description Successful Response */
200: {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["ServiceCredentialGroupResponse"];
};
};
/** @description Request Error */
"4XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
/** @description Server Error */
"5XX": {
headers: {
[name: string]: unknown;
};
content: {
"application/json": components["schemas"]["MessageExceptionModel"];
};
};
};
};
delete_credentials_api_users__user_id__credentials__user_credentials_id__group__group_id__delete: {
parameters: {
query?: never;
header?: {
@@ -384,7 +384,7 @@ async function updateGroup(groupId: string): Promise<void> {
saveButtonText.value = busyMessage.value;
await saveUserCredentials(groupId, groupToUpdate.groupPayload);
await saveUserCredentials(props.serviceDefinition, groupId, groupToUpdate.groupPayload);
discardGroupChanges(groupId);
@@ -198,7 +198,11 @@ async function onSaveChanges(): Promise<void> {
saveButtonText.value = busyMessage.value;
await saveUserCredentials(groupToUpdate.groupData.groupId, groupToUpdate.groupData.groupPayload);
await saveUserCredentials(
groupToUpdate.serviceDefinition,
groupToUpdate.groupData.groupId,
groupToUpdate.groupData.groupPayload,
);
Toast.success("Credentials group updated successfully");
@@ -185,14 +185,14 @@ describe("useUserToolCredentials", () => {
return response(200).json(TEST_CREDENTIALS_GROUP);
}),
http.put("/api/users/{user_id}/credentials/group/{group_id}", ({ response }) => {
http.put("/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}", ({ response }) => {
return response(200).json({
...TEST_CREDENTIALS_GROUP,
variables: [{ name: "bucket_name", value: "updated-bucket" }],
});
}),
http.delete("/api/users/{user_id}/credentials/{user_credentials_id}/{group_id}", ({ response }) => {
http.delete("/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}", ({ response }) => {
return response(204).empty();
}),
@@ -268,12 +268,14 @@ export function useUserToolCredentials(toolId: string, toolVersion: string) {
/**
* Save user credentials for the tool.
* @param {ServiceCredentialsIdentifier} serviceIdentifier - Service name and version identifier.
* @param {string} groupId - Group ID to update.
* @param {ServiceCredentialGroupPayload} serviceGroupPayload - Service group payload to save.
* @returns {Promise<void>}
* @throws {Error} If the credential save fails.
*/
async function saveUserCredentials(
serviceIdentifier: ServiceCredentialsIdentifier,
groupId: string,
serviceGroupPayload: ServiceCredentialGroupPayload,
): Promise<void> {
@@ -281,6 +283,7 @@ export function useUserToolCredentials(toolId: string, toolVersion: string) {
await userToolsServiceCredentialsStore.updateUserCredentialsForTool(
toolId,
toolVersion,
serviceIdentifier,
groupId,
serviceGroupPayload,
);
@@ -368,6 +368,7 @@ export const useUserToolsServiceCredentialsStore = defineStore("userToolsService
* Updates existing credentials for a tool.
* @param {string} toolId - Tool identifier.
* @param {string} toolVersion - Tool version.
* @param {ServiceCredentialsIdentifier} serviceIdentifier - Service name and version.
* @param {string} groupId - Group ID to update.
* @param {ServiceCredentialGroupPayload} serviceGroupPayload - Updated credential data.
* @returns {Promise<ServiceCredentialGroupResponse>} Updated credential group.
@@ -376,11 +377,19 @@ export const useUserToolsServiceCredentialsStore = defineStore("userToolsService
async function updateUserCredentialsForTool(
toolId: string,
toolVersion: string,
serviceIdentifier: ServiceCredentialsIdentifier,
groupId: string,
serviceGroupPayload: ServiceCredentialGroupPayload,
): Promise<ServiceCredentialGroupResponse> {
const userId = ensureUserIsRegistered();
const toolService = getToolService(toolId, toolVersion, serviceIdentifier);
const toolServicesId = toolService?.id;
if (!toolServicesId) {
const userToolKey = getUserToolKey(toolId, toolVersion);
throw new Error(`No service found for tool ${userToolKey}`);
}
const serviceGroupPayloadCopy = structuredClone(serviceGroupPayload);
removeSecretPlaceholders(serviceGroupPayloadCopy);
@@ -388,12 +397,15 @@ export const useUserToolsServiceCredentialsStore = defineStore("userToolsService
isBusy.value = true;
try {
const { data, error } = await GalaxyApi().PUT("/api/users/{user_id}/credentials/group/{group_id}", {
params: {
path: { user_id: userId, group_id: groupId },
const { data, error } = await GalaxyApi().PUT(
"/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}",
{
params: {
path: { user_id: userId, user_credentials_id: toolServicesId, group_id: groupId },
},
body: serviceGroupPayloadCopy,
},
body: serviceGroupPayloadCopy,
});
);
if (error) {
const userToolKey = getUserToolKey(toolId, toolVersion);
@@ -442,7 +454,7 @@ export const useUserToolsServiceCredentialsStore = defineStore("userToolsService
try {
const { error } = await GalaxyApi().DELETE(
"/api/users/{user_id}/credentials/{user_credentials_id}/{group_id}",
"/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}",
{
params: {
path: { user_id: userId, user_credentials_id: toolServicesId, group_id: groupId },
+5 -4
View File
@@ -84,17 +84,18 @@ class FastAPICredentials:
return self.service.provide_credential(trans, user_id, payload)
@router.put(
"/api/users/{user_id}/credentials/group/{group_id}",
"/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}",
summary="Updates user credentials",
)
def update_user_credentials(
self,
user_id: FlexibleUserIdType,
user_credentials_id: DecodedDatabaseIdField,
group_id: DecodedDatabaseIdField,
payload: ServiceCredentialGroupPayload,
trans: ProvidesUserContext = DependsOnTrans,
) -> ServiceCredentialGroupResponse:
return self.service.update_user_credentials(trans, user_id, group_id, payload)
return self.service.update_user_credentials(trans, user_id, user_credentials_id, group_id, payload)
@router.put(
"/api/users/{user_id}/credentials",
@@ -125,8 +126,8 @@ class FastAPICredentials:
return Response(status_code=status.HTTP_204_NO_CONTENT)
@router.delete(
"/api/users/{user_id}/credentials/{user_credentials_id}/{group_id}",
summary="Deletes a specific credential",
"/api/users/{user_id}/credentials/{user_credentials_id}/group/{group_id}",
summary="Deletes a specific credential group",
status_code=status.HTTP_204_NO_CONTENT,
)
def delete_credentials(
@@ -92,12 +92,13 @@ class CredentialsService:
self,
trans: ProvidesUserContext,
user_id: FlexibleUserIdType,
user_credentials_id: DecodedDatabaseIdField,
group_id: DecodedDatabaseIdField,
payload: ServiceCredentialGroupPayload,
) -> ServiceCredentialGroupResponse:
"""Updates user credentials for a specific group."""
user = self._ensure_user_access(trans, user_id)
return self._update_credentials(trans.sa_session, user, group_id, payload)
return self._update_credentials(trans.sa_session, user, user_credentials_id, group_id, payload)
def update_user_credentials_group(
self,
@@ -351,11 +352,14 @@ class CredentialsService:
self,
session: scoped_session,
user: User,
user_credentials_id: DecodedDatabaseIdField,
group_id: DecodedDatabaseIdField,
payload: ServiceCredentialGroupPayload,
) -> ServiceCredentialGroupResponse:
group_name, variables, secrets = payload.name, payload.variables, payload.secrets
existing_user_credentials = self.credentials_manager.get_user_credentials(user.id, group_id=group_id)
existing_user_credentials = self.credentials_manager.get_user_credentials(
user.id, user_credentials_id=user_credentials_id, group_id=group_id
)
if not existing_user_credentials:
raise ObjectNotFound(f"No credentials found for user {user.id} in group {group_id}.")
+45 -15
View File
@@ -94,19 +94,22 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
created_group = self._provide_user_credentials(payload)
created_group_id = created_group["id"]
list_user_credentials = self._check_credentials_exist()
user_credentials_id = list_user_credentials[0]["id"]
update_payload = self._build_update_credentials_payload(group_name=random_name())
group_name_updated_group = self._update_credentials(created_group_id, update_payload)
group_name_updated_group = self._update_credentials(user_credentials_id, created_group_id, update_payload)
assert group_name_updated_group["update_time"] > created_group["update_time"]
update_payload = self._build_update_credentials_payload(variables=[{"name": "server", "value": random_name()}])
variable_updated_group = self._update_credentials(created_group_id, update_payload)
variable_updated_group = self._update_credentials(user_credentials_id, created_group_id, update_payload)
assert variable_updated_group["update_time"] > group_name_updated_group["update_time"]
assert variable_updated_group["update_time"] > created_group["update_time"]
update_payload = self._build_update_credentials_payload(
secrets=[{"name": "username", "value": random_name()}, {"name": "password", "value": None}]
)
secret_updated_group = self._update_credentials(created_group_id, update_payload)
secret_updated_group = self._update_credentials(user_credentials_id, created_group_id, update_payload)
assert secret_updated_group["update_time"] > variable_updated_group["update_time"]
assert secret_updated_group["update_time"] > group_name_updated_group["update_time"]
assert secret_updated_group["update_time"] > created_group["update_time"]
@@ -117,19 +120,28 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
initial_group = self._provide_user_credentials()
group_id = initial_group["id"]
list_user_credentials = self._check_credentials_exist()
user_credentials_id = list_user_credentials[0]["id"]
# Update only group name
new_name = random_name()
result = self._update_credentials(group_id, self._build_update_credentials_payload(group_name=new_name))
result = self._update_credentials(
user_credentials_id, group_id, self._build_update_credentials_payload(group_name=new_name)
)
assert result["name"] == new_name
# Update only variables
new_variables = [{"name": "server", "value": "https://new-server.com"}]
result = self._update_credentials(group_id, self._build_update_credentials_payload(variables=new_variables))
result = self._update_credentials(
user_credentials_id, group_id, self._build_update_credentials_payload(variables=new_variables)
)
assert result["variables"][0]["value"] == "https://new-server.com"
# Update only secrets
new_secrets = [{"name": "username", "value": "newuser"}, {"name": "password", "value": "newpass"}]
result = self._update_credentials(group_id, self._build_update_credentials_payload(secrets=new_secrets))
result = self._update_credentials(
user_credentials_id, group_id, self._build_update_credentials_payload(secrets=new_secrets)
)
assert any(s["name"] == "username" and s["is_set"] for s in result["secrets"])
# Update all fields at once
@@ -137,6 +149,7 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
final_variables = [{"name": "server", "value": "https://final-server.com"}]
final_secrets = [{"name": "username", "value": "finaluser"}, {"name": "password", "value": "finalpass"}]
result = self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(
group_name=final_name, variables=final_variables, secrets=final_secrets
@@ -152,14 +165,22 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
group = self._provide_user_credentials()
group_id = group["id"]
list_user_credentials = self._check_credentials_exist()
user_credentials_id = list_user_credentials[0]["id"]
# Invalid group name - use empty string which should be invalid
self._update_credentials(group_id, self._build_update_credentials_payload(group_name=""), status_code=400)
self._update_credentials(
user_credentials_id, group_id, self._build_update_credentials_payload(group_name=""), status_code=400
)
# Missing required variable
self._update_credentials(group_id, self._build_update_credentials_payload(variables=[]), status_code=400)
self._update_credentials(
user_credentials_id, group_id, self._build_update_credentials_payload(variables=[]), status_code=400
)
# Missing required secret
self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(secrets=[{"name": "password", "value": "pass"}]),
status_code=400,
@@ -167,11 +188,13 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
# Invalid credential names
self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(variables=[{"name": "invalid_name", "value": "value"}]),
status_code=400,
)
self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(secrets=[{"name": "invalid_secret", "value": "value"}]),
status_code=400,
@@ -179,11 +202,13 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
# Empty required values
self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(variables=[{"name": "server", "value": ""}]),
status_code=400,
)
self._update_credentials(
user_credentials_id,
group_id,
self._build_update_credentials_payload(
secrets=[{"name": "username", "value": ""}, {"name": "password", "value": "pass"}]
@@ -193,7 +218,10 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
# Test non-existent group ID
self._update_credentials(
"f2db41e1fa331b3e", self._build_update_credentials_payload(group_name="test"), status_code=400
user_credentials_id,
"f2db41e1fa331b3e",
self._build_update_credentials_payload(group_name="test"),
status_code=400,
)
@skip_without_tool(CREDENTIALS_TEST_TOOL)
@@ -253,7 +281,7 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
assert list_user_credentials[0]["current_group_id"] == target_group_id
# Delete the group
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/{target_group_id}")
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/group/{target_group_id}")
self._assert_status_code_is(response, 204)
# Check group is deleted - should only have our initial group left
@@ -302,7 +330,7 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
self._assert_status_code_is(response, 400)
def test_delete_nonexistent_credentials_group(self):
response = self._delete("/api/users/current/credentials/f2db41e1fa331b3e/f2db41e1fa331b3e")
response = self._delete("/api/users/current/credentials/f2db41e1fa331b3e/group/f2db41e1fa331b3e")
self._assert_status_code_is(response, 400)
@skip_without_tool(CREDENTIALS_TEST_TOOL)
@@ -315,7 +343,7 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
user_credentials_list = self._check_credentials_exist()
user_credentials_id = user_credentials_list[0]["id"]
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/{group_id}")
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/group/{group_id}")
self._assert_status_code_is(response, 204)
@skip_without_tool(CREDENTIALS_TEST_TOOL)
@@ -422,7 +450,7 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
# Delete the credentials group
user_credentials_id = credentials_list[0]["id"]
group_id = group["id"]
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/{group_id}")
response = self._delete(f"/api/users/current/credentials/{user_credentials_id}/group/{group_id}")
self._assert_status_code_is(response, 204)
# Check that secrets are removed from the vault
@@ -466,9 +494,11 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
},
}
def _update_credentials(self, group_id, payload=None, status_code=200):
def _update_credentials(self, user_credentials_id, group_id, payload=None, status_code=200):
payload = payload or self._build_update_credentials_payload()
response = self._put(f"/api/users/current/credentials/group/{group_id}", data=payload, json=True)
response = self._put(
f"/api/users/current/credentials/{user_credentials_id}/group/{group_id}", data=payload, json=True
)
self._assert_status_code_is(response, status_code)
return response.json()