Hi Manfred I don't see how you can 'keep it' for the LDAP case. Please create new Improvement ticket, such that we can look for a proper solution.
What you actually wanted to do but which IMO got never explicitly mentioned due to mixing implementation details with the generic sync-stuff: "Ability to resolve principal name from ExternalIdentityRef without IDP roundtrip" IMO such an improvement would make sense for all cases not just for the LDAP integration. But I want to have that done properly and without hacks and relying on implementation details of one particular impl for the generic code base. Based on the experience we made here, I think it would make sense, to get more reviews and feedback on changes for the mentioned modules. Specially when API extensions are involved that are really cumbersome to fix later on. In general I think this is a very good practise and it may help spotting issues early in the cycle. Kind regards Angela On 01/12/16 13:46, "Manfred Baedke" <[email protected]> wrote: >Hi Davide, Angela, > >Note that this is not an issue with oak-auth-ldap, but rather with >oak-auth-external. > >The ExternalIdentity implementation used by oak-auth-ldap uses the DN as >both the id and the principal name, so it's working fine. Other >implementations of the external auth mechanism may run into problems >(btw: are other implementations known?). > >I'm going to revert the change now, but since the performance >improvement was striking on customer site, I'd like to keep it for the >LDAP case. Anyone feel free to give suggestions on the most elegant way >to do this. > >Best regards, >Manfred > >On 12/1/2016 12:52 PM, Angela Schreiber wrote: >> Hi Davide >> >> IMO the fix for OAK-5200 is straight forward: >> >> 1. revert changes made to DynamicSyncContext: it's here where the bug >>was >> introduced. >> that should be doable today. >> >> 2. have a quick discussion on oak-dev on how to deal with the new, >> exported class ExternalGroupRef >> which was introduced (and was already backported): this one would >>not >> be used any >> more after 1) but removing it again would (afaik) lead to a major >> version bump. >> i want to get more opinions on that, because i am not aware that we >>had >> the problem >> before (or missed/don't remember how we dealt with it). >> if we see that it's not doable or too cumbersome reverting this >>class, >> we may decide >> to leave it... we might find it useful later on or end up >>deprecating >> it for 1.8. >> but i want to have a conscious decision here and some broader >>agreement >> from the team. >> >> - if we leave it -> changes in auth-ldap code base don't need to be >> reverted >> - if we revert it -> changes in auth-ldap also would need to be >>reverted >> >> Hope that helps >> Angela >> >> PS: as far as the original improvement is concerned that was the >>intention >> of OAK-4930 >> (but which is not obvious from the subject, which is not correct), we >>need >> to have a new >> Improvement ticket, where we can discuss the options... I want that kind >> of improvements >> to be properly tracked in jira (and ultimately in the documentation and >> benchmarked) such >> that everyone can understand why a given change was made... i had a hard >> time yesterday to >> understand what was the actual aim of OAK-4930 and it was just by >> coincidence that I hours >> later spotted the issue... just because I could not understand how I >>could >> have mixed up >> ID and principal name in the original code (and which turned out wasn't >> the case). >> >> >> On 01/12/16 12:15, "Davide Giannella" <[email protected]> wrote: >> >>> Hello team, >>> >>> I'm planning to cut Oak 1.5.15 on Monday 5th December. >>> >>> If there are any objections please let me know. Otherwise I will >>> re-schedule any non-resolved issue for the next iteration. >>> >>> Angela, Manfred, is https://issues.apache.org/jira/browse/OAK-5200 >>> really a blocker for 1.5.15? Or is it rather for 1.6? If it's for >>>1.5.15 >>> how long for the fix to get done? >>> >>> Thanks >>> Davide >>> >>> >
