Allow specifying a fixed prefix and maximum allowed length for accounts generated by the Active Directory backend

stackabletech / secret-operator

Other

11 stars 6 forks source link

Allow specifying a fixed prefix and maximum allowed length for accounts generated by the Active Directory backend #449

Closed soenkeliebau closed 2 months ago

soenkeliebau commented 3 months ago

We should enhance the secret operator to give a certain degree of influence over the accounts that it generates in active directory, when using this backend for Kerberos principals.

Necessary functionality:

specify a fixed prefix that should be prepended to all samaccountnames of like for example "svc01"
specify a maximum length for the overall length of samaccountname (if used in conjunction with the prefix, the overall length counted together with the prefix and the generated portion of the name should not exceed this value)

nightkr commented 3 months ago

I'm curious about the max length one, we should already be hard-coded to fit within AD's limits. Do they systems that limit it further?

There's also the problem that the shorter it gets, the more likely collissions get... :/

soenkeliebau commented 3 months ago

Do we think collisions are a real issue? For example what this user wants would give us 10 characters for the random part, with numbers, upper- and lowercase letters that gives us 62^10 combinations (839299365868340224) - sounds okay, but I know that for this type of things large numbers can be deceiving :)

ChatGPT says this:

The probability of at least one collision occurring when randomly generating 10,000 hash values from a possible 839299365868340224 is approximately 5.96×10 ^ −11 , which is extremely small. This suggests that collisions are highly unlikely with such a large number of possibilities and relatively few generated values.

Not sure about the length restriction, I can check, but I don't think there is anything we can do there, if we touch this anyway, it might be best to just accept it, make it configurable and add a note to the docs about "you shouldn't need to set this, but if you do, please make sure you consider possible collisions"..

nightkr commented 3 months ago

I was confusing CN and samAccountName here. CN is problematic, sAN is just random anyway so we can set it to whatever.

lfrancke commented 3 weeks ago

Can you please include a link to the docs and include a snippet we can use for the release notes? And is this a CRD change requiring any actions by the users?

nightkr commented 2 weeks ago

Docs: https://docs.stackable.tech/home/nightly/secret-operator/secretclass#ad-samaccountname

Release notes: "Added support for customizing sAMAccountName generation"

The CRD change is purely additive, by default it will keep the old behaviour.

maxgruber19 commented 2 weeks ago

Just wanted to let you know that this is perfectly fine, we're looking forward to 24.11 :-) thanks for your quick solution