From 710e7a443025346253508d280b110b89a282b8c8 Mon Sep 17 00:00:00 2001 From: Joe Steele Date: Sun, 13 Jul 2014 14:08:24 -0400 Subject: [PATCH] Notify user of invalid account-setup settings combo Specifically, warn and block them when attempting to configure Client Certificate Authentication in combination with Connection Security = None. If this were not made obvious to the user, they might not understand why they are not permitted to tap "Next". Also, move the initialization of all view listeners out of onCreate() into initializeViewListeners() which is then called near the end of onCreate(), helping to assure that the listeners won't be triggered during the initialization of views inside onCreate(). --- res/values/strings.xml | 2 + .../activity/setup/AccountSetupIncoming.java | 143 +++++++++++------ .../activity/setup/AccountSetupOutgoing.java | 145 ++++++++++++------ 3 files changed, 204 insertions(+), 86 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 2da3e9489..5ab142dd1 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -390,6 +390,7 @@ Please submit bug reports, contribute new features and ask questions at None SSL/TLS STARTTLS + \"%1$s = %2$s\" is not valid with \"%3$s = %4$s\" When I delete a message Do not delete on server @@ -439,6 +440,7 @@ Please submit bug reports, contribute new features and ask questions at Username Password Authentication + \"%1$s = %2$s\" is not valid with \"%3$s = %4$s\" Invalid setup: %s diff --git a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java index ea2c1ec2f..493a682de 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupIncoming.java @@ -12,6 +12,7 @@ import android.util.Log; import android.view.View; import android.view.View.OnClickListener; import android.widget.*; +import android.widget.AdapterView.OnItemSelectedListener; import android.widget.CompoundButton.OnCheckedChangeListener; import com.fsck.k9.*; @@ -59,8 +60,11 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener private TextView mPasswordLabelView; private EditText mServerView; private EditText mPortView; + private String mCurrentPortViewSetting; private Spinner mSecurityTypeView; + private int mCurrentSecurityTypeViewPosition; private Spinner mAuthTypeView; + private int mCurrentAuthTypeViewPosition; private CheckBox mImapAutoDetectNamespaceView; private EditText mImapPathPrefixView; private EditText mWebdavPathPrefixView; @@ -136,26 +140,9 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } }); - mAuthTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { - @Override - public void onItemSelected(AdapterView parent, View view, int position, - long id) { - updateViewFromAuthType(); - } - - @Override - public void onNothingSelected(AdapterView parent) { /* unused */ } - }); - mAuthTypeAdapter = AuthType.getArrayAdapter(this, SslHelper.isClientCertificateSupportAvailable()); mAuthTypeView.setAdapter(mAuthTypeAdapter); - mUsernameView.addTextChangedListener(validationTextWatcher); - mPasswordView.addTextChangedListener(validationTextWatcher); - mServerView.addTextChangedListener(validationTextWatcher); - mPortView.addTextChangedListener(validationTextWatcher); - mClientCertificateSpinner.setOnClientCertificateChangedListener(clientCertificateChangedListener); - /* * Only allow digits in the port field. */ @@ -182,14 +169,15 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mSecurityTypeView.setAdapter(securityTypesAdapter); // Select currently configured security type - int index = securityTypesAdapter.getPosition(settings.connectionSecurity); - mSecurityTypeView.setSelection(index, false); + mCurrentSecurityTypeViewPosition = securityTypesAdapter.getPosition(settings.connectionSecurity); + mSecurityTypeView.setSelection(mCurrentSecurityTypeViewPosition, false); updateAuthPlainTextFromSecurityType(settings.connectionSecurity); // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter - int position = mAuthTypeAdapter.getPosition(settings.authenticationType); - mAuthTypeView.setSelection(position, false); + mCurrentAuthTypeViewPosition = mAuthTypeAdapter.getPosition(settings.authenticationType); + mAuthTypeView.setSelection(mCurrentAuthTypeViewPosition, false); + updateViewFromAuthType(); if (settings.username != null) { mUsernameView.setText(settings.username); @@ -272,27 +260,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener throw new Exception("Unknown account type: " + mAccount.getStoreUri()); } - /* - * Updates the port when the user changes the security type. This allows - * us to show a reasonable default which the user can change. - * - * Note: It's important that we set the listener *after* an initial option has been - * selected by the code above. Otherwise the listener might be called after - * onCreate() has been processed and the current port value set later in this - * method is overridden with the default port for the selected security type. - */ - mSecurityTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { - @Override - public void onItemSelected(AdapterView parent, View view, int position, - long id) { - // this indirectly triggers validateFields because the port text is watched - updatePortFromSecurityType(); - } - - @Override - public void onNothingSelected(AdapterView parent) { /* unused */ } - }); - mCompressionMobile.setChecked(mAccount.useCompression(Account.TYPE_MOBILE)); mCompressionWifi.setChecked(mAccount.useCompression(Account.TYPE_WIFI)); mCompressionOther.setChecked(mAccount.useCompression(Account.TYPE_OTHER)); @@ -306,15 +273,67 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener } else { updatePortFromSecurityType(); } + mCurrentPortViewSetting = mPortView.getText().toString(); mSubscribedFoldersOnly.setChecked(mAccount.subscribedFoldersOnly()); + initializeViewListeners(); + validateFields(); } catch (Exception e) { failure(e); } } + /** + * Called at the end of {@code onCreate()}, after the views have been + * initialized, so that the listeners are not triggered during the view + * initialization. This avoids needless calls to {@code validateFields()} + * which is called at the end of {@code onCreate()}. + */ + private void initializeViewListeners() { + + /* + * Updates the port when the user changes the security type. This allows + * us to show a reasonable default which the user can change. + * + * Note: It's important that we set this listener *after* an initial + * mSecurityTypeView option has been selected by the code in onCreate(). + * Otherwise the listener might be called after onCreate() is finished + * which would wipe out the initial port value set in onCreate(), + * replacing it with the default port for the selected security type. + */ + mSecurityTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { + @Override + public void onItemSelected(AdapterView parent, View view, int position, + long id) { + // this indirectly triggers validateFields because the port text is watched + updatePortFromSecurityType(); + } + + @Override + public void onNothingSelected(AdapterView parent) { /* unused */ } + }); + + mAuthTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { + @Override + public void onItemSelected(AdapterView parent, View view, int position, + long id) { + updateViewFromAuthType(); + validateFields(); + } + + @Override + public void onNothingSelected(AdapterView parent) { /* unused */ } + }); + + mClientCertificateSpinner.setOnClientCertificateChangedListener(clientCertificateChangedListener); + mUsernameView.addTextChangedListener(validationTextWatcher); + mPasswordView.addTextChangedListener(validationTextWatcher); + mServerView.addTextChangedListener(validationTextWatcher); + mPortView.addTextChangedListener(validationTextWatcher); + } + @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); @@ -343,7 +362,6 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener mClientCertificateLabelView.setVisibility(View.GONE); mClientCertificateSpinner.setVisibility(View.GONE); } - validateFields(); } private void validateFields() { @@ -352,6 +370,45 @@ public class AccountSetupIncoming extends K9Activity implements OnClickListener ConnectionSecurity connectionSecurity = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); boolean hasConnectionSecurity = (!connectionSecurity.equals(ConnectionSecurity.NONE)); + + if (isAuthTypeExternal && !hasConnectionSecurity) { + + // Notify user of an invalid combination of AuthType.EXTERNAL & ConnectionSecurity.NONE + String toastText = getString(R.string.account_setup_incoming_invalid_setting_combo_notice, + getString(R.string.account_setup_incoming_auth_type_label), + AuthType.EXTERNAL.toString(), + getString(R.string.account_setup_incoming_security_label), + ConnectionSecurity.NONE.toString()); + Toast.makeText(this, toastText, Toast.LENGTH_LONG).show(); + + // Reset the views back to their previous settings without recursing through here again + OnItemSelectedListener onItemSelectedListener = mAuthTypeView.getOnItemSelectedListener(); + mAuthTypeView.setOnItemSelectedListener(null); + mAuthTypeView.setSelection(mCurrentAuthTypeViewPosition, false); + mAuthTypeView.setOnItemSelectedListener(onItemSelectedListener); + updateViewFromAuthType(); + + onItemSelectedListener = mSecurityTypeView.getOnItemSelectedListener(); + mSecurityTypeView.setOnItemSelectedListener(null); + mSecurityTypeView.setSelection(mCurrentSecurityTypeViewPosition, false); + mSecurityTypeView.setOnItemSelectedListener(onItemSelectedListener); + updateAuthPlainTextFromSecurityType((ConnectionSecurity) mSecurityTypeView.getSelectedItem()); + + mPortView.removeTextChangedListener(validationTextWatcher); + mPortView.setText(mCurrentPortViewSetting); + mPortView.addTextChangedListener(validationTextWatcher); + + authType = (AuthType) mAuthTypeView.getSelectedItem(); + isAuthTypeExternal = AuthType.EXTERNAL.equals(authType); + + connectionSecurity = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); + hasConnectionSecurity = (!connectionSecurity.equals(ConnectionSecurity.NONE)); + } else { + mCurrentAuthTypeViewPosition = mAuthTypeView.getSelectedItemPosition(); + mCurrentSecurityTypeViewPosition = mSecurityTypeView.getSelectedItemPosition(); + mCurrentPortViewSetting = mPortView.getText().toString(); + } + boolean hasValidCertificateAlias = mClientCertificateSpinner.getAlias() != null; boolean hasValidUserName = Utility.requiredFieldValid(mUsernameView); diff --git a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java index f59abf4e9..dec9b17d1 100644 --- a/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java +++ b/src/com/fsck/k9/activity/setup/AccountSetupOutgoing.java @@ -12,6 +12,7 @@ import android.view.View; import android.view.View.OnClickListener; import android.view.ViewGroup; import android.widget.*; +import android.widget.AdapterView.OnItemSelectedListener; import android.widget.CompoundButton.OnCheckedChangeListener; import com.fsck.k9.*; @@ -46,10 +47,13 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, private TextView mPasswordLabelView; private EditText mServerView; private EditText mPortView; + private String mCurrentPortViewSetting; private CheckBox mRequireLoginView; private ViewGroup mRequireLoginSettingsView; private Spinner mSecurityTypeView; + private int mCurrentSecurityTypeViewPosition; private Spinner mAuthTypeView; + private int mCurrentAuthTypeViewPosition; private ArrayAdapter mAuthTypeAdapter; private Button mNextButton; private Account mAccount; @@ -106,30 +110,12 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mNextButton = (Button)findViewById(R.id.next); mNextButton.setOnClickListener(this); - mRequireLoginView.setOnCheckedChangeListener(this); mSecurityTypeView.setAdapter(ConnectionSecurity.getArrayAdapter(this)); mAuthTypeAdapter = AuthType.getArrayAdapter(this, SslHelper.isClientCertificateSupportAvailable()); mAuthTypeView.setAdapter(mAuthTypeAdapter); - mUsernameView.addTextChangedListener(validationTextWatcher); - mPasswordView.addTextChangedListener(validationTextWatcher); - mServerView.addTextChangedListener(validationTextWatcher); - mPortView.addTextChangedListener(validationTextWatcher); - mClientCertificateSpinner.setOnClientCertificateChangedListener(clientCertificateChangedListener); - - mAuthTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { - @Override - public void onItemSelected(AdapterView parent, View view, int position, - long id) { - updateViewFromAuthType(); - } - - @Override - public void onNothingSelected(AdapterView parent) { /* unused */ } - }); - /* * Only allow digits in the port field. */ @@ -155,15 +141,18 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, updateAuthPlainTextFromSecurityType(settings.connectionSecurity); // The first item is selected if settings.authenticationType is null or is not in mAuthTypeAdapter - int position = mAuthTypeAdapter.getPosition(settings.authenticationType); - mAuthTypeView.setSelection(position, false); + mCurrentAuthTypeViewPosition = mAuthTypeAdapter.getPosition(settings.authenticationType); + mAuthTypeView.setSelection(mCurrentAuthTypeViewPosition, false); + updateViewFromAuthType(); // Select currently configured security type - mSecurityTypeView.setSelection(settings.connectionSecurity.ordinal(), false); + mCurrentSecurityTypeViewPosition = settings.connectionSecurity.ordinal(); + mSecurityTypeView.setSelection(mCurrentSecurityTypeViewPosition, false); if (settings.username != null) { mUsernameView.setText(settings.username); mRequireLoginView.setChecked(true); + mRequireLoginSettingsView.setVisibility(View.VISIBLE); } if (settings.password != null) { @@ -174,27 +163,6 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mClientCertificateSpinner.setAlias(settings.clientCertificateAlias); } - /* - * Updates the port when the user changes the security type. This allows - * us to show a reasonable default which the user can change. - * - * Note: It's important that we set the listener *after* an initial option has been - * selected by the code above. Otherwise the listener might be called after - * onCreate() has been processed and the current port value set later in this - * method is overridden with the default port for the selected security type. - */ - mSecurityTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { - @Override - public void onItemSelected(AdapterView parent, View view, int position, - long id) { - // this indirectly triggers validateFields because the port text is watched - updatePortFromSecurityType(); - } - - @Override - public void onNothingSelected(AdapterView parent) { /* unused */ } - }); - if (settings.host != null) { mServerView.setText(settings.host); } @@ -204,6 +172,9 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } else { updatePortFromSecurityType(); } + mCurrentPortViewSetting = mPortView.getText().toString(); + + initializeViewListeners(); validateFields(); } catch (Exception e) { @@ -215,6 +186,56 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, } + /** + * Called at the end of {@code onCreate()}, after the views have been + * initialized, so that the listeners are not triggered during the view + * initialization. This avoids needless calls to {@code validateFields()} + * which is called at the end of {@code onCreate()}. + */ + private void initializeViewListeners() { + + /* + * Updates the port when the user changes the security type. This allows + * us to show a reasonable default which the user can change. + * + * Note: It's important that we set this listener *after* an initial + * mSecurityTypeView option has been selected by the code in onCreate(). + * Otherwise the listener might be called after onCreate() is finished + * which would wipe out the initial port value set in onCreate(), + * replacing it with the default port for the selected security type. + */ + mSecurityTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { + @Override + public void onItemSelected(AdapterView parent, View view, int position, + long id) { + // this indirectly triggers validateFields because the port text is watched + updatePortFromSecurityType(); + } + + @Override + public void onNothingSelected(AdapterView parent) { /* unused */ } + }); + + mAuthTypeView.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() { + @Override + public void onItemSelected(AdapterView parent, View view, int position, + long id) { + updateViewFromAuthType(); + validateFields(); + } + + @Override + public void onNothingSelected(AdapterView parent) { /* unused */ } + }); + + mRequireLoginView.setOnCheckedChangeListener(this); + mClientCertificateSpinner.setOnClientCertificateChangedListener(clientCertificateChangedListener); + mUsernameView.addTextChangedListener(validationTextWatcher); + mPasswordView.addTextChangedListener(validationTextWatcher); + mServerView.addTextChangedListener(validationTextWatcher); + mPortView.addTextChangedListener(validationTextWatcher); + } + @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); @@ -243,7 +264,6 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, mClientCertificateLabelView.setVisibility(View.GONE); mClientCertificateSpinner.setVisibility(View.GONE); } - validateFields(); } private void validateFields() { @@ -252,6 +272,45 @@ public class AccountSetupOutgoing extends K9Activity implements OnClickListener, ConnectionSecurity connectionSecurity = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); boolean hasConnectionSecurity = (!connectionSecurity.equals(ConnectionSecurity.NONE)); + + if (isAuthTypeExternal && !hasConnectionSecurity) { + + // Notify user of an invalid combination of AuthType.EXTERNAL & ConnectionSecurity.NONE + String toastText = getString(R.string.account_setup_outgoing_invalid_setting_combo_notice, + getString(R.string.account_setup_incoming_auth_type_label), + AuthType.EXTERNAL.toString(), + getString(R.string.account_setup_incoming_security_label), + ConnectionSecurity.NONE.toString()); + Toast.makeText(this, toastText, Toast.LENGTH_LONG).show(); + + // Reset the views back to their previous settings without recursing through here again + OnItemSelectedListener onItemSelectedListener = mAuthTypeView.getOnItemSelectedListener(); + mAuthTypeView.setOnItemSelectedListener(null); + mAuthTypeView.setSelection(mCurrentAuthTypeViewPosition, false); + mAuthTypeView.setOnItemSelectedListener(onItemSelectedListener); + updateViewFromAuthType(); + + onItemSelectedListener = mSecurityTypeView.getOnItemSelectedListener(); + mSecurityTypeView.setOnItemSelectedListener(null); + mSecurityTypeView.setSelection(mCurrentSecurityTypeViewPosition, false); + mSecurityTypeView.setOnItemSelectedListener(onItemSelectedListener); + updateAuthPlainTextFromSecurityType((ConnectionSecurity) mSecurityTypeView.getSelectedItem()); + + mPortView.removeTextChangedListener(validationTextWatcher); + mPortView.setText(mCurrentPortViewSetting); + mPortView.addTextChangedListener(validationTextWatcher); + + authType = (AuthType) mAuthTypeView.getSelectedItem(); + isAuthTypeExternal = AuthType.EXTERNAL.equals(authType); + + connectionSecurity = (ConnectionSecurity) mSecurityTypeView.getSelectedItem(); + hasConnectionSecurity = (!connectionSecurity.equals(ConnectionSecurity.NONE)); + } else { + mCurrentAuthTypeViewPosition = mAuthTypeView.getSelectedItemPosition(); + mCurrentSecurityTypeViewPosition = mSecurityTypeView.getSelectedItemPosition(); + mCurrentPortViewSetting = mPortView.getText().toString(); + } + boolean hasValidCertificateAlias = mClientCertificateSpinner.getAlias() != null; boolean hasValidUserName = Utility.requiredFieldValid(mUsernameView);