Merge 54c91a7 into 2055fda

Wikidata · Aug 20, 2019 · 884d796 · 884d796
2 parents 2055fda + 54c91a7
commit 884d796
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 59 deletions.
diff --git a/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java b/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/ApiConnection.java
@@ -189,6 +189,11 @@ public class ApiConnection {
 	 */
 	final Map<String, String> cookies;
 
+	/**
+	 * Map of requested tokens.
+	 */
+	final Map<String, String> tokens;
+
 	/**
 	 * Mapper object used for deserializing JSON data.
 	 */
@@ -206,6 +211,7 @@ public class ApiConnection {
 	public ApiConnection(String apiBaseUrl) {
 		this.apiBaseUrl = apiBaseUrl;
 		this.cookies = new HashMap<>();
+		this.tokens = new HashMap<>();
 	}
 
 	/**
@@ -217,18 +223,22 @@ public ApiConnection(String apiBaseUrl) {
 	 * 		map of cookies used for this session
 	 * @param loggedIn
 	 * 		true if login succeeded.
+	 * @param tokens
+	 * 		map of tokens used for this session
 	 */
 	@JsonCreator
 	@Deprecated
 	protected ApiConnection(
 			@JsonProperty("baseUrl") String apiBaseUrl,
 			@JsonProperty("cookies") Map<String, String> cookies,
 			@JsonProperty("username") String username,
-			@JsonProperty("loggedIn") boolean loggedIn) {
+			@JsonProperty("loggedIn") boolean loggedIn,
+			@JsonProperty("tokens") Map<String, String> tokens) {
 		this.apiBaseUrl = apiBaseUrl;
 		this.username = username;
 		this.cookies = cookies;
 		this.loggedIn = loggedIn;
+		this.tokens = tokens;
 	}
 
 	/**
@@ -335,6 +345,7 @@ public void logout() throws IOException {
 			Map<String, String> params = new HashMap<>();
 			params.put("action", "logout");
 			params.put("format", "json"); // reduce the output
+			params.put("token", getOrFetchToken("csrf"));
 			try {
 				sendJsonRequest("POST", params);
 			} catch (MediaWikiApiErrorException e) {
@@ -355,6 +366,31 @@ public void logout() throws IOException {
 	public void clearCookies() throws IOException {
 		logout();
 		this.cookies.clear();
+		this.tokens.clear();
+	}
+
+	/**
+	 * Return a token of given type.
+	 * @param tokenType The kind of token to retrieve like "csrf" or "login"
+	 * @return can return null if token can not be retrieved
+	 */
+	String getOrFetchToken(String tokenType) {
+		if (tokens.containsKey(tokenType)) {
+			return tokens.get(tokenType);
+		}
+		String value = fetchToken(tokenType);
+		tokens.put(tokenType, value);
+		// TODO if this is null, we could try to recover here:
+		// (1) Check if we are still logged in; maybe log in again
+		// (2) If there is another error, maybe just run the operation again
+		return value;
+	}
+
+	/**
+	 * Remove fetched value of given token.
+	 */
+	void clearToken(String tokenType) {
+		tokens.remove(tokenType);
 	}
 
 	/**
@@ -365,7 +401,7 @@ public void clearCookies() throws IOException {
 	 * @param tokenType The kind of token to retrieve like "csrf" or "login"
 	 * @return newly retrieved token or null if no token was retrieved
 	 */
-	String fetchToken(String tokenType) throws IOException, MediaWikiApiErrorException {
+	private String fetchToken(String tokenType) {
 		Map<String, String> params = new HashMap<>();
 		params.put(ApiConnection.PARAM_ACTION, "query");
 		params.put("meta", "tokens");

diff --git a/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnection.java b/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnection.java
@@ -57,14 +57,17 @@ public BasicApiConnection(String apiBaseUrl) {
 	 * 		map of cookies used for this session
 	 * @param loggedIn
 	 * 		true if login succeeded.
+	 * @param tokens
+	 * 		map of tokens used for this session
 	 */
 	@JsonCreator
 	private BasicApiConnection(
 			@JsonProperty("baseUrl") String apiBaseUrl,
 			@JsonProperty("cookies") Map<String, String> cookies,
 			@JsonProperty("username") String username,
-			@JsonProperty("loggedIn") boolean loggedIn) {
-		super(apiBaseUrl, cookies, username, loggedIn);
+			@JsonProperty("loggedIn") boolean loggedIn,
+			@JsonProperty("tokens") Map<String, String> tokens) {
+		super(apiBaseUrl, cookies, username, loggedIn, tokens);
 	}
 
 	/**

diff --git a/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/WbEditingAction.java b/wdtk-wikibaseapi/src/main/java/org/wikidata/wdtk/wikibaseapi/WbEditingAction.java
@@ -67,12 +67,6 @@ public class WbEditingAction {
 	 */
 	final ObjectMapper mapper;
 
-	/**
-	 * Current CSRF (Cross-Site Request Forgery) token, or null if no valid
-	 * token is known.
-	 */
-	String csrfToken = null;
-
 	/**
 	 * Value in seconds of MediaWiki's maxlag parameter. Shorter is nicer,
 	 * longer is more aggressive.
@@ -696,7 +690,7 @@ private JsonNode performAPIAction(
 		}
 
 		parameters.put("maxlag", Integer.toString(this.maxLag));
-		parameters.put("token", getCsrfToken());
+		parameters.put("token", connection.getOrFetchToken("csrf"));
 
 		if (this.remainingEdits > 0) {
 			this.remainingEdits--;
@@ -717,8 +711,8 @@ private JsonNode performAPIAction(
 				break;
 			} catch (TokenErrorException e) { // try again with a fresh token
 				lastException = e;
-				refreshCsrfToken();
-				parameters.put("token", getCsrfToken());
+				connection.clearToken("csrf");
+				parameters.put("token", connection.getOrFetchToken("csrf"));
 			} catch (MaxlagErrorException e) { // wait for 5 seconds
 				lastException = e;
 				logger.warn(e.getMessage() + " -- pausing for 5 seconds.");
@@ -783,46 +777,6 @@ private EntityDocument parseJsonResponse(JsonNode entityNode) throws IOException
 				.readValue(entityNode);
 	}
 
-	/**
-	 * Returns a CSRF (Cross-Site Request Forgery) token as required to edit
-	 * data.
-	 */
-	private String getCsrfToken() {
-		if (this.csrfToken == null) {
-			refreshCsrfToken();
-		}
-		return this.csrfToken;
-	}
-
-	/**
-	 * Obtains and sets a new CSRF token, whether or not there is already a
-	 * token set right now.
-	 */
-	private void refreshCsrfToken() {
-
-		this.csrfToken = fetchCsrfToken();
-		// TODO if this is null, we could try to recover here:
-		// (1) Check if we are still logged in; maybe log in again
-		// (2) If there is another error, maybe just run the operation again
-	}
-
-	/**
-	 * Executes a API query action to get a new CSRF (Cross-Site Request
-	 * Forgery) token. The method only executes the action, without doing any
-	 * checks first. If errors occur, they are logged and null is returned.
-	 *
-	 * @return newly retrieved token or null if no token was retrieved
-	 */
-	private String fetchCsrfToken() {
-		try {
-			return connection.fetchToken("csrf");
-		} catch (IOException | MediaWikiApiErrorException e) {
-			logger.error("Error when trying to fetch csrf token: "
-					+ e.toString());
-		}
-		return null;
-	}
-
 	/**
 	 * Makes sure that we are not editing too fast. The method stores the last
 	 * {@link WbEditingAction#editTimeWindow} time points when an edit was

diff --git a/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java b/wdtk-wikibaseapi/src/test/java/org/wikidata/wdtk/wikibaseapi/BasicApiConnectionTest.java
@@ -77,6 +77,7 @@ public void setUp() throws Exception {
 		params.put("meta", "tokens");
 		params.put("type", "csrf");
 		params.put("format", "json");
+		params.put("assert", "user");
 		this.con.setWebResourceFromPath(params, this.getClass(),
 				"/query-csrf-token-loggedin-response.json", CompressionType.NONE);
 		params.clear();
@@ -117,8 +118,10 @@ public void setUp() throws Exception {
 		params.put("action", "logout");
 		params.put("assert", "user");
 		params.put("format", "json");
+		params.put("token", "42307b93c79b0cb558d2dfb4c3c92e0955e06041+\\");
 		this.con.setWebResource(params, "{}");
-
+
+		params.clear();
 		params.put("action", "query");
 		params.put("assert", "user");
 		params.put("format", "json");
@@ -128,18 +131,24 @@ public void setUp() throws Exception {
 	}
 
 	@Test
-	public void testGetToken() throws IOException, MediaWikiApiErrorException {
-		assertTrue(this.con.fetchToken("csrf") != null);
+	public void testGetToken() throws LoginFailedException {
+		this.con.login("username", "password");
+		assertTrue(this.con.getOrFetchToken("csrf") != null);
+	}
+
+	@Test
+	public void testGetTokenWithoutLogin() throws LoginFailedException {
+		assertTrue(this.con.getOrFetchToken("csrf") == null);
 	}
 
 	@Test
-	public void testGetLoginToken() throws IOException, MediaWikiApiErrorException {
-		assertTrue(this.con.fetchToken("login") != null);
+	public void testGetLoginToken() {
+		assertTrue(this.con.getOrFetchToken("login") != null);
 	}
 
 	@Test
 	public void testConfirmLogin() throws LoginFailedException, IOException, MediaWikiApiErrorException {
-		String token = this.con.fetchToken("login");
+		String token = this.con.getOrFetchToken("login");
 		this.con.confirmLogin(token, "username", "password");
 	}