Split Client Active Directory model from data-transfer model when publishing CloudEvents

[stijnmoreels]

Is your feature request related to a problem? Please describe.
When receiving secrets from the Azure Active Directory, the same model is used to both retrieve the correct secrets as well to send CloudEvents to Azure Event Grid. This can possible lead to security issues when we want to enhance the AzureApplication model because it would immediately mean we enhance the transfer object for the event, making it public.

Describe the solution you'd like
Split the models into 2 so we have more control over what needs to be internal and what can be published.

[stijnmoreels]

Something you can agree on, @pim-simons ?

[pim-simons]

Just to check, you want a seperate model to use here:

arcus.backgroundjobs/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJobOptions.cs

Line 86 in e392e5d

internal CloudEvent CreateEvent(AzureApplication application, ClientSecretExpirationEventType type, Uri eventUri)

I don't really see a problem with that, if it removes a possible issue later on than I'm all for it 👍🏻

[stijnmoreels]

Just to check, you want a seperate model to use here:

Yep, that's the one. Some separated event as in AzureApplicationEvent or so.
Now we use the KeyId of the secret. Is that a possible vulnerability risk?

[pim-simons]

Just to check, you want a seperate model to use here:

Yep, that's the one. Some separated event as in AzureApplicationEvent or so. Now we use the KeyId of the secret. Is that a possible vulnerability risk?

I'm not a security person 👀 but I don't see that much harm in using the KeyId. As far as I know it is just an identifier of the secret that has expired.

[stijnmoreels]

This can also be my paranoia speaking, but it still feels like a good idea. Splitting the domain and the transfer models.

[stijnmoreels]

I'll add @fgheysels to ask his view too 😄 .

[stijnmoreels]

On the other hand, the internal model here is rather the PasswordCredential and the AzureApplication is already a sub-set of those values, so in that context it is already split. An extra description on the AzureApplication model could be useful to explain that this model is used for event tracking.