You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by "coheigea (via GitHub)" <gi...@apache.org> on 2026/10/14 08:30:04 UTC
[PR] Apply search authorization to persistent search notifications [directory-server]
coheigea opened a new pull request, #382:
URL: https://github.com/apache/directory-server/pull/382
Apply search authorization to persistent search notifications
Entries sent to a persistent search client were taken straight from the
operation context, bypassing the authentication and ACI checks applied
to a normal search, as well as the userPassword hiding. With changesOnly
set, no check at all was done on the client before registering it.
- PersistentSearchListener now reads every changed entry again through
the client's own session, with a base level search, and only sends it
if it is visible, with the attributes the client is allowed to read.
userPassword is removed when the server hides passwords.
- As a deleted entry can't be read anymore, DirectoryListener gets a
beforeEntryDeleted() callback, called by the EventInterceptor before
the entry is deleted.
- A changesOnly persistent search now does a base level search through
the client's session before the listener is registered, so that an
unauthenticated client is rejected when anonymous access is disabled.
- The EventInterceptor executor queue is now unbounded: the listeners
wait for the end of the operation which notifies them, and a rejected
notification made that operation fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on code in PR #382:
URL: https://github.com/apache/directory-server/pull/382#discussion_r4372893839
##########
protocol-ldap/src/main/java/org/apache/directory/server/ldap/handlers/request/SearchRequestHandler.java:
##########
@@ -149,6 +150,19 @@ private void handlePersistentSearch( LdapSession session, SearchRequest req, Per
return;
}
}
+ else
+ {
+ // Even if only the changes are requested, the client must be allowed to
+ // search the base : do a base level search through the client's session,
+ // which will throw an exception if it isn't authenticated as required, or
+ // if the base does not exist
+ try ( Cursor<Entry> cursor = session.getCoreSession().search( req.getBase(), SearchScope.OBJECT,
+ new PresenceNode( SchemaConstants.OBJECT_CLASS_AT ), AliasDerefMode.NEVER_DEREF_ALIASES,
+ SchemaConstants.NO_ATTRIBUTE ) )
+ {
Review Comment:
Not sure it's necessary. We can let the cursor being auto-closed without doing anything with it, teh idea is just to check that the user is authorized.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub) via dev" <de...@directory.apache.org>.
elecharny commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6564910444
I have added a unit test that demonstrates the issue. Bottom line we directly write back each modified entry to teh user who have started a persistent search, with no authz check.
Adding this chekc is not trivial: first we need to have the identity of the user who has started the PSearch, second we must activate the authz filter before sending back an entry, but we shoudl also avoid doing a search: we have the full entry at hand.
Working on that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on code in PR #382:
URL: https://github.com/apache/directory-server/pull/382#discussion_r4372881534
##########
interceptors/event/src/main/java/org/apache/directory/server/core/event/EventInterceptor.java:
##########
@@ -110,8 +109,13 @@ public Thread newThread( Runnable runnable )
}
};
- executor = new ThreadPoolExecutor( 1, 10, 1000, TimeUnit.MILLISECONDS, new ArrayBlockingQueue<Runnable>( 100 ),
- threadFactory );
+ // The listeners may have to wait for the end of the operation which notifies them
+ // (persistent search listeners read the modified entry), so the queue must not be
+ // bounded : a rejected notification would make this operation fail.
+ ThreadPoolExecutor threadPoolExecutor = new ThreadPoolExecutor( 10, 10, 1000, TimeUnit.MILLISECONDS,
+ new LinkedBlockingQueue<Runnable>(), threadFactory );
Review Comment:
Not sure this is a good idea: persistent search can last for days, without any update operations being done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6405112177
The raised issue is really critical: we must absolutely check the authorization for each entry potentially returned to the client. Now, I'm not sure the proposed solution is ideal: it costs a supplemental search for each modified entry, something we want to avoid at all.
Ideally speaking, we should go through the interceptor chains up to the Authorization interceptor, but no farther. It would require the addition of a flag in the Operation context that would tell the Authz interceptor to just verify the resuqested authorization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6468785750
OK thanks @elecharny . Should I close this Pr if you want to pursue a different way of fixing it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6286348285
Sorry for the delay, lot of $dayjob atm. Will review your PR soon!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6482769991
I still have to add some unit test for this one. I have little time atm, but I'll try to dedicate a part of it to this PR, so please, keep it open.
Thanks :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [PR] Apply search authorization to persistent search notifications [directory-server]
Posted by "elecharny (via GitHub)" <gi...@apache.org>.
elecharny commented on PR #382:
URL: https://github.com/apache/directory-server/pull/382#issuecomment-6437854243
It's even more complex than what I thought. Results are sent to the client when a modification is applied, through code like:
```
public void modify( final ModifyOperationContext modifyContext ) throws LdapException
{
Entry oriEntry = modifyContext.getEntry();
// modification was already done when this flag is turned on, move to sending the events
if ( !modifyContext.isPushToEvtInterceptor() )
{
next( modifyContext );
}
List<RegistrationEntry> selecting = getSelectingRegistrations( modifyContext.getDn(), oriEntry );
if ( selecting.isEmpty() )
{
return;
}
// Get the modified entry
CoreSession session = modifyContext.getSession();
LookupOperationContext lookupContext = new LookupOperationContext( session, modifyContext.getDn(),
SchemaConstants.ALL_ATTRIBUTES_ARRAY );
lookupContext.setPartition( modifyContext.getPartition() );
lookupContext.setTransaction( modifyContext.getTransaction() );
Entry alteredEntry = directoryService.getPartitionNexus().lookup( lookupContext );
modifyContext.setAlteredEntry( alteredEntry );
for ( final RegistrationEntry registration : selecting )
{
if ( EventType.isModify( registration.getCriteria().getEventMask() ) )
{
fire( modifyContext, EventType.MODIFY, registration.getListener() );
}
}
}
```
As you can see, before passing into the listener (list code line) we first fetch the entry ("get the modified entry") which do a lookup using the context. I do think we then pass through the authorization interceptor.
To be verified... (and for all operations)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org