Skip to content

Commit 69f13b7

Browse files
CopilotAvery-Dunn
Copilot
and
Avery-Dunn
committed
Add correlation ID to additional exception paths and create tests
Co-authored-by: Avery-Dunn <62066438+Avery-Dunn@users.noreply.github.com>
1 parent 1bbb9cb commit 69f13b7

4 files changed

Lines changed: 131 additions & 10 deletions

File tree

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByDeviceCodeFlowSupplier.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33

44
package com.microsoft.aad.msal4j;
55

6+
import org.slf4j.Logger;
7+
import org.slf4j.LoggerFactory;
8+
69
import java.util.concurrent.TimeUnit;
710

811
import static com.microsoft.aad.msal4j.AuthenticationErrorCode.AUTHORIZATION_PENDING;
912

1013
class AcquireTokenByDeviceCodeFlowSupplier extends AuthenticationResultSupplier {
14+
private static final Logger LOG = LoggerFactory.getLogger(AcquireTokenByDeviceCodeFlowSupplier.class);
1115

1216
private DeviceCodeFlowRequest deviceCodeFlowRequest;
1317

@@ -68,7 +72,10 @@ private AuthenticationResult acquireTokenWithDeviceCode(DeviceCode deviceCode,
6872
}
6973
}
7074
}
71-
throw new MsalClientException("Expired Device code", AuthenticationErrorCode.CODE_EXPIRED);
75+
String message = "Expired Device code";
76+
LOG.error(LogHelper.createMessage(message, deviceCodeFlowRequest.requestContext().correlationId()));
77+
throw new MsalClientException(message, AuthenticationErrorCode.CODE_EXPIRED,
78+
deviceCodeFlowRequest.requestContext().correlationId());
7279
}
7380

