Add Exchange #2

danielberkompas · 2017-12-11T18:28:10Z

This exchange module will be used to exchange one set of credentials for
another, for example:

email/password => api token
email => reset password token

Here's what you would to to allow it to support password reset tokens (only good for password resets) and identity tokens.

1. Update Store Module

You update your existing store module to exchange users for tokens, and validate them.

defmodule MyApp.Accounts.Store do
  alias MyApp.{
    Accounts.User,
    Accounts.Token,
    Repo
  }

  use Authority.Authentication.EctoStore,
    repo: Repo,
    schema: User,
    identity_fields: [:username, :email],
    credential_field: [:password],
    credential_type: :hash,
    hash_algorithm: :bcrypt

  @behaviour Authority.Exchange.Store

  # Define the `exchange/2` callback to create a credential, a token in this case
  def exchange(%User{} = user, opts) do
     MyApp.Repo.insert(%Token{context: opts[:context], user_id: user.id})
  end

  # Override identify/2 to support the new token type
  def identify(%Token{token: token}, opts) do
    token = 
      Token
      |> Repo.get_by(token: token)
      |> Repo.preload(:user)

    if token do
      {:ok, user}
    else
      {:error, :invalid_token}
    end
  end

  # Fallback to EctoStore's implementation of identify/2
  def identify(identifier, opts) do
    super(identifier, opts)
  end

  # Override validate/3 to support the new token type
  def validate(%Token{} = token, _user, opts) do
    if token.context == opts[:context] do
      :ok
    else
      {:error, :invalid_token_for_context}
    end
  end

  # Override validate/3 to allow email address only in the :recovery context.
  # Otherwise, fallback to EctoStore's implementation of validate/3
  def validate(credential, user, opts) do
    if is_binary(credential) && credential =~ "@" && opts[:context] == :recovery do
      {:ok, user}
    else
      super(credential, user, opts)
    end
  end
end

2. Update Authentication Module

defmodule MyApp.Accounts.Authentication do
  use Authority.Authentication, store: MyApp.Accounts.Store
  use Authority.Exchange, 
    store: MyApp.Accounts.Store,
    authentication: __MODULE__
end

3. Update Accounts Domain

defdelegate exchange(credential, opts \\ []), to: Authentication

3. Use `exchange/2`

Now that everything is set up, you can easily exchange an email address for a recovery token:

Accounts.exchange("user@email.com", context: :recovery)
# => {:ok, token}

You can also exchange email/password for an identity token:

Accounts.exchange({"user@email.com", "password"}, context: :identity)
# => {:ok, token}

These tokens can be passed into authenticate:

Accounts.authenticate(token, context: :recovery)
# => {:ok, user}

If the given context doesn't match the token's context, you'll get an error:

Accounts.authenticate(token, context: :identity)
# => {:error, :invalid_token_for_context}

Caveats

You need to pass structs as arguments. If the mobile app sends you a token, you need to wrap it with a Token struct to prevent the authentication system from thinking it's an email address.
This isn't quite as straightforward as I would like, but I'm sure we could build a layer on top of this (like EctoStore) that would do all this for the most common use cases.

This exchange module will be used to exchange one set of credentials for another, such as "user/password => token".

ryanlntn · 2017-12-11T19:06:23Z

@danielberkompas I like this but the API seems a bit confusing to me. Perhaps we could create recover and identify functions on Accounts that wrap exchange calling it with the appropriate context? Then base cases are simpler but if you need another context exchange is still there.

Also is context required to authenticate now? Shouldn't just having the token be enough?

danielberkompas · 2017-12-11T19:29:09Z

@ryanlntn I'm working on making the API a lot less confusing by making EctoStore able to do all of this without overrides.

Context probably needs to be required for tokens, because a token might not be valid for everything. A recovery token isn't a login token, so everywhere that needs the user to be logged in should be asserting that the token is an :identity token.

ryanlntn · 2017-12-11T19:31:37Z

@danielberkompas Right but does it need to be passed to authenticate or could that just be checked against the token itself since we're still storing them with the context.

This will massively simplify how you integrate `Exchange` if you are using `EctoStore`.

danielberkompas · 2017-12-11T22:15:16Z

I've massively simplified the steps and now allow you to set a default context for EctoStore. The new steps to integrate Exchange with your project are:

