CESNET
fix(core): lock user uid attribute value generation
- uid-namespace attribute fillAttribute method now locks in order to prevent duplicate values. This was an issue when multiple addMember calls were executed on groups which had the attribute as required.
Note before merging (after discussion with @zlamalp)
- A duplicate value can theoretically still occur when someone manually sets the attribute's value while the attribute is being filled automatically for another user. This way, the attribute module is not used, and the lock does not take effect. We should keep this in mind, discuss whether this issue needs solving, and maybe consider a better alternative to ensuring unique values of attributes in the future.
- While manual testing has been done (successfully), a runtime test should be considered in order to check for deadlocks. However, through code analysis I could not find a scenario in which a transaction would first acquire the attribute lock and then try to acquire the group structure lock (only the ordering of 'group lock' > 'attribute lock' or only 'attribute lock' happens). This should mean that circular wait cannot happen. Nonetheless, as part of the review, it would be nice for someone to go through this again and make sure.
Regarding the potential duplicate value, can we set the attribute as unique to prevent duplicity?
After discussion with @zlamalp @balcirakpeter @vyskocilpavel, we will try to set the attributes to unique which prevents duplicity and does not have deadlock potential. The drawback here is that actions (mostly application approval) can fail if they cannot assign UID value and need to be repeated. It is hard to say how often will actions fail beforehand so if it turns out to be a problem we can try the lock approach. With locks there is a potential of deadlocks since the lock itself is on a attribute for namespace and not the "general" attribute. Again it might not be a problem and is not possible to test with certainty.