7481
private Long getCurrentSystemTimeInSeconds() {

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,14 @@ class AcquireTokenByManagedIdentitySupplier extends AuthenticationResultSupplier
2727
AuthenticationResult execute() throws Exception {
2828

2929
if (StringHelper.isNullOrBlank(managedIdentityParameters.resource)) {
30+
String message = MsalErrorMessage.SCOPES_REQUIRED;
31+
LOG.error(LogHelper.createMessage(
32+
"[Managed Identity] " + message,
33+
msalRequest.requestContext().correlationId()));
3034
throw new MsalClientException(
3135
MsalError.RESOURCE_REQUIRED_MANAGED_IDENTITY,
32-
MsalErrorMessage.SCOPES_REQUIRED);
36+
message,
37+
msalRequest.requestContext().correlationId());
3338
}
3439

3540
TokenRequestExecutor tokenRequestExecutor = new TokenRequestExecutor(

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/InteractiveRequest.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33

44
package com.microsoft.aad.msal4j;
55

6+
import org.slf4j.Logger;
7+
import org.slf4j.LoggerFactory;
8+
69
import java.net.InetAddress;
710
import java.net.URI;
811
import java.net.URL;
@@ -14,6 +17,7 @@
1417
import java.util.concurrent.atomic.AtomicReference;
1518

1619
class InteractiveRequest extends MsalRequest {
20+
private static final Logger LOG = LoggerFactory.getLogger(InteractiveRequest.class);
1721

1822
private AtomicReference<CompletableFuture<IAuthenticationResult>> futureReference;
1923

@@ -53,25 +57,34 @@ private void validateRedirectUrl(URI redirectUri) {
5357

5458
//Validate URI scheme. Only http is valid, as determined by the HttpListener created in AcquireTokenByInteractiveFlowSupplier.startHttpListener()
5559
if (scheme == null || !scheme.equals("http")) {
56-
throw new MsalClientException(String.format(
57-
"Only http://localhost or http://localhost:port is supported for the redirect URI of an interactive request using a browser, but \"%s\" was found. For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request", scheme),
58-
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI);
60+
String message = String.format(
61+
"Only http://localhost or http://localhost:port is supported for the redirect URI of an interactive request using a browser, but \"%s\" was found. For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request", scheme);
62+
LOG.error(LogHelper.createMessage(message, this.requestContext().correlationId()));
63+
throw new MsalClientException(message,
64+
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI,
65+
this.requestContext().correlationId());
5966
}
6067

6168
//Ensure that the given redirect URI has a known address
6269
try {
6370
address = InetAddress.getByName(host);
6471
} catch (UnknownHostException e) {
65-
throw new MsalClientException(String.format(
66-
"Unknown host exception for host \"%s\". For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request", host),
67-
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI);
72+
String message = String.format(
73+
"Unknown host exception for host \"%s\". For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request", host);
74+
LOG.error(LogHelper.createMessage(message, this.requestContext().correlationId()));
75+
throw new MsalClientException(message,
76+
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI,
77+
this.requestContext().correlationId());
6878
}
6979

7080
//Ensure that the redirect URI is considered a loopback address
7181
if (address == null || !address.isLoopbackAddress()) {
82+
String message = "Only loopback redirect URI is supported for interactive requests. For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request";
83+
LOG.error(LogHelper.createMessage(message, this.requestContext().correlationId()));
7284
throw new MsalClientException(
73-
"Only loopback redirect URI is supported for interactive requests. For more information about redirect URI formats, see https://aka.ms/msal4j-interactive-request",
74-
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI);
85+
message,
86+
AuthenticationErrorCode.LOOPBACK_REDIRECT_URI,
87+
this.requestContext().correlationId());
7588
}
7689
}
7790

‎msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/CorrelationIdExceptionTest.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.microsoft.aad.msal4j;
5+
6+
import org.junit.jupiter.api.Test;
7+
import static org.junit.jupiter.api.Assertions.*;
8+
9+
/**
10+
* Test class to verify correlation IDs are included in exceptions
11+
*/
12+
class CorrelationIdExceptionTest {
13+
14+
@Test
15+
void testMsalExceptionWithCorrelationId() {
16+
String testCorrelationId = "test-correlation-id-123";
17+
String message = "Test error message";
18+
String errorCode = "test_error";
19+
20+
MsalException exception = new MsalException(message, errorCode, testCorrelationId);
21+
22+
assertNotNull(exception.correlationId(), "Correlation ID should not be null");
23+
assertEquals(testCorrelationId, exception.correlationId(), "Correlation ID should match");
24+
assertTrue(exception.getMessage().contains(testCorrelationId),
25+
"Exception message should contain correlation ID");
26+
assertTrue(exception.getMessage().contains("[Correlation ID: " + testCorrelationId + "]"),
27+
"Exception message should be formatted with correlation ID");
28+
}
29+
30+
@Test
31+
void testMsalClientExceptionWithCorrelationId() {
32+
String testCorrelationId = "client-error-correlation-id";
33+
String message = "Client error occurred";
34+
String errorCode = "client_error";
35+
36+
MsalClientException exception = new MsalClientException(message, errorCode, testCorrelationId);
37+
38+
assertNotNull(exception.correlationId(), "Correlation ID should not be null");
39+
assertEquals(testCorrelationId, exception.correlationId(), "Correlation ID should match");
40+
assertTrue(exception.getMessage().contains(testCorrelationId),
41+
"Exception message should contain correlation ID");
42+
}
43+
44+
@Test
45+
void testMsalServiceExceptionWithCorrelationId() {
46+
String testCorrelationId = "service-error-correlation-id";
47+
String message = "Service error occurred";
48+
String errorCode = "service_error";
49+
50+
MsalServiceException exception = new MsalServiceException(message, errorCode, testCorrelationId);
51+
52+
assertNotNull(exception.correlationId(), "Correlation ID should not be null");
53+
assertEquals(testCorrelationId, exception.correlationId(), "Correlation ID should match");
54+
assertTrue(exception.getMessage().contains(testCorrelationId),
55+
"Exception message should contain correlation ID");
56+
}
57+
58+
@Test
59+
void testMsalThrottlingExceptionWithCorrelationId() {
60+
String testCorrelationId = "throttling-correlation-id";
61+
long retryInMs = 5000;
62+
63+
MsalThrottlingException exception = new MsalThrottlingException(retryInMs, testCorrelationId);
64+
65+
assertNotNull(exception.correlationId(), "Correlation ID should not be null");
66+
assertEquals(testCorrelationId, exception.correlationId(), "Correlation ID should match");
67+
assertTrue(exception.getMessage().contains(testCorrelationId),
68+
"Exception message should contain correlation ID");
69+
assertEquals(retryInMs, exception.retryInMs(), "Retry time should match");
70+
}
71+
72+
@Test
73+
void testMsalExceptionWithoutCorrelationId() {
74+
String message = "Test error message";
75+
String errorCode = "test_error";
76+
77+
MsalException exception = new MsalException(message, errorCode);
78+
79+
assertNull(exception.correlationId(), "Correlation ID should be null when not provided");
80+
assertFalse(exception.getMessage().contains("[Correlation ID:"),
81+
"Exception message should not contain correlation ID prefix");
82+
}
83+
84+
@Test
85+
void testLogHelperCreateMessage() {
86+
String message = "Test message";
87+
String correlationId = "test-corr-id";
88+
89+
String formattedMessage = LogHelper.createMessage(message, correlationId);
90+
91+
assertTrue(formattedMessage.contains("[Correlation ID: " + correlationId + "]"),
92+
"Formatted message should contain correlation ID in correct format");
93+
assertTrue(formattedMessage.contains(message),
94+
"Formatted message should contain original message");
95+
}
96+
}

0 commit comments

Comments
 (0)