Skip to content

Commit e2fe377

Browse files
golbigolbi
committed
UY-1150 fix for wrong credential reset code expiration maths, add tests
1 parent bc3c725 commit e2fe377

File tree

5 files changed

+176
-9
lines changed

5 files changed

+176
-9
lines changed

‎engine-api/src/main/java/pl/edu/icm/unity/engine/api/authn/EntityWithCredential.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ public class EntityWithCredential
1414
private String credentialValue;
1515
private long entityId;
1616

17+
public EntityWithCredential()
18+
{
19+
}
20+
21+
public EntityWithCredential(String credentialName, String credentialValue, long entityId)
22+
{
23+
this.credentialName = credentialName;
24+
this.credentialValue = credentialValue;
25+
this.entityId = entityId;
26+
}
1727
public String getCredentialName()
1828
{
1929
return credentialName;

‎std-plugins/src/main/java/pl/edu/icm/unity/stdext/credential/CredentialResetBase.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55
package pl.edu.icm.unity.stdext.credential;
66

7+
import java.time.Duration;
8+
import java.time.LocalDateTime;
79
import java.util.HashMap;
810
import java.util.Locale;
911
import java.util.Map;
@@ -39,7 +41,8 @@ public abstract class CredentialResetBase implements CredentialReset
3941
private static final Logger log = Log.getLogger(Log.U_SERVER_AUTHN, CredentialResetBase.class);
4042
protected static final int MAX_ANSWER_ATTEMPTS = 2;
4143
private static final int MAX_RESENDS = 3;
42-
private static final long MAX_CODE_VALIDITY = 30*3600;
44+
public static final Duration DEFAULT_MAX_CODE_VALIDITY = Duration.ofMinutes(30);
45+
private Duration maxCodeValidity;
4346

4447
private NotificationProducer notificationProducer;
4548
private IdentityResolver identityResolver;
@@ -52,7 +55,7 @@ public abstract class CredentialResetBase implements CredentialReset
5255
private ObjectNode completeCredentialConfiguration;
5356

5457
private String codeSent;
55-
private long codeValidityEnd;
58+
private LocalDateTime codeValidityEnd;
5659
private int dynamicAnswerAttempts = 0;
5760
private int codeSendingAttempts = 0;
5861
private AuthenticationSubject requestedSubject;
@@ -62,14 +65,16 @@ public CredentialResetBase(NotificationProducer notificationProducer,
6265
LocalCredentialVerificator localVerificator,
6366
CredentialHelper credentialHelper,
6467
String credentialId,
65-
ObjectNode completeCredentialConfiguration)
68+
ObjectNode completeCredentialConfiguration,
69+
Duration maxCodeValidity)
6670
{
6771
this.notificationProducer = notificationProducer;
6872
this.credentialHelper = credentialHelper;
6973
this.identityResolver = identityResolver;
7074
this.credentialId = credentialId;
7175
this.localCredentialHandler = localVerificator;
7276
this.completeCredentialConfiguration = completeCredentialConfiguration;
77+
this.maxCodeValidity = maxCodeValidity;
7378
}
7479

7580
public void setSubject(AuthenticationSubject subject, String[] idTypes)
@@ -135,7 +140,7 @@ private void createCode(boolean onlyNumberCode)
135140
{
136141
codeSent = CodeGenerator.generateNumberCode(codeLen);
137142
}
138-
codeValidityEnd = System.currentTimeMillis() + MAX_CODE_VALIDITY;
143+
codeValidityEnd = LocalDateTime.now().plus(maxCodeValidity);
139144
}
140145

141146
protected abstract int getCodeLength();
@@ -161,13 +166,18 @@ public void sendCode(String msgTemplate, boolean onlyNumberCode) throws EngineEx
161166
msgTemplate, params, locale, username, true);
162167
}
163168

