Add GET method to deprovision route

[phavekes]

To match other Openconext components, the deprovision API should also accept GET requetst

[pmeulen]

Hmm. The app can both read (HTTP verb GET) and delete (HTTP verb DELETE) to the deprovision endpoint:

https://github.com/OpenConext/OpenConext-user-lifecycle/blob/7ac0445cb17cbb791d4fbd95491d9ecf049eae2c/src/OpenConext/UserLifecycle/Infrastructure/UserLifecycleBundle/Client/DeprovisionClient.php#L93

https://github.com/OpenConext/OpenConext-user-lifecycle/blob/7ac0445cb17cbb791d4fbd95491d9ecf049eae2c/src/OpenConext/UserLifecycle/Infrastructure/UserLifecycleBundle/Client/DeprovisionClient.php#L118

This is what that should do: https://github.com/OpenConext/OpenConext-user-lifecycle/blob/7ac0445cb17cbb791d4fbd95491d9ecf049eae2c/docs/deprovision-information.md#required-api-endpoints

This does not look like a the correct solution because the middleware function that is being called now does not distinguish between GET and DELETE:

Stepup-Middleware/src/Surfnet/StepupMiddleware/ApiBundle/Controller/DeprovisionController.php

Lines 35 to 54 in 7230660

	public function deprovision(string $collabPersonId): JsonResponse
	{
	$this->denyAccessUnlessGrantedOneOff(['ROLE_DEPROVISION']);
	$errors = [];
	try {
	$userData = $this->deprovisionService->readUserData($collabPersonId);
	if ($userData !== []) {
	$this->deprovisionService->deprovision($collabPersonId);
	}
	} catch (DomainException $e) {
	// On domain exceptions, like when the identity is forgotten, we return OK, with empty data
	// just so the deprovision run does not end prematurely. At this point, no other domain exceptions
	// are thrown.
	$userData = [];
	$errors = [$e->getMessage()];
	} catch (Exception $e) {
	$userData = [];
	$errors = [$e->getMessage()];
	}
	return new JsonResponse($this->formatHelper->format($userData, $errors));

[pmeulen]

See previous comment. It looks like the MW deprovision API does not adhere to the API that lifecycle expects (a GET only returns the user's info, it does not deprovision the user)

[pmeulen]

@johanib What do you think of this?

[johanib]

Lifecycle: GET /deprovision/<collabPersonId>
> Middleware GET deprovision/<collabPersonId> 🔴 Does not exist.

DELETE /deprovision/<collabPersonId>
> DELETE deprovision/<collabPersonId> ✔️

DELETE /deprovision/<collabPersonId>/dry-run
> DELETE deprovision/<collabPersonId>/dry-run ✔️

Without taking the new GET from Peter into consideration, Pieter is right:
The GET deprovision/<collabPersonId> does not exist in middleware. User lifecycle implements in in code & documentation, but it does not work. So, if this is needed, we should add the new GET route to middleware, and return the data.
Usually, GET should not perform mutations, especially in the this context where DELETE is also used.

In summary, I think we should follow the lifecycle specifications.

@phavekes what's the reason for this PR? Is there another component besides user lifecycle that calls the deprovision endpoints on Middleware?

...

On the other hand, why is the GET even needed, if the DRY RUN returns the same data? (But then the dry run should just return no data)

---

Another tweak:
The DELETE deprovision/<collabPersonId>/dry-run route in middleware should only accept DELETE requests. It currently accepts all http verbs.