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

Commit 9c61c0c

Browse files
authored
Handle errors when calling Okta to get user groups (#1068)
Co-authored-by: Shawn Sherwood <shawn-sher@users.noreply.github.com>
1 parent 7db244b commit 9c61c0c

4 files changed

Lines changed: 190 additions & 2 deletions

File tree

cerberus-auth-connector-okta/src/main/java/com/nike/cerberus/auth/connector/okta/OktaAuthConnector.java

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@
3838
import com.okta.sdk.authc.credentials.TokenClientCredentials;
3939
import com.okta.sdk.client.Client;
4040
import com.okta.sdk.client.Clients;
41+
import com.okta.sdk.resource.ResourceException;
42+
import com.okta.sdk.resource.group.Group;
4143
import com.okta.sdk.resource.group.GroupList;
44+
import com.okta.sdk.resource.group.GroupProfile;
4245
import com.okta.sdk.resource.user.User;
4346
import java.util.HashSet;
4447
import java.util.Map;
@@ -217,20 +220,60 @@ public AuthResponse mfaCheck(String stateToken, String deviceId, String otpToken
217220
}
218221
}
219222

223+
/**
224+
* Get a valid user from the identity provider if possible
225+
*
226+
* @param userId
227+
* @return User corresponding to the id
228+
* @throws ApiException if user cannot be resolved
229+
*/
230+
protected User getUserFromIDP(String userId) {
231+
try {
232+
return sdkClient.getUser(userId);
233+
} catch (IllegalStateException ise) {
234+
throw ApiException.newBuilder()
235+
.withExceptionCause(ise)
236+
.withApiErrors(DefaultApiError.IDENTITY_PROVIDER_BAD_GATEWAY)
237+
.withExceptionMessage("Could not communicate properly with identity provider")
238+
.build();
239+
} catch (ResourceException rexc) {
240+
String msg =
241+
String.format("Got invalid response from identity providers: %s", rexc.getMessage());
242+
throw ApiException.newBuilder()
243+
.withExceptionCause(rexc)
244+
.withApiErrors(DefaultApiError.IDENTITY_PROVIDER_BAD_GATEWAY)
245+
.withExceptionMessage(msg)
246+
.build();
247+
} catch (Exception exc) {
248+
throw ApiException.newBuilder()
249+
.withExceptionCause(exc)
250+
.withApiErrors(DefaultApiError.INTERNAL_SERVER_ERROR)
251+
.withExceptionMessage("Unknown error trying to getUser from identity provider")
252+
.build();
253+
}
254+
}
255+
220256
/** Obtains groups user belongs to. */
221257
@Override
222258
public Set<String> getGroups(AuthData authData) {
223259

224260
Preconditions.checkNotNull(authData, "auth data cannot be null.");
225261

226-
User user = sdkClient.getUser(authData.getUserId());
262+
String userId = authData.getUserId();
263+
User user = getUserFromIDP(userId);
227264
GroupList userGroups = user.listGroups();
228265

229266
final Set<String> groups = new HashSet<>();
230267
if (userGroups == null) {
231268
return groups;
232269
}
233-
userGroups.forEach(group -> groups.add(group.getProfile().getName()));
270+
271+
for (Group group : userGroups) {
272+
GroupProfile profile = group.getProfile();
273+
if (profile != null) {
274+
groups.add(profile.getName());
275+
}
276+
}
234277

235278
return groups;
236279
}

cerberus-auth-connector-okta/src/test/java/com/nike/cerberus/auth/connector/okta/OktaAuthConnectorTest.java

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,34 @@
2323
import static org.mockito.Mockito.*;
2424
import static org.mockito.MockitoAnnotations.initMocks;
2525

