On Tue, Jul 12, 2011 at 9:19 AM, Kiran Ayyagari <[email protected]> wrote: > On Tue, Jul 12, 2011 at 11:23 AM, Emmanuel Lécharny > <[email protected]> wrote: >> On 7/12/11 2:21 AM, Alex Karasulu wrote: >>> >>> On Mon, Jul 11, 2011 at 9:55 AM, Emmanuel Lecharny<[email protected]> >>> wrote: >>>> >>>> I'm not sure it"s a good idea to setup a default session, at least to >>>> admin. >>>> If we consider the normal (ie, not embedded) server, we don't set any >>>> session, the default session is Anonymous (of course if allowed). IMO, >>>> this >>>> might be a security breach too. >>>> >>>> What was the rational for this modificatioon, Alex ? >>> >>> First there was a big null pointer exception due to this not being >>> set. Second taking a big step back I thought about it and if I have a >>> handle on DirectoryService I can pretty much do anything anyway. If >>> I'm using CoreSessions and DirectoryServices I can use any kind of >>> session there's no security barrier there. So IMO there's no security >>> issue here to defaulting to an admin session. >> >> Make sense. I'm just wondering if we shouldn't mimic the way the LDAP server >> works by forcing the session to use an anonymous principal by default, >> instead of an admin one. I shouldn't have used the term 'security issue', >> it's not really a problem in this case, what I had in mind is that if >> someone want to use a Admin session, it's probably better to require that he >> explicitly create such a session. Call it 'protection against stupid >> move'... >> >> PS : NPE ? ouch... >> > this NPE is a valid one in this case, note that this is a connection > implementation and a user is supposed to call bind() > but the constructor LdapCoreSessionConnection( CoreSession session ) > is entirely different, in this case the user knows what he is > doing, so my preference would be to move the assignment to this > constructor and assign the passed session (i.e., > > this.session = session;
There were actually a few places where this could happen. If session was passed in as a method argument sure we can just set it as you did above. But I don't think we had that option in the method where the change was induced. > rather than > > this.session = directoryService.getAdminSession(); in setDirectoryService()) > > what we already know is that DS is available and user/app can do > anything if has got access to, but more important is > the usage from an app developer's POV, if I have a web app that allows > users to connect to the server using LdapCoreSessionConnection > then assigning admin session by default during initialization will be > a serious security issue. LDAP applications rarely align their authorization schema with LDAP security. Most applications just connect as admin and handle lookups on behalf of their users. But I think you and Emmanuel both make a good case here. We need to solve this better since some applications like the self service applications we've spoken about writing might use direct LDAP security. However I think we don't just go with an anonymous session or a admin session. We need a means to make this decision better. We should require a bind to set the exact session. -- Best Regards, -- Alex
