Skip to content

Refactor PersistentLogin for readability and maintainability #5460

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

Draft
wants to merge 1 commit into
base: develop-6.x.x
Choose a base branch
from
Draft

Refactor PersistentLogin for readability and maintainability #5460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 30 additions & 28 deletions ...rsistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@

import java.security.SecureRandom;
import java.util.*;
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be replaced by a list of package imports from java.util?

Copy link
Member

Choose a reason for hiding this comment

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

yeas please

Copy link
Member

Choose a reason for hiding this comment

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

please expand the *

import java.util.concurrent.ConcurrentHashMap;

/**
* A persistent login feature ("remember me") similar to the implementation in <a href="https://github.com/SpringSource/spring-security">Spring Security</a>,
* which is based on <a href="http://jaspan.com/improved_persistent_login_cookie_best_practice">Improved Persistent Login Cookie
* Best Practice</a> .
*
* <p>
* The one-time tokens generated by this class are purely random and do not contain a user name or other information. For security reasons,
* tokens and user information are not stored anywhere, so if the database is shut down, registered tokens will be gone.
*
* <p>
* The one-time token approach has the negative effect that requests need to be made in sequence, which is sometimes difficult if an app uses
* concurrent AJAX requests. Unfortunately, this is the price we have to pay for a sufficiently secure protection against
* cookie stealing attacks.
Expand All @@ -60,11 +61,11 @@

public final static int DEFAULT_TOKEN_LENGTH = 16;

public final static int INVALIDATION_TIMEOUT = 20000;
public final static int INVALIDATION_TIMEOUT = 10000;

Check notice on line 64 in extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java#L64 

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

private Map<String, LoginDetails> seriesMap = Collections.synchronizedMap(new HashMap<>());
private final Map<String, LoginDetails> seriesMap = new ConcurrentHashMap<>();

Check notice on line 66 in extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java#L66 

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

private SecureRandom random;
private final SecureRandom random;

Check notice on line 68 in extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

extensions/modules/persistentlogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLogin.java#L68 

Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.

public PersistentLogin() {
random = new SecureRandom();
Expand All @@ -73,10 +74,10 @@
/**
* Register the user and generate a first login token which will be valid for the next
* call to {@link #lookup(String)}.
*
* <p>
* The generated token will have the format base64(series-hash):base64(token-hash).
*
* @param user the user name
* @param user the username
* @param password the password
* @param timeToLive timeout of the token
* @return a first login token
Expand Down Expand Up @@ -154,17 +155,18 @@

public class LoginDetails {

private String userName;
private String password;
private String token;
private String series;
private long expires;
private DurationValue timeToLive;

private final String userName;
private final String password;
private final String series;
private final long expires;
private final DurationValue timeToLive;

// disable sequential token checking by default
private boolean seqBehavior = false;
private final boolean seqBehavior = false;

private Map<String, Long> invalidatedTokens = new HashMap<>();
private final Map<String, Long> invalidatedTokens = new HashMap<>();

public LoginDetails(String user, String password, DurationValue timeToLive, long expires) {
this.userName = user;
Expand All @@ -176,19 +178,19 @@
}

public String getToken() {
return this.token;
return token;
}

public String getSeries() {
return this.series;
return series;
}

public String getUser() {
return this.userName;
return userName;
}

public String getPassword() {
return this.password;
return password;
}

public DurationValue getTimeToLive() {
Expand All @@ -197,13 +199,15 @@

public boolean checkAndUpdateToken(String token) {
if (this.token.equals(token)) {
timeoutCheck();
update();
return true;
}
// check map of invalidating tokens
Long timeout = invalidatedTokens.get(token);
if (timeout == null)
final Long timeout = invalidatedTokens.get(token);
if (timeout == null) {
return false;
}
// timed out: remove
if (System.currentTimeMillis() > timeout) {
invalidatedTokens.remove(token);
Expand All @@ -213,23 +217,21 @@
return true;
}

public String update() {
timeoutCheck();
// leave a small time window until previous token is deleted
public void update() {
// leave a small time-window until previous token is deleted
// to allow for concurrent requests
invalidatedTokens.put(this.token, System.currentTimeMillis() + INVALIDATION_TIMEOUT);
this.token = generateToken();
return this.token;
invalidatedTokens.put(token, System.currentTimeMillis() + INVALIDATION_TIMEOUT);
token = generateToken();
}

private void timeoutCheck() {
long now = System.currentTimeMillis();
final long now = System.currentTimeMillis();
invalidatedTokens.entrySet().removeIf(entry -> entry.getValue() < now);
}

@Override
public String toString() {
return this.series + ":" + this.token;
return series + ":" + token;
}
}
}
173 changes: 106 additions & 67 deletions ...ogin/src/main/java/org/exist/xquery/modules/persistentlogin/PersistentLoginFunctions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,36 @@
import org.exist.security.SecurityManager;
import org.exist.security.Subject;
import org.exist.storage.BrokerPool;
import org.exist.xquery.*;
import org.exist.xquery.value.*;
import org.exist.xquery.AnalyzeContextInfo;
import org.exist.xquery.Cardinality;
import org.exist.xquery.ErrorCodes;
import org.exist.xquery.Function;
import org.exist.xquery.FunctionSignature;
import org.exist.xquery.UserSwitchingBasicFunction;
import org.exist.xquery.XPathException;
import org.exist.xquery.XQueryContext;
import org.exist.xquery.value.DurationValue;
import org.exist.xquery.value.FunctionParameterSequenceType;
import org.exist.xquery.value.FunctionReference;
import org.exist.xquery.value.FunctionReturnSequenceType;
import org.exist.xquery.value.Sequence;
import org.exist.xquery.value.SequenceType;
import org.exist.xquery.value.StringValue;
import org.exist.xquery.value.Type;

import javax.annotation.Nullable;

import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;

/**
* Functions to access the persistent login module.
*/
public class PersistentLoginFunctions extends UserSwitchingBasicFunction {

public final static FunctionSignature signatures[] = {
public final static FunctionSignature[] signatures = {
new FunctionSignature(
new QName("register", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
PersistentLoginFn.REGISTER.getQName(),
"Try to log in the user and create a one-time login token. The token can be stored to a cookie and used to log in " +
"(via the login function) as the same user without " +
"providing credentials. However, for security reasons the token will be valid only for " +
Expand All @@ -55,7 +74,7 @@ public class PersistentLoginFunctions extends UserSwitchingBasicFunction {
new FunctionReturnSequenceType(Type.ITEM, Cardinality.ZERO_OR_MORE, "result of the callback function or the empty sequence")
),
new FunctionSignature(
new QName("login", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
PersistentLoginFn.LOGIN.getQName(),
"Try to log in the user based on the supplied token. If the login succeeds, the provided callback function " +
"is called with 4 arguments: $token as xs:string, $user as xs:string, $password as xs:string, $timeToLive as duration. " +
"$token will be a new token which can be used for the next request. The old token is deleted.",
Expand All @@ -67,109 +86,94 @@ public class PersistentLoginFunctions extends UserSwitchingBasicFunction {
new FunctionReturnSequenceType(Type.ITEM, Cardinality.ZERO_OR_MORE, "result of the callback function or the empty sequence")
),
new FunctionSignature(
new QName("invalidate", PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX),
PersistentLoginFn.INVALIDATE.getQName(),
"Invalidate the supplied one-time token, so it can no longer be used to log in.",
new SequenceType[]{
new FunctionParameterSequenceType("token", Type.STRING, Cardinality.EXACTLY_ONE, "a valid one-time token")
},
new FunctionReturnSequenceType(Type.EMPTY, Cardinality.EXACTLY_ONE, "empty sequence")
)
};

private AnalyzeContextInfo cachedContextInfo;

public PersistentLoginFunctions(final XQueryContext context, final FunctionSignature signature) {
super(context, signature);
}

private static Sequence invalidate(Sequence[] args) throws XPathException {
PersistentLogin.getInstance().invalidate(args[0].getStringValue());
return Sequence.EMPTY_SEQUENCE;
}

@Override
public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException {
super.analyze(contextInfo);
this.cachedContextInfo = new AnalyzeContextInfo(contextInfo);
cachedContextInfo = new AnalyzeContextInfo(contextInfo);
}

@Override
public Sequence eval(final Sequence[] args, final Sequence contextSequence) throws XPathException {
if (isCalledAs("register")) {
final String user = args[0].getStringValue();
final String pass;
if (!args[1].isEmpty()) {
pass = args[1].getStringValue();
} else {
pass = null;
}
final DurationValue timeToLive = (DurationValue) args[2].itemAt(0);
final FunctionReference callback;
if (!args[3].isEmpty()) {
callback = (FunctionReference) args[3].itemAt(0);
} else {
callback = null;
}
try {
return register(user, pass, timeToLive, callback);
} finally {
if (callback != null) {
callback.close();
}
}
} else if (isCalledAs("login")) {
final String token = args[0].getStringValue();
final FunctionReference callback;
if (!args[1].isEmpty()) {
callback = (FunctionReference) args[1].itemAt(0);
} else {
callback = null;
}
try {
return authenticate(token, callback);
} finally {
if (callback != null) {
callback.close();
}
}
} else {
PersistentLogin.getInstance().invalidate(args[0].getStringValue());
return Sequence.EMPTY_SEQUENCE;
switch (PersistentLoginFn.get(this)) {
case REGISTER:
return register(args);
case LOGIN:
return login(args);
case INVALIDATE:
return invalidate(args);
default:
throw new XPathException(this, ErrorCodes.ERROR, "Unknown function: " + getName());
}
}

private Sequence register(final String user, final String pass, final DurationValue timeToLive, final FunctionReference callback) throws XPathException {
if (login(user, pass)) {
final PersistentLogin.LoginDetails details = PersistentLogin.getInstance().register(user, pass, timeToLive);
return callback(callback, null, details);
private Sequence register(Sequence[] args) throws XPathException {
final String user = args[0].getStringValue();

final String pass;
if (args[1].isEmpty()) {
pass = null;
} else {
pass = args[1].getStringValue();
}
return Sequence.EMPTY_SEQUENCE;
}

private Sequence authenticate(final String token, final FunctionReference callback) throws XPathException {
final PersistentLogin.LoginDetails data = PersistentLogin.getInstance().lookup(token);
final DurationValue timeToLive = (DurationValue) args[2].itemAt(0);

if (data == null) {
return Sequence.EMPTY_SEQUENCE;
try (FunctionReference callback = getCallBack(args[3])) {
if (unauthenticated(user, pass)) {
return Sequence.EMPTY_SEQUENCE;
}
final PersistentLogin.LoginDetails details = PersistentLogin.getInstance().register(user, pass, timeToLive);
return call(callback, null, details);
}
}

if (login(data.getUser(), data.getPassword())) {
return callback(callback, token, data);
}
private Sequence login(Sequence[] args) throws XPathException {
final String token = args[0].getStringValue();
try (FunctionReference callback = getCallBack(args[1])) {
final PersistentLogin.LoginDetails data = PersistentLogin.getInstance().lookup(token);

return Sequence.EMPTY_SEQUENCE;
if (data == null || unauthenticated(data.getUser(), data.getPassword())) {
return Sequence.EMPTY_SEQUENCE;
}
return call(callback, token, data);
}
}

private boolean login(final String user, final String pass) throws XPathException {
private boolean unauthenticated(final String user, final String pass) {
try {
final SecurityManager sm = BrokerPool.getInstance().getSecurityManager();
final Subject subject = sm.authenticate(user, pass);

//switch the user of the current broker
switchUser(subject);

return true;
} catch (final AuthenticationException | EXistException e) {
return false;
} catch (final AuthenticationException | EXistException e) {
return true;
}
}

private Sequence callback(final FunctionReference func, final String oldToken, final PersistentLogin.LoginDetails details) throws XPathException {
private Sequence call(@Nullable final FunctionReference func, final String oldToken, final PersistentLogin.LoginDetails details) throws XPathException {
if (func == null) return Sequence.EMPTY_SEQUENCE;
final Sequence[] args = new Sequence[4];
final String newToken = details.toString();

Expand All @@ -185,4 +189,39 @@ private Sequence callback(final FunctionReference func, final String oldToken, f
func.analyze(cachedContextInfo);
return func.evalFunction(null, null, args);
}

private @Nullable FunctionReference getCallBack(final Sequence arg) {
if (arg.isEmpty()) {
return null;
}
return (FunctionReference) arg.itemAt(0);
}

private enum PersistentLoginFn {
REGISTER("register"),
LOGIN("login"),
INVALIDATE("invalidate");

final static Map<QName, PersistentLoginFn> lookup = new HashMap<>();

static {
for (PersistentLoginFn persistentLoginFn : EnumSet.allOf(PersistentLoginFn.class)) {
lookup.put(persistentLoginFn.getQName(), persistentLoginFn);
}
}

private final QName qname;

PersistentLoginFn(String name) {
qname = new QName(name, PersistentLoginModule.NAMESPACE, PersistentLoginModule.PREFIX);
}

static PersistentLoginFn get(Function f) {
return lookup.get(f.getName());
}

public QName getQName() {
return qname;
}
}
}
Loading