26+
import com.google.common.collect.ImmutableMap;
27+
import com.google.common.collect.Lists;
28+
import com.nike.backstopper.apierror.ApiError;
2629
import com.nike.backstopper.exception.ApiException;
2730
import com.nike.cerberus.auth.connector.AuthData;
2831
import com.nike.cerberus.auth.connector.AuthResponse;
2932
import com.nike.cerberus.auth.connector.AuthStatus;
3033
import com.nike.cerberus.auth.connector.okta.statehandlers.InitialLoginStateHandler;
3134
import com.nike.cerberus.auth.connector.okta.statehandlers.MfaStateHandler;
35+
import com.nike.cerberus.error.DefaultApiError;
3236
import com.okta.authn.sdk.client.AuthenticationClient;
3337
import com.okta.authn.sdk.impl.resource.DefaultVerifyPassCodeFactorRequest;
3438
import com.okta.jwt.AccessTokenVerifier;
3539
import com.okta.jwt.Jwt;
3640
import com.okta.jwt.JwtVerificationException;
3741
import com.okta.sdk.client.Client;
42+
import com.okta.sdk.impl.client.DefaultClient;
43+
import com.okta.sdk.impl.error.DefaultError;
44+
import com.okta.sdk.resource.ResourceException;
45+
import com.okta.sdk.resource.group.Group;
46+
import com.okta.sdk.resource.group.GroupList;
47+
import com.okta.sdk.resource.group.GroupProfile;
48+
import com.okta.sdk.resource.user.User;
3849
import java.util.HashMap;
50+
import java.util.HashSet;
51+
import java.util.List;
3952
import java.util.Map;
53+
import java.util.Set;
4054
import org.junit.Before;
4155
import org.junit.Test;
4256
import org.mockito.Mock;
@@ -327,4 +341,130 @@ public void testGetAccessTokenVerifier() {
327341
AccessTokenVerifier verifier = this.oktaAuthConnector.getAccessTokenVerifier();
328342
assertNotNull(verifier);
329343
}
344+
345+
@Test
346+
public void testGetGroups() {
347+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
348+
349+
GroupProfile groupProfile = mock(GroupProfile.class);
350+
when(groupProfile.getName()).thenReturn("testGroup");
351+
352+
Group fakeGroup = mock(Group.class);
353+
when(fakeGroup.getProfile()).thenReturn(groupProfile);
354+
355+
List<Group> groupIteraterList = Lists.newArrayList(fakeGroup);
356+
GroupList groupList = mock(GroupList.class);
357+
when(groupList.iterator()).thenReturn(groupIteraterList.iterator());
358+
359+
User mockUser = mock(User.class);
360+
when(mockUser.listGroups()).thenReturn(groupList);
361+
362+
DefaultClient mockClient = mock(DefaultClient.class);
363+
when(mockClient.getUser(anyString())).thenReturn(mockUser);
364+
365+
OktaAuthConnector connector =
366+
new OktaAuthConnector(
367+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
368+
AuthData authData = AuthData.builder().userId("deadbeef").build();
369+
Set<String> groups = connector.getGroups(authData);
370+
assertEquals(groups, Set.of("testGroup"));
371+
}
372+
373+
@Test
374+
public void testGetGroupsMissingProfile() {
375+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
376+
377+
Group fakeGroup = mock(Group.class);
378+
when(fakeGroup.getProfile()).thenReturn(null);
379+
380+
List<Group> groupIteraterList = Lists.newArrayList(fakeGroup);
381+
GroupList groupList = mock(GroupList.class);
382+
when(groupList.iterator()).thenReturn(groupIteraterList.iterator());
383+
384+
User mockUser = mock(User.class);
385+
when(mockUser.listGroups()).thenReturn(groupList);
386+
387+
DefaultClient mockClient = mock(DefaultClient.class);
388+
when(mockClient.getUser(anyString())).thenReturn(mockUser);
389+
390+
OktaAuthConnector connector =
391+
new OktaAuthConnector(
392+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
393+
AuthData authData = AuthData.builder().userId("deadbeef").build();
394+
Set<String> groups = connector.getGroups(authData);
395+
assertEquals(groups, new HashSet<String>());
396+
}
397+
398+
@Test
399+
public void testGetGroupsNullGroups() {
400+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
401+
402+
User mockUser = mock(User.class);
403+
when(mockUser.listGroups()).thenReturn(null);
404+
405+
DefaultClient mockClient = mock(DefaultClient.class);
406+
when(mockClient.getUser(anyString())).thenReturn(mockUser);
407+
408+
OktaAuthConnector connector =
409+
new OktaAuthConnector(
410+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
411+
412+
AuthData authData = AuthData.builder().userId("deadbeef").build();
413+
Set<String> groups = connector.getGroups(authData);
414+
assertEquals(groups, new HashSet<>());
415+
}
416+
417+
@Test(expected = ApiException.class)
418+
public void testBadGetUser() {
419+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
420+
Client mockClient = mock(Client.class);
421+
when(mockClient.getUser(anyString())).thenThrow(new RuntimeException("it's broke"));
422+
OktaAuthConnector connector =
423+
new OktaAuthConnector(
424+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
425+
AuthData authData = AuthData.builder().userId("deadbeef").build();
426+
connector.getGroups(authData);
427+
}
428+
429+
@Test
430+
public void testGetUserFromIdpCompletelyBrokenOkta() {
431+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
432+
Client mockClient = mock(Client.class);
433+
String exceptionMessage = "who knows what broke?";
434+
when(mockClient.getUser(anyString())).thenThrow(new IllegalStateException(exceptionMessage));
435+
OktaAuthConnector connector =
436+
new OktaAuthConnector(
437+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
438+
try {
439+
connector.getUserFromIDP("fooUser");
440+
} catch (ApiException exc) {
441+
String actualMessage = exc.getMessage();
442+
assertEquals(actualMessage, "Could not communicate properly with identity provider");
443+
ApiError apiError = exc.getApiErrors().get(0);
444+
assertEquals(apiError, DefaultApiError.IDENTITY_PROVIDER_BAD_GATEWAY);
445+
String causeMessage = exc.getCause().getMessage();
446+
assertEquals(causeMessage, exceptionMessage);
447+
}
448+
}
449+
450+
@Test
451+
public void testGetUserFromIdpOktaProblem() {
452+
AccessTokenVerifier verifier = mock(AccessTokenVerifier.class);
453+
Client mockClient = mock(Client.class);
454+
String excMessage = "A specific thing had a problem";
455+
String excpetionPrefix = "Got invalid response from identity providers";
456+
ResourceException resourceException =
457+
new ResourceException(new DefaultError(ImmutableMap.of("message", excMessage)));
458+
when(mockClient.getUser(anyString())).thenThrow(resourceException);
459+
OktaAuthConnector connector =
460+
new OktaAuthConnector(
461+
client, mockClient, "https://foo.bar/oauth2/skiddleydee", "dogs", verifier);
462+
try {
463+
connector.getUserFromIDP("fooUser");
464+
} catch (ApiException exc) {
465+
String actualMessage = exc.getMessage();
466+
assert actualMessage.startsWith(excpetionPrefix);
467+
assertEquals(exc.getApiErrors().get(0), DefaultApiError.IDENTITY_PROVIDER_BAD_GATEWAY);
468+
}
469+
}
330470
}

cerberus-core/src/main/java/com/nike/cerberus/error/DefaultApiError.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ public enum DefaultApiError implements ApiError {
291291
/** Generic bad requests. This is useful because the blueprint error handling sucks. */
292292
GENERIC_BAD_REQUEST(99999, "Request will not be completed.", SC_BAD_REQUEST),
293293

294+
/** Bad response from identity provider */
295+
IDENTITY_PROVIDER_BAD_GATEWAY(99988, "Bad response from identity provider", SC_BAD_GATEWAY),
296+
294297
/**
295298
* If we encounter an error where something expected is not setup correctly, meaning the service
296299
* is not functional.

cerberus-web/src/main/java/com/nike/cerberus/error/DefaultApiErrorsImpl.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.nike.backstopper.apierror.ApiErrorConstants.*;
2020
import static com.nike.backstopper.apierror.projectspecificinfo.ProjectSpecificErrorCodeRange.ALLOW_ALL_ERROR_CODES;
21+
import static javax.servlet.http.HttpServletResponse.SC_BAD_GATEWAY;
2122
import static javax.servlet.http.HttpServletResponse.SC_NOT_IMPLEMENTED;
2223

2324
import com.nike.backstopper.apierror.ApiError;
@@ -34,6 +35,7 @@ public class DefaultApiErrorsImpl extends SampleProjectApiErrorsBase {
3435
Arrays.asList(
3536
HTTP_STATUS_CODE_FORBIDDEN,
3637
HTTP_STATUS_CODE_UNAUTHORIZED,
38+
SC_BAD_GATEWAY,
3739
HTTP_STATUS_CODE_SERVICE_UNAVAILABLE,
3840
HTTP_STATUS_CODE_TOO_MANY_REQUESTS,
3941
HTTP_STATUS_CODE_INTERNAL_SERVER_ERROR,

0 commit comments

Comments
 (0)