#94 feat(usermanagement): add secure self-delete endpoint with confirmation and test coverage - #97
Conversation
There was a problem hiding this comment.
Testes realizados
Registro e login de usuários:
- criação usuários de teste (delete_test, user1, user2, integrity_test)
- login funciona antes da deleção
Deleção em si:
- endpoint testado manualmente
- somente o próprio usuário pode se deletar (tentativa de deletar outro usuário retorna 403 Forbidden)
- confirmação de deleção obrigatória: "Yes, delete my user" (texto exato). Diferente disso, dá erro
- deleção sem o texto correto retorna 400 Invalid Delete Confirmation
- usuário deletado não consegue mais logar (INVALID_CREDENTIALS)
Integridade entre serviços:
- usermanagement-service: usuário realmente removido do banco
- tournament-service: token antigo não funciona e o usuário deletado não acessa dados
Outros:
- tentativa de deletar um usuário errado
- um usuário tentando deletar o outro
- logs OK
- confirmação de deleção incorreta → 400 Invalid Delete Confirmation
- login com usuário deletado → INVALID_CREDENTIALS
- uso de token antigo para acessar outros serviços → falha de autenticação
**** testes unitários e de integração rodaram 100% também
There was a problem hiding this comment.
Pull request overview
Adds a secure self-service account deletion capability to the usermanagement-service, enforcing both ownership (JWT sub must match user_id) and an explicit confirmation phrase before permanently deleting the user record.
Changes:
- Introduces
DELETE /users/{user_id}with ownership enforcement and confirmation-text validation. - Adds a domain command (
DeleteUserProfileCommand) plus service/contract/schema updates to support deletion. - Expands exception mapping and adds unit + integration tests for the delete flow.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/usermanagement-service/src/controller/user.py | Adds the DELETE /users/{user_id} endpoint and ownership check via JWT subject. |
| backend/usermanagement-service/src/core/exception_handlers.py | Maps new/previous domain errors to correct HTTP status codes and error types. |
| backend/usermanagement-service/src/domain/contracts.py | Extends the user service contract with delete_user_profile. |
| backend/usermanagement-service/src/domain/exceptions.py | Adds InvalidDeleteConfirmationError. |
| backend/usermanagement-service/src/domain/schemas/user.py | Adds DeleteUserRequest schema for confirmation payload. |
| backend/usermanagement-service/src/domain/schemas/init.py | Exposes DeleteUserRequest from the schema package. |
| backend/usermanagement-service/src/domain/services/commands/delete_user_profile.py | Implements the delete command with phrase validation + DB delete/commit. |
| backend/usermanagement-service/src/domain/services/commands/init.py | Exports the new delete command. |
| backend/usermanagement-service/src/domain/services/user.py | Adds UserService.delete_user_profile to execute the delete command. |
| backend/usermanagement-service/tests/conftest.py | Extends MockSession to track deletes for unit tests. |
| backend/usermanagement-service/tests/unit/test_delete_user_profile.py | Unit tests for the delete command and service method. |
| backend/usermanagement-service/tests/integration/test_user_delete.py | Integration tests for success/403/400 delete scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| REQUIRED_DELETE_CONFIRMATION = "Yes, delete my user" | ||
|
|
||
|
|
||
| class DeleteUserProfileCommand(Command): | ||
| """Command to delete a user profile with explicit confirmation.""" | ||
|
|
||
| def __init__(self, session: AsyncSession, user_id: str, confirmation_text: str): | ||
| self.session = session | ||
| self.user_id = user_id | ||
| self.confirmation_text = confirmation_text | ||
|
|
||
| async def execute(self) -> None: | ||
| self._validate_confirmation_phrase() | ||
| user = await self._get_user() | ||
|
|
||
| await self.session.delete(user) | ||
| await self.session.commit() | ||
|
|
||
| logger.info("User profile deleted", user_id=self.user_id) | ||
|
|
||
| def _validate_confirmation_phrase(self) -> None: | ||
| if self.confirmation_text != REQUIRED_DELETE_CONFIRMATION: | ||
| raise InvalidDeleteConfirmationError( | ||
| f"Invalid confirmation text. Required: '{REQUIRED_DELETE_CONFIRMATION}'" | ||
| ) |
| @pytest.mark.asyncio | ||
| class TestUserDelete: | ||
| """Integration tests for /users/{user_id} delete endpoint.""" | ||
|
|
||
| async def test_delete_user_success_with_valid_confirmation( | ||
| self, client: AsyncClient, db_session: AsyncSession | ||
| ): |
|
Só não sei se é uma boa expor a frase de deleção.. já tinha pensado nisso e Copilot deu uma reforçada |
Em que sentido você fala sobre essa exposição da frase? Sobre o copilot: |
Context
This PR introduces secure account deletion support in the usermanagement-service.
The endpoint allows full user removal, but only when:
What changed
1) Feature implementation (commit: 03621e9)
Added a new protected DELETE flow in usermanagement-service:
DELETE /users/{user_id}submust matchuser_idFiles included:
2) Test coverage (commit: 6fd1a06)
Added unit and integration tests covering the full delete behavior:
Files included:
API behavior
Endpoint
DELETE /users/{user_id}Request body
{ "confirmation_text": "Yes, delete my user" }Responses
204 No Content: user deleted successfully400 Bad Request: invalid delete confirmation phrase403 Forbidden: authenticated user is not the target user404 Not Found: target user does not existValidation and security rules
Testing
Executed targeted tests for this feature and all passed:
tests/unit/test_delete_user_profile.pytests/integration/test_user_delete.py