1. Add `Exchange` settings to your `EctoStore`

defmodule MyApp.Accounts.Store do
  use Authority.EctoStore,
    repo: MyApp.Repo,
    authentication: %{
      schema: MyApp.Accounts.User,
      identity_field: :email,
      credential_field: :password,
      credential_type: :hash,
      hash_algorithm: :bcrypt
    },
    exchange: %{
      schema: MyApp.Accounts.Token,
      identity_assoc: :user,
      token_field: :token,
      token_type: :uuid,
      expiry_field: :expires_at,
      context_field: :context,
      contexts: %{
        identity: %{
          default: true,
          expires_in_seconds: 60,
        },
        recovery: %{
          expires_in_seconds: 60,
          skip_validation: [~r/@/] # allows emails to be exchanged for tokens in recovery context
        }
      }
    }
end

2. Add `use Exchange`

You can put this on any module you want. I've chosen to put it on the Authorization module here, but you could put it on your Accounts domain module directly if you want.

defmodule MyApp.Accounts.Authentication do
  use Authority.Authentication, store: MyApp.Accounts.Store
  use Authority.Exchange,
    store: MyApp.Accounts.Store,
    authentication: __MODULE__
end

3. Call `exchange/2`

Accounts.exchange({"my@email.com", "password"})
# => {:ok, %Token{context: :identity}}

Accounts.exchange({"my@email.com", "password"}, context: :recovery)
# => {:ok, %Token{context: :recovery}}

Accounts.exchange("my@email.com", context: :recovery)
# => {:ok, %Token{context: :recovery}}

You can add `skip_validation` patterns which will exclude a credential from validations. The patterns can either be a regex or an Elixir pattern like a struct. Also, they're limited to a specific context.

darinwilson · 2017-12-12T20:15:38Z

I think this last round of refactoring is an improvement, but after thinking about this overnight, I still find the API a little confusing. I tried to think through what I find confusing, and how I might improve it, and came up with the following laundry list. These are all just highly biased ramblings, so please feel free to take or leave any of it.

First, I wonder if token management really needs to be in a separate module, and could instead be part of Authentication. I have a feeling most folks will do as Daniel did in the example and just drop it into the Accounts domain. It seems like it makes some sense to keep them integrated unless or until there's a really compelling reason to separate them.

Also, as a user of the API, I'd prefer to see more descriptive function names, rather than a single exchange function with the context passed in as a parameter. If we integrated the tokens with auth, the API could look something like this, which feels more intuitive to me:

alias MyApp.Accounts

{:ok, token} = Accounts.authenticate("my@email.com", "password")

{:ok, %User{}} = Accounts.identify(token)

{:ok, recovery_token} = Accounts.generate_recovery("my@email.com")

{:ok, token} = Account.validate_recovery("my@email.com", recovery_token)

I'm not sure we need to make the client think too much about context, other than calling the right function. If they pass a recovery token to identify, it will just fail - that seems like reasonable behavior.

This is a smaller point, but I'd also suggest renaming context to token_type, just because it seems more immediately grokable. The exchange config could then look something like this:

tokens: %{
  schema: MyApp.Accounts.Token,
  identity_assoc: :user,
  token_field: :token,
  token_data_type: :uuid,
  expiry_field: :expires_at,
  token_type_field: :token_type,
  token_types: %{
    identity: %{
      expires_in_seconds: 60
    },
    recovery: %{
      expires_in_seconds: 60
    }
  }
}

Again, I'm just kind of spitballing here. Feel free to ignore 😄

danielberkompas · 2017-12-12T21:55:35Z

@darinwilson I agree that this still doesn't feel quite right. I think that before merging this, we should discuss what we want the public API of a full-featured Accounts domain to be. Once we know that, it should become a lot more obvious how to implement it.

I'll put this PR on hold and create a separate discussion issue for this.

