Skip to content
This repository was archived by the owner on Dec 12, 2018. It is now read-only.

Commit f699791

Browse files
dogeareddogeared
authored
Merge pull request #1235 from stormpath/issue/1233
1233 Respect createChallenge field when creating Google Authenticator factors.
2 parents a066be8 + deeb872 commit f699791

File tree

8 files changed

+135
-106
lines changed

8 files changed

+135
-106
lines changed

‎api/src/main/java/com/stormpath/sdk/factor/CreateFactorRequest.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@
1818
/**
1919
* Represents an attempt to create a new {@link com.stormpath.sdk.factor.Factor} record in Stormpath.
2020
*
21-
* @see com.stormpath.sdk.account.Account#createFactor(CreateFactorRequest)
22-
*
2321
* @param <T> a subclass of {@link Factor} specifying the kind of {@code Factor} created by this {@code CreateFactorRequest}.
2422
* @param <O> a subclass of {@link FactorOptions} specifying the kind of {@code FactorOptions} to be used for creating a {@link Factor}.
23+
* @see com.stormpath.sdk.account.Account#createFactor(CreateFactorRequest)
2524
* @since 1.1.0
2625
*/
27-
public interface CreateFactorRequest<T extends Factor, O extends FactorOptions>{
26+
public interface CreateFactorRequest<T extends Factor, O extends FactorOptions> {
2827

2928
/**
3029
* Returns the Factor instance for which a new record will be created in Stormpath.
@@ -43,7 +42,16 @@ public interface CreateFactorRequest<T extends Factor, O extends FactorOptions>{
4342
/**
4443
* Returns the {@link FactorOptions}.
4544
*
46-
* @return {@link FactorOptions}.
45+
* @return {@link FactorOptions}.
4746
*/
4847
O getFactorOptions() throws IllegalStateException;
48+
49+
/**
50+
* Returns true in case Factor should be challenged upon creation.
51+
* In which case a challenge resource is also created.
52+
*
53+
* @return true in case Factor should be challenged upon creation..
54+
*/
55+
boolean isCreateChallenge();
56+
4957
}

‎api/src/main/java/com/stormpath/sdk/factor/sms/CreateSmsFactorRequest.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,5 @@
2323
* @see com.stormpath.sdk.account.Account#createFactor(CreateFactorRequest)
2424
* @since 1.1.0
2525
*/
26-
public interface CreateSmsFactorRequest<T extends SmsFactor, O extends SmsFactorOptions> extends CreateFactorRequest<T,O>{
27-
/**
28-
* Returns true in case Factor should be challenged upon creation.
29-
* In which case a challenge resource is also created.
30-
*
31-
* @return rtue in case Factor should be challenged upon creation..
32-
*/
33-
boolean isCreateChallenge();
26+
public interface CreateSmsFactorRequest<T extends SmsFactor, O extends SmsFactorOptions> extends CreateFactorRequest<T, O> {
3427
}

‎extensions/httpclient/src/test/groovy/com/stormpath/sdk/impl/factor/GoogleAuthenticatorFactorIT.groovy‎

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ import static org.testng.Assert.assertTrue
3636
/**
3737
* @since 1.1.0
3838
*/
39-
class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
39+
class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT {
4040

4141
@Test
4242
void testQueryMultipleFactorsWithPotentiallyMissingProperties() {
4343
Directory dir = client.instantiate(Directory)
44-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
44+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
4545
dir = client.currentTenant.createDirectory(dir);
4646
deleteOnTeardown(dir)
4747
Account account = createTempAccountInDir(dir)
@@ -79,7 +79,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
7979
@Test
8080
void testCreateAndGetFactorWithNulIssuerAndNullAccountName() {
8181
Directory dir = client.instantiate(Directory)
82-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
82+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
8383
dir = client.currentTenant.createDirectory(dir);
8484
deleteOnTeardown(dir)
8585
Account account = createTempAccountInDir(dir)
@@ -99,7 +99,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
9999
@Test
100100
void testCreateAndGetFactorWithNullIssuerAndNoAccountName() {
101101
Directory dir = client.instantiate(Directory)
102-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
102+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
103103
dir = client.currentTenant.createDirectory(dir);
104104
deleteOnTeardown(dir)
105105
Account account = createTempAccountInDir(dir)
@@ -116,7 +116,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
116116
@Test
117117
void testCreateAndGetFactorWithIssuerAndNullAccountName() {
118118
Directory dir = client.instantiate(Directory)
119-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
119+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
120120
dir = client.currentTenant.createDirectory(dir);
121121
deleteOnTeardown(dir)
122122
Account account = createTempAccountInDir(dir)
@@ -137,7 +137,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
137137
@Test
138138
void testCreateAndGetFactorWithIssuerAndNoAccountName() {
139139
Directory dir = client.instantiate(Directory)
140-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
140+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
141141
dir = client.currentTenant.createDirectory(dir);
142142
deleteOnTeardown(dir)
143143
Account account = createTempAccountInDir(dir)
@@ -155,7 +155,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
155155
@Test
156156
void testCreateAndGetFactorWithInvalidIssuer() {
157157
Directory dir = client.instantiate(Directory)
158-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
158+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
159159
dir = client.currentTenant.createDirectory(dir);
160160
deleteOnTeardown(dir)
161161
Account account = createTempAccountInDir(dir)
@@ -177,7 +177,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
177177
@Test
178178
void testCreateAndGetFactorWithDifficultIssuer() {
179179
Directory dir = client.instantiate(Directory)
180-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
180+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
181181
dir = client.currentTenant.createDirectory(dir);
182182
deleteOnTeardown(dir)
183183
Account account = createTempAccountInDir(dir)
@@ -194,7 +194,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
194194
@Test
195195
void testCreateAndGetFactorWithInvalidAccountName() {
196196
Directory dir = client.instantiate(Directory)
197-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
197+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
198198
dir = client.currentTenant.createDirectory(dir);
199199
deleteOnTeardown(dir)
200200
Account account = createTempAccountInDir(dir)
@@ -215,7 +215,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
215215
@Test
216216
void testCreateAndGetFactorWithDifficultAccountName() {
217217
Directory dir = client.instantiate(Directory)
218-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
218+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
219219
dir = client.currentTenant.createDirectory(dir);
220220
deleteOnTeardown(dir)
221221
Account account = createTempAccountInDir(dir)
@@ -231,7 +231,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
231231
@Test
232232
void testCreateAndGetFactorWithAccountNameAndNullIssuer() {
233233
Directory dir = client.instantiate(Directory)
234-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
234+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
235235
dir = client.currentTenant.createDirectory(dir);
236236
deleteOnTeardown(dir)
237237
Account account = createTempAccountInDir(dir)
@@ -247,7 +247,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
247247
@Test
248248
void testCreateAndGetFactorWithBothIssuerAndAccountName() {
249249
Directory dir = client.instantiate(Directory)
250-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
250+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
251251
dir = client.currentTenant.createDirectory(dir);
252252
deleteOnTeardown(dir)
253253
Account account = createTempAccountInDir(dir)
@@ -261,10 +261,27 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
261261
assertGoogleAuthenticatorFactorFields(factor, randomIssuer, randomAccountName)
262262
}
263263

264+
@Test
265+
void testChallengeOnFactorCreation() {
266+
Directory dir = client.instantiate(Directory)
267+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
268+
dir = client.currentTenant.createDirectory(dir);
269+
deleteOnTeardown(dir)
270+
Account account = createTempAccountInDir(dir)
271+
272+
def randomAccountName = uniquify("Random AccountName")
273+
def randomIssuer = uniquify("Random Issuer")
274+
def factor = createGoogleAuthenticatorFactor(account, randomIssuer, randomAccountName, true, true)
275+
assertGoogleAuthenticatorFactorFields(factor, randomIssuer, randomAccountName, true, true)
276+
277+
factor = client.getResource(factor.getHref(), GoogleAuthenticatorFactor.class)
278+
assertGoogleAuthenticatorFactorFields(factor, randomIssuer, randomAccountName, true, true)
279+
}
280+
264281
@Test
265282
void testSearchForFactor() {
266283
Directory dir = client.instantiate(Directory)
267-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
284+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
268285
dir = client.currentTenant.createDirectory(dir);
269286
deleteOnTeardown(dir)
270287
Account account = createTempAccountInDir(dir)
@@ -273,22 +290,22 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
273290
FactorList factors = account.getFactors()
274291
assertEquals(factors.size, 2)
275292

276-
factors = account.getFactors(["type":"google-authenticator"])
293+
factors = account.getFactors(["type": "google-authenticator"])
277294
assertEquals(factors.size, 1)
278295

279-
factors = account.getFactors(["verificationStatus":"UNVERIFIED"])
296+
factors = account.getFactors(["verificationStatus": "UNVERIFIED"])
280297
assertEquals(factors.size, 2)
281298

282-
factors = account.getFactors(["issuer":"issuer"])
299+
factors = account.getFactors(["issuer": "issuer"])
283300
assertEquals(factors.size, 1)
284301

285-
factors = account.getFactors(["issuer":"iss*"])
302+
factors = account.getFactors(["issuer": "iss*"])
286303
assertEquals(factors.size, 1)
287304

288-
factors = account.getFactors(["accountName":"accountName*"])
305+
factors = account.getFactors(["accountName": "accountName*"])
289306
assertEquals(factors.size, 1)
290307

291-
factors = account.getFactors(["accountName":"account*"])
308+
factors = account.getFactors(["accountName": "account*"])
292309
assertEquals(factors.size, 1)
293310
}
294311

@@ -298,7 +315,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
298315
// todo: Enable this test once the issue is fixed
299316
void testDeleteFactorDeletesChallenges() {
300317
Directory dir = client.instantiate(Directory)
301-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
318+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
302319
dir = client.currentTenant.createDirectory(dir);
303320
deleteOnTeardown(dir)
304321
Account account = createTempAccountInDir(dir)
@@ -316,7 +333,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
316333

317334
factor.delete()
318335

319-
for(def currentChallenge : challenges) {
336+
for (def currentChallenge : challenges) {
320337
Throwable e = null
321338
try {
322339
client.getResource(currentChallenge.href, GoogleAuthenticatorChallenge.class)
@@ -338,7 +355,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
338355
// todo: Enable this test once the issue is fixed
339356
void testDeleteAccountDeletesFactorsAndChallenges() {
340357
Directory dir = client.instantiate(Directory)
341-
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object(){}.getClass().getEnclosingMethod().getName()}")
358+
dir.name = uniquify("Java SDK: ${this.getClass().getSimpleName()}.${new Object() {}.getClass().getEnclosingMethod().getName()}")
342359
dir = client.currentTenant.createDirectory(dir);
343360
deleteOnTeardown(dir)
344361
Account account = createTempAccountInDir(dir)
@@ -368,7 +385,7 @@ class GoogleAuthenticatorFactorIT extends AbstractMultiFactorIT{
368385
}
369386
assertTrue(e instanceof ResourceException)
370387

371-
for(def currentChallenge : challenges) {
388+
for (def currentChallenge : challenges) {
372389
e = null
373390
try {
374391
client.getResource(currentChallenge.href, GoogleAuthenticatorChallenge.class)

‎extensions/httpclient/src/test/groovy/com/stormpath/sdk/impl/multifactor/AbstractMultiFactorIT.groovy‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import com.stormpath.sdk.client.ClientIT
2727
import com.stormpath.sdk.factor.FactorStatus
2828
import com.stormpath.sdk.factor.FactorType
2929
import com.stormpath.sdk.factor.FactorVerificationStatus
30+
import com.stormpath.sdk.factor.Factors
3031
import com.stormpath.sdk.factor.google.GoogleAuthenticatorFactor
3132
import com.stormpath.sdk.factor.sms.SmsFactor
3233
import com.stormpath.sdk.phone.Phone
@@ -41,38 +42,35 @@ import static org.testng.Assert.assertNull
4142
/**
4243
* @since 1.1.0
4344
*/
44-
abstract class AbstractMultiFactorIT extends ClientIT{
45+
abstract class AbstractMultiFactorIT extends ClientIT {
4546

4647
protected static final String VALID_PHONE_NUMBER = "+15005550006"
4748
protected static final String INVALID_PHONE_NUMBER = "+15005550001"
4849

49-
protected void assertGoogleAuthenticatorFactorFields(GoogleAuthenticatorFactor factor, String expectedIssuer = null, String expectedAccountName = null, boolean enabled = true) {
50+
protected void assertGoogleAuthenticatorFactorFields(GoogleAuthenticatorFactor factor, String expectedIssuer = null, String expectedAccountName = null, boolean enabled = true, boolean createChallenge = false) {
5051
assertNotNull factor.href
5152
assertEquals(factor.type, FactorType.GOOGLE_AUTHENTICATOR)
5253
assertEquals(factor.factorVerificationStatus, FactorVerificationStatus.UNVERIFIED)
53-
if(enabled) {
54+
if (enabled) {
5455
assertEquals(factor.status, FactorStatus.ENABLED)
55-
}
56-
else{
56+
} else {
5757
assertEquals(factor.status, FactorStatus.DISABLED)
5858
}
5959

6060
def actualAccountName = factor.accountName
6161

6262
if (expectedAccountName == null) {
6363
assertNull(actualAccountName)
64-
}
65-
else {
64+
} else {
6665
assertEquals(actualAccountName, expectedAccountName)
6766
}
6867

6968
def actualIssuer = factor.issuer
7069

7170
if (expectedIssuer == null) {
7271
assertNull(actualIssuer)
73-
}
74-
else {
75-
assertEquals(actualIssuer,expectedIssuer)
72+
} else {
73+
assertEquals(actualIssuer, expectedIssuer)
7674
}
7775

7876
assertNotNull(factor.secret)
@@ -105,7 +103,12 @@ abstract class AbstractMultiFactorIT extends ClientIT{
105103
assertNotNull(factor.getAccount())
106104
assertNotNull(factor.getAccount().href)
107105

108-
assertNull(factor.getMostRecentChallenge())
106+
if (createChallenge) {
107+
assertNotNull(factor.getMostRecentChallenge())
108+
assertNotNull(factor.getMostRecentChallenge().getHref())
109+
} else {
110+
assertNull(factor.getMostRecentChallenge())
111+
}
109112

110113
assertNotNull(factor.getChallenges())
111114
assertNotNull(factor.getChallenges().href)
@@ -141,14 +144,20 @@ abstract class AbstractMultiFactorIT extends ClientIT{
141144
assertNotNull(challenge.account.href)
142145
}
143146

144-
protected GoogleAuthenticatorFactor createGoogleAuthenticatorFactor(Account account, String issuer = null, String accountName = null, boolean enabled = true) {
145-
def factor = client.instantiate(GoogleAuthenticatorFactor.class)
147+
protected GoogleAuthenticatorFactor createGoogleAuthenticatorFactor(Account account, String issuer = null, String accountName = null, boolean enabled = true, boolean createChallenge = false) {
148+
GoogleAuthenticatorFactor factor = client.instantiate(GoogleAuthenticatorFactor.class)
146149
factor.accountName = accountName
147150
factor.issuer = issuer
148-
if(!enabled){
151+
if (!enabled) {
149152
factor.status = FactorStatus.DISABLED
150153
}
151-
factor = account.createFactor(factor)
154+
155+
def builder = Factors.GOOGLE_AUTHENTICATOR.newCreateRequestFor(factor)
156+
if (createChallenge) {
157+
builder = builder.createChallenge()
158+
}
159+
160+
factor = account.createFactor(builder.build())
152161
factor
153162
}
154163

@@ -162,7 +171,7 @@ abstract class AbstractMultiFactorIT extends ClientIT{
162171
factor
163172
}
164173

165-
protected Account createGoogleAuthenticatorFactorAndSmsFactor(String issuer = null, String accountName = null, Account account){
174+
protected Account createGoogleAuthenticatorFactorAndSmsFactor(String issuer = null, String accountName = null, Account account) {
166175
createSmsFactor(account)
167176
createGoogleAuthenticatorFactor(account, issuer, accountName)
168177
account

0 commit comments

Comments
 (0)