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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/freeotp-devel