I reworked `Authenticate` to rely on internal callbacks rather than an external "store" module. This feels more natural to me, because it's more like a GenServer. The distinction between the authentication module and the store felt arbitrary. My goal is to get the callback system right, because that's what you'll need to use whenever you want to do something custom. After that, we can examine if we need to add an additional layer on top for common configurations. Here's an example of how you might implement an `Accounts` domain which supports email/password and tokenization. ```elixir defmodule MyApp.Accounts do alias MyApp.Accounts.{ Authentication, Tokenization } defdelegate authenticate(credential, purpose \\ :any), to: Authentication defdelegate tokenize(credential, purpose \\ :any), to: Tokenization end defmodule MyApp.Accounts.Authentication do use Authority.Authentication alias MyApp.{ Accounts.User, Accounts.Token, Repo } # Refresh the token from the `token` attribute, so that you # don't have to pass the full token def before_lookup(%Token{token: token}) do token = Token |> Repo.get_by(token: token) |> Repo.preload(:user) case token do nil -> {:error, :invalid_token} token -> {:ok, token} end end def before_lookup(identifier), do: {:ok, identifier} def lookup(%Token{user: %User{} = user}) do {:ok, user} end def lookup(email) do case Repo.get_by(User, email: email) do nil -> {:error, :invalid_email} user -> {:ok, user} end end def validate(%Token{expires_at: expires_at, purpose: _purpose}, _user, _purpose) do if DateTime.compare(DateTime.utc_now(), expires_at) == :lt do :ok else {:error, :expired_token} end end def validate(%Token{}, _user, _purpose) do {:error, :invalid_token_for_purpose} end def validate(password, user, _purpose) do case Comeonin.Bcrypt.checkpw(password, user.encrypted_password) do true -> :ok false -> {:error, :invalid_password} end end end defmodule MyApp.Accounts.Tokenization do use Authority.Tokenization alias MyApp.Accounts.Authentication def tokenize({email, password}, purpose) do with {:ok, user} <- Authentication.authenticate({email, password}, purpose) do do_tokenize(user, purpose) end end def tokenize(email, :recovery) do do_tokenize(user, purpose) end def tokenize(_other, _purpose) do {:error, :invalid_credential_for_purpose} end defp do_tokenize(user, purpose) do %Token{user: user} |> Token.insert_changeset(%{purpose: purpose}) |> Repo.insert() end end ``` As you can see, it isn't a lot of code. Perhaps we could do a few projects this way, and then see if we feel like we really need an extra layer on top for common configurations.

Authentication Draft #2

Add Exchange

ba1b6ee

This exchange module will be used to exchange one set of credentials for another, such as "user/password => token".

Support Exchange in EctoStore

29da8f4

This will massively simplify how you integrate `Exchange` if you are using `EctoStore`.

danielberkompas added 2 commits December 11, 2017 14:46

Add support for exchanging email in recovery context

526b302

You can add `skip_validation` patterns which will exclude a credential from validations. The patterns can either be a regex or an Elixir pattern like a struct. Also, they're limited to a specific context.

Rename attributes to reduce likelihood of collisions

da71db6

danielberkompas added a commit that referenced this pull request Dec 15, 2017

Merge pull request #4 from infinitered/behaviourize

8094112

Authentication Draft #2

danielberkompas closed this Dec 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Exchange #2

Add Exchange #2

Uh oh!

danielberkompas commented Dec 11, 2017

Uh oh!

ryanlntn commented Dec 11, 2017

Uh oh!

danielberkompas commented Dec 11, 2017

Uh oh!

ryanlntn commented Dec 11, 2017

Uh oh!

danielberkompas commented Dec 11, 2017 •

edited

Loading

Uh oh!

darinwilson commented Dec 12, 2017

Uh oh!

danielberkompas commented Dec 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add Exchange #2

Add Exchange #2

Uh oh!

Conversation

danielberkompas commented Dec 11, 2017

1. Update Store Module

2. Update Authentication Module

3. Update Accounts Domain

3. Use exchange/2

Caveats

Uh oh!

ryanlntn commented Dec 11, 2017

Uh oh!

danielberkompas commented Dec 11, 2017

Uh oh!

ryanlntn commented Dec 11, 2017

Uh oh!

danielberkompas commented Dec 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Add Exchange settings to your EctoStore

2. Add use Exchange

3. Call exchange/2

Uh oh!

darinwilson commented Dec 12, 2017

Uh oh!

danielberkompas commented Dec 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

3. Use `exchange/2`

danielberkompas commented Dec 11, 2017 •

edited

Loading

1. Add `Exchange` settings to your `EctoStore`

2. Add `use Exchange`

3. Call `exchange/2`