169+
public String getSentCode()
170+
{
171+
return codeSent;
172+
}
173+
164174
@Override
165175
public void verifyDynamicData(String answer) throws WrongArgumentException, TooManyAttempts
166176
{
167177
if (dynamicAnswerAttempts >= MAX_ANSWER_ATTEMPTS)
168178
throw new TooManyAttempts();
169179
dynamicAnswerAttempts++;
170-
if (System.currentTimeMillis() > codeValidityEnd)
180+
if (LocalDateTime.now().isAfter(codeValidityEnd))
171181
throw new TooManyAttempts();
172182
if (codeSent == null || !codeSent.equals(answer))
173183
throw new WrongArgumentException("The code is invalid");

‎std-plugins/src/main/java/pl/edu/icm/unity/stdext/credential/pass/PasswordCredentialResetImpl.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
package pl.edu.icm.unity.stdext.credential.pass;
66

7+
import java.time.Duration;
78
import java.util.List;
89

910
import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -38,13 +39,27 @@ public PasswordCredentialResetImpl(NotificationProducer notificationProducer,
3839
CredentialHelper credentialHelper, String credentialId,
3940
ObjectNode completeCredentialConfiguration,
4041
PasswordCredentialResetSettings settings,
41-
PasswordEngine passwordEngine)
42+
PasswordEngine passwordEngine,
43+
Duration maxCodeValidity)
4244
{
4345
super(notificationProducer, identityResolver, localVerificator, credentialHelper,
44-
credentialId, completeCredentialConfiguration);
46+
credentialId, completeCredentialConfiguration, maxCodeValidity);
4547
this.settings = settings;
4648
this.passwordEngine = passwordEngine;
4749
}
50+
51+
public PasswordCredentialResetImpl(NotificationProducer notificationProducer,
52+
IdentityResolver identityResolver,
53+
LocalCredentialVerificator localVerificator,
54+
CredentialHelper credentialHelper, String credentialId,
55+
ObjectNode completeCredentialConfiguration,
56+
PasswordCredentialResetSettings settings,
57+
PasswordEngine passwordEngine)
58+
{
59+
this(notificationProducer, identityResolver, localVerificator, credentialHelper, credentialId,
60+
completeCredentialConfiguration, settings, passwordEngine,
61+
CredentialResetBase.DEFAULT_MAX_CODE_VALIDITY);
62+
}
4863

4964
@Override
5065
protected String getCredentialSettings()

‎std-plugins/src/main/java/pl/edu/icm/unity/stdext/credential/sms/SMSCredentialResetImpl.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55
package pl.edu.icm.unity.stdext.credential.sms;
66

7+
import java.time.Duration;
8+
79
import com.fasterxml.jackson.databind.node.ObjectNode;
810

911
import pl.edu.icm.unity.Constants;
@@ -30,11 +32,26 @@ public SMSCredentialResetImpl(NotificationProducer notificationProducer,
3032
CredentialHelper credentialHelper,
3133
String credentialId,
3234
ObjectNode completeCredentialConfiguration,
33-
SMSCredentialRecoverySettings settings)
35+
SMSCredentialRecoverySettings settings,
36+
Duration maxCodeValidity)
3437
{
35-
super(notificationProducer, identityResolver, localVerificator, credentialHelper, credentialId, completeCredentialConfiguration);
38+
super(notificationProducer, identityResolver, localVerificator, credentialHelper, credentialId,
39+
completeCredentialConfiguration, maxCodeValidity);
3640
this.settings = settings;
3741
}
42+
43+
public SMSCredentialResetImpl(NotificationProducer notificationProducer,
44+
IdentityResolver identityResolver,
45+
LocalCredentialVerificator localVerificator,
46+
CredentialHelper credentialHelper,
47+
String credentialId,
48+
ObjectNode completeCredentialConfiguration,
49+
SMSCredentialRecoverySettings settings)
50+
{
51+
this(notificationProducer, identityResolver, localVerificator, credentialHelper, credentialId,
52+
completeCredentialConfiguration, settings, CredentialResetBase.DEFAULT_MAX_CODE_VALIDITY);
53+
}
54+
3855

3956
@Override
4057
protected String getCredentialSettings()

