Projects tigase _server server-core Issues #1142
After registration inform the client that the account activation (email) is required (#1142)
Closed
wojciech.kapcia@tigase.net opened 4 years ago

Currently, after registration, if the e-mail confirmation/activation is required, there is not programmatic way to know it, which can cause in some clients undesirable states (constant error about connection with "policy violation" because account is inactive, or disabling the account).

Objective: include additional marker indicating that the further action is required to use the account.


Include also information in auth result that verification is required. Most likely use account-disabled with additional text information, eg. for required confirmation but can be adjusted for disabled accounts:

<failure xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>
    <account-disabled/>
    <text xml:lang='en'>Account needs to be confirmed -- please check your email</text>
</failure>
wojciech.kapcia@tigase.net commented 4 years ago

@andrzej.wojcik let's move that (and related mobile client's tasks) to 8.2.0

wojciech.kapcia@tigase.net commented 3 years ago

I added the changes to inform the client about the conditions of the failed authentication. <email/> field was already marked as required which combined with better sasl-error should be enough for the clients to properly handle the situation. I also was pondering returning query with instruction and custom element <email-confirmation-required/> upon successful registration, but I'm not sure if client would handle it correctly (https://github.com/tigase/tigase-server/pull/122/files#diff-4647cde966d78e616f28e0d8edb7854dac9d2020c9f8fc154d222dede5e25cc2R650) but we can enable it if that would also help with handling it in Beagle/Siskin.

@andrzej.wojcik could you check and comment on the changes?

Andrzej Wójcik (Tigase) commented 3 years ago

I've added my comments in the review on GitHub.

wojciech.kapcia@tigase.net commented 3 years ago

After reviving nightly packages generation it turns out that Bruteforce tests are failing:

  • tigase.tests.server.TestBruteforcePrevention: testDisableUser

Probably due to changes in error returned:

2021-10-27 22:16:17 |  >> <failure xmlns="urn:ietf:params:xml:ns:xmpp-sasl"><account-disabled/><text xml:lang="en">Your account has been disabled, please contact the administration</text></failure>
2021-10-27 22:16:17 | user :: user_rnawgm0027@test-domain.com << <failure xmlns="urn:ietf:params:xml:ns:xmpp-sasl"><account-disabled/><text xml:lang="en">Your account has been disabled, please contact the administration</text></failure>

ST:

java.lang.AssertionError
tigase.tests.utils.JaxmppBuilder.build(JaxmppBuilder.java:102)
tigase.tests.server.TestBruteforcePrevention.testDisableUser(TestBruteforcePrevention.java:158)
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
java.base/java.lang.reflect.Method.invoke(Method.java:566)
org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
org.testng.TestRunner.privateRun(TestRunner.java:764)
org.testng.TestRunner.run(TestRunner.java:585)
org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
org.testng.SuiteRunner.run(SuiteRunner.java:286)
org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
org.testng.TestNG.runSuites(TestNG.java:1069)
org.testng.TestNG.run(TestNG.java:1037)
org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:283)
org.apache.maven.surefire.testng.TestNGXmlTestSuite.execute(TestNGXmlTestSuite.java:75)
org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:120)
org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
wojciech.kapcia@tigase.net commented 3 years ago

Adding new SASL error conditions in JaXMPP solved the issue (proper exception was generated).

@bmalkow JaXMPP is probably legacy at this point but if StorkIM depends on it (and it's behaviour) the most likely bumping dependency version in stork would be advised.

Please wait...
Page is in error, reload to recover