Skip to content

Conversation

dwd
Copy link
Member

@dwd dwd commented Jun 2, 2025

This is a "rebase and recovery" of work originally done for Surevine back in the Depths Of Time, and published in their open repository.

It is, currently, entirely untested (though the unit tests pass, so... success?).

@akrherz akrherz changed the title SASL2 (2025 Edition) OF-2535 SASL2 (2025 Edition) Jun 2, 2025
@guusdk
Copy link
Member

guusdk commented Jun 4, 2025

Some off-the-cuff remarks:

  • Lets either squash the commits, or rewrite history to not have that addition then removal of database driver stuff in this changeset
  • Is it worth introducing an enum definition of the used authentication type (eg: SASL, SASL2, EXTERNAL) instead of having a boolean that differentiates between using or not using SASL2?
  • needs fixing the 'TODO' that it introduces
  • can we add some unit tests (static methods seem to be low-hanging fruit)
  • the various tasks that are introduced (PasswordResetTask et all): shouldn't those be:
    -- split off in a separate effort
    -- be an interface rather than a class
    -- have some kind of manager that easily allows extension through the plugin API?

@dwd
Copy link
Member Author

dwd commented Jun 19, 2025

So, yes, I'll rewrite history to tidy things up, for sure.

The Boolean is SASL vs SASL2; EXTERNAL is just a mechanism and can be used with either. This code does support SASL2 server-side for S2S.

There are now some tests.

I should probably strip out the tasks; they've not been standardised aside from TOTP, and it feels like a distinct problem to solve, anyway.

@dwd
Copy link
Member Author

dwd commented Jun 19, 2025

Bit of a think - I think this is functionally complete and working, though it's entirely ignoring the element and has no support for tasks (and therefore will never send ).

I'll sort out the history and trim off deadcode tomorrow, and hide it behind a feature flag.

@guusdk
Copy link
Member

guusdk commented Jul 8, 2025

From a (very) cursory read: does it make sense to expose the UserAgentInfo to other cluster nodes? This can probably be achieved by adding it to org.jivesoftware.openfire.session.RemoteSessionTask.

@dwd
Copy link
Member Author

dwd commented Jul 8, 2025

From a (very) cursory read: does it make sense to expose the UserAgentInfo to other cluster nodes? This can probably be achieved by adding it to org.jivesoftware.openfire.session.RemoteSessionTask.

Sounds sensible; I'll see what I can do when I next hit this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants