Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MojoAuth Module #525

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benlangfeld
Copy link
Contributor

Permits use of MojoAuth (http://mojoauth.mojolingo.com/) in ejabberd. MojoAuth is a set of standard approaches to cross-app authentication based on HMAC which is specified in RFC2104.

@benlangfeld
Copy link
Contributor Author

This should be good to merge along with #524. Would it be possible to include in 15.05?

@benlangfeld
Copy link
Contributor Author

Is there any chance this might be reviewed for inclusion?

erszcz pushed a commit to erszcz/ejabberd that referenced this pull request Nov 7, 2015
@benlangfeld
Copy link
Contributor Author

@badlop I really need this in several of my apps and running on an ejabberd fork is becoming tricky. Can this and #524 please be considered?

@@ -128,7 +128,7 @@ register_mechanism(Mechanism, Module, PasswordType) ->
%% end.

check_credentials(_State, Props) ->
User = proplists:get_value(username, Props, <<>>),
User = proplists:get_value(authzid, Props, <<>>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing parameters name: Doesn't it break other form of authentication ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested both internal auth and LDAP auth, along with anonymous using this change and didn't have any problems.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's strange that changing parameter name has no impact :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. These parameters are normally the same.

@mremond
Copy link
Member

mremond commented Mar 1, 2016

I think the main problem is that patch is big and may break some other form of authentication. The second problem is that the ejabberd community does probably not know how to test the patch to check it is working, even less how to maintain it and fix bugs in case of issues.

We want to consider it, but we would need to understand and master it. We had never heard of MojoAuthentication before.

@benlangfeld
Copy link
Contributor Author

We had never heard of MojoAuthentication before.

Even if the PR this is based on could be merged, that would be something. That PR does not introduce MojoAuth, but simply makes ejabberd more SASL compatible by allowing authentication modules to differentiate between authcid and authzid.

@mremond
Copy link
Member

mremond commented Mar 1, 2016

ok, we will try to compare with the SASL part. However, probably considering integration would probably be in two parts: API changes on ejabberd core and mojo authentication module itself possibly at first in ejabberd contribs.

@alexeyshch alexeyshch self-assigned this Mar 1, 2016
@benlangfeld
Copy link
Contributor Author

probably considering integration would probably be in two parts

👍

@benlangfeld
Copy link
Contributor Author

Rebased for an eventual merge. In the meantime I'll prep a PR to get this into ejabberd-contrib until such time as it's eligible for core.

Permits use of MojoAuth (http://mojoauth.mojolingo.com/) in ejabberd. MojoAuth is a set of standard approaches to cross-app authentication based on HMAC which is specified in RFC2104.
@mremond
Copy link
Member

mremond commented Mar 25, 2016

@benlangfeld Maybe this helps ?
46568fb

@benlangfeld
Copy link
Contributor Author

I saw that, thank you to everyone involved in getting that merged @mremond :)

@Ashaman-
Copy link

@benlangfeld Is there any ETA on MojoAuth? I'm sure I'm not the only one that hates having to choose between SIP/TURN authentication and SCRAM passwords...

@Neustradamus
Copy link
Contributor

@benlangfeld Always possible for 18.12+?

@benlangfeld
Copy link
Contributor Author

I don’t have a need for this right now, but if anyone else has the time to push it across the finish line, be my guest.

@cromain
Copy link
Contributor

cromain commented Dec 20, 2018

If needed for faster inclusion, it can be applied to ejabberd-contrib instead.

@Neustradamus
Copy link
Contributor

@badlop: It can be added in ejabberd-contrib?

@badlop
Copy link
Member

badlop commented Dec 28, 2020

It can be added in ejabberd-contrib?

If it works correctly with recent ejabberd, yes, it can be added.

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

Successfully merging this pull request may close these issues.

7 participants