Merge lp:~karni/ubuntu-sso-java-library/fix-oauth-time-drift into lp:ubuntu-sso-java-library

[Michał Karnicki]

Fixed OAuth time drift issue by overriding getTimestamp method of OAuth consumer. Before use, OAuthAuthorizer.fixTimeDrift() should be called once for given server before the authorizer is used.

Fixed email validation regex utility, that did not accept some correct emails.

[lp:~karni/ubuntu-sso-java-library/fix-oauth-time-drift]

17. By Michał Karnicki on 2011-08-29

Format the device time in logs.

[Chad Miller (cmiller)]

Do we really need email validation outside of actually trying it?

I'm pretty sure "[1539137169]!chad.miller" should be valid. The mail system implements mail standards better than we can. We should be able to dump a message into it, and let the transit itself be our validation.

Who owns that? Can we add @Deprecated on that boolean isValidEmail(String email) method?

review: Needs Information

[Michał Karnicki (karni)]

I didn't want to make an unnecessary request, if the e-mail isn't valid. If you think we shouldn't validate the email (alternatively, if you think the regex is wrong), I can turn it off.

About adding @Deprecated - it's used right after registration (which is currently turned off and launches the browser) and right after login (before the requests are made). Perhaps I shouldn't be checking the email, but sending an invalid email doesn't make sense to me either, does it?

[Chad Miller (cmiller)]

I don't think we should be peeking at the string to validate. The only validation that matters is whether it can traverse the mail transport system.

As an example of how hard it is to peek, for the class of email addresses that contain an "@", this is close to acceptable. (And there are more valid addresses than this format!)

^(?:(?:(?:[^@,"\[\]\x5c\x00-\x20\x7f-\xff\.]|\x5c(?=[@,"\[\]\x5c\x00-\x20\x7f-\xff]))(?:[^@,"\[\]\x5c\x00-\x20\x7f-\xff\.]|(?<=\x5c)[@,"\[\]\x5c\x00-\x20\x7f-\xff]|\x5c(?=[@,"\[\]\x5c\x00-\x20\x7f-\xff])|\.(?=[^\.])){1,62}(?:[^@,"\[\]\x5c\x00-\x20\x7f-\xff\.]|(?<=\x5c)[@,"\[\]\x5c\x00-\x20\x7f-\xff])|[^@,"\[\]\x5c\x00-\x20\x7f-\xff\.]{1,2})|"(?:[^"]|(?<=\x5c)"){1,62}")@(?:(?!.{64})(?:[a-zA-Z0-9][a-zA-Z0-9-]{1,61}[a-zA-Z0-9]\.?|[a-zA-Z0-9]\.?)+\.(?:xn--[a-zA-Z0-9]+|[a-zA-Z]{2,6})|\[(?:[0-1]?\d?\d|2[0-4]\d|25[0-5])(?:\.(?:[0-1]?\d?\d|2[0-4]\d|25[0-5])){3}\])$

Let me try to dig up the regex from the ORA book from the '90s, which consisted of most of a printed page of text.

[Chad Miller (cmiller)]

http://ex-parrot.com/~pdw/Mail-RFC822-Address.html

