Project

General

Profile

Task #1080

Avatar?id=6023&size=50x50 Avatar?id=6023&size=22x22

Rewrite ResourceBind and authentication plugins as preprocessors

Added by Artur Hefczyc Tigase team member about 5 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
Start date:
2013-02-24
Due date:
2015-01-24
% Done:

100%

Estimated time:
16.00 h
Database:
n/a
Additional charges approved:
No

Description

In a similar way as StartTLS required is implemented through preprocessing API to block communication before TLS is activated, the resource bind and authentication should be implemented the same way.

The code from DefaultPacketHandler should be removed and the conditions should be moved over to plugins responsible for functionality.

Perhaps there should be a common, abstract auth plugin to do the checking and all others should extend it with auth mechanism implementation.

Associated revisions

Revision 9ba5880c (diff)
Added by Andrzej Wójcik Tigase team member over 3 years ago

Issue #1080 - Rewrite ResourceBind and authentication plugins as preprocessors

History

#1 Avatar?id=6023&size=24x24 Updated by Artur Hefczyc Tigase team member about 5 years ago

  • Target version changed from tigase-server-5.2.0 to tigase-server-5.2.1

#2 Avatar?id=6023&size=24x24 Updated by Artur Hefczyc Tigase team member about 4 years ago

  • Target version changed from tigase-server-5.2.1 to tigase-server-7.0.0

#3 Avatar?id=6023&size=24x24 Updated by Artur Hefczyc Tigase team member over 3 years ago

  • Due date set to 2015-01-24
  • Assignee changed from Artur Hefczyc to Andrzej Wójcik

#4 Updated by Andrzej Wójcik Tigase team member over 3 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

  1. In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

  2. Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

#5 Avatar?id=6023&size=24x24 Updated by Artur Hefczyc Tigase team member over 3 years ago

Andrzej Wójcik wrote:

I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

Yes, I think it should be left as it is, unless you have a different suggestion.

Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

To be honest I did not think of a case where we have multiple auth plugins. Please look for some way to optimize it.

Maybe we should have an abstract plugin for all auth plugins. This abstract plugin could have a checking logic implemented, so other plugins would not need to reimplement it. Then it would be easier to optimize the checking code. As far as I remember, preprocessors are executed sequentially in SM. So as soon as the first preprocessors returns "false" we could stop processing the packet by other preprocessors.

#6 Updated by Andrzej Wójcik Tigase team member over 3 years ago

Artur Hefczyc wrote:

Andrzej Wójcik wrote:

I started working on this separation as specified but few questions appeared after I started working on this, maybe %kobit could answer this questions:

In PacketDefaultHandler there is also code responsible for counting stanzas and ignoring stream:features sent from client with type = error@, I assume this should be left in @PacketDeafultHandler as it is?

Yes, I think it should be left as it is, unless you have a different suggestion.

OK

Creation of abstract class extending XMPPProcessor and implementing XMPPPreprocessorIfc which will be responsible for filtering stanzas from not yet authenticated connections is a good idea, but when this code will be moved and every auth-related processor will extend it then authentication will be check for single packet more than once if more that one authentication processor will be loaded.

Is it ok? or should I look for some way to optimize this check? as in generally check is rather simple.

To be honest I did not think of a case where we have multiple auth plugins. Please look for some way to optimize it.

We already have at least 2 (I found 2 of them):

  • JabberIqAuth

  • SaslAuth

Maybe we should have an abstract plugin for all auth plugins. This abstract plugin could have a checking logic implemented, so other plugins would not need to reimplement it. Then it would be easier to optimize the checking code. As far as I remember, preprocessors are executed sequentially in SM. So as soon as the first preprocessors returns "false" we could stop processing the packet by other preprocessors.

I already started creation of abstract preprocessor for all auth plugins but I cannot find any solution how to optimize this. You are right, first one which will return false will stop processing but for most of packets (on authenticated connections) this will be checked twice for both plugins active, but since checking is rather fast, I do not think we would need to look for a way to optimize this.

#7 Updated by Andrzej Wójcik Tigase team member over 3 years ago

  • % Done changed from 10 to 60

#8 Updated by Andrzej Wójcik Tigase team member over 3 years ago

  • Status changed from In Progress to In QA
  • Assignee changed from Andrzej Wójcik to Artur Hefczyc
  • % Done changed from 60 to 100

I moved implementation checking conditions from DefaultPacketHandler to proper XMPPProcessors (implementing XMPPPreprocessor interface) and verified that moved code is working properly. I needed to apply some changed to make it work but in result code works as before but is now located in proper locations.

#9 Avatar?id=6023&size=24x24 Updated by Artur Hefczyc Tigase team member over 3 years ago

  • Status changed from In QA to Closed

Also available in: Atom PDF