Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ public class IdentityConstants {
public static final String UNKNOWN = "unknown";
public static final String USER_IP = "user-ip";

// User-Agent header constants.
public static final String USER_AGENT = "User-Agent";
public static final String X_FORWARDED_USER_AGENT = "X-Forwarded-User-Agent";

// Service provider constants
public static final String SKIP_CONSENT_DISPLAY_NAME="Skip Consent";
public static final String SKIP_CONSENT="skipConsent";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,37 @@
public class Request {

private final List<Header> headers = new ArrayList<>();
private final String ipAddress;

private Request(Builder builder) {

this.headers.addAll(builder.headers);
this.ipAddress = builder.ipAddress;
}

public List<Header> getHeaders() {

return Collections.unmodifiableList(headers);
}

public String getIpAddress() {
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docstring for public method. All public methods should have a docstring. Consider adding:

/**
 * Gets the IP address of the request.
 *
 * @return The IP address.
 */
public String getIpAddress() {

Copilot generated this review using guidance from repository custom instructions.

return ipAddress;
}

/**
* Builder for the Request.
*/
public static class Builder {

private final List<Header> headers = new ArrayList<>();
private String ipAddress;

public Builder fromHttpRequest(HttpServletRequest request) {

if (request != null) {
resolveHeaders(request);
this.ipAddress = request.getRemoteAddr();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Verify null handling in IdentityContextUtil.

The request.getRemoteAddr() call can return null according to the Servlet API specification. Ensure that IdentityContextUtil properly handles the case where ipAddress is null.

Run the following script to verify how IdentityContextUtil handles null IP addresses:


🏁 Script executed:

#!/bin/bash
# Description: Verify that IdentityContextUtil handles null ipAddress properly

# Search for getClientIpAddress implementations and check null handling
ast-grep --pattern $'getClientIpAddress($$$) {
  $$$
}'

# Also check for direct usage of Request.getIpAddress()
rg -nP -A 5 'getIpAddress\(\)' --type=java

Length of output: 1824


🏁 Script executed:

#!/bin/bash
# Get full context around IdentityContextUtil line 79 and find getClientIpAddress method

echo "=== Context around line 79 in IdentityContextUtil ==="
rg -nP -B 10 -A 10 'request\.getIpAddress\(\)' \
  components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

echo -e "\n=== Looking for getClientIpAddress method ==="
rg -nP 'getClientIpAddress|ClientIpAddress' \
  components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

echo -e "\n=== Full IdentityContextUtil file to understand structure ==="
cat components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

Length of output: 6846


Handle potential null return from IP address fallback path.

Verification confirms that request.getRemoteAddr() can return null per the Servlet API specification. When IdentityContextUtil.getClientIpAddress(Request) falls back to request.getIpAddress() (line 79), this can return null without explicit handling or documentation. Consider either:

  • Documenting that the method can return null and require callers to handle it, or
  • Providing a fallback/default value when both header-based resolution and getRemoteAddr() return null
🤖 Prompt for AI Agents
In
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/model/Request.java
around line 65, request.getRemoteAddr() can return null; update the assignment
to handle null by providing a safe fallback (e.g., set ipAddress to an explicit
default like "unknown" or empty string) and add a brief Javadoc or inline
comment noting that the field may be a default when no client IP is available so
callers know it can be non-specific.

}
Comment on lines 62 to 66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 1

Suggested change
if (request != null) {
resolveHeaders(request);
this.ipAddress = request.getRemoteAddr();
}
if (request != null) {
resolveHeaders(request);
this.ipAddress = request.getRemoteAddr();
}
log.debug("Request builder initialized with IP address: {}", this.ipAddress);


return this;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
* in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.wso2.carbon.identity.core.context.util;

import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.identity.base.IdentityConstants;
import org.wso2.carbon.identity.core.context.IdentityContext;
import org.wso2.carbon.identity.core.context.model.Header;
import org.wso2.carbon.identity.core.context.model.Request;
import org.wso2.carbon.identity.core.util.IdentityUtil;

import java.util.List;

/**
* Utility class for IdentityContext related operations.
*/
public class IdentityContextUtil {

Check warning on line 36 in components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add a private constructor to hide the implicit public one.

See more on https://sonarcloud.io/project/issues?id=wso2_carbon-identity-framework&issues=AZq7A2rhGFM5CyxkqT1h&open=AZq7A2rhGFM5CyxkqT1h&pullRequest=7632

private static final Log LOG = LogFactory.getLog(IdentityContextUtil.class);

/**
* Resolves the client IP address from the given IdentityContext.
*
* @return The client IP address if found, else falls back to the request IP address.
*/
public static String getClientIpAddress() {

Check failure on line 45 in components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=wso2_carbon-identity-framework&issues=AZq7OmJTVKNDerBrzgMI&open=AZq7OmJTVKNDerBrzgMI&pullRequest=7632

Request request = IdentityContext.getThreadLocalIdentityContext().getRequest();
if (request == null) {
LOG.debug("IdentityContext or Request object is null. Cannot resolve client IP address.");
return null;
}

List<Header> headerList = request.getHeaders();
if (CollectionUtils.isEmpty(headerList)) {
LOG.debug("Request headers are empty. Falling back to request IP address.");
return request.getIpAddress();
}

for (String ipHeader : IdentityConstants.HEADERS_WITH_IP) {
for (Header header : headerList) {
if (ipHeader.equalsIgnoreCase(header.getName())) {
List<String> values = header.getValue();
if (CollectionUtils.isNotEmpty(values)) {
if (LOG.isDebugEnabled() && values.size() > 1) {
LOG.debug("Multiple values found for header: " + ipHeader +

Check failure on line 65 in components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/context/util/IdentityContextUtil.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal "Multiple values found for header: " 3 times.

See more on https://sonarcloud.io/project/issues?id=wso2_carbon-identity-framework&issues=AZq7A2rhGFM5CyxkqT1g&open=AZq7A2rhGFM5CyxkqT1g&pullRequest=7632
". Using the first value as the client IP address.");
}
String ip = values.get(0);
if (StringUtils.isNotEmpty(ip) && !IdentityConstants.UNKNOWN.equalsIgnoreCase(ip)) {
return IdentityUtil.getFirstIP(ip);
}
}
}
}
}

LOG.debug("Cannot resolve client IP address from the request headers. Falling back to request IP address.");
return request.getIpAddress();
}

/**
* Resolves the client User-Agent from the given Request.
*
* @return The client User-Agent if found, else null.
*/
public static String getClientUserAgent() {

Request request = IdentityContext.getThreadLocalIdentityContext().getRequest();
if (request == null) {
LOG.debug("IdentityContext or Request object is null. Cannot resolve User-Agent.");
return null;
}

List<Header> headerList = request.getHeaders();
if (CollectionUtils.isEmpty(headerList)) {
LOG.debug("Request headers are empty. User-Agent cannot be resolved.");
return null;
}

List<String> forwardedUserAgent = null;
List<String> userAgent = null;
for (Header header : headerList) {
String name = header.getName();
if (IdentityConstants.X_FORWARDED_USER_AGENT.equalsIgnoreCase(name)) {
forwardedUserAgent = header.getValue();
} else if (IdentityConstants.USER_AGENT.equalsIgnoreCase(name)) {
userAgent = header.getValue();
}
}

if (CollectionUtils.isNotEmpty(forwardedUserAgent)) {
if (LOG.isDebugEnabled() && forwardedUserAgent.size() > 1) {
LOG.debug("Multiple values found for header: " + IdentityConstants.X_FORWARDED_USER_AGENT +
". Using the first value as the User-Agent.");
Comment on lines +112 to +114
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug logging should be wrapped in an if (LOG.isDebugEnabled()) check when performing string concatenation. The current condition if (LOG.isDebugEnabled() && forwardedUserAgent.size() > 1) still performs the string concatenation in the LOG.debug() call even when the size check fails but debug is enabled. Consider restructuring:

if (LOG.isDebugEnabled()) {
    if (forwardedUserAgent.size() > 1) {
        LOG.debug("Multiple values found for header: " + IdentityConstants.X_FORWARDED_USER_AGENT +
                ". Using the first value as the User-Agent.");
    }
}

Copilot generated this review using guidance from repository custom instructions.
}
return forwardedUserAgent.get(0);
} else if (CollectionUtils.isNotEmpty(userAgent)) {
if (LOG.isDebugEnabled() && userAgent.size() > 1) {
LOG.debug("Multiple values found for header: " + IdentityConstants.USER_AGENT +
". Using the first value as the User-Agent.");
Comment on lines +118 to +120
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug logging should be wrapped in an if (LOG.isDebugEnabled()) check when performing string concatenation. The current condition if (LOG.isDebugEnabled() && userAgent.size() > 1) still performs the string concatenation in the LOG.debug() call even when the size check fails but debug is enabled. Consider restructuring:

if (LOG.isDebugEnabled()) {
    if (userAgent.size() > 1) {
        LOG.debug("Multiple values found for header: " + IdentityConstants.USER_AGENT +
                ". Using the first value as the User-Agent.");
    }
}

Copilot generated this review using guidance from repository custom instructions.
}
return userAgent.get(0);
}

LOG.debug("User-Agent header not found in the request.");
return null;
}
}