=== modified file 'src/com/ubuntu/sso/UbuntuSingleSignOnAPI.java'
--- src/com/ubuntu/sso/UbuntuSingleSignOnAPI.java	2011-08-02 17:11:58 +0000
+++ src/com/ubuntu/sso/UbuntuSingleSignOnAPI.java	2011-08-29 15:54:22 +0000
@@ -28,6 +28,10 @@
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import oauth.signpost.signature.HmacSha1MessageSigner;
 
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHeaders;
@@ -73,6 +77,8 @@
 import com.ubuntu.sso.util.HttpUtil.QueryStringBuilder;
 
 public class UbuntuSingleSignOnAPI implements SingleSignOnAPI {
+	private static final Logger LOGGER = Logger.getLogger("com.ubuntu.sso");
+	
 	private static final String DEFAULT_SERVICE_ROOT	= "localhost:8000";
 	private static final String DEFAULT_CONTENT_TYPE	= "application/x-www-form-urlencoded";
 	// API complains about trailing "; charset=UTF-8";
@@ -109,6 +115,8 @@
 	 */
 	public UbuntuSingleSignOnAPI(HttpClient httpClient, String scheme, String host,
 			Authorizer authorizer) {
+		LOGGER.setLevel(Level.INFO);
+		
 		if (httpClient == null) {
 			final HttpParams httpParams = new BasicHttpParams();
 			// 30 second timeout. Sometimes pingU1() can take a while.
@@ -123,6 +131,20 @@
 		this.authorizer = authorizer;
 	}
 	
+	public static void fixTimeDrift(HttpClient client) {
+		try {
+			if (client == null) {
+				client = new DefaultHttpClient();
+			}
+			OAuthAuthorizer.fixTimeDrift(client,
+					"http://login.ubuntu.com",
+					"E, dd MMM yyyy HH:mm:ss z");
+		} catch (Exception e) {
+			LOGGER.severe(e.getMessage());
+			e.printStackTrace();
+		}
+	}
+	
 	/* (non-Javadoc)
 	 * @see com.ubuntu.sso.SingleSignOnAPI#setAuthorizer(com.ubuntu.sso.authorizer.Authorizer)
 	 */

=== modified file 'src/com/ubuntu/sso/authorizer/OAuthAuthorizer.java'
--- src/com/ubuntu/sso/authorizer/OAuthAuthorizer.java	2011-07-18 22:09:38 +0000
+++ src/com/ubuntu/sso/authorizer/OAuthAuthorizer.java	2011-08-29 15:54:22 +0000
@@ -22,19 +22,34 @@
 
 package com.ubuntu.sso.authorizer;
 
+import java.io.IOException;
+import java.text.DateFormat;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.Date;
 import java.util.StringTokenizer;
+import java.util.logging.Logger;
 
 import oauth.signpost.commonshttp.CommonsHttpOAuthConsumer;
 import oauth.signpost.http.HttpParameters;
 import oauth.signpost.signature.OAuthMessageSigner;
 import oauth.signpost.signature.PlainTextMessageSigner;
 
+import org.apache.http.Header;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.client.ClientProtocolException;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpHead;
 import org.apache.http.client.methods.HttpUriRequest;
 
 import com.ubuntu.sso.entry.AuthenticateResponse;
 
-public class OAuthAuthorizer implements Authorizer {
-	private final CommonsHttpOAuthConsumer consumer;
+public class OAuthAuthorizer extends CommonsHttpOAuthConsumer
+		implements Authorizer {
+	private static final long serialVersionUID = -5429816815093116827L;
+	
+	private static long timeDriftMillis = 0L;
 
 	/**
 	 * Creates new OAuth {@link Authorizer} with given
@@ -53,12 +68,12 @@
 	 */
 	public OAuthAuthorizer(String consumerKey, String consumerSecret,
 			String tokenKey, String tokenSecret, OAuthMessageSigner signer) {
-		consumer = new CommonsHttpOAuthConsumer(consumerKey, consumerSecret);
-		consumer.setMessageSigner(signer);
-		consumer.setTokenWithSecret(tokenKey, tokenSecret);
+		super(consumerKey, consumerSecret);
+		setMessageSigner(signer);
+		setTokenWithSecret(tokenKey, tokenSecret);
 		final HttpParameters params = new HttpParameters();
 		params.put("realm", "");
-		consumer.setAdditionalParameters(params);
+		setAdditionalParameters(params);
 	}
 	
 	/**
@@ -107,10 +122,54 @@
 	public void signRequest(HttpUriRequest request)
 			throws AuthorizerException {
 		try {
-			consumer.sign(request);
+			sign(request);
 		} catch (Exception e) {
 			e.printStackTrace();
 			throw new AuthorizerException(e);
 		}
 	}
+	
+	// SSO Date header format: "E, dd MMM yyyy HH:mm:ss z"
+	public static void fixTimeDrift(HttpClient client, String uri, String format)
+			throws ClientProtocolException, IOException {
+		final HttpUriRequest head = new HttpHead(uri);
+		final HttpResponse response = client.execute(head);
+		final Logger logger = Logger.getLogger("com.ubuntu.sso");
+		
+		if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
+			final Header dateHeader = response.getFirstHeader("Date");
+			assert dateHeader != null;
+			
+			logger.info("Server time: " + dateHeader.getValue());
+			try {
+				final DateFormat formatter = new SimpleDateFormat(format);
+				final Date serverDate = formatter.parse(dateHeader.getValue());
+				final long server = serverDate.getTime();
+				final long device = System.currentTimeMillis();
+				timeDriftMillis = server - device;
+				if (timeDriftMillis > 300*1000) { // 300 seconds
+					logger.warning("Device time DRIFTED: "
+							+ formatter.format(new Date(device)));
+				}
+			} catch (ParseException e) {
+				logger.severe("Couldn't parse server time!");
+				e.printStackTrace();
+			}
+		} else {
+			logger.warning("Could not check time drift: " + response.getStatusLine());
+		}
+	}
+
+	public long getTimeDriftMillis() {
+		return timeDriftMillis;
+	}
+	
+	public void setTimeDriftMillis(long millis) {
+		timeDriftMillis = millis;
+	}
+
+	@Override
+	protected String generateTimestamp() {
+		return Long.toString((System.currentTimeMillis() + timeDriftMillis) / 1000L);
+	}
 }

=== modified file 'src/com/ubuntu/sso/util/ValidationUtil.java'
--- src/com/ubuntu/sso/util/ValidationUtil.java	2011-07-12 12:24:19 +0000
+++ src/com/ubuntu/sso/util/ValidationUtil.java	2011-08-29 15:54:22 +0000
@@ -34,7 +34,8 @@
 	private static Pattern containsDigitPattern;
 	
 	static {
-		isEmailPattern = Pattern.compile("\\w+.*@\\w+\\.[a-z]+"); // simple check
+		isEmailPattern = Pattern.compile(
+				"^[\\w\\-]+(\\.[\\w\\-]+)*@([A-Za-z0-9-]+\\.)+[A-Za-z]{2,4}$");
 		containsUppercasePattern = Pattern.compile(".*[A-Z]+.*");
 		containsDigitPattern = Pattern.compile(".*[0-9]+.*");
 	}