From: David A. Velasco Date: Thu, 22 Aug 2013 17:03:21 +0000 (+0200) Subject: Avoid that a user can recover an expirated SAML session with a different userid than... X-Git-Tag: oc-android-1.4.6~17^2~10 X-Git-Url: http://git.linex4red.de/pub/Android/ownCloud.git/commitdiff_plain/b2f18e0f129f97f75b29dc9e3a9145038f5b6e31 Avoid that a user can recover an expirated SAML session with a different userid than the used to create it --- diff --git a/res/values/strings.xml b/res/values/strings.xml index 0176ba0e..0872084a 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -191,6 +191,7 @@ Malformed server configuration It seems that your server instance is not correctly configured. Contact your administrator for more details. An account for the same user and server already exists in the device + The entered user does not match the user of this account Unknown error occurred! An unknown error occurred. Please contact support and include logs from your device. Couldn\'t find host diff --git a/src/com/owncloud/android/authentication/AuthenticatorActivity.java b/src/com/owncloud/android/authentication/AuthenticatorActivity.java index 58c0c979..37ff4912 100644 --- a/src/com/owncloud/android/authentication/AuthenticatorActivity.java +++ b/src/com/owncloud/android/authentication/AuthenticatorActivity.java @@ -1021,6 +1021,9 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList case ACCOUNT_NOT_NEW: mAuthStatusText = R.string.auth_account_not_new; break; + case ACCOUNT_NOT_THE_SAME: + mAuthStatusText = R.string.auth_account_not_the_same; + break; case UNHANDLED_HTTP_CODE: case UNKNOWN_ERROR: mAuthStatusText = R.string.auth_unknown_error_title; @@ -1085,12 +1088,12 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList if (result.isSuccess()) { Log_OC.d(TAG, "Successful access - time to save the account"); - boolean success = true; + boolean success = false; if (mAction == ACTION_CREATE) { success = createAccount(); } else { - updateToken(); + success = updateToken(); } if (success) { @@ -1135,7 +1138,7 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList * Sets the proper response to get that the Account Authenticator that started this activity saves * a new authorization token for mAccount. */ - private void updateToken() { + private boolean updateToken() { Bundle response = new Bundle(); response.putString(AccountManager.KEY_ACCOUNT_NAME, mAccount.name); response.putString(AccountManager.KEY_ACCOUNT_TYPE, mAccount.type); @@ -1146,9 +1149,21 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList mAccountMgr.setAuthToken(mAccount, mCurrentAuthTokenType, mAuthToken); } else if (AccountAuthenticator.AUTH_TOKEN_TYPE_SAML_WEB_SSO_SESSION_COOKIE.equals(mCurrentAuthTokenType)) { + String username = getUserNameForSamlSso(); + if (!mUsernameInput.getText().toString().equals(username)) { + // fail - not a new account, but an existing one; disallow + RemoteOperationResult result = new RemoteOperationResult(ResultCode.ACCOUNT_NOT_THE_SAME); + updateAuthStatusIconAndText(result); + showAuthStatus(); + Log_OC.d(TAG, result.getLogMessage()); + + return false; + } + response.putString(AccountManager.KEY_AUTHTOKEN, mAuthToken); // the next line is necessary; by now, notifications are calling directly to the AuthenticatorActivity to update, without AccountManager intervention mAccountMgr.setAuthToken(mAccount, mCurrentAuthTokenType, mAuthToken); + Log_OC.e(TAG, "saving auth token: " + mAuthToken); } else { response.putString(AccountManager.KEY_AUTHTOKEN, mPasswordInput.getText().toString()); @@ -1156,8 +1171,7 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList } setAccountAuthenticatorResult(response); - // Sync Account - syncAccount(); + return true; } @@ -1194,7 +1208,6 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList Log_OC.d(TAG, result.getLogMessage()); return false; - } else { if (isOAuth || isSaml) { @@ -1222,6 +1235,7 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList intent.putExtra(AccountManager.KEY_USERDATA, username); if (isOAuth || isSaml) { mAccountMgr.setAuthToken(mAccount, mCurrentAuthTokenType, mAuthToken); + Log_OC.e(TAG, "saving auth token: " + mAuthToken); } /// add user data to the new account; TODO probably can be done in the last parameter addAccountExplicitly, or in KEY_USERDATA mAccountMgr.setUserData(mAccount, AccountAuthenticator.KEY_OC_VERSION, mDiscoveredVersion.toString()); @@ -1562,12 +1576,12 @@ implements OnRemoteOperationListener, OnSslValidatorListener, OnFocusChangeList if (sessionCookie != null && sessionCookie.length() > 0) { Log_OC.d(TAG, "Successful SSO - time to save the account"); mAuthToken = sessionCookie; - boolean success = true; + boolean success = false; if (mAction == ACTION_CREATE) { success = createAccount(); } else { - updateToken(); + success = updateToken(); } if (success) { finish(); diff --git a/src/com/owncloud/android/network/OwnCloudClientUtils.java b/src/com/owncloud/android/network/OwnCloudClientUtils.java index 75aed4ac..7148d2d0 100644 --- a/src/com/owncloud/android/network/OwnCloudClientUtils.java +++ b/src/com/owncloud/android/network/OwnCloudClientUtils.java @@ -102,6 +102,7 @@ public class OwnCloudClientUtils { } else if (isSamlSso) { // TODO avoid a call to getUserData here String accessToken = am.blockingGetAuthToken(account, AccountAuthenticator.AUTH_TOKEN_TYPE_SAML_WEB_SSO_SESSION_COOKIE, false); client.setSsoSessionCookie(accessToken); + Log_OC.e(TAG, "client with auth token: " + accessToken); } else { String username = account.name.substring(0, account.name.lastIndexOf('@')); @@ -134,6 +135,7 @@ public class OwnCloudClientUtils { String accessToken = result.getString(AccountManager.KEY_AUTHTOKEN); if (accessToken == null) throw new AuthenticatorException("WTF!"); client.setSsoSessionCookie(accessToken); + Log_OC.e(TAG, "client with auth token: " + accessToken); } else { String username = account.name.substring(0, account.name.lastIndexOf('@')); diff --git a/src/com/owncloud/android/operations/RemoteOperationResult.java b/src/com/owncloud/android/operations/RemoteOperationResult.java index ff0167cc..999974cf 100644 --- a/src/com/owncloud/android/operations/RemoteOperationResult.java +++ b/src/com/owncloud/android/operations/RemoteOperationResult.java @@ -51,7 +51,7 @@ import com.owncloud.android.network.CertificateCombinedException; public class RemoteOperationResult implements Serializable { /** Generated - should be refreshed every time the class changes!! */ - private static final long serialVersionUID = 3267227833178885664L; + private static final long serialVersionUID = -4415103901492836870L; private static final String TAG = "RemoteOperationResult"; @@ -86,7 +86,8 @@ public class RemoteOperationResult implements Serializable { QUOTA_EXCEEDED, ACCOUNT_NOT_FOUND, ACCOUNT_EXCEPTION, - ACCOUNT_NOT_NEW + ACCOUNT_NOT_NEW, + ACCOUNT_NOT_THE_SAME } private boolean mSuccess = false; @@ -301,6 +302,9 @@ public class RemoteOperationResult implements Serializable { } else if (mCode == ResultCode.ACCOUNT_NOT_NEW) { return "Account already existing when creating a new one"; + + } else if (mCode == ResultCode.ACCOUNT_NOT_THE_SAME) { + return "Authenticated with a different account than the one updating"; } return "Operation finished with HTTP status code " + mHttpCode + " (" + (isSuccess() ? "success" : "fail") + ")"; @@ -324,8 +328,9 @@ public class RemoteOperationResult implements Serializable { } public boolean isIdPRedirection() { - return (mRedirectedLocation.toUpperCase().contains("SAML") || - mRedirectedLocation.toLowerCase().contains("wayf")); + return (mRedirectedLocation != null && + (mRedirectedLocation.toUpperCase().contains("SAML") || + mRedirectedLocation.toLowerCase().contains("wayf"))); } } diff --git a/src/com/owncloud/android/ui/dialog/SamlWebViewDialog.java b/src/com/owncloud/android/ui/dialog/SamlWebViewDialog.java index 9ed8a446..a5219a22 100644 --- a/src/com/owncloud/android/ui/dialog/SamlWebViewDialog.java +++ b/src/com/owncloud/android/ui/dialog/SamlWebViewDialog.java @@ -29,6 +29,7 @@ import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; import android.webkit.CookieManager; +import android.webkit.CookieSyncManager; import android.webkit.WebBackForwardList; import android.webkit.WebSettings; import android.webkit.WebView; @@ -115,6 +116,8 @@ public class SamlWebViewDialog extends SherlockDialogFragment { public void onCreate(Bundle savedInstanceState) { Log_OC.d(TAG, "onCreate"); super.onCreate(savedInstanceState); + + CookieSyncManager.createInstance(getActivity()); if (savedInstanceState == null) { mInitialUrl = getArguments().getString(ARG_INITIAL_URL);