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

Wrong FORM_TYPE for IBR form with CAPTCHA #3045

Closed
linkmauve opened this issue Sep 29, 2019 · 9 comments
Closed

Wrong FORM_TYPE for IBR form with CAPTCHA #3045

linkmauve opened this issue Sep 29, 2019 · 9 comments

Comments

@linkmauve
Copy link

Environment

  • ejabberd version: unknown
  • Erlang version: unknown
  • OS: unknown
  • Installed from: unknown

Configuration (only if needed): unknown

Errors from error.log/crash.log

No errors

Bug description

When doing in-band registration with some Ejabberd server with a CAPTCHA, such as creep.im or patchcord.be, the FORM_TYPE is urn:xmpp:captcha instead of jabber:iq:registration like it should be as defined by XEP-0077 and extended by XEP-0158.

@zinid zinid added this to the ejabberd 19.11 milestone Oct 1, 2019
@badlop
Copy link
Member

badlop commented Oct 3, 2019

Maybe I'm missing something... What you consider a bug is in fact described in example 11 from https://xmpp.org/extensions/xep-0158.html#register , and is similarly provided by ejabberd 19.09:

<iq xmlns="jabber:client"
    to="localhost"
    type="get"
    id="4620ac0b-09a7-4c02-92bf-573652f5cdb5">
  <query xmlns="jabber:iq:register" />
</iq>

<iq xml:lang='ca'
    from='localhost'
    type='result'
    id='4620ac0b-09a7-4c02-92bf-573652f5cdb5'>
  <query xmlns='jabber:iq:register'>
    <x type='form'
       xmlns='jabber:x:data'>
      <instructions>Tria nom d'usuari i contrasenya per a
      registrar-te en aquest servidor</instructions>
      <field var='FORM_TYPE'
             type='hidden'>
        <value>urn:xmpp:captcha</value>
      </field>
      <field var='username'
             type='text-single'
             label='Usuari'>
        <required />
      </field>
      ...
    </x>
  </query>
</iq>

@zinid
Copy link
Contributor

zinid commented Oct 9, 2019

I close the ticket because ejabberd respects namespace requirements from XEP-0158. If there are any discrepancies in the XEPs, this should be addressed at the XSF.

@zinid zinid closed this as completed Oct 9, 2019
@linkmauve
Copy link
Author

linkmauve commented Nov 7, 2019

This is being addressed in xsf/xeps#852, the example was obviously wrong.

It is now part of XEP-0158 version 1.0.1, you can reopen this issue.

@Neustradamus
Copy link
Contributor

@processone, @mremond, @zinid: Can you reopen it?

@badlop
Copy link
Member

badlop commented Nov 15, 2019

Oh, now that XEP-0158 changed, ejabberd no longer respects it. And I wonder how this change should be implemented:

@zinid
Copy link
Contributor

zinid commented Nov 25, 2019

I don't see CAPTCHA related fields registered in 'jabber:iq:register' form type: https://xmpp.org/extensions/xep-0077.html#registrar-formtypes
So putting 'jabber:iq:register' in 'FORM_TYPE' is also wrong.

@lovetox
Copy link

lovetox commented Nov 25, 2019

i think this XEP is a bit confusing, the registrated fields are not the fields for the dataform, these are the fields we can expect within the jabber:iq:register namespace, means

<query xmlns="jabber:iq:register">
  <username/>
  <password/>
</query>

The only rules which apply to dataforms are written down under https://xmpp.org/extensions/xep-0077.html#extensibility

Reading this, we can extend the dataform anyway we like.
To be fair the FORM_TYPE is only mentioned in examples, but its obvious that this should be a well known value, otherwise there would be no way to discover if a form is a register form or not.

There is another ejabberd specific component to this.
Ejabberd includes captcha fallback fields for clients who dont support captcha.
Now you cant expect clients that don't support captcha to search for a form of type urn:xmpp:captcha, rather they will always look for jabber:iq:register as 0077 shows in its examples

@lovetox
Copy link

lovetox commented Oct 18, 2020

Any chance this can be looked at? the current impl needs clients to specifically support ejabberds impl

@badlop
Copy link
Member

badlop commented Oct 20, 2020

The easiest solution that I could find is the third option that I mentioned in #3045 (comment)

That proposed patch is in badlop@49431b5

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

No branches or pull requests

5 participants