cloudfoundry / cloud_controller_ng

Cloud Foundry Cloud Controller
Apache License 2.0
187 stars 356 forks source link

UAA Client Improvements #3851

Open philippthun opened 1 month ago

philippthun commented 1 month ago

Validation of User "Guids"

Cloud Controller accepts arbitrary strings (only rule: 1 to 200 characters long) as input for the "guid" of a user - which can either be a real UUID or an (OAuth) client ID. Although the POST /v3/users endpoint is for admins only, all org managers can create users by assigning them a role within their org. Currently there is no further validation with regards to allowed characters in the "guid"; there is also no check if the added "guid" already exists in UAA (either as a user's guid or a client's id).

Question: Should there be further validations (e.g. as part of the UserCreateMessage)? To be considered: v2 endpoints don't use this message class, but directly create the user (e.g. User.create).

Question: Would it make sense to send a request to UAA to validate the provided "guid"?

Categorization of UAA Errors

All UAA errors are presented as UaaUnavailable errors. This is misleading and hides some real errors coming from UAA (that might be caused by an incorrect call done by Cloud Controller). For other external components there are different error types, i.e. StagerUnavailable vs. StagerError, RunnerUnavailable vs. RunnerError and BlobstoreUnavailable vs. BlobstoreError.

Question: Would it make sense to introduce an UaaError (HTTP status code 500) and distinguish between temporary (e.g. connection) issues and other concrete (permanent) errors?

Retry Mechanism for Transient Errors

Currently all UAA errors are retried. Although this reduces problems due to temporary issues, it seems rather unnecessary for errors caused by a wrong request. Having a better categorization of errors (see above) would make it easier to decide which errors should be retried and which not.