‎std-plugins/src/test/java/pl/edu/icm/unity/stdext/credential/pass/PasswordCredentialResetImplTest.java

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* Copyright (c) 2021 Bixbit - Krzysztof Benedyczak. All rights reserved.
3+
* See LICENCE.txt file for licensing information.
4+
*/
5+
package pl.edu.icm.unity.stdext.credential.pass;
6+
7+
import static org.assertj.core.api.Assertions.assertThat;
8+
import static org.assertj.core.api.Assertions.catchThrowable;
9+
import static org.mockito.ArgumentMatchers.any;
10+
import static org.mockito.ArgumentMatchers.eq;
11+
import static org.mockito.Mockito.mock;
12+
import static org.mockito.Mockito.when;
13+
14+
import java.time.Duration;
15+
16+
import org.junit.Test;
17+
18+
import pl.edu.icm.unity.engine.api.authn.AuthenticationSubject;
19+
import pl.edu.icm.unity.engine.api.authn.EntityWithCredential;
20+
import pl.edu.icm.unity.engine.api.identity.IdentityResolver;
21+
import pl.edu.icm.unity.engine.api.notification.NotificationProducer;
22+
import pl.edu.icm.unity.exceptions.EngineException;
23+
import pl.edu.icm.unity.exceptions.IllegalGroupValueException;
24+
import pl.edu.icm.unity.exceptions.IllegalIdentityValueException;
25+
import pl.edu.icm.unity.exceptions.IllegalTypeException;
26+
import pl.edu.icm.unity.exceptions.TooManyAttempts;
27+
import pl.edu.icm.unity.exceptions.WrongArgumentException;
28+
29+
public class PasswordCredentialResetImplTest
30+
{
31+
private AuthenticationSubject subject = AuthenticationSubject.identityBased("identity");
32+
private NotificationProducer notificationProducer;
33+
private IdentityResolver identityResolver;
34+
35+
@Test
36+
public void shouldAcceptCorrectCode() throws EngineException
37+
{
38+
initMocks();
39+
PasswordCredentialResetImpl resetImpl = new PasswordCredentialResetImpl(notificationProducer,
40+
identityResolver,
41+
null,
42+
null,
43+
"credentialId",
44+
null,
45+
mock(PasswordCredentialResetSettings.class),
46+
null);
47+
resetImpl.setSubject(subject);
48+
resetImpl.sendCode("msgTemplate", false);
49+
50+
String sentCode = resetImpl.getSentCode();
51+
assertThat(sentCode).isNotNull();
52+
53+
resetImpl.verifyDynamicData(sentCode);
54+
}
55+
56+
@Test
57+
public void shouldNotAcceptIncorrectCode() throws EngineException
58+
{
59+
initMocks();
60+
PasswordCredentialResetImpl resetImpl = new PasswordCredentialResetImpl(notificationProducer,
61+
identityResolver,
62+
null,
63+
null,
64+
"credentialId",
65+
null,
66+
mock(PasswordCredentialResetSettings.class),
67+
null);
68+
resetImpl.setSubject(subject);
69+
resetImpl.sendCode("msgTemplate", false);
70+
71+
String sentCode = resetImpl.getSentCode();
72+
assertThat(sentCode).isNotNull();
73+
74+
Throwable error = catchThrowable(() -> resetImpl.verifyDynamicData(sentCode + "foo"));
75+
76+
assertThat(error).isNotNull().isInstanceOf(WrongArgumentException.class);
77+
}
78+
79+
@Test
80+
public void shouldNotAcceptExpiredCode() throws EngineException, InterruptedException
81+
{
82+
initMocks();
83+
PasswordCredentialResetImpl resetImpl = new PasswordCredentialResetImpl(notificationProducer,
84+
identityResolver,
85+
null,
86+
null,
87+
"credentialId",
88+
null,
89+
mock(PasswordCredentialResetSettings.class),
90+
null,
91+
Duration.ofMillis(10));
92+
resetImpl.setSubject(subject);
93+
resetImpl.sendCode("msgTemplate", false);
94+
95+
String sentCode = resetImpl.getSentCode();
96+
assertThat(sentCode).isNotNull();
97+
98+
Thread.sleep(11);
99+
100+
Throwable error = catchThrowable(() -> resetImpl.verifyDynamicData(sentCode));
101+
102+
assertThat(error).isNotNull().isInstanceOf(TooManyAttempts.class);
103+
}
104+
105+
private void initMocks() throws IllegalIdentityValueException, IllegalTypeException, IllegalGroupValueException,
106+
EngineException
107+
{
108+
notificationProducer = mock(NotificationProducer.class);
109+
identityResolver = mock(IdentityResolver.class);
110+
when(identityResolver.resolveSubject(eq(subject), any(), eq("credentialId"))).thenReturn(
111+
new EntityWithCredential("credentialId", "credVal", 123));
112+
}
113+
114+
}
115+

0 commit comments

Comments
 (0)