Skip to content

Prepare 1.5.0 #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nilsbehlen wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Prepare 1.5.0 #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
125990d
small changes, prepare v1.5.0
nilsbehlen Oct 20, 2025
b9e6856
v1.5.0
nilsbehlen Mar 5, 2026
016bc8c
Merge branch 'master' into prepare-1.5.0
nilsbehlen Mar 5, 2026
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<artifactId>maven-surefire-plugin</artifactId>
<version>3.5.3</version>
<configuration>
<skipTests>false</skipTests>
<skipTests>true</skipTests>
</configuration>
Comment on lines +21 to 22
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

Setting Surefire’s <skipTests> to true disables unit tests for all builds, which makes it easy to ship regressions (especially for a release bump). If the intent is only to speed up local packaging, consider removing this change or gating it behind a Maven profile / CI property instead of committing it as the default.

Copilot uses AI. Check for mistakes.
</plugin>
<plugin>
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/privacyidea/AsyncRequestCallable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ public void onFailure(@NotNull Call call, @NotNull IOException e)
@Override
public void onResponse(@NotNull Call call, @NotNull Response response) throws IOException
{
// Only response.body() is available in OkHttp; ensure it is closed and consumed only once to prevent resource leaks.
// The body of the response can be in `body()` for success cases or in `errorBody()` for error cases.
// We need to handle both and ensure the body is closed to prevent resource leaks.
// The body can only be consumed once.
Comment on lines +80 to 82
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The comment mentions errorBody(), but okhttp3.Response doesn’t have an errorBody() API (that’s Retrofit). This is misleading documentation; either adjust the comment to OkHttp semantics (body is available regardless of status) or implement the intended behavior in the appropriate layer.

Suggested change
// The body of the response can be in `body()` for success cases or in `errorBody()` for error cases.
// We need to handle both and ensure the body is closed to prevent resource leaks.
// The body can only be consumed once.
// For OkHttp, the response body is always available via `body()`, regardless of HTTP status.
// We must ensure the body is closed to prevent resource leaks, and it can only be consumed once.
// Using try-with-resources guarantees the body is properly closed after reading.

Copilot uses AI. Check for mistakes.
try (ResponseBody responseBody = response.body())
{
if (responseBody != null)
{
String s = responseBody.string();
if (!privacyIDEA.logExcludedEndpoints().contains(path) && !ENDPOINT_AUTH.equals(path))
if (!privacyIDEA.logExcludedEndpoints().contains(path))
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

This change will log the full response body for all endpoints not in logExcludedEndpoints(). Since /auth is not excluded by default (and returns JWT/token material), this can leak sensitive credentials into logs. Please re-add an explicit /auth exclusion (or add it to the default excluded list) and/or redact tokens before logging.

Suggested change
if (!privacyIDEA.logExcludedEndpoints().contains(path))
if (!privacyIDEA.logExcludedEndpoints().contains(path) && !ENDPOINT_AUTH.equals(path))

Copilot uses AI. Check for mistakes.
{
privacyIDEA.log(path + " (" + response.code() + "):\n" + privacyIDEA.parser.formatJson(s));
}
Expand Down
48 changes: 39 additions & 9 deletions src/main/java/org/privacyidea/PIResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonDeserializationContext;
import com.google.gson.JsonDeserializer;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;
Expand Down Expand Up @@ -84,10 +89,11 @@ public boolean authenticationSuccessful()
*/
public boolean pushAvailable()
{
return multiChallenge.stream().anyMatch(c -> isPushOrSmartphoneContainer(c.getType()));
return multiChallenge.stream().anyMatch(c -> isPushOrSmartphoneContainer(c.getType()) && "poll".equals(c.getClientMode()));
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

pushAvailable() now requires clientMode to be exactly "poll". Many server responses don’t include client_mode per challenge (it parses as an empty string), so this can incorrectly return false even when a push challenge is present. Consider detecting push based on token type alone or falling back to the response’s preferredClientMode when clientMode is missing.

Suggested change
return multiChallenge.stream().anyMatch(c -> isPushOrSmartphoneContainer(c.getType()) && "poll".equals(c.getClientMode()));
if (multiChallenge == null || multiChallenge.isEmpty())
{
return false;
}
return multiChallenge.stream().anyMatch(c -> {
if (!isPushOrSmartphoneContainer(c.getType()))
{
return false;
}
String clientMode = c.getClientMode();
if (clientMode == null || clientMode.isEmpty())
{
// Fall back to the response's preferred client mode when per-challenge clientMode is missing
return "poll".equals(preferredClientMode);
}
return "poll".equals(clientMode);
});

Copilot uses AI. Check for mistakes.
}

private boolean isPushOrSmartphoneContainer(String type) {
private boolean isPushOrSmartphoneContainer(String type)
{
return TOKEN_TYPE_PUSH.equals(type) || CONTAINER_TYPE_SMARTPHONE.equals(type);
}

Expand All @@ -113,7 +119,8 @@ public String otpTransactionId()
return null;
}

public String pushTransactionId() {
public String pushTransactionId()
{
for (Challenge challenge : multiChallenge)
{
if (isPushOrSmartphoneContainer(challenge.getType()))
Expand Down Expand Up @@ -142,16 +149,27 @@ private boolean isNotBlank(String str) {
*/
public String otpMessage()
{
return reduceChallengeMessagesWhere(c -> !(isPushOrSmartphoneContainer(c.getType())));
return reduceChallengeMessagesWhere(c ->
{
String cm = c.getClientMode();
System.out.println(
"challenge for " + c.getType() + " " + c.getSerial() + " with mode: " + cm);
boolean yes = "interactive".equals(cm);
return yes;
Comment on lines +154 to +158
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

otpMessage() now filters challenges by clientMode == "interactive". In existing responses/fixtures, client_mode is often absent for OTP challenges (parsed as empty string), which would make otpMessage() return an empty message even when OTP challenges exist. Consider keeping the previous logic (exclude push/smartphone/webauthn) or falling back to preferredClientMode / treating missing client_mode as interactive for OTP challenges.

Suggested change
String cm = c.getClientMode();
System.out.println(
"challenge for " + c.getType() + " " + c.getSerial() + " with mode: " + cm);
boolean yes = "interactive".equals(cm);
return yes;
String clientMode = c.getClientMode();
String type = c.getType();
// If client_mode is explicitly "interactive", this is an OTP-style interactive challenge.
if ("interactive".equals(clientMode))
{
return true;
}
// For legacy responses where client_mode is missing or empty, fall back to the
// previous behavior: include all non-push / non-smartphone / non-webauthn challenges.
if (clientMode == null || clientMode.trim().isEmpty())
{
if (isPushOrSmartphoneContainer(type) || TOKEN_TYPE_WEBAUTHN.equals(type))
{
return false;
}
return true;
}
// Any other explicit client_mode is not considered an OTP-input challenge.
return false;

Copilot uses AI. Check for mistakes.
});
Comment on lines +152 to +159
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

Library code should not write directly to stdout. The System.out.println(...) debug output in otpMessage() will spam application logs and may leak token/challenge details; please remove it or route it through the configured logger behind a debug flag.

Suggested change
return reduceChallengeMessagesWhere(c ->
{
String cm = c.getClientMode();
System.out.println(
"challenge for " + c.getType() + " " + c.getSerial() + " with mode: " + cm);
boolean yes = "interactive".equals(cm);
return yes;
});
return reduceChallengeMessagesWhere(c -> "interactive".equals(c.getClientMode()));

Copilot uses AI. Check for mistakes.
}

private String reduceChallengeMessagesWhere(Predicate<Challenge> predicate)
{
StringBuilder sb = new StringBuilder();
sb.append(
multiChallenge.stream().filter(predicate).map(Challenge::getMessage).distinct().reduce("", (a, s) -> a + s + ", ").trim());

if (sb.length() > 0)
sb.append(this.multiChallenge.stream()
.filter(predicate)
.map(Challenge::getMessage)
.distinct()
.reduce("", (a, s) -> a + s + ", ")
.trim());

if (!sb.isEmpty())
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

StringBuilder in Java 14 (per pom.xml) does not have an isEmpty() method, so this will not compile. Use sb.length() > 0 (or check the appended string) before deleting the trailing separator.

Suggested change
if (!sb.isEmpty())
if (sb.length() > 0)

Copilot uses AI. Check for mistakes.
{
sb.deleteCharAt(sb.length() - 1);
}
Expand Down Expand Up @@ -198,7 +216,19 @@ public String toJSON()

public static PIResponse fromJSON(String json)
{
return new Gson().fromJson(json, PIResponse.class);
JsonDeserializer<Challenge> challengeDeserializer = (jsonElement, type, ctx) ->
{
JsonObject obj = jsonElement.getAsJsonObject();
String serial = obj.has("serial") && !obj.get("serial").isJsonNull() ? obj.get("serial").getAsString() : "";
String message = obj.has("message") && !obj.get("message").isJsonNull() ? obj.get("message").getAsString() : "";
String clientMode = obj.has("clientMode") && !obj.get("clientMode").isJsonNull() ? obj.get("clientMode").getAsString() : "";
String image = obj.has("image") && !obj.get("image").isJsonNull() ? obj.get("image").getAsString() : "";
String transactionID = obj.has("transactionID") && !obj.get("transactionID").isJsonNull() ? obj.get("transactionID").getAsString() : "";
String tokenType = obj.has("type") && !obj.get("type").isJsonNull() ? obj.get("type").getAsString() : "";
return new Challenge(serial, message, clientMode, image, transactionID, tokenType);
};
Gson gson = new GsonBuilder().registerTypeAdapter(Challenge.class, challengeDeserializer).create();
return gson.fromJson(json, PIResponse.class);
}

@Override
Expand Down