Skip to content
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.

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

feat(agama): improve flows timeout #1447

Merged
merged 6 commits into from
May 25, 2022
Merged
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 @@ -6,6 +6,7 @@
import io.jans.agama.engine.service.FlowService;
import io.jans.agama.engine.service.WebContext;
import io.jans.agama.engine.servlet.ExecutionServlet;
import io.jans.agama.engine.script.LogUtils;
import io.jans.agama.model.EngineConfig;

import jakarta.inject.Inject;
Expand Down Expand Up @@ -45,7 +46,7 @@ public Boolean prepareFlow(String sessionId, String qname, String jsonInput) thr

logger.info("Preparing flow '{}'", qname);
Boolean alreadyRunning = null;
if (fs.isEnabled(qname)) {
if (aps.flowEnabled(qname)) {

FlowStatus st = aps.getFlowStatus(sessionId);
alreadyRunning = st != null;
Expand All @@ -56,11 +57,19 @@ public Boolean prepareFlow(String sessionId, String qname, String jsonInput) thr
st = null;
}
if (st == null) {

int timeout = aps.getEffectiveFlowTimeout(qname);
if (timeout <= 0) throw new Exception("Flow timeout negative or zero. " +
"Check your AS configuration or flow definition");
long expireAt = System.currentTimeMillis() + 1000L * timeout;

st = new FlowStatus();
st.setStartedAt(FlowStatus.PREPARED);
st.setQname(qname);
st.setJsonInput(jsonInput);
aps.createFlowRun(sessionId, st);
st.setFinishBefore(expireAt);
aps.createFlowRun(sessionId, st, expireAt);
LogUtils.log("@w Effective timeout for this flow will be % seconds", timeout);
}
}
return alreadyRunning;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ public class FlowUtils {

@Inject
private EngineConfig engineConfig;

private int effectiveInterruptionTime;

public int getEffectiveInterruptionTime() {
return effectiveInterruptionTime;
}

public void setEffectiveInterruptionTime(int effectiveInterruptionTime) {
this.effectiveInterruptionTime = effectiveInterruptionTime;
}

public boolean serviceEnabled() {
return engineConfig.isEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class FlowStatus {
private String qname;
private String templatePath;
private long startedAt;
private long finishBefore;
private Object templateDataModel;
private LinkedList<ParentFlowData> parentsData = new LinkedList<>();
private String externalRedirectUrl;
Expand Down Expand Up @@ -45,6 +46,14 @@ public void setStartedAt(long startedAt) {
this.startedAt = startedAt;
}

public long getFinishBefore() {
return finishBefore;
}

public void setFinishBefore(long finishBefore) {
this.finishBefore = finishBefore;
}

public String getQname() {
return qname;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.jans.agama.engine.serialize.ContinuationSerializer;
import io.jans.agama.model.Flow;
import io.jans.agama.model.ProtoFlow;
import io.jans.as.model.configuration.AppConfiguration;
import io.jans.orm.PersistenceEntryManager;
import io.jans.orm.search.filter.Filter;
import io.jans.util.Pair;
Expand All @@ -18,6 +19,7 @@
import java.util.Base64;
import java.util.Date;
import java.util.List;
import java.util.Optional;

import org.mozilla.javascript.NativeContinuation;
import org.mozilla.javascript.Scriptable;
Expand Down Expand Up @@ -45,6 +47,9 @@ public class AgamaPersistenceService {
@Inject
private FlowUtils flowUtils;

@Inject
private AppConfiguration appConfiguration;

public FlowStatus getFlowStatus(String sessionId) throws IOException {

try {
Expand All @@ -71,19 +76,17 @@ public void persistFlowStatus(String sessionId, FlowStatus fst) throws IOExcepti
} catch(Exception e) {
throw new IOException(e);
}

}

public void createFlowRun(String id, FlowStatus fst) throws Exception {
public void createFlowRun(String id, FlowStatus fst, long expireAt) throws Exception {

long expireAt = System.currentTimeMillis() + 1000L * flowUtils.getEffectiveInterruptionTime();

FlowRun fr = new FlowRun();
fr.setBaseDn(String.format("%s=%s,%s", FlowRun.ATTR_NAMES.ID, id, AGAMA_FLOWRUNS_BASE));
fr.setId(id);
fr.setStatus(fst);
fr.setDeletableAt(new Date(expireAt));

logger.info("Creating flow run");
entryManager.persist(fr);

Expand All @@ -108,6 +111,19 @@ public boolean flowEnabled(String flowName) {

}

public int getEffectiveFlowTimeout(String flowName) {

Flow fl = entryManager.findEntries(AGAMA_FLOWS_BASE, Flow.class,
Filter.createEqualityFilter(Flow.ATTR_NAMES.QNAME, flowName),
new String[]{ Flow.ATTR_NAMES.META }, 1).get(0);

int unauth = appConfiguration.getSessionIdUnauthenticatedUnusedLifetime();
Integer flowTimeout = fl.getMetadata().getTimeout();
int timeout = Optional.ofNullable(flowTimeout).map(Integer::intValue).orElse(unauth);
return Math.min(unauth, timeout);

}

public Flow getFlow(String flowName, boolean full) throws IOException {

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ public FlowStatus getRunningFlowStatus() throws IOException {
return aps.getFlowStatus(sessionId);
}

public boolean isEnabled(String flowName) {
return aps.flowEnabled(flowName);
}

public FlowStatus startFlow(FlowStatus status) throws FlowCrashException {

try {
Expand Down Expand Up @@ -208,13 +204,10 @@ public Pair<Function, NativeJavaObject> prepareSubflow(String subflowName, Strin

public void ensureTimeNotExceeded(FlowStatus flstatus) throws FlowTimeoutException {

int time = flowUtils.getEffectiveInterruptionTime();
//Use some seconds to account for the potential time difference due to redirections:
//jython script -> agama, agama -> jython script. This helps agama flows to timeout
//before the unauthenticated unused time
if (time > 0 &&
System.currentTimeMillis() - flstatus.getStartedAt() + TIMEOUT_SKEW > 1000 * time) {

if (System.currentTimeMillis() + TIMEOUT_SKEW > flstatus.getFinishBefore()) {
throw new FlowTimeoutException("You have exceeded the amount of time required " +
"to complete your authentication process", flstatus.getQname());
//"Your authentication attempt has run for more than " + time + " seconds"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.fasterxml.jackson.databind.ObjectMapper;

import io.jans.agama.engine.misc.FlowUtils;
import io.jans.agama.model.EngineConfig;
import io.jans.as.model.configuration.AppConfiguration;
import io.jans.service.cdi.event.ConfigurationUpdate;
Expand All @@ -22,9 +21,6 @@ public class ServicesFactory {
@Inject
private Logger logger;

@Inject
private FlowUtils futils;

private ObjectMapper mapper;

private EngineConfig econfig;
Expand All @@ -45,16 +41,6 @@ public void updateConfiguration(@Observes @ConfigurationUpdate AppConfiguration
try {
logger.info("Refreshing Agama configuration...");
BeanUtils.copyProperties(econfig, appConfiguration.getAgamaConfiguration());

int unauth = appConfiguration.getSessionIdUnauthenticatedUnusedLifetime();
int effectiveInterruptionTime = econfig.getInterruptionTime();
if (effectiveInterruptionTime == 0 || effectiveInterruptionTime > unauth) {
//Ensure interruption time is lower than or equal to unauthenticated unused
effectiveInterruptionTime = unauth;
logger.warn("Agama flow interruption time modified to {}", unauth);
}
futils.setEffectiveInterruptionTime(effectiveInterruptionTime);

} catch (Exception e) {
logger.error(e.getMessage(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ public void doPost(HttpServletRequest request, HttpServletResponse response)
flowService.terminateFlow();

logger.debug("Sending user's browser for a flow start");
//This relies on the (unathenticated) session id being still alive (so the
//flow name and its inputs can be remembered
//This redirection relies on the (unauthenticated) session id being still alive
//(so the flow name and its inputs can be remembered in the bridge script)
//see AgamaBridge.py#prepareForStep
String url = bridge.scriptPageUrl().replaceFirst("\\.xhtml", ".htm");
response.sendRedirect(request.getContextPath() + "/" + url);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import io.jans.agama.dsl.TranspilationResult;
import io.jans.agama.dsl.Transpiler;
import io.jans.agama.dsl.TranspilerException;
import io.jans.agama.dsl.error.SyntaxException;
Expand Down Expand Up @@ -37,7 +38,7 @@
public class Transpilation {

private static final int DELAY = 10 + (int) (10 * Math.random()); //seconds
private static final int INTERVAL = 30; //TODO: adjust seconds
private static final int INTERVAL = 45; // seconds
private static final double PR = 0.25;

@Inject
Expand Down Expand Up @@ -94,7 +95,8 @@ private Map<String, Integer> makeSimpleMap(Map<String, ProtoFlow> map) {
}

/**
* This method assumes that when a flow is created, attribute revision is set to a negative value
* This method assumes that when a flow is created (eg. via an administrative tool),
* attribute revision is set to a negative value
* @throws IOException
*/
public void process() throws IOException {
Expand Down Expand Up @@ -158,15 +160,15 @@ public void process() throws IOException {

String error = null, shortError = null;
try {
List<String> strings = Transpiler.transpile(qname, map.keySet(), fl.getSource());
TranspilationResult result = Transpiler.transpile(qname, map.keySet(), fl.getSource());
logger.debug("Successful transpilation");
String fname = strings.remove(0);
String compiled = strings.remove(0);

FlowMetadata meta = fl.getMetadata();
meta.setFuncName(fname);
meta.setInputs(strings);

meta.setFuncName(result.getFuncName());
meta.setInputs(result.getInputs());
meta.setTimeout(result.getTimeout());

String compiled = result.getCode();
fl.setMetadata(meta);
fl.setTranspiled(compiled);
fl.setTransHash(futils.hash(compiled));
Expand Down Expand Up @@ -203,6 +205,5 @@ public void process() throws IOException {
traces = makeSimpleMap(map);

}


}
12 changes: 0 additions & 12 deletions agama/model/src/main/java/io/jans/agama/model/EngineConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ public class EngineConfig {
private String scriptsPath = "/scripts";

private Type serializerType = Type.KRYO;

//Time used to determine if flow timeout has occurred in Agama. A negative
//value means no timeout will ever occur
private int interruptionTime;

private int maxItemsLoggedInCollections = 3;

Expand Down Expand Up @@ -96,14 +92,6 @@ public Type getSerializerType() {
public void setSerializerType(Type serializerType) {
this.serializerType = serializerType;
}

public int getInterruptionTime() {
return interruptionTime;
}

public void setInterruptionTime(int interruptionTime) {
this.interruptionTime = interruptionTime;
}

public String getInterruptionErrorPage() {
return interruptionErrorPage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class FlowMetadata {

private String funcName;
private List<String> inputs;
private Integer timeout;
private String displayName;
private String author;
private long timestamp;
Expand All @@ -33,6 +34,14 @@ public void setInputs(List<String> inputs) {
this.inputs = inputs;
}

public Integer getTimeout() {
return timeout;
}

public void setTimeout(Integer timeout) {
this.timeout = timeout;
}

public String getDisplayName() {
return displayName;
}
Expand Down
8 changes: 6 additions & 2 deletions agama/transpiler/grammar/AuthnFlow.g4
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Whenever this file is edited, Class org.gluu.flowless.dsl.Transpiler and its dependants MUST BE reviewed
// Whenever this file is edited, Class io.jans.agama.dsl.Transpiler and its dependants MUST BE reviewed
grammar AuthnFlow;

/*
Expand Down Expand Up @@ -32,13 +32,15 @@ NL: '\r'? '\n' [\t ]* ;

flow: header statement+ ;

header: FLOWSTART WS qname WS? INDENT base configs? inputs? DEDENT NL* ;
header: FLOWSTART WS qname WS? INDENT base timeout? configs? inputs? DEDENT NL* ;
// header always ends in a NL

qname: ALPHANUM | QNAME ; //if flow name is a single word, it is not identified as QNAME but ALPHANUM by parser

base: BASE WS STRING WS? NL;

timeout: TIMEOUT WS UINT WS SECS WS? NL ;

configs: CONFIGS WS short_var WS? NL ;

inputs: FLOWINPUTS (WS short_var)+ WS? NL ;
Expand Down Expand Up @@ -130,6 +132,8 @@ FLOWSTART: 'Flow' ;

BASE: 'Basepath' ;

TIMEOUT: 'Timeout' ;

CONFIGS: 'Configs' ;

FLOWINPUTS: 'Inputs' ;
Expand Down
5 changes: 4 additions & 1 deletion agama/transpiler/grammar/AuthnFlow.interp

Large diffs are not rendered by default.

Loading