Skip to content

Commit

Permalink
CLDR-16838 Fix login bug: exclude JWT if query has email and password
Browse files Browse the repository at this point in the history
-New set of excluded JWT in KeepLoggedInManager.java

-Undeprecate UserRegistry.internalGetPassword, accessed not only by JSP but also by Auth.java

-Fix some compiler warnings for Auth.java
  • Loading branch information
btangmu committed Nov 14, 2023
1 parent dace6c6 commit 393d030
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Date;
import java.util.HashSet;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.crypto.SecretKey;
Expand All @@ -31,6 +33,8 @@ public class KeepLoggedInManager {
private final File keyFile;
private SecretKey key;

private final Set<String> excludedJwtId = new HashSet<>();

public static File getDefaultParent() {
final CLDRConfig config = CLDRConfig.getInstance();
if (config instanceof CLDRConfigImpl) {
Expand Down Expand Up @@ -175,4 +179,12 @@ public Jws<Claims> getClaims(String jwt) {
return null;
}
}

public boolean jwtIsInExcludedSet(String jwtId) {
return excludedJwtId.contains(jwtId);
}

public void addToExcludedSet(String jwtId) {
excludedJwtId.add(jwtId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,7 @@ String getPassword() {
return password;
}

/** here to allow one JSP to get at the password, but otherwise keep the field hidden */
@Deprecated
/* Accessed by admin-usersWithOldVotes.jsp as well as by Auth.java */
public String internalGetPassword() {
return getPassword();
}
Expand Down
17 changes: 11 additions & 6 deletions tools/cldr-apps/src/main/java/org/unicode/cldr/web/WebContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -1328,12 +1328,17 @@ public void setSession() {
{
final String jwt = getCookieValue(SurveyMain.COOKIE_SAVELOGIN);
if (jwt != null && !jwt.isBlank()) {
final String jwtId = CookieSession.sm.klm.getSubject(jwt);
if (jwtId != null && !jwtId.isBlank()) {
User jwtInfo = CookieSession.sm.reg.getInfo(Integer.parseInt(jwtId));
if (jwtInfo != null) {
user = jwtInfo;
logger.fine("Logged in " + jwtInfo + " #" + jwtId + " using JWT");
KeepLoggedInManager klm = CookieSession.sm.klm;
final String jwtId = klm.getSubject(jwt);
if (jwtId != null && !jwtId.isBlank() && !klm.jwtIsInExcludedSet(jwtId)) {
if (!email.isEmpty() && !password.isEmpty()) {
klm.addToExcludedSet(jwtId);
} else {
User jwtInfo = CookieSession.sm.reg.getInfo(Integer.parseInt(jwtId));
if (jwtInfo != null) {
user = jwtInfo;
logger.fine("Logged in " + jwtInfo + " #" + jwtId + " using JWT");
}
}
}
}
Expand Down
24 changes: 10 additions & 14 deletions tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/Auth.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
import org.eclipse.microprofile.openapi.annotations.tags.Tag;
import org.unicode.cldr.web.CookieSession;
import org.unicode.cldr.web.KeepLoggedInManager;
import org.unicode.cldr.web.SurveyLog;
import org.unicode.cldr.web.SurveyMain;
import org.unicode.cldr.web.UserRegistry;
Expand Down Expand Up @@ -56,22 +57,18 @@ public Response login(
@Context HttpServletRequest hreq,
@Context HttpServletResponse hresp,
@QueryParam("remember")
@Schema(
required = false,
defaultValue = "false",
description = "If true, remember login")
@Schema(defaultValue = "false", description = "If true, remember login")
boolean remember,
LoginRequest request) {

// If there's no user/pass, try to fill one in from cookies.
if (request.isEmpty()) {
// No option to ignore the cookies.
// If you want to logout, use the /logout endpoint first.
// Also compare WebContext.setSession()
final String jwt = WebContext.getCookieValue(hreq, SurveyMain.COOKIE_SAVELOGIN);
if (jwt != null && !jwt.isBlank()) {
final String jwtId = CookieSession.sm.klm.getSubject(jwt);
if (jwtId != null && !jwtId.isBlank()) {
KeepLoggedInManager klm = CookieSession.sm.klm;
final String jwtId = klm.getSubject(jwt);
if (jwtId != null && !jwtId.isBlank() && !klm.jwtIsInExcludedSet(jwtId)) {
User jwtInfo = CookieSession.sm.reg.getInfo(Integer.parseInt(jwtId));
if (jwtInfo != null) {
request.password = jwtInfo.internalGetPassword();
Expand All @@ -96,7 +93,7 @@ public Response login(
if (session == null) {
session = CookieSession.newSession(user, userIP);
}
if (remember == true && user != null) {
if (remember) {
WebContext.loginRemember(hresp, user);
}
} else {
Expand Down Expand Up @@ -155,8 +152,8 @@ public Response login(
* Create a LoginResponse, given a session. Put this here and not in LoginResponse because of
* serialization
*
* @param session
* @return
* @param session the cookie session
* @return the response
*/
private LoginResponse createLoginResponse(CookieSession session) {
LoginResponse resp = new LoginResponse();
Expand Down Expand Up @@ -212,7 +209,6 @@ public Response info(
final String session,
@QueryParam("touch")
@Schema(
required = false,
defaultValue = "false",
description = "Whether to mark the session as updated")
final boolean touch) {
Expand Down Expand Up @@ -294,7 +290,7 @@ public Response lock(
/**
* Extract a CookieSession from a session string
*
* @param session
* @param session the session string, or null
* @return session or null
*/
public static CookieSession getSession(String session) {
Expand All @@ -306,7 +302,7 @@ public static CookieSession getSession(String session) {
/**
* Convenience function for returning the response when there's no session
*
* @return
* @return the response
*/
public static Response noSessionResponse() {
return Response.status(Status.UNAUTHORIZED).build();
Expand Down

0 comments on commit 393d030

Please sign in to comment.