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,

On Tue, Jan 13, 2015 at 8:30 PM, Nathaniel McCallum <npmccallum@redhat.com> wrote:

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

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