Hi (again) everyone,
(As you might have noticed my previous patches got eaten by the mailer daemon :-) I have now split my patches in to additional commits and I hope they will be accepted by the system. I also agree with Nathaniel that they are way easier to follow now as well. For convenience I also include the original cover letter below.)
My name is Petter and I am working for the Swedish company Fidesm AB. We are building a platform for managing Secure Elements (such as NFC cards) via the network. One of the first services that can be deliverd via our platform is an OTP application developed by Yubico (https://github.com/Yubico/ykneo-oath). To enable us to use this application to store our OTP credentials we wanted to integrate it into a good OTP Android App. This patch enables any supported external NFC token to be used with FreeOTP. Currently the only two tokens using ths protocol that we know of are the Fidesmo Card and the Yubico Yubikey. I am more than willing to add support for any other device which is using the same protocol.
The strategy taken to integrate this into FreeOTP was first to generalize the TokenPersistence and Token classes to accomodate both internal and external tokens. This is the first commit/patch. On top of this the NFC functionality was built. To manage the NFC tag efficiently (i.e. not to have the user tap it more than neccessary), we decided to handle it only in the main activity. This required the other activities to return their results rather than manage the TokenPersistence directly. This is a very common pattern in Android and we believe that end result was actually quite nice, clearly separating managing the tokens from the other activities.
If you want to try it out you will require an external NFC token. Please let me know if you want a Fidesmo Card and I can provide instructions on how to get one for free. As earlier stated it can also be tested with a Yubikey from Yubico.
I hope you like the implementation and that you have some good feedack for me on what needs to be changed and improved!
Please beware that I am a bit new to git send-email, so let me know if you want any changes to how the patches were generated.
Best regards, Petter
Petter Arvidsson (4): Change Token to Internal token Add interfaces for Token and TokenPersistence Add External tokens Extend app to utilize new external tokens
app/build.gradle | 6 + app/src/main/AndroidManifest.xml | 9 +- .../freeotp/BaseReorderableAdapter.java | 97 ++++--- .../fedorahosted/freeotp/ExternalMainActivity.java | 142 +++++++++ .../org/fedorahosted/freeotp/ExternalToken.java | 130 +++++++++ .../freeotp/ExternalTokenPersistence.java | 129 +++++++++ .../fedorahosted/freeotp/InternalMainActivity.java | 126 ++++++++ .../org/fedorahosted/freeotp/InternalToken.java | 318 +++++++++++++++++++++ .../freeotp/InternalTokenPersistence.java | 140 +++++++++ .../org/fedorahosted/freeotp/MainActivity.java | 83 +++--- .../java/org/fedorahosted/freeotp/NfcHelpers.java | 60 ++++ .../main/java/org/fedorahosted/freeotp/Token.java | 277 +----------------- .../org/fedorahosted/freeotp/TokenAdapter.java | 62 ++-- .../org/fedorahosted/freeotp/TokenPersistence.java | 131 ++------- .../org/fedorahosted/freeotp/add/AddActivity.java | 10 +- .../org/fedorahosted/freeotp/add/BaseActivity.java | 7 + .../org/fedorahosted/freeotp/add/ScanActivity.java | 23 +- .../fedorahosted/freeotp/edit/BaseActivity.java | 22 -- .../fedorahosted/freeotp/edit/DeleteActivity.java | 19 +- .../fedorahosted/freeotp/edit/EditActivity.java | 46 +-- app/src/main/res/layout/main.xml | 4 +- app/src/main/res/menu/editable_token.xml | 33 +++ app/src/main/res/menu/token.xml | 6 - app/src/main/res/values/strings.xml | 16 +- 24 files changed, 1339 insertions(+), 557 deletions(-) create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalToken.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java delete mode 100644 app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java create mode 100644 app/src/main/res/menu/editable_token.xml
-- 1.9.1
- Rename Token -> InternalToken - Rename TokenPersistence -> InternalTokenPersistence - Instantiate token persistence whenever needed - Introduce notion of an internal token - Let TokenAdapter hide edit menu item for non-internal tokens --- .../freeotp/BaseReorderableAdapter.java | 97 +++++++++++----------- .../freeotp/{Token.java => InternalToken.java} | 14 ++-- ...sistence.java => InternalTokenPersistence.java} | 49 +++++++---- .../org/fedorahosted/freeotp/MainActivity.java | 7 +- .../org/fedorahosted/freeotp/TokenAdapter.java | 23 +++-- .../java/org/fedorahosted/freeotp/TokenLayout.java | 8 +- .../org/fedorahosted/freeotp/add/AddActivity.java | 4 +- .../org/fedorahosted/freeotp/add/ScanActivity.java | 6 +- .../fedorahosted/freeotp/edit/DeleteActivity.java | 11 +-- .../fedorahosted/freeotp/edit/EditActivity.java | 13 +-- app/src/main/res/menu/editable_token.xml | 33 ++++++++ app/src/main/res/menu/token.xml | 6 -- 12 files changed, 168 insertions(+), 103 deletions(-) rename app/src/main/java/org/fedorahosted/freeotp/{Token.java => InternalToken.java} (95%) rename app/src/main/java/org/fedorahosted/freeotp/{TokenPersistence.java => InternalTokenPersistence.java} (65%) create mode 100644 app/src/main/res/menu/editable_token.xml
diff --git a/app/src/main/java/org/fedorahosted/freeotp/BaseReorderableAdapter.java b/app/src/main/java/org/fedorahosted/freeotp/BaseReorderableAdapter.java index 3a46587..ad32b16 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/BaseReorderableAdapter.java +++ b/app/src/main/java/org/fedorahosted/freeotp/BaseReorderableAdapter.java @@ -45,57 +45,58 @@ public abstract class BaseReorderableAdapter extends BaseAdapter { if (convertView == null) { int type = getItemViewType(position); convertView = createView(parent, type); + if(isMovable()) { + convertView.setOnDragListener(new OnDragListener() { + @Override + public boolean onDrag(View dstView, DragEvent event) { + Reference<View> ref = (Reference<View>) event.getLocalState(); + final View srcView = ref.reference; + + switch (event.getAction()) { + case DragEvent.ACTION_DRAG_ENTERED: + srcView.setVisibility(View.VISIBLE); + dstView.setVisibility(View.INVISIBLE); + + move(((Integer) srcView.getTag(KEY)).intValue(), + ((Integer) dstView.getTag(KEY)).intValue()); + ref.reference = dstView; + break; + + case DragEvent.ACTION_DRAG_ENDED: + srcView.post(new Runnable() { + @Override + public void run() { + srcView.setVisibility(View.VISIBLE); + } + }); + break; + }
- convertView.setOnDragListener(new OnDragListener() { - @Override - public boolean onDrag(View dstView, DragEvent event) { - Reference<View> ref = (Reference<View>) event.getLocalState(); - final View srcView = ref.reference; - - switch (event.getAction()) { - case DragEvent.ACTION_DRAG_ENTERED: - srcView.setVisibility(View.VISIBLE); - dstView.setVisibility(View.INVISIBLE); - - move(((Integer) srcView.getTag(KEY)).intValue(), - ((Integer) dstView.getTag(KEY)).intValue()); - ref.reference = dstView; - break; - - case DragEvent.ACTION_DRAG_ENDED: - srcView.post(new Runnable() { - @Override - public void run() { - srcView.setVisibility(View.VISIBLE); - } - }); - break; + return true; } - - return true; - } - }); - - convertView.setOnLongClickListener(new OnLongClickListener() { - @Override - public boolean onLongClick(final View view) { - // Force a reset of any states - notifyDataSetChanged(); - - // Start the drag on the main loop to allow - // the above state reset to settle. - view.post(new Runnable() { - @Override - public void run() { - ClipData data = ClipData.newPlainText("", ""); - DragShadowBuilder sb = new View.DragShadowBuilder(view); - view.startDrag(data, sb, new Reference<View>(view), 0); - } });
- return true; - } - }); + convertView.setOnLongClickListener(new OnLongClickListener() { + @Override + public boolean onLongClick(final View view) { + // Force a reset of any states + notifyDataSetChanged(); + + // Start the drag on the main loop to allow + // the above state reset to settle. + view.post(new Runnable() { + @Override + public void run() { + ClipData data = ClipData.newPlainText("", ""); + DragShadowBuilder sb = new View.DragShadowBuilder(view); + view.startDrag(data, sb, new Reference<View>(view), 0); + } + }); + + return true; + } + }); + } }
convertView.setTag(KEY, Integer.valueOf(position)); @@ -103,6 +104,8 @@ public abstract class BaseReorderableAdapter extends BaseAdapter { return convertView; }
+ protected abstract boolean isMovable(); + protected abstract void move(int fromPosition, int toPosition);
protected abstract void bindView(View view, int position); diff --git a/app/src/main/java/org/fedorahosted/freeotp/Token.java b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java similarity index 95% rename from app/src/main/java/org/fedorahosted/freeotp/Token.java rename to app/src/main/java/org/fedorahosted/freeotp/InternalToken.java index 98febfb..bec97de 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/Token.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java @@ -33,7 +33,7 @@ import android.net.Uri; import com.google.android.apps.authenticator.Base32String; import com.google.android.apps.authenticator.Base32String.DecodingException;
-public class Token { +public class InternalToken { public static class TokenUriInvalidException extends Exception { private static final long serialVersionUID = -1108624734612362345L; } @@ -56,7 +56,7 @@ public class Token { private long counter; private int period;
- private Token(Uri uri, boolean internal) throws TokenUriInvalidException { + private InternalToken(Uri uri, boolean internal) throws TokenUriInvalidException { if (!uri.getScheme().equals("otpauth")) throw new TokenUriInvalidException();
@@ -182,15 +182,15 @@ public class Token { return ""; }
- public Token(String uri, boolean internal) throws TokenUriInvalidException { + public InternalToken(String uri, boolean internal) throws TokenUriInvalidException { this(Uri.parse(uri), internal); }
- public Token(Uri uri) throws TokenUriInvalidException { + public InternalToken(Uri uri) throws TokenUriInvalidException { this(uri, false); }
- public Token(String uri) throws TokenUriInvalidException { + public InternalToken(String uri) throws TokenUriInvalidException { this(Uri.parse(uri)); }
@@ -303,4 +303,8 @@ public class Token {
return null; } + + public boolean isInternal() { + return true; + } } diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java similarity index 65% rename from app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java rename to app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java index 3d450f9..78af8c8 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java @@ -1,11 +1,28 @@ +/* + * FreeOTP + * + * Authors: Nathaniel McCallum npmccallum@redhat.com + * + * Copyright (C) 2013 Nathaniel McCallum, Red Hat + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.fedorahosted.freeotp;
import java.lang.reflect.Type; import java.util.LinkedList; import java.util.List;
-import org.fedorahosted.freeotp.Token.TokenUriInvalidException; - import android.content.Context; import android.content.SharedPreferences; import android.net.Uri; @@ -15,7 +32,7 @@ import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; import com.google.gson.reflect.TypeToken;
-public class TokenPersistence { +public class InternalTokenPersistence { private static final String NAME = "tokens"; private static final String ORDER = "tokenOrder"; private final SharedPreferences prefs; @@ -32,12 +49,12 @@ public class TokenPersistence { return prefs.edit().putString(ORDER, gson.toJson(order)); }
- public static Token addWithToast(Context ctx, String uri) { + public InternalToken addWithToast(Context ctx, String uri) { try { - Token token = new Token(uri); - new TokenPersistence(ctx).add(token); + InternalToken token = new InternalToken(uri); + add(token); return token; - } catch (TokenUriInvalidException e) { + } catch (InternalToken.TokenUriInvalidException e) { Toast.makeText(ctx, R.string.invalid_token, Toast.LENGTH_SHORT).show(); e.printStackTrace(); } @@ -45,7 +62,7 @@ public class TokenPersistence { return null; }
- public TokenPersistence(Context ctx) { + public InternalTokenPersistence(Context ctx) { prefs = ctx.getApplicationContext().getSharedPreferences(NAME, Context.MODE_PRIVATE); gson = new Gson(); } @@ -54,17 +71,17 @@ public class TokenPersistence { return getTokenOrder().size(); }
- public Token get(int position) { + public InternalToken get(int position) { String key = getTokenOrder().get(position); String str = prefs.getString(key, null);
try { - return gson.fromJson(str, Token.class); + return gson.fromJson(str, InternalToken.class); } catch (JsonSyntaxException jse) { // Backwards compatibility for URL-based persistence. try { - return new Token(str, true); - } catch (TokenUriInvalidException tuie) { + return new InternalToken(str, true); + } catch (InternalToken.TokenUriInvalidException tuie) { tuie.printStackTrace(); } } @@ -72,7 +89,7 @@ public class TokenPersistence { return null; }
- public void add(Token token) throws TokenUriInvalidException { + public void add(InternalToken token) throws InternalToken.TokenUriInvalidException { String key = token.getID();
if (prefs.contains(key)) @@ -103,7 +120,11 @@ public class TokenPersistence { setTokenOrder(order).remove(key).apply(); }
- public void save(Token token) { + public void save(InternalToken token) { prefs.edit().putString(token.getID(), gson.toJson(token)).apply(); } + + public boolean isMovable() { + return true; + } } diff --git a/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java b/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java index f1cc81b..1b9812d 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java @@ -53,6 +53,7 @@ import android.widget.GridView;
public class MainActivity extends Activity implements OnMenuItemClickListener { private TokenAdapter mTokenAdapter; + private InternalTokenPersistence mTokenPersistence; private DataSetObserver mDataSetObserver;
@Override @@ -60,8 +61,8 @@ public class MainActivity extends Activity implements OnMenuItemClickListener { super.onCreate(savedInstanceState); onNewIntent(getIntent()); setContentView(R.layout.main); - - mTokenAdapter = new TokenAdapter(this); + mTokenPersistence = new InternalTokenPersistence(this); + mTokenAdapter = new TokenAdapter(this, mTokenPersistence); ((GridView) findViewById(R.id.grid)).setAdapter(mTokenAdapter);
// Don't permit screenshots since these might contain OTP codes. @@ -134,6 +135,6 @@ public class MainActivity extends Activity implements OnMenuItemClickListener {
Uri uri = intent.getData(); if (uri != null) - TokenPersistence.addWithToast(this, uri.toString()); + mTokenPersistence.addWithToast(this, uri.toString()); } } diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java index e7adf36..b2f035e 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java @@ -39,13 +39,13 @@ import java.util.HashMap; import java.util.Map;
public class TokenAdapter extends BaseReorderableAdapter { - private final TokenPersistence mTokenPersistence; + private final InternalTokenPersistence mTokenPersistence; private final LayoutInflater mLayoutInflater; private final ClipboardManager mClipMan; private final Map<String, TokenCode> mTokenCodes;
- public TokenAdapter(Context ctx) { - mTokenPersistence = new TokenPersistence(ctx); + public TokenAdapter(Context ctx, InternalTokenPersistence tokenPersistence) { + mTokenPersistence = tokenPersistence; mLayoutInflater = (LayoutInflater) ctx.getSystemService(Context.LAYOUT_INFLATER_SERVICE); mClipMan = (ClipboardManager) ctx.getSystemService(Context.CLIPBOARD_SERVICE); mTokenCodes = new HashMap<String, TokenCode>(); @@ -68,7 +68,7 @@ public class TokenAdapter extends BaseReorderableAdapter { }
@Override - public Token getItem(int position) { + public InternalToken getItem(int position) { return mTokenPersistence.get(position); }
@@ -87,9 +87,11 @@ public class TokenAdapter extends BaseReorderableAdapter { protected void bindView(View view, final int position) { final Context ctx = view.getContext(); TokenLayout tl = (TokenLayout) view; - Token token = getItem(position); + InternalToken token = getItem(position);
- tl.bind(token, R.menu.token, new PopupMenu.OnMenuItemClickListener() { + int menu = token.isInternal() ? R.menu.editable_token : R.menu.token; + + tl.bind(token, menu, new PopupMenu.OnMenuItemClickListener() { @Override public boolean onMenuItemClick(MenuItem item) { Intent i; @@ -115,10 +117,10 @@ public class TokenAdapter extends BaseReorderableAdapter { tl.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { - TokenPersistence tp = new TokenPersistence(ctx); + InternalTokenPersistence tp = TokenAdapter.this.mTokenPersistence;
// Increment the token. - Token token = tp.get(position); + InternalToken token = tp.get(position); TokenCode codes = token.generateCodes(); tp.save(token);
@@ -142,4 +144,9 @@ public class TokenAdapter extends BaseReorderableAdapter { protected View createView(ViewGroup parent, int type) { return mLayoutInflater.inflate(R.layout.token, parent, false); } + + @Override + protected boolean isMovable() { + return mTokenPersistence.isMovable(); + } } diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java b/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java index f2dbbd0..a4660ad 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java @@ -26,7 +26,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru private PopupMenu mPopupMenu;
private TokenCode mCodes; - private Token.TokenType mType; + private InternalToken.TokenType mType; private String mPlaceholder; private long mStartTime;
@@ -58,7 +58,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru mMenu.setOnClickListener(this); }
- public void bind(Token token, int menu, PopupMenu.OnMenuItemClickListener micl) { + public void bind(InternalToken token, int menu, PopupMenu.OnMenuItemClickListener micl) { mCodes = null;
// Setup menu. @@ -106,7 +106,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru view.startAnimation(a); }
- public void start(Token.TokenType type, TokenCode codes, boolean animate) { + public void start(InternalToken.TokenType type, TokenCode codes, boolean animate) { mCodes = codes; mType = type;
@@ -147,7 +147,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru // Update the fields mCode.setText(code); mProgressInner.setProgress(mCodes.getCurrentProgress()); - if (mType != Token.TokenType.HOTP) + if (mType != InternalToken.TokenType.HOTP) mProgressOuter.setProgress(mCodes.getTotalProgress());
postDelayed(this, 100); diff --git a/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java b/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java index 6c6d3dd..d996a4d 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java @@ -25,7 +25,7 @@ import java.net.URLEncoder; import java.util.Locale;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.TokenPersistence; +import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.app.Activity; import android.content.Intent; @@ -130,7 +130,7 @@ public class AddActivity extends Activity implements View.OnClickListener, Compo }
// Add the token - if (TokenPersistence.addWithToast(this, uri) != null) + if (new InternalTokenPersistence(this).addWithToast(this, uri) != null) finish();
break; diff --git a/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java b/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java index 263b267..68fa5cf 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java @@ -23,8 +23,8 @@ package org.fedorahosted.freeotp.add; import java.util.List;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.Token; -import org.fedorahosted.freeotp.TokenPersistence; +import org.fedorahosted.freeotp.InternalToken; +import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.annotation.TargetApi; import android.app.Activity; @@ -98,7 +98,7 @@ public class ScanActivity extends Activity implements SurfaceHolder.Callback { @Override protected void onPostExecute(String result) { super.onPostExecute(result); - Token token = TokenPersistence.addWithToast(ScanActivity.this, result); + InternalToken token = new InternalTokenPersistence(ScanActivity.this).addWithToast(ScanActivity.this, result); if (token == null || token.getImage() == null) { finish(); return; diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java index 4da2013..edad048 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java @@ -1,8 +1,8 @@ package org.fedorahosted.freeotp.edit;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.Token; -import org.fedorahosted.freeotp.TokenPersistence; +import org.fedorahosted.freeotp.InternalToken; +import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.os.Bundle; import android.view.View; @@ -12,12 +12,13 @@ import android.widget.TextView; import com.squareup.picasso.Picasso;
public class DeleteActivity extends BaseActivity { + private InternalTokenPersistence mTokenPersistence; @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setContentView(R.layout.delete); - - Token token = new TokenPersistence(this).get(getPosition()); + mTokenPersistence = new InternalTokenPersistence(this); + InternalToken token = mTokenPersistence.get(getPosition()); ((TextView) findViewById(R.id.issuer)).setText(token.getIssuer()); ((TextView) findViewById(R.id.label)).setText(token.getLabel()); Picasso.with(this) @@ -35,7 +36,7 @@ public class DeleteActivity extends BaseActivity { findViewById(R.id.delete).setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { - new TokenPersistence(DeleteActivity.this).delete(getPosition()); + mTokenPersistence.delete(getPosition()); finish(); } }); diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java index 575d1ad..5f61379 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java @@ -21,8 +21,8 @@ package org.fedorahosted.freeotp.edit;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.Token; -import org.fedorahosted.freeotp.TokenPersistence; +import org.fedorahosted.freeotp.InternalToken; +import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.content.Intent; import android.net.Uri; @@ -50,6 +50,7 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl private Uri mImageCurrent; private Uri mImageDefault; private Uri mImageDisplay; + private InternalTokenPersistence mTokenPersistence;
private void showImage(Uri uri) { mImageDisplay = uri; @@ -73,7 +74,8 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl setContentView(R.layout.edit);
// Get token values. - Token token = new TokenPersistence(this).get(getPosition()); + mTokenPersistence = new InternalTokenPersistence(this); + InternalToken token = mTokenPersistence.get(getPosition()); mIssuerCurrent = token.getIssuer(); mLabelCurrent = token.getLabel(); mImageCurrent = token.getImage(); @@ -150,12 +152,11 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl break;
case R.id.save: - TokenPersistence tp = new TokenPersistence(this); - Token token = tp.get(getPosition()); + InternalToken token = mTokenPersistence.get(getPosition()); token.setIssuer(mIssuer.getText().toString()); token.setLabel(mLabel.getText().toString()); token.setImage(mImageDisplay); - tp.save(token); + mTokenPersistence.save(token);
case R.id.cancel: finish(); diff --git a/app/src/main/res/menu/editable_token.xml b/app/src/main/res/menu/editable_token.xml new file mode 100644 index 0000000..a5a28e9 --- /dev/null +++ b/app/src/main/res/menu/editable_token.xml @@ -0,0 +1,33 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + - FreeOTP + - + - Authors: Nathaniel McCallum <npmccallum@redhat.com> + - + - Copyright (C) 2014 Nathaniel McCallum, Red Hat + - + - Licensed under the Apache License, Version 2.0 (the "License"); + - you may not use this file except in compliance with the License. + - You may obtain a copy of the License at + - + - http://www.apache.org/licenses/LICENSE-2.0 + - + - Unless required by applicable law or agreed to in writing, software + - distributed under the License is distributed on an "AS IS" BASIS, + - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + - See the License for the specific language governing permissions and + - limitations under the License. + --> +<menu xmlns:android="http://schemas.android.com/apk/res/android" > + <item + android:id="@+id/action_edit" + android:icon="@drawable/ic_action_edit" + android:title="@string/edit" + /> + + <item + android:id="@+id/action_delete" + android:icon="@android:drawable/ic_menu_delete" + android:title="@string/delete" + /> +</menu> diff --git a/app/src/main/res/menu/token.xml b/app/src/main/res/menu/token.xml index a5a28e9..0af483c 100644 --- a/app/src/main/res/menu/token.xml +++ b/app/src/main/res/menu/token.xml @@ -20,12 +20,6 @@ --> <menu xmlns:android="http://schemas.android.com/apk/res/android" > <item - android:id="@+id/action_edit" - android:icon="@drawable/ic_action_edit" - android:title="@string/edit" - /> - - <item android:id="@+id/action_delete" android:icon="@android:drawable/ic_menu_delete" android:title="@string/delete"
- Add interface for Token - Add interface for TokenPersistence - Update other classes to use interface to a greater extent --- .../org/fedorahosted/freeotp/InternalToken.java | 10 +--- .../freeotp/InternalTokenPersistence.java | 4 +- .../main/java/org/fedorahosted/freeotp/Token.java | 56 ++++++++++++++++++++++ .../org/fedorahosted/freeotp/TokenAdapter.java | 12 ++--- .../java/org/fedorahosted/freeotp/TokenLayout.java | 8 ++-- .../org/fedorahosted/freeotp/TokenPersistence.java | 40 ++++++++++++++++ 6 files changed, 109 insertions(+), 21 deletions(-) create mode 100644 app/src/main/java/org/fedorahosted/freeotp/Token.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java
diff --git a/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java index bec97de..103f21a 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java @@ -33,15 +33,7 @@ import android.net.Uri; import com.google.android.apps.authenticator.Base32String; import com.google.android.apps.authenticator.Base32String.DecodingException;
-public class InternalToken { - public static class TokenUriInvalidException extends Exception { - private static final long serialVersionUID = -1108624734612362345L; - } - - public static enum TokenType { - HOTP, TOTP - } - +public class InternalToken implements Token { private String issuerInt; private String issuerExt; private String issuerAlt; diff --git a/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java index 78af8c8..883a139 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java @@ -32,7 +32,7 @@ import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; import com.google.gson.reflect.TypeToken;
-public class InternalTokenPersistence { +public class InternalTokenPersistence implements TokenPersistence { private static final String NAME = "tokens"; private static final String ORDER = "tokenOrder"; private final SharedPreferences prefs; @@ -120,7 +120,7 @@ public class InternalTokenPersistence { setTokenOrder(order).remove(key).apply(); }
- public void save(InternalToken token) { + public void save(Token token) { prefs.edit().putString(token.getID(), gson.toJson(token)).apply(); }
diff --git a/app/src/main/java/org/fedorahosted/freeotp/Token.java b/app/src/main/java/org/fedorahosted/freeotp/Token.java new file mode 100644 index 0000000..2709bd2 --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/Token.java @@ -0,0 +1,56 @@ +/* + * FreeOTP + * + * Authors: Nathaniel McCallum npmccallum@redhat.com + * + * Copyright (C) 2013 Nathaniel McCallum, Red Hat + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import android.net.Uri; + +public interface Token { + public static class TokenUriInvalidException extends Exception { + private static final long serialVersionUID = -1108624734612362345L; + } + + public static enum TokenType { + HOTP, TOTP + } + + public String getID(); + + public String getIssuer(); + + public void setIssuer(String issuer); + + public String getLabel(); + + public void setLabel(String label); + + public int getDigits(); + + // NOTE: This may change internal data. You MUST save the token immediately. + public TokenCode generateCodes(); + + public TokenType getType(); + + public Uri getImage(); + + public void setImage(Uri image); + + public boolean isInternal(); +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java index b2f035e..2e521f8 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java @@ -39,12 +39,12 @@ import java.util.HashMap; import java.util.Map;
public class TokenAdapter extends BaseReorderableAdapter { - private final InternalTokenPersistence mTokenPersistence; + private final TokenPersistence mTokenPersistence; private final LayoutInflater mLayoutInflater; private final ClipboardManager mClipMan; private final Map<String, TokenCode> mTokenCodes;
- public TokenAdapter(Context ctx, InternalTokenPersistence tokenPersistence) { + public TokenAdapter(Context ctx, TokenPersistence tokenPersistence) { mTokenPersistence = tokenPersistence; mLayoutInflater = (LayoutInflater) ctx.getSystemService(Context.LAYOUT_INFLATER_SERVICE); mClipMan = (ClipboardManager) ctx.getSystemService(Context.CLIPBOARD_SERVICE); @@ -68,7 +68,7 @@ public class TokenAdapter extends BaseReorderableAdapter { }
@Override - public InternalToken getItem(int position) { + public Token getItem(int position) { return mTokenPersistence.get(position); }
@@ -87,7 +87,7 @@ public class TokenAdapter extends BaseReorderableAdapter { protected void bindView(View view, final int position) { final Context ctx = view.getContext(); TokenLayout tl = (TokenLayout) view; - InternalToken token = getItem(position); + Token token = getItem(position);
int menu = token.isInternal() ? R.menu.editable_token : R.menu.token;
@@ -117,10 +117,10 @@ public class TokenAdapter extends BaseReorderableAdapter { tl.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { - InternalTokenPersistence tp = TokenAdapter.this.mTokenPersistence; + TokenPersistence tp = TokenAdapter.this.mTokenPersistence;
// Increment the token. - InternalToken token = tp.get(position); + Token token = tp.get(position); TokenCode codes = token.generateCodes(); tp.save(token);
diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java b/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java index a4660ad..f2dbbd0 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenLayout.java @@ -26,7 +26,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru private PopupMenu mPopupMenu;
private TokenCode mCodes; - private InternalToken.TokenType mType; + private Token.TokenType mType; private String mPlaceholder; private long mStartTime;
@@ -58,7 +58,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru mMenu.setOnClickListener(this); }
- public void bind(InternalToken token, int menu, PopupMenu.OnMenuItemClickListener micl) { + public void bind(Token token, int menu, PopupMenu.OnMenuItemClickListener micl) { mCodes = null;
// Setup menu. @@ -106,7 +106,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru view.startAnimation(a); }
- public void start(InternalToken.TokenType type, TokenCode codes, boolean animate) { + public void start(Token.TokenType type, TokenCode codes, boolean animate) { mCodes = codes; mType = type;
@@ -147,7 +147,7 @@ public class TokenLayout extends FrameLayout implements View.OnClickListener, Ru // Update the fields mCode.setText(code); mProgressInner.setProgress(mCodes.getCurrentProgress()); - if (mType != InternalToken.TokenType.HOTP) + if (mType != Token.TokenType.HOTP) mProgressOuter.setProgress(mCodes.getTotalProgress());
postDelayed(this, 100); diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java new file mode 100644 index 0000000..889b9fb --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java @@ -0,0 +1,40 @@ +/* + * FreeOTP + * + * Authors: Nathaniel McCallum npmccallum@redhat.com + * + * Copyright (C) 2013 Nathaniel McCallum, Red Hat + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.fedorahosted.freeotp; + +import java.lang.reflect.Type; +import java.util.LinkedList; +import java.util.List; + +import android.content.Context; +import android.content.SharedPreferences; + +import com.google.gson.reflect.TypeToken; + +public interface TokenPersistence { + public Token addWithToast(Context ctx, String uri); + public int length(); + public Token get(int position); + public void add(InternalToken token) throws Token.TokenUriInvalidException; + public void move(int fromPosition, int toPosition); + public void delete(int position); + public void save(Token token); + public boolean isMovable(); +}
- Include OTP NFC client library - Add NfcHelpers to implement common NFC tasks - Add ExternalToken implementation of Token - Add ExternalTokenPersistence implementation of TokenPersistence - Update interfaces to include new functionality - Update other classes to support new interfaces --- app/build.gradle | 6 + app/src/main/AndroidManifest.xml | 2 + .../org/fedorahosted/freeotp/ExternalToken.java | 130 +++++++++++++++++++++ .../freeotp/ExternalTokenPersistence.java | 129 ++++++++++++++++++++ .../org/fedorahosted/freeotp/InternalToken.java | 16 +++ .../freeotp/InternalTokenPersistence.java | 20 +++- .../java/org/fedorahosted/freeotp/NfcHelpers.java | 60 ++++++++++ .../main/java/org/fedorahosted/freeotp/Token.java | 9 +- .../org/fedorahosted/freeotp/TokenAdapter.java | 28 +++-- .../org/fedorahosted/freeotp/TokenPersistence.java | 14 +-- .../fedorahosted/freeotp/edit/DeleteActivity.java | 4 +- .../fedorahosted/freeotp/edit/EditActivity.java | 4 +- app/src/main/res/values/strings.xml | 3 +- 13 files changed, 389 insertions(+), 36 deletions(-) create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java
diff --git a/app/build.gradle b/app/build.gradle index 0faef46..58499f3 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -1,9 +1,15 @@ +apply plugin: 'maven' apply plugin: 'com.android.application'
+repositories { + maven { url 'http://releases.marmeladburk.fidesmo.com/' } +} + dependencies { compile 'com.google.zxing:core:3.1.0' compile 'com.google.code.gson:gson:2.3' compile 'com.squareup.picasso:picasso:2.3.3' + compile 'com.fidesmo:otp-client:0.0.2' }
android { diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index fd6c7a5..0eeaa19 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -38,9 +38,11 @@
<uses-permission android:name="android.permission.CAMERA" /> <uses-permission android:name="android.permission.INTERNET" /> + <uses-permission android:name="android.permission.NFC" /> <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" /> <uses-feature android:name="android.hardware.camera" android:required="false" /> <uses-feature android:name="android.hardware.camera.autofocus" android:required="false" /> + <uses-feature android:name="android.hardware.nfc" android:required="false" />
<application android:allowBackup="true" diff --git a/app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java b/app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java new file mode 100644 index 0000000..0aaec1f --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java @@ -0,0 +1,130 @@ +/* + * FreeOTP + * + * Authors: Petter Arvidsson petter.arvidsson@fidesmo.com + * + * Copyright (C) 2014 Petter Arvidsson, Fidesmo AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import java.io.IOException; +import android.net.Uri; +import com.fidesmo.oath.hardware.HardwareToken; +import com.fidesmo.oath.hardware.TokenMeta; + +public class ExternalToken implements Token { + private String issuer; + private String label; + private String id; + private TokenType type; + private int digits; + private String algorithm; + private int period = 30; + + public ExternalToken(TokenMeta meta) { + this.id = meta.getLabel(); + String s[] = meta.getLabel().split(":", 2); + if(s.length == 1) { + label = s[0]; + } else { + issuer = s[0]; + label = s[1]; + } + + switch(meta.getType()) { + case TOTP: + this.type = TokenType.TOTP; + break; + case HOTP: + this.type = TokenType.HOTP; + break; + } + switch(meta.getAlgorithm()) { + case SHA1: + algorithm = "SHA1"; + break; + case SHA256: + algorithm = "SHA256"; + break; + } + this.digits = meta.getDigits(); + } + + public String getID() { + return id; + } + + public String getIssuer() { + return issuer != null ? issuer : ""; + } + + public void setIssuer(String issuer) { + throw new UnsupportedOperationException(); + } + + public String getLabel() { + return label; + } + + public void setLabel(String label) { + throw new UnsupportedOperationException(); + } + + public int getDigits() { + return digits; + } + + private TokenCode generateTotpCode(HardwareToken token, long counter, TokenCode next) throws IOException { + return new TokenCode(token.readTotpCode(id, counter), counter * period * 1000, (counter + 1) * period * 1000, next); + } + + private TokenCode generateHotpCode(HardwareToken token, long cur) throws IOException { + return new TokenCode(token.readHotpCode(id), cur, cur + (period * 1000)); + } + + public TokenCode generateCodes(HardwareToken token) throws IOException { + long cur = System.currentTimeMillis(); + switch(type) { + case TOTP: + long counter = cur / 1000 / period; + return generateTotpCode(token, counter, generateTotpCode(token, counter + 1, null)); + case HOTP: + return generateHotpCode(token, cur); + default: + throw new IOException("Invalid token type"); + } + } + + public TokenType getType() { + return type; + } + + public String getAlgorithm() { + return algorithm; + } + + public Uri getImage() { + return null; + } + + public void setImage(Uri image) { + throw new UnsupportedOperationException(); + } + + public boolean isInternal() { + return false; + } +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java new file mode 100644 index 0000000..d9ded71 --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java @@ -0,0 +1,129 @@ +/* + * FreeOTP + * + * Authors: Petter Arvidsson petter.arvidsson@fidesmo.com + * + * Copyright (C) 2014 Petter Arvidsson, Fidesmo AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import java.util.*; +import java.io.IOException; +import com.fidesmo.oath.hardware.*; +import com.yubico.yubioath.model.*; +import android.content.Context; +import android.content.SharedPreferences; +import android.widget.Toast; +import android.nfc.tech.IsoDep; + +public class ExternalTokenPersistence implements TokenPersistence { + HardwareToken store; + byte[] id; + List<TokenMeta> tokens; + + public ExternalTokenPersistence(IsoDep card) throws IOException{ + store = new YkneoOath(card); + long timestamp = (System.currentTimeMillis() / 1000 + 10) / 30; + store.open(); + tokens = store.getTokens(timestamp); + store.close(); + } + + public Token addWithToast(Context ctx, String uri) { + try { + InternalToken token = new InternalToken(uri); + add(token); + return token; + } catch (Token.TokenUriInvalidException e) { + Toast.makeText(ctx, R.string.invalid_token, Toast.LENGTH_SHORT).show(); + } catch (IOException e) { + Toast.makeText(ctx, R.string.failed_to_read_external_token, Toast.LENGTH_SHORT).show(); + } + return null; + + } + + public int length() { + return tokens.size(); + } + + public Token get(int position) { + return new ExternalToken(tokens.get(position)); + } + + private static TokenMeta tokenMeta(InternalToken token) { + TokenMeta.Type type = null; + switch(token.getType()) { + case HOTP: + type = TokenMeta.Type.HOTP; + break; + case TOTP: + type = TokenMeta.Type.TOTP; + break; + } + + if(token.getAlgorithm().equals("SHA1")) { + return new TokenMeta(token.getID(), token.getDigits(), type, TokenMeta.Algorithm.SHA1); + } else if(token.getAlgorithm().equals("SHA256")) { + return new TokenMeta(token.getID(), token.getDigits(), type, TokenMeta.Algorithm.SHA256); + } else { + return null; + } + } + + public void add(InternalToken token) throws Token.TokenUriInvalidException, IOException { + try { + store.open(); + if(token.getType() == Token.TokenType.HOTP) + store.storeCode(tokenMeta(token), token.getSecret(), (int)token.getCounter()); + else + store.storeCode(tokenMeta(token), token.getSecret(), token.getPeriod()); + long timestamp = (System.currentTimeMillis() / 1000 + 10) / 30; + tokens = store.getTokens(timestamp); + } finally { + store.close(); + } + } + + public void move(int fromPosition, int toPosition) { + throw new UnsupportedOperationException(); + } + + public void delete(int position) throws IOException { + try { + Token token = new ExternalToken(tokens.get(position)); + store.open(); + store.deleteCode(token.getID()); + long timestamp = (System.currentTimeMillis() / 1000 + 10) / 30; + tokens = store.getTokens(timestamp); + } finally { + store.close(); + } + } + + public TokenCode generateCodes(int position) throws IOException { + try { + store.open(); + return new ExternalToken(tokens.get(position)).generateCodes(store); + } finally { + store.close(); + } + } + + public boolean isMovable() { + return false; + } +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java index 103f21a..5486c14 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalToken.java @@ -296,6 +296,22 @@ public class InternalToken implements Token { return null; }
+ public byte[] getSecret() { + return secret; + } + + public long getCounter() { + return counter; + } + + public int getPeriod() { + return period; + } + + public String getAlgorithm() { + return algo; + } + public boolean isInternal() { return true; } diff --git a/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java index 883a139..81dd73e 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java @@ -54,7 +54,7 @@ public class InternalTokenPersistence implements TokenPersistence { InternalToken token = new InternalToken(uri); add(token); return token; - } catch (InternalToken.TokenUriInvalidException e) { + } catch (Token.TokenUriInvalidException e) { Toast.makeText(ctx, R.string.invalid_token, Toast.LENGTH_SHORT).show(); e.printStackTrace(); } @@ -71,7 +71,7 @@ public class InternalTokenPersistence implements TokenPersistence { return getTokenOrder().size(); }
- public InternalToken get(int position) { + private InternalToken getInternalToken(int position) { String key = getTokenOrder().get(position); String str = prefs.getString(key, null);
@@ -81,15 +81,18 @@ public class InternalTokenPersistence implements TokenPersistence { // Backwards compatibility for URL-based persistence. try { return new InternalToken(str, true); - } catch (InternalToken.TokenUriInvalidException tuie) { + } catch (Token.TokenUriInvalidException tuie) { tuie.printStackTrace(); } } - return null; }
- public void add(InternalToken token) throws InternalToken.TokenUriInvalidException { + public Token get(int position) { + return getInternalToken(position); + } + + public void add(InternalToken token) throws Token.TokenUriInvalidException { String key = token.getID();
if (prefs.contains(key)) @@ -124,6 +127,13 @@ public class InternalTokenPersistence implements TokenPersistence { prefs.edit().putString(token.getID(), gson.toJson(token)).apply(); }
+ public TokenCode generateCodes(int position) { + InternalToken token = getInternalToken(position); + TokenCode codes = token.generateCodes(); + save(token); + return codes; + } + public boolean isMovable() { return true; } diff --git a/app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java b/app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java new file mode 100644 index 0000000..8a809cb --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java @@ -0,0 +1,60 @@ +/* + * FreeOTP + * + * Authors: Petter Arvidsson petter.arvidsson@fidesmo.com + * + * Copyright (C) 2014 Petter Arvidsson, Fidesmo AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import android.app.Activity; +import android.app.PendingIntent; +import android.content.Intent; +import android.content.IntentFilter; +import android.nfc.NfcAdapter; +import android.nfc.tech.IsoDep; +import android.nfc.Tag; + +public class NfcHelpers { + + public static void enableForegroundDispatch(Activity activity, NfcAdapter adapter) { + Intent intent = activity.getIntent(); + intent.setFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP); + enableForegroundDispatch(activity, adapter, intent); + } + + public static void enableForegroundDispatch(Activity activity, NfcAdapter adapter, Intent intent) { + if(adapter.isEnabled()) { + PendingIntent tagIntent = PendingIntent.getActivity(activity, 0, intent, PendingIntent.FLAG_CANCEL_CURRENT); + IntentFilter iso = new IntentFilter(NfcAdapter.ACTION_TECH_DISCOVERED); + adapter.enableForegroundDispatch(activity, tagIntent, new IntentFilter[]{iso}, + new String[][]{new String[]{IsoDep.class.getName()}}); + } + } + + public static void disableForegroundDispatch(Activity activity, NfcAdapter adapter) { + adapter.disableForegroundDispatch(activity); + } + + public static IsoDep getIsoTag(Intent intent) { + Tag tag = intent.getParcelableExtra(NfcAdapter.EXTRA_TAG); + if(tag != null) { + return IsoDep.get(tag); + } else { + return null; + } + } +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/Token.java b/app/src/main/java/org/fedorahosted/freeotp/Token.java index 2709bd2..eaa8ab1 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/Token.java +++ b/app/src/main/java/org/fedorahosted/freeotp/Token.java @@ -20,9 +20,11 @@
package org.fedorahosted.freeotp;
+import java.io.IOException; +import java.io.Serializable; import android.net.Uri;
-public interface Token { +public interface Token extends Serializable { public static class TokenUriInvalidException extends Exception { private static final long serialVersionUID = -1108624734612362345L; } @@ -43,14 +45,13 @@ public interface Token {
public int getDigits();
- // NOTE: This may change internal data. You MUST save the token immediately. - public TokenCode generateCodes(); - public TokenType getType();
public Uri getImage();
public void setImage(Uri image);
+ public String getAlgorithm(); + public boolean isInternal(); } diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java index 2e521f8..94baaab 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java @@ -37,6 +37,7 @@ import org.fedorahosted.freeotp.edit.EditActivity;
import java.util.HashMap; import java.util.Map; +import java.io.IOException;
public class TokenAdapter extends BaseReorderableAdapter { private final TokenPersistence mTokenPersistence; @@ -121,17 +122,22 @@ public class TokenAdapter extends BaseReorderableAdapter {
// Increment the token. Token token = tp.get(position); - TokenCode codes = token.generateCodes(); - tp.save(token); - - // Copy code to clipboard. - mClipMan.setPrimaryClip(ClipData.newPlainText(null, codes.getCurrentCode())); - Toast.makeText(v.getContext().getApplicationContext(), - R.string.code_copied, - Toast.LENGTH_SHORT).show(); - - mTokenCodes.put(token.getID(), codes); - ((TokenLayout) v).start(token.getType(), codes, true); + try { + TokenCode codes = tp.generateCodes(position); + + // Copy code to clipboard. + mClipMan.setPrimaryClip(ClipData.newPlainText(null, codes.getCurrentCode())); + Toast.makeText(v.getContext().getApplicationContext(), + R.string.code_copied, + Toast.LENGTH_SHORT).show(); + + mTokenCodes.put(token.getID(), codes); + ((TokenLayout) v).start(token.getType(), codes, true); + } catch(IOException e) { + Toast.makeText(v.getContext().getApplicationContext(), + R.string.external_token_needed, + Toast.LENGTH_SHORT).show(); + } } });
diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java b/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java index 889b9fb..84ab32e 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenPersistence.java @@ -19,22 +19,14 @@ */ package org.fedorahosted.freeotp;
-import java.lang.reflect.Type; -import java.util.LinkedList; -import java.util.List; - +import java.io.IOException; import android.content.Context; -import android.content.SharedPreferences; - -import com.google.gson.reflect.TypeToken;
public interface TokenPersistence { - public Token addWithToast(Context ctx, String uri); public int length(); public Token get(int position); - public void add(InternalToken token) throws Token.TokenUriInvalidException; + public void add(InternalToken token) throws Token.TokenUriInvalidException, IOException; public void move(int fromPosition, int toPosition); - public void delete(int position); - public void save(Token token); + public TokenCode generateCodes(int position) throws IOException; public boolean isMovable(); } diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java index edad048..7da16fb 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java @@ -1,7 +1,7 @@ package org.fedorahosted.freeotp.edit;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.InternalToken; +import org.fedorahosted.freeotp.Token; import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.os.Bundle; @@ -18,7 +18,7 @@ public class DeleteActivity extends BaseActivity { super.onCreate(savedInstanceState); setContentView(R.layout.delete); mTokenPersistence = new InternalTokenPersistence(this); - InternalToken token = mTokenPersistence.get(getPosition()); + Token token = mTokenPersistence.get(getPosition()); ((TextView) findViewById(R.id.issuer)).setText(token.getIssuer()); ((TextView) findViewById(R.id.label)).setText(token.getLabel()); Picasso.with(this) diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java index 5f61379..d55a657 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java @@ -75,7 +75,7 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl
// Get token values. mTokenPersistence = new InternalTokenPersistence(this); - InternalToken token = mTokenPersistence.get(getPosition()); + InternalToken token = (InternalToken)mTokenPersistence.get(getPosition()); mIssuerCurrent = token.getIssuer(); mLabelCurrent = token.getLabel(); mImageCurrent = token.getImage(); @@ -152,7 +152,7 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl break;
case R.id.save: - InternalToken token = mTokenPersistence.get(getPosition()); + InternalToken token = (InternalToken)mTokenPersistence.get(getPosition()); token.setIssuer(mIssuer.getText().toString()); token.setLabel(mLabel.getText().toString()); token.setImage(mImageDisplay); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index de39f41..359026f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -37,7 +37,8 @@
<string name="delete_summary">This is your last chance: if you delete this token, it will be gone forever. It will not be disabled on the server.</string> <string name="delete_question">Delete this token?</string> - + <string name="external_token_needed">You need the NFC device to generate a code</string> + <string name="failed_to_read_external_token">Failed to read the NFC device</string> <string-array name="algorithms"> <item>MD5</item> <item>SHA1</item>
- Create an abstract base class for MainActivity - Rename old MainActivity InternalMainActivity - Add ExternalMainActivity to show external tokens - Extend InternalMainActivity to scan for NFC devices and start ExternalMainActivity if presented - Centralize TokenPersistence to *MainActivity - Extend all other activities to execute actions and return their results - Add related strings --- app/src/main/AndroidManifest.xml | 7 +- .../fedorahosted/freeotp/ExternalMainActivity.java | 142 +++++++++++++++++++++ .../fedorahosted/freeotp/InternalMainActivity.java | 126 ++++++++++++++++++ .../org/fedorahosted/freeotp/MainActivity.java | 84 +++++------- .../org/fedorahosted/freeotp/TokenAdapter.java | 21 +-- .../org/fedorahosted/freeotp/add/AddActivity.java | 10 +- .../org/fedorahosted/freeotp/add/BaseActivity.java | 7 + .../org/fedorahosted/freeotp/add/ScanActivity.java | 23 +++- .../fedorahosted/freeotp/edit/BaseActivity.java | 22 ---- .../fedorahosted/freeotp/edit/DeleteActivity.java | 22 +++- .../fedorahosted/freeotp/edit/EditActivity.java | 45 ++++--- app/src/main/res/layout/main.xml | 4 +- app/src/main/res/values/strings.xml | 15 ++- 13 files changed, 402 insertions(+), 126 deletions(-) create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java delete mode 100644 app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 0eeaa19..39a9cec 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -80,7 +80,12 @@ />
<activity - android:name=".MainActivity" + android:name=".ExternalMainActivity" + android:label="@string/nfc_app_name" + /> + + <activity + android:name=".InternalMainActivity" android:label="@string/app_name" android:clearTaskOnLaunch="true" android:launchMode="singleTask" diff --git a/app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java b/app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java new file mode 100644 index 0000000..d357f59 --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java @@ -0,0 +1,142 @@ +/* + * FreeOTP + * + * Authors: Petter Arvidsson petter.arvidsson@fidesmo.com + * + * Copyright (C) 2014 Petter Arvidsson, Fidesmo AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import java.io.IOException; + +import static org.fedorahosted.freeotp.NfcHelpers.*; + +import android.content.Intent; +import android.widget.Toast; +import android.widget.TextView; +import android.widget.GridView; +import android.view.View; +import android.os.Bundle; +import android.nfc.NfcAdapter; +import android.nfc.tech.IsoDep; +import android.app.AlertDialog; +import android.content.DialogInterface; + +public class ExternalMainActivity extends MainActivity { + private NfcAdapter mAdapter; + int tokenToDelete = -1; + String tokenToAdd = null; + AlertDialog dialog; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + mAdapter = NfcAdapter.getDefaultAdapter(this); + onNewIntent(getIntent()); + } + + @Override + protected void onResume() { + super.onResume(); + Intent intent = new Intent(this, ExternalMainActivity.class); + intent.setFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP); + enableForegroundDispatch(this, mAdapter, intent); + } + + @Override + protected void onPause() { + super.onPause(); + disableForegroundDispatch(this, mAdapter); + } + + protected void deleteToken(int position) { + tokenToDelete = position; + dialog = new AlertDialog.Builder(this) + .setTitle(R.string.tap_dialog_title) + .setMessage(R.string.tap_to_delete) + .setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() { + public void onClick(DialogInterface dialog, int wich) { + tokenToDelete = -1; + } + }) + .show(); + } + + protected void addToken(String tokenUri) { + tokenToAdd = tokenUri; + dialog = new AlertDialog.Builder(this) + .setTitle(R.string.tap_dialog_title) + .setMessage(R.string.tap_to_add) + .setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() { + public void onClick(DialogInterface dialog, int wich) { + tokenToAdd = null; + } + }) + .show(); + } + + private void setUpTokenList(IsoDep tag) { + ExternalTokenPersistence persistence = null; + TextView view = (TextView)findViewById(android.R.id.empty); + if(tag != null) { + try { + persistence = new ExternalTokenPersistence(tag); + } catch(IOException ioe) { + Toast.makeText(this, R.string.failed_to_read_external_token, Toast.LENGTH_SHORT).show(); + } + } + + if(persistence != null) { + if(dialog != null) dialog.cancel(); + view.setText(R.string.no_keys); + getActionBar().setBackgroundDrawable(getResources().getDrawable(android.R.color.black)); + /* + * The following two lines are a fix for the following bug: + * http://stackoverflow.com/questions/17076958/change-actionbar-color-programma... + */ + getActionBar().setDisplayShowTitleEnabled(false); + getActionBar().setDisplayShowTitleEnabled(true); + if(tokenToDelete != -1) { + try { + persistence.delete(tokenToDelete); + } catch(IOException ioe) { + Toast.makeText(this, R.string.failed_to_read_external_token, Toast.LENGTH_SHORT).show(); + } + tokenToDelete = -1; + } else if(tokenToAdd != null) { + persistence.addWithToast(this, tokenToAdd); + tokenToAdd = null; + } + + TokenAdapter adapter = new TokenAdapter(this, persistence); + ((GridView) findViewById(R.id.grid)).setAdapter(adapter); + if (adapter.getCount() == 0) + view.setVisibility(View.VISIBLE); + else + view.setVisibility(View.GONE); + } else { + view.setText(R.string.tap_external_token); + view.setVisibility(View.VISIBLE); + } + } + + @Override + protected void onNewIntent(Intent intent) { + super.onNewIntent(intent); + setUpTokenList(getIsoTag(intent)); + } + +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java b/app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java new file mode 100644 index 0000000..e6c3021 --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java @@ -0,0 +1,126 @@ +/* + * FreeOTP + * + * Authors: Nathaniel McCallum npmccallum@redhat.com + * + * Copyright (C) 2013 Nathaniel McCallum, Red Hat + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Portions Copyright 2009 ZXing authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.fedorahosted.freeotp; + +import static org.fedorahosted.freeotp.NfcHelpers.*; +import org.fedorahosted.freeotp.edit.EditActivity; + +import android.app.Activity; +import android.content.Intent; +import android.database.DataSetObserver; +import android.net.Uri; +import android.os.Bundle; +import android.nfc.NfcAdapter; +import android.nfc.tech.IsoDep; +import android.widget.GridView; +import android.view.View; +import android.widget.Toast; + +public class InternalMainActivity extends MainActivity { + private InternalTokenPersistence mTokenPersistence; + private TokenAdapter mTokenAdapter; + private DataSetObserver mDataSetObserver; + private NfcAdapter mAdapter; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + mAdapter = NfcAdapter.getDefaultAdapter(this); + mTokenPersistence = new InternalTokenPersistence(this); + mTokenAdapter = new TokenAdapter(this, mTokenPersistence); + ((GridView) findViewById(R.id.grid)).setAdapter(mTokenAdapter); + + mDataSetObserver = new DataSetObserver() { + @Override + public void onChanged() { + super.onChanged(); + if (mTokenAdapter.getCount() == 0) + findViewById(android.R.id.empty).setVisibility(View.VISIBLE); + else + findViewById(android.R.id.empty).setVisibility(View.GONE); + } + }; + mTokenAdapter.registerDataSetObserver(mDataSetObserver); + } + + @Override + protected void onResume() { + super.onResume(); + mTokenAdapter.notifyDataSetChanged(); + enableForegroundDispatch(this, mAdapter, new Intent(this, ExternalMainActivity.class)); + } + + @Override + protected void onPause() { + super.onPause(); + mTokenAdapter.notifyDataSetChanged(); + disableForegroundDispatch(this, mAdapter); + } + + @Override + protected void onDestroy() { + super.onDestroy(); + mTokenAdapter.unregisterDataSetObserver(mDataSetObserver); + } + + @Override + protected void onNewIntent(Intent intent) { + super.onNewIntent(intent); + Uri uri = intent.getData(); + if (uri != null) { + mTokenPersistence.addWithToast(this, uri.toString()); + } + } + + protected void deleteToken(int position) { + mTokenPersistence.delete(position); + } + + protected void addToken(String tokenUri) { + mTokenPersistence.addWithToast(this, tokenUri); + } + + @Override + protected void onActivityResult(int requestCode, int resultCode, Intent data) { + super.onActivityResult(requestCode, resultCode, data); + if(requestCode == EDIT_REQUEST_CODE && resultCode == RESULT_OK) { + InternalToken token = (InternalToken)data.getSerializableExtra(EditActivity.EXTRA_TOKEN); + mTokenPersistence.save(token); + } + } + +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java b/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java index 1b9812d..968e140 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/MainActivity.java @@ -36,67 +36,36 @@
package org.fedorahosted.freeotp;
-import org.fedorahosted.freeotp.add.AddActivity; -import org.fedorahosted.freeotp.add.ScanActivity; +import java.io.IOException;
+import org.fedorahosted.freeotp.add.ScanActivity; +import org.fedorahosted.freeotp.add.AddActivity; +import org.fedorahosted.freeotp.add.BaseActivity; +import org.fedorahosted.freeotp.edit.DeleteActivity; import android.app.Activity; import android.content.Intent; -import android.database.DataSetObserver; import android.net.Uri; import android.os.Bundle; import android.view.Menu; import android.view.MenuItem; import android.view.MenuItem.OnMenuItemClickListener; -import android.view.View; import android.view.WindowManager.LayoutParams; -import android.widget.GridView; +import android.widget.Toast;
-public class MainActivity extends Activity implements OnMenuItemClickListener { - private TokenAdapter mTokenAdapter; - private InternalTokenPersistence mTokenPersistence; - private DataSetObserver mDataSetObserver; +public abstract class MainActivity extends Activity implements OnMenuItemClickListener { + public static final int DELETE_REQUEST_CODE = 1; + public static final int SCAN_REQUEST_CODE = 2; + public static final int ADD_REQUEST_CODE = 3; + public static final int EDIT_REQUEST_CODE = 4; + protected abstract void deleteToken(int position); + protected abstract void addToken(String tokenUri);
@Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - onNewIntent(getIntent()); setContentView(R.layout.main); - mTokenPersistence = new InternalTokenPersistence(this); - mTokenAdapter = new TokenAdapter(this, mTokenPersistence); - ((GridView) findViewById(R.id.grid)).setAdapter(mTokenAdapter); - // Don't permit screenshots since these might contain OTP codes. getWindow().setFlags(LayoutParams.FLAG_SECURE, LayoutParams.FLAG_SECURE); - - mDataSetObserver = new DataSetObserver() { - @Override - public void onChanged() { - super.onChanged(); - if (mTokenAdapter.getCount() == 0) - findViewById(android.R.id.empty).setVisibility(View.VISIBLE); - else - findViewById(android.R.id.empty).setVisibility(View.GONE); - } - }; - mTokenAdapter.registerDataSetObserver(mDataSetObserver); - } - - @Override - protected void onResume() { - super.onResume(); - mTokenAdapter.notifyDataSetChanged(); - } - - @Override - protected void onPause() { - super.onPause(); - mTokenAdapter.notifyDataSetChanged(); - } - - @Override - protected void onDestroy() { - super.onDestroy(); - mTokenAdapter.unregisterDataSetObserver(mDataSetObserver); }
@Override @@ -111,14 +80,16 @@ public class MainActivity extends Activity implements OnMenuItemClickListener {
@Override public boolean onMenuItemClick(MenuItem item) { + Intent i; switch (item.getItemId()) { case R.id.action_scan: - startActivity(new Intent(this, ScanActivity.class)); + i = new Intent(this, ScanActivity.class); + startActivityForResult(i, SCAN_REQUEST_CODE); overridePendingTransition(R.anim.fadein, R.anim.fadeout); return true;
case R.id.action_add: - startActivity(new Intent(this, AddActivity.class)); + startActivityForResult(new Intent(this, AddActivity.class), ADD_REQUEST_CODE); return true;
case R.id.action_about: @@ -130,11 +101,20 @@ public class MainActivity extends Activity implements OnMenuItemClickListener { }
@Override - protected void onNewIntent(Intent intent) { - super.onNewIntent(intent); - - Uri uri = intent.getData(); - if (uri != null) - mTokenPersistence.addWithToast(this, uri.toString()); + protected void onActivityResult(int requestCode, int resultCode, Intent data) { + if(resultCode == RESULT_OK) { + switch(requestCode) { + case DELETE_REQUEST_CODE: + int position = data.getIntExtra(DeleteActivity.EXTRA_POSITION, -1); + deleteToken(position); + break; + case ADD_REQUEST_CODE: + case SCAN_REQUEST_CODE: + String tokenUri = data.getStringExtra(BaseActivity.EXTRA_TOKEN_URI); + addToken(tokenUri); + break; + } + } } + } diff --git a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java index 94baaab..e2c64b4 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java +++ b/app/src/main/java/org/fedorahosted/freeotp/TokenAdapter.java @@ -20,6 +20,9 @@
package org.fedorahosted.freeotp;
+import java.io.IOException; + +import android.app.Activity; import android.content.ClipData; import android.content.ClipboardManager; import android.content.Context; @@ -44,8 +47,10 @@ public class TokenAdapter extends BaseReorderableAdapter { private final LayoutInflater mLayoutInflater; private final ClipboardManager mClipMan; private final Map<String, TokenCode> mTokenCodes; + private final Activity mCtx;
- public TokenAdapter(Context ctx, TokenPersistence tokenPersistence) { + public TokenAdapter(Activity ctx, TokenPersistence tokenPersistence) { + mCtx = ctx; mTokenPersistence = tokenPersistence; mLayoutInflater = (LayoutInflater) ctx.getSystemService(Context.LAYOUT_INFLATER_SERVICE); mClipMan = (ClipboardManager) ctx.getSystemService(Context.CLIPBOARD_SERVICE); @@ -86,9 +91,8 @@ public class TokenAdapter extends BaseReorderableAdapter {
@Override protected void bindView(View view, final int position) { - final Context ctx = view.getContext(); TokenLayout tl = (TokenLayout) view; - Token token = getItem(position); + final Token token = getItem(position);
int menu = token.isInternal() ? R.menu.editable_token : R.menu.token;
@@ -99,15 +103,16 @@ public class TokenAdapter extends BaseReorderableAdapter {
switch (item.getItemId()) { case R.id.action_edit: - i = new Intent(ctx, EditActivity.class); - i.putExtra(EditActivity.EXTRA_POSITION, position); - ctx.startActivity(i); + i = new Intent(mCtx, EditActivity.class); + i.putExtra(EditActivity.EXTRA_TOKEN, token); + mCtx.startActivityForResult(i, MainActivity.EDIT_REQUEST_CODE); break;
case R.id.action_delete: - i = new Intent(ctx, DeleteActivity.class); + i = new Intent(mCtx, DeleteActivity.class); i.putExtra(DeleteActivity.EXTRA_POSITION, position); - ctx.startActivity(i); + i.putExtra(DeleteActivity.EXTRA_TOKEN, token); + mCtx.startActivityForResult(i, MainActivity.DELETE_REQUEST_CODE); break; }
diff --git a/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java b/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java index d996a4d..66b12e8 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/add/AddActivity.java @@ -25,7 +25,6 @@ import java.net.URLEncoder; import java.util.Locale;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.app.Activity; import android.content.Intent; @@ -41,7 +40,7 @@ import android.widget.Spinner;
import com.squareup.picasso.Picasso;
-public class AddActivity extends Activity implements View.OnClickListener, CompoundButton.OnCheckedChangeListener { +public class AddActivity extends BaseActivity implements View.OnClickListener, CompoundButton.OnCheckedChangeListener { private final int SHA1_OFFSET = 1; private ImageButton mImage; private EditText mIssuer; @@ -129,9 +128,10 @@ public class AddActivity extends Activity implements View.OnClickListener, Compo } }
- // Add the token - if (new InternalTokenPersistence(this).addWithToast(this, uri) != null) - finish(); + Intent resultData = new Intent(); + resultData.putExtra(EXTRA_TOKEN_URI, uri); + setResult(RESULT_OK, resultData); + finish();
break; } diff --git a/app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java b/app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java new file mode 100644 index 0000000..f68ab4b --- /dev/null +++ b/app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java @@ -0,0 +1,7 @@ +package org.fedorahosted.freeotp.add; + +import android.app.Activity; + +public abstract class BaseActivity extends Activity { + public static final String EXTRA_TOKEN_URI = "tokenUri"; +} diff --git a/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java b/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java index 68fa5cf..9e29aa4 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/add/ScanActivity.java @@ -23,14 +23,14 @@ package org.fedorahosted.freeotp.add; import java.util.List;
import org.fedorahosted.freeotp.R; -import org.fedorahosted.freeotp.InternalToken; -import org.fedorahosted.freeotp.InternalTokenPersistence;
import android.annotation.TargetApi; import android.app.Activity; +import android.content.Intent; import android.hardware.Camera; import android.hardware.Camera.CameraInfo; import android.hardware.Camera.Parameters; +import android.net.Uri; import android.os.Bundle; import android.os.Handler; import android.os.Message; @@ -44,7 +44,7 @@ import android.widget.ProgressBar; import com.squareup.picasso.Callback; import com.squareup.picasso.Picasso;
-public class ScanActivity extends Activity implements SurfaceHolder.Callback { +public class ScanActivity extends BaseActivity implements SurfaceHolder.Callback { private final CameraInfo mCameraInfo = new CameraInfo(); private final ScanAsyncTask mScanAsyncTask; private final int mCameraId; @@ -98,15 +98,22 @@ public class ScanActivity extends Activity implements SurfaceHolder.Callback { @Override protected void onPostExecute(String result) { super.onPostExecute(result); - InternalToken token = new InternalTokenPersistence(ScanActivity.this).addWithToast(ScanActivity.this, result); - if (token == null || token.getImage() == null) { + + Uri uri = Uri.parse(result); + + final Intent resultData = new Intent(); + resultData.putExtra(EXTRA_TOKEN_URI, result); + + String imageParameter = uri.getQueryParameter("image"); + if(imageParameter == null) { + setResult(RESULT_OK, resultData); finish(); return; } - + Uri imageUri = Uri.parse(imageParameter); final ImageView image = (ImageView) findViewById(R.id.image); Picasso.with(ScanActivity.this) - .load(token.getImage()) + .load(imageUri) .placeholder(R.drawable.scan) .into(image, new Callback() { @Override @@ -116,6 +123,7 @@ public class ScanActivity extends Activity implements SurfaceHolder.Callback { image.postDelayed(new Runnable() { @Override public void run() { + setResult(RESULT_OK, resultData); finish(); } }, 2000); @@ -123,6 +131,7 @@ public class ScanActivity extends Activity implements SurfaceHolder.Callback {
@Override public void onError() { + setResult(RESULT_CANCELED); finish(); } }); diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java deleted file mode 100644 index d81010e..0000000 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java +++ /dev/null @@ -1,22 +0,0 @@ -package org.fedorahosted.freeotp.edit; - -import android.app.Activity; -import android.os.Bundle; - -public abstract class BaseActivity extends Activity { - public static final String EXTRA_POSITION = "position"; - private int mPosition; - - @Override - protected void onCreate(Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - - // Get the position of the token. This MUST exist. - mPosition = getIntent().getIntExtra(EXTRA_POSITION, -1); - assert mPosition >= 0; - } - - protected int getPosition() { - return mPosition; - } -} diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java index 7da16fb..0ad2549 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/DeleteActivity.java @@ -2,23 +2,30 @@ package org.fedorahosted.freeotp.edit;
import org.fedorahosted.freeotp.R; import org.fedorahosted.freeotp.Token; -import org.fedorahosted.freeotp.InternalTokenPersistence;
+import android.app.Activity; import android.os.Bundle; +import android.content.Intent; import android.view.View; import android.widget.ImageView; import android.widget.TextView;
import com.squareup.picasso.Picasso;
-public class DeleteActivity extends BaseActivity { - private InternalTokenPersistence mTokenPersistence; +public class DeleteActivity extends Activity { + public static final String EXTRA_TOKEN = "token"; + public static final String EXTRA_POSITION = "position"; + private int mPosition; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + // Get the position of the token. This MUST exist. + mPosition = getIntent().getIntExtra(EXTRA_POSITION, -1); + assert mPosition >= 0; + Token token = (Token)getIntent().getSerializableExtra(EXTRA_TOKEN); setContentView(R.layout.delete); - mTokenPersistence = new InternalTokenPersistence(this); - Token token = mTokenPersistence.get(getPosition()); + ((TextView) findViewById(R.id.issuer)).setText(token.getIssuer()); ((TextView) findViewById(R.id.label)).setText(token.getLabel()); Picasso.with(this) @@ -29,6 +36,7 @@ public class DeleteActivity extends BaseActivity { findViewById(R.id.cancel).setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { + setResult(RESULT_CANCELED); finish(); } }); @@ -36,7 +44,9 @@ public class DeleteActivity extends BaseActivity { findViewById(R.id.delete).setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { - mTokenPersistence.delete(getPosition()); + Intent resultData = new Intent(); + resultData.putExtra(EXTRA_POSITION, mPosition); + setResult(RESULT_OK, resultData); finish(); } }); diff --git a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java index d55a657..5c495d3 100644 --- a/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java +++ b/app/src/main/java/org/fedorahosted/freeotp/edit/EditActivity.java @@ -22,8 +22,8 @@ package org.fedorahosted.freeotp.edit;
import org.fedorahosted.freeotp.R; import org.fedorahosted.freeotp.InternalToken; -import org.fedorahosted.freeotp.InternalTokenPersistence;
+import android.app.Activity; import android.content.Intent; import android.net.Uri; import android.os.Bundle; @@ -36,7 +36,10 @@ import android.widget.ImageButton;
import com.squareup.picasso.Picasso;
-public class EditActivity extends BaseActivity implements TextWatcher, View.OnClickListener { +public class EditActivity extends Activity implements TextWatcher, View.OnClickListener { + public static final String EXTRA_TOKEN = "token"; + + private InternalToken mToken; private EditText mIssuer; private EditText mLabel; private ImageButton mImage; @@ -50,7 +53,6 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl private Uri mImageCurrent; private Uri mImageDefault; private Uri mImageDisplay; - private InternalTokenPersistence mTokenPersistence;
private void showImage(Uri uri) { mImageDisplay = uri; @@ -72,19 +74,18 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setContentView(R.layout.edit); - + mToken = (InternalToken)getIntent().getSerializableExtra(EXTRA_TOKEN); // Get token values. - mTokenPersistence = new InternalTokenPersistence(this); - InternalToken token = (InternalToken)mTokenPersistence.get(getPosition()); - mIssuerCurrent = token.getIssuer(); - mLabelCurrent = token.getLabel(); - mImageCurrent = token.getImage(); - token.setIssuer(null); - token.setLabel(null); - token.setImage(null); - mIssuerDefault = token.getIssuer(); - mLabelDefault = token.getLabel(); - mImageDefault = token.getImage(); + + mIssuerCurrent = mToken.getIssuer(); + mLabelCurrent = mToken.getLabel(); + mImageCurrent = mToken.getImage(); + mToken.setIssuer(null); + mToken.setLabel(null); + mToken.setImage(null); + mIssuerDefault = mToken.getIssuer(); + mLabelDefault = mToken.getLabel(); + mImageDefault = mToken.getImage();
// Get references to widgets. mIssuer = (EditText) findViewById(R.id.issuer); @@ -152,13 +153,17 @@ public class EditActivity extends BaseActivity implements TextWatcher, View.OnCl break;
case R.id.save: - InternalToken token = (InternalToken)mTokenPersistence.get(getPosition()); - token.setIssuer(mIssuer.getText().toString()); - token.setLabel(mLabel.getText().toString()); - token.setImage(mImageDisplay); - mTokenPersistence.save(token); + mToken.setIssuer(mIssuer.getText().toString()); + mToken.setLabel(mLabel.getText().toString()); + mToken.setImage(mImageDisplay); + Intent resultData = new Intent(); + resultData.putExtra(EXTRA_TOKEN, mToken); + setResult(RESULT_OK, resultData); + finish(); + break;
case R.id.cancel: + setResult(RESULT_CANCELED); finish(); break; } diff --git a/app/src/main/res/layout/main.xml b/app/src/main/res/layout/main.xml index a1770ae..1045568 100644 --- a/app/src/main/res/layout/main.xml +++ b/app/src/main/res/layout/main.xml @@ -31,7 +31,7 @@ android:layout_height="match_parent" android:padding="8dp" android:gravity="center" - android:text="@string/no_keys" + android:text="@string/no_keys_or_tap" />
<GridView @@ -48,4 +48,4 @@ android:verticalSpacing="8dp" android:background="#E5E5E5" /> -</LinearLayout> \ No newline at end of file +</LinearLayout> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 359026f..c4b55d3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1,13 +1,17 @@ <?xml version="1.0" encoding="utf-8"?> <resources> <string name="app_name">FreeOTP</string> + <string name="nfc_app_name">FreeOTP (NFC)</string> <string name="add">Add</string> <string name="invalid_token">The token specified was invalid!</string> <string name="delete">Delete</string> <string name="edit">Edit</string> <string name="restore_defaults">Restore Defaults</string> - <string name="no_keys">No OTP keys installed.</string> - <string name="add_token">Add Token</string> + <string name="no_keys_or_tap">No OTP tokens on this device. To list the tokens on an NFC device, tap it!</string> + <string name="no_keys">No OTP tokens on this device.</string> + <string name="add_token">Add token</string> + <string name="external_add_token">Tap NFC device to add token</string> + <string name="external_add_token_validation_wrong">Fields are not entered correctly</string> <string name="scan_qr_code">Scan QR Code</string> <string name="interval">Interval</string> <string name="counter">Counter</string> @@ -37,8 +41,13 @@
<string name="delete_summary">This is your last chance: if you delete this token, it will be gone forever. It will not be disabled on the server.</string> <string name="delete_question">Delete this token?</string> - <string name="external_token_needed">You need the NFC device to generate a code</string> <string name="failed_to_read_external_token">Failed to read the NFC device</string> + <string name="tap_external_token">Tap the NFC device to refresh the list of tokens</string> + <string name="external_token_needed">You need the NFC device to generate a code</string> + <string name="cancel">Cancel</string> + <string name="tap_to_delete">Tap your NFC device to delete the token.</string> + <string name="tap_to_add">Tap your NFC device to add the token.</string> + <string name="tap_dialog_title">Tap NFC device</string> <string-array name="algorithms"> <item>MD5</item> <item>SHA1</item>
Petter,
Thanks for your patches! I spent a few hours reviewing these patches this morning. I am very excited to finally getting around to having NFC support.
Unfortunately, these patches were largely written from the perspective of a NFC card vendor and not from the user's perspective. This is probably what I would have done if I was in your shoes, so please don't take offense. I just need to guard the user experience.
So the current behavior of these patches is to have a special NFC mode which you enter by touching the tag. From this mode, you can view the tokens on the NFC device, add/delete tokens and activate them. You exit the mode by pressing back. Let's look at two common scenarios.
First, adding a token: 1. Touch the tag. 2. Enter NFC mode. 3. Add the token. 4. Touch the tag. 5. Click to exit NFC mode.
Second, activating a token: 1. Touch the tag. 2. Enter NFC mode. 3. Click the token. 4. Touch the tag. 5. Click to exit NFC mode.
Notice that a lot of the user action is just handling the NFC mode.
Another problem arises, which is that tokens in NFC mode behave differently than those in non-NFC mode. They can't really be edited or moved. Nor can they have images assigned to them.
Let me propose an alternative: kill NFC mode. NFC tokens aren't really a special kind of token but rather a special way of calculating the token code. In the future we'll have to implement something like this for encrypted token secrets anyway. So all the metadata for the tokens should live just like a normal token. The only difference is where the secret is stored.
Instead of NFC mode, this is how I see it working.
Adding a token: 1. Add a token like normal. 2. Click "Move to NFC" drop-down menu item. 3. Touch the tag.
Deleting a token: 1. Delete like normal. 2. Touch the tag.
Activating a token: 1. Touch a tag with only one token on it (this idea needs refinement). = OR = 1. Click the token. 2. Touch the tag.
Doing the UI/code this way lets NFC tokens get all the drag-drop/image/editing goodness of regular tokens. It also significantly reduces the number of steps for all activities. You can even show a little NFC icon in the top right of the token to indicate to the user that this is an NFC token.
A corner case arises when structuring the code this way. Namely, what happens if a phone is lost/erased and thus FreeOTP has lost its "linkage" to the NFC entry. A simple "Import from NFC" method (parallel to the ScanQRCode and Manual methods) would solve this case. This "Import from NFC" method for adding tokens to FreeOTP's database could detect if unmanaged tokens exist on the NFC card and prompt to import them automatically.
Structuring this code also has another benefit. Namely, NFC codes could be available on remote devices such as a smartwatch. I need to think through the interaction here, but I think the idea has merit.
One other thing that needs thought is what to do if the NFC is touched to the phone while FreeOTP is not running/foregrounded. I think that we should try to detect to see if it has tokens that FreeOTP manages and, if so, is started/foregrounded.
This reduces... 1. Start FreeOTP 2. Click token. 3. Touch tag. = OR = 1. Start FreeOTP 2. Touch tag (single token only).
... to ... 1. Touch tag. 2. Click token. = OR = 1. Touch tag (single token only).
For users with only a single token, that is by far the most optimal experience. We could even implement this with a widget so that the app doesn't even need to open.
I know I've dumped a lot on you. I hope it is helpful! Any thoughts?
On Tue, 2014-11-18 at 16:55 +0100, Petter Arvidsson wrote:
Hi (again) everyone,
(As you might have noticed my previous patches got eaten by the mailer daemon :-) I have now split my patches in to additional commits and I hope they will be accepted by the system. I also agree with Nathaniel that they are way easier to follow now as well. For convenience I also include the original cover letter below.)
My name is Petter and I am working for the Swedish company Fidesm AB. We are building a platform for managing Secure Elements (such as NFC cards) via the network. One of the first services that can be deliverd via our platform is an OTP application developed by Yubico (https://github.com/Yubico/ykneo-oath). To enable us to use this application to store our OTP credentials we wanted to integrate it into a good OTP Android App. This patch enables any supported external NFC token to be used with FreeOTP. Currently the only two tokens using ths protocol that we know of are the Fidesmo Card and the Yubico Yubikey. I am more than willing to add support for any other device which is using the same protocol.
The strategy taken to integrate this into FreeOTP was first to generalize the TokenPersistence and Token classes to accomodate both internal and external tokens. This is the first commit/patch. On top of this the NFC functionality was built. To manage the NFC tag efficiently (i.e. not to have the user tap it more than neccessary), we decided to handle it only in the main activity. This required the other activities to return their results rather than manage the TokenPersistence directly. This is a very common pattern in Android and we believe that end result was actually quite nice, clearly separating managing the tokens from the other activities.
If you want to try it out you will require an external NFC token. Please let me know if you want a Fidesmo Card and I can provide instructions on how to get one for free. As earlier stated it can also be tested with a Yubikey from Yubico.
I hope you like the implementation and that you have some good feedack for me on what needs to be changed and improved!
Please beware that I am a bit new to git send-email, so let me know if you want any changes to how the patches were generated.
Best regards, Petter
Petter Arvidsson (4): Change Token to Internal token Add interfaces for Token and TokenPersistence Add External tokens Extend app to utilize new external tokens
app/build.gradle | 6 + app/src/main/AndroidManifest.xml | 9 +- .../freeotp/BaseReorderableAdapter.java | 97 ++++--- .../fedorahosted/freeotp/ExternalMainActivity.java | 142 +++++++++ .../org/fedorahosted/freeotp/ExternalToken.java | 130 +++++++++ .../freeotp/ExternalTokenPersistence.java | 129 +++++++++ .../fedorahosted/freeotp/InternalMainActivity.java | 126 ++++++++ .../org/fedorahosted/freeotp/InternalToken.java | 318 +++++++++++++++++++++ .../freeotp/InternalTokenPersistence.java | 140 +++++++++ .../org/fedorahosted/freeotp/MainActivity.java | 83 +++--- .../java/org/fedorahosted/freeotp/NfcHelpers.java | 60 ++++ .../main/java/org/fedorahosted/freeotp/Token.java | 277 +----------------- .../org/fedorahosted/freeotp/TokenAdapter.java | 62 ++-- .../org/fedorahosted/freeotp/TokenPersistence.java | 131 ++------- .../org/fedorahosted/freeotp/add/AddActivity.java | 10 +- .../org/fedorahosted/freeotp/add/BaseActivity.java | 7 + .../org/fedorahosted/freeotp/add/ScanActivity.java | 23 +- .../fedorahosted/freeotp/edit/BaseActivity.java | 22 -- .../fedorahosted/freeotp/edit/DeleteActivity.java | 19 +- .../fedorahosted/freeotp/edit/EditActivity.java | 46 +-- app/src/main/res/layout/main.xml | 4 +- app/src/main/res/menu/editable_token.xml | 33 +++ app/src/main/res/menu/token.xml | 6 - app/src/main/res/values/strings.xml | 16 +- 24 files changed, 1339 insertions(+), 557 deletions(-) create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalToken.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java create mode 100644 app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java delete mode 100644 app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java create mode 100644 app/src/main/res/menu/editable_token.xml
-- 1.9.1 _______________________________________________ freeotp-devel mailing list freeotp-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel
Hi Nathaniel and everyone else,
Thank you for your comments! I agree to them and will get started making the updates as soon as I have some time available (Hopefully after the current project is finished, some weeks from now).
Some comments regarding usability and why we choose to use a separate list for the card and not a list where you "sync" the contents of the card with the phone: - Conflicting tokens. Possibly several cards containing the same token id (but different cryptographic keys stored) may be presented to the phone. This could be quite confusing since they would show up as the same token. - How to know if a token was removed from the card? Commonly (especially in the case of the Yubikey) the external card will be managed by another application and not FreeOTP. This will leave dead tokens in the FreeOTP app whenever tokens are removed using a client PC. Probably it's easily solved by the user simply removing the token also in the FreeOTP App.
Both of these are probably not big issues in reality.
Regarding the single token touch. We can also generate all tokens at the same time when they are TOTP. This is done by e.g. Yubicos own app. This way, we could generate all TOTP tokens on a card when the card is presented? TOTP being most common protocol this could be fairly convenient?
What are you thoughts here?
Best regards, Petter
On Tue, Jan 13, 2015 at 8:30 PM, Nathaniel McCallum npmccallum@redhat.com wrote:
Petter,
Thanks for your patches! I spent a few hours reviewing these patches this morning. I am very excited to finally getting around to having NFC support.
Unfortunately, these patches were largely written from the perspective of a NFC card vendor and not from the user's perspective. This is probably what I would have done if I was in your shoes, so please don't take offense. I just need to guard the user experience.
So the current behavior of these patches is to have a special NFC mode which you enter by touching the tag. From this mode, you can view the tokens on the NFC device, add/delete tokens and activate them. You exit the mode by pressing back. Let's look at two common scenarios.
First, adding a token:
- Touch the tag.
- Enter NFC mode.
- Add the token.
- Touch the tag.
- Click to exit NFC mode.
Second, activating a token:
- Touch the tag.
- Enter NFC mode.
- Click the token.
- Touch the tag.
- Click to exit NFC mode.
Notice that a lot of the user action is just handling the NFC mode.
Another problem arises, which is that tokens in NFC mode behave differently than those in non-NFC mode. They can't really be edited or moved. Nor can they have images assigned to them.
Let me propose an alternative: kill NFC mode. NFC tokens aren't really a special kind of token but rather a special way of calculating the token code. In the future we'll have to implement something like this for encrypted token secrets anyway. So all the metadata for the tokens should live just like a normal token. The only difference is where the secret is stored.
Instead of NFC mode, this is how I see it working.
Adding a token:
- Add a token like normal.
- Click "Move to NFC" drop-down menu item.
- Touch the tag.
Deleting a token:
- Delete like normal.
- Touch the tag.
Activating a token:
- Touch a tag with only one token on it (this idea needs refinement).
= OR =
- Click the token.
- Touch the tag.
Doing the UI/code this way lets NFC tokens get all the drag-drop/image/editing goodness of regular tokens. It also significantly reduces the number of steps for all activities. You can even show a little NFC icon in the top right of the token to indicate to the user that this is an NFC token.
A corner case arises when structuring the code this way. Namely, what happens if a phone is lost/erased and thus FreeOTP has lost its "linkage" to the NFC entry. A simple "Import from NFC" method (parallel to the ScanQRCode and Manual methods) would solve this case. This "Import from NFC" method for adding tokens to FreeOTP's database could detect if unmanaged tokens exist on the NFC card and prompt to import them automatically.
Structuring this code also has another benefit. Namely, NFC codes could be available on remote devices such as a smartwatch. I need to think through the interaction here, but I think the idea has merit.
One other thing that needs thought is what to do if the NFC is touched to the phone while FreeOTP is not running/foregrounded. I think that we should try to detect to see if it has tokens that FreeOTP manages and, if so, is started/foregrounded.
This reduces...
- Start FreeOTP
- Click token.
- Touch tag.
= OR =
- Start FreeOTP
- Touch tag (single token only).
... to ...
- Touch tag.
- Click token.
= OR =
- Touch tag (single token only).
For users with only a single token, that is by far the most optimal experience. We could even implement this with a widget so that the app doesn't even need to open.
I know I've dumped a lot on you. I hope it is helpful! Any thoughts?
On Tue, 2014-11-18 at 16:55 +0100, Petter Arvidsson wrote:
Hi (again) everyone,
(As you might have noticed my previous patches got eaten by the mailer daemon :-) I have now split my patches in to additional commits and I hope they will be accepted by the system. I also agree with Nathaniel that they are way easier to follow now as well. For convenience I also include the original cover letter below.)
My name is Petter and I am working for the Swedish company Fidesm AB. We are building a platform for managing Secure Elements (such as NFC cards) via the network. One of the first services that can be deliverd via our platform is an OTP application developed by Yubico (https://github.com/Yubico/ykneo-oath). To enable us to use this application to store our OTP credentials we wanted to integrate it into a good OTP Android App. This patch enables any supported external NFC token to be used with FreeOTP. Currently the only two tokens using ths protocol that we know of are the Fidesmo Card and the Yubico Yubikey. I am more than willing to add support for any other device which is using the same protocol.
The strategy taken to integrate this into FreeOTP was first to generalize the TokenPersistence and Token classes to accomodate both internal and external tokens. This is the first commit/patch. On top of this the NFC functionality was built. To manage the NFC tag efficiently (i.e. not to have the user tap it more than neccessary), we decided to handle it only in the main activity. This required the other activities to return their results rather than manage the TokenPersistence directly. This is a very common pattern in Android and we believe that end result was actually quite nice, clearly separating managing the tokens from the other activities.
If you want to try it out you will require an external NFC token. Please let me know if you want a Fidesmo Card and I can provide instructions on how to get one for free. As earlier stated it can also be tested with a Yubikey from Yubico.
I hope you like the implementation and that you have some good feedack for me on what needs to be changed and improved!
Please beware that I am a bit new to git send-email, so let me know if you want any changes to how the patches were generated.
Best regards, Petter
Petter Arvidsson (4): Change Token to Internal token Add interfaces for Token and TokenPersistence Add External tokens Extend app to utilize new external tokens
app/build.gradle | 6 + app/src/main/AndroidManifest.xml | 9 +- .../freeotp/BaseReorderableAdapter.java | 97 ++++--- .../fedorahosted/freeotp/ExternalMainActivity.java | 142 +++++++++ .../org/fedorahosted/freeotp/ExternalToken.java | 130 +++++++++ .../freeotp/ExternalTokenPersistence.java | 129 +++++++++ .../fedorahosted/freeotp/InternalMainActivity.java | 126 ++++++++ .../org/fedorahosted/freeotp/InternalToken.java | 318
+++++++++++++++++++++
.../freeotp/InternalTokenPersistence.java | 140 +++++++++ .../org/fedorahosted/freeotp/MainActivity.java | 83 +++--- .../java/org/fedorahosted/freeotp/NfcHelpers.java | 60 ++++ .../main/java/org/fedorahosted/freeotp/Token.java | 277
+-----------------
.../org/fedorahosted/freeotp/TokenAdapter.java | 62 ++-- .../org/fedorahosted/freeotp/TokenPersistence.java | 131 ++------- .../org/fedorahosted/freeotp/add/AddActivity.java | 10 +- .../org/fedorahosted/freeotp/add/BaseActivity.java | 7 + .../org/fedorahosted/freeotp/add/ScanActivity.java | 23 +- .../fedorahosted/freeotp/edit/BaseActivity.java | 22 -- .../fedorahosted/freeotp/edit/DeleteActivity.java | 19 +- .../fedorahosted/freeotp/edit/EditActivity.java | 46 +-- app/src/main/res/layout/main.xml | 4 +- app/src/main/res/menu/editable_token.xml | 33 +++ app/src/main/res/menu/token.xml | 6 - app/src/main/res/values/strings.xml | 16 +- 24 files changed, 1339 insertions(+), 557 deletions(-) create mode 100644
app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/InternalToken.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java
create mode 100644
app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java
delete mode 100644
app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java
create mode 100644 app/src/main/res/menu/editable_token.xml
-- 1.9.1 _______________________________________________ freeotp-devel mailing list freeotp-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel
freeotp-devel mailing list freeotp-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel
On Mon, 2015-01-19 at 10:21 +0100, Petter Arvidsson wrote:
Hi Nathaniel and everyone else,
Thank you for your comments! I agree to them and will get started making the updates as soon as I have some time available (Hopefully after the current project is finished, some weeks from now).
Some comments regarding usability and why we choose to use a separate list for the card and not a list where you "sync" the contents of the card with the phone:
- Conflicting tokens. Possibly several cards containing the same token
id (but different cryptographic keys stored) may be presented to the phone. This could be quite confusing since they would show up as the same token.
Make sure to assign each token a sufficiently unique id. Something like a random UUID comes to mind. Are there length restrictions on the token id?
- How to know if a token was removed from the card? Commonly
(especially in the case of the Yubikey) the external card will be managed by another application and not FreeOTP. This will leave dead tokens in the FreeOTP app whenever tokens are removed using a client PC. Probably it's easily solved by the user simply removing the token also in the FreeOTP App.
Yup.
Both of these are probably not big issues in reality.
Regarding the single token touch. We can also generate all tokens at the same time when they are TOTP. This is done by e.g. Yubicos own app. This way, we could generate all TOTP tokens on a card when the card is presented? TOTP being most common protocol this could be fairly convenient?
Good idea!
What are you thoughts here?
I think this all makes sense.
On Tue, Jan 13, 2015 at 8:30 PM, Nathaniel McCallum npmccallum@redhat.com wrote: Petter,
Thanks for your patches! I spent a few hours reviewing these patches this morning. I am very excited to finally getting around to having NFC support. Unfortunately, these patches were largely written from the perspective of a NFC card vendor and not from the user's perspective. This is probably what I would have done if I was in your shoes, so please don't take offense. I just need to guard the user experience. So the current behavior of these patches is to have a special NFC mode which you enter by touching the tag. From this mode, you can view the tokens on the NFC device, add/delete tokens and activate them. You exit the mode by pressing back. Let's look at two common scenarios. First, adding a token: 1. Touch the tag. 2. Enter NFC mode. 3. Add the token. 4. Touch the tag. 5. Click to exit NFC mode. Second, activating a token: 1. Touch the tag. 2. Enter NFC mode. 3. Click the token. 4. Touch the tag. 5. Click to exit NFC mode. Notice that a lot of the user action is just handling the NFC mode. Another problem arises, which is that tokens in NFC mode behave differently than those in non-NFC mode. They can't really be edited or moved. Nor can they have images assigned to them. Let me propose an alternative: kill NFC mode. NFC tokens aren't really a special kind of token but rather a special way of calculating the token code. In the future we'll have to implement something like this for encrypted token secrets anyway. So all the metadata for the tokens should live just like a normal token. The only difference is where the secret is stored. Instead of NFC mode, this is how I see it working. Adding a token: 1. Add a token like normal. 2. Click "Move to NFC" drop-down menu item. 3. Touch the tag. Deleting a token: 1. Delete like normal. 2. Touch the tag. Activating a token: 1. Touch a tag with only one token on it (this idea needs refinement). = OR = 1. Click the token. 2. Touch the tag. Doing the UI/code this way lets NFC tokens get all the drag-drop/image/editing goodness of regular tokens. It also significantly reduces the number of steps for all activities. You can even show a little NFC icon in the top right of the token to indicate to the user that this is an NFC token. A corner case arises when structuring the code this way. Namely, what happens if a phone is lost/erased and thus FreeOTP has lost its "linkage" to the NFC entry. A simple "Import from NFC" method (parallel to the ScanQRCode and Manual methods) would solve this case. This "Import from NFC" method for adding tokens to FreeOTP's database could detect if unmanaged tokens exist on the NFC card and prompt to import them automatically. Structuring this code also has another benefit. Namely, NFC codes could be available on remote devices such as a smartwatch. I need to think through the interaction here, but I think the idea has merit. One other thing that needs thought is what to do if the NFC is touched to the phone while FreeOTP is not running/foregrounded. I think that we should try to detect to see if it has tokens that FreeOTP manages and, if so, is started/foregrounded. This reduces... 1. Start FreeOTP 2. Click token. 3. Touch tag. = OR = 1. Start FreeOTP 2. Touch tag (single token only). ... to ... 1. Touch tag. 2. Click token. = OR = 1. Touch tag (single token only). For users with only a single token, that is by far the most optimal experience. We could even implement this with a widget so that the app doesn't even need to open. I know I've dumped a lot on you. I hope it is helpful! Any thoughts? On Tue, 2014-11-18 at 16:55 +0100, Petter Arvidsson wrote: > Hi (again) everyone, > > (As you might have noticed my previous patches got eaten by the mailer > daemon :-) I have now split my patches in to additional commits and I > hope they will be accepted by the system. I also agree with Nathaniel > that they are way easier to follow now as well. For convenience I also > include the original cover letter below.) > > My name is Petter and I am working for the Swedish company Fidesm > AB. We are building a platform for managing Secure Elements (such as > NFC cards) via the network. One of the first services that can be > deliverd via our platform is an OTP application developed by Yubico > (https://github.com/Yubico/ykneo-oath). To enable us to use this > application to store our OTP credentials we wanted to integrate it > into a good OTP Android App. This patch enables any supported external > NFC token to be used with FreeOTP. Currently the only two tokens using > ths protocol that we know of are the Fidesmo Card and the Yubico > Yubikey. I am more than willing to add support for any other device > which is using the same protocol. > > The strategy taken to integrate this into FreeOTP was first to > generalize the TokenPersistence and Token classes to accomodate both > internal and external tokens. This is the first commit/patch. On top > of this the NFC functionality was built. To manage the NFC tag > efficiently (i.e. not to have the user tap it more than neccessary), > we decided to handle it only in the main activity. This required the > other activities to return their results rather than manage the > TokenPersistence directly. This is a very common pattern in Android > and we believe that end result was actually quite nice, clearly > separating managing the tokens from the other activities. > > If you want to try it out you will require an external NFC > token. Please let me know if you want a Fidesmo Card and I can provide > instructions on how to get one for free. As earlier stated it can also > be tested with a Yubikey from Yubico. > > I hope you like the implementation and that you have some good feedack > for me on what needs to be changed and improved! > > Please beware that I am a bit new to git send-email, so let me know if > you want any changes to how the patches were generated. > > Best regards, > Petter > > Petter Arvidsson (4): > Change Token to Internal token > Add interfaces for Token and TokenPersistence > Add External tokens > Extend app to utilize new external tokens > > app/build.gradle | 6 + > app/src/main/AndroidManifest.xml | 9 +- > .../freeotp/BaseReorderableAdapter.java | 97 ++++--- > .../fedorahosted/freeotp/ExternalMainActivity.java | 142 +++++++++ > .../org/fedorahosted/freeotp/ExternalToken.java | 130 +++++++++ > .../freeotp/ExternalTokenPersistence.java | 129 +++++++++ > .../fedorahosted/freeotp/InternalMainActivity.java | 126 ++++++++ > .../org/fedorahosted/freeotp/InternalToken.java | 318 +++++++++++++++++++++ > .../freeotp/InternalTokenPersistence.java | 140 +++++++++ > .../org/fedorahosted/freeotp/MainActivity.java | 83 +++--- > .../java/org/fedorahosted/freeotp/NfcHelpers.java | 60 ++++ > .../main/java/org/fedorahosted/freeotp/Token.java | 277 +----------------- > .../org/fedorahosted/freeotp/TokenAdapter.java | 62 ++-- > .../org/fedorahosted/freeotp/TokenPersistence.java | 131 ++------- > .../org/fedorahosted/freeotp/add/AddActivity.java | 10 +- > .../org/fedorahosted/freeotp/add/BaseActivity.java | 7 + > .../org/fedorahosted/freeotp/add/ScanActivity.java | 23 +- > .../fedorahosted/freeotp/edit/BaseActivity.java | 22 -- > .../fedorahosted/freeotp/edit/DeleteActivity.java | 19 +- > .../fedorahosted/freeotp/edit/EditActivity.java | 46 +-- > app/src/main/res/layout/main.xml | 4 +- > app/src/main/res/menu/editable_token.xml | 33 +++ > app/src/main/res/menu/token.xml | 6 - > app/src/main/res/values/strings.xml | 16 +- > 24 files changed, 1339 insertions(+), 557 deletions(-) > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalMainActivity.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalToken.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/ExternalTokenPersistence.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalMainActivity.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalToken.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/InternalTokenPersistence.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/NfcHelpers.java > create mode 100644 app/src/main/java/org/fedorahosted/freeotp/add/BaseActivity.java > delete mode 100644 app/src/main/java/org/fedorahosted/freeotp/edit/BaseActivity.java > create mode 100644 app/src/main/res/menu/editable_token.xml > > -- > 1.9.1 > _______________________________________________ > freeotp-devel mailing list > freeotp-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel _______________________________________________ freeotp-devel mailing list freeotp-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel
freeotp-devel mailing list freeotp-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel
freeotp-devel@lists.fedorahosted.org