From e05fbd574f3314a0082bb613b1bf015aa330a227 Mon Sep 17 00:00:00 2001
From: Marcin Kulik <m@ku1ik.com>
Date: Thu, 2 Apr 2015 09:45:39 +0000
Subject: [PATCH] Add ability to revoke recorder tokens

---
 app/assets/stylesheets/users.sass             |  4 ++
 app/controllers/api_tokens_controller.rb      |  9 +++-
 app/controllers/users_controller.rb           | 14 +++---
 app/models/api_token.rb                       |  7 +++
 app/models/user.rb                            |  8 ++++
 app/policies/api_token_policy.rb              |  9 ++++
 app/presenters/user_edit_page_presenter.rb    | 27 ++++++++++++
 app/views/docs/usage.html.md                  |  8 ++--
 app/views/users/edit.html.slim                | 43 +++++++++++++++++--
 config/routes.rb                              |  1 +
 ...0401161102_add_revoked_at_to_api_tokens.rb |  5 +++
 db/schema.rb                                  |  3 +-
 spec/features/tokens_spec.rb                  | 38 ++++++++++++++++
 spec/policies/api_token_policy_spec.rb        | 27 ++++++++++++
 14 files changed, 187 insertions(+), 16 deletions(-)
 create mode 100644 app/policies/api_token_policy.rb
 create mode 100644 app/presenters/user_edit_page_presenter.rb
 create mode 100644 db/migrate/20150401161102_add_revoked_at_to_api_tokens.rb
 create mode 100644 spec/features/tokens_spec.rb
 create mode 100644 spec/policies/api_token_policy_spec.rb

diff --git a/app/assets/stylesheets/users.sass b/app/assets/stylesheets/users.sass
index 549a6d0..433f93d 100644
--- a/app/assets/stylesheets/users.sass
+++ b/app/assets/stylesheets/users.sass
@@ -30,3 +30,7 @@
 
     .actions
       margin: 20px 0 0 0
+
+.edit-page
+  > .row:last-child
+    margin-top: 30px
diff --git a/app/controllers/api_tokens_controller.rb b/app/controllers/api_tokens_controller.rb
index 20cb09e..8c4bf20 100644
--- a/app/controllers/api_tokens_controller.rb
+++ b/app/controllers/api_tokens_controller.rb
@@ -5,10 +5,17 @@ class ApiTokensController < ApplicationController
   def create
     current_user.assign_api_token(params[:api_token])
     redirect_to profile_path(current_user),
-      notice: "Successfully registered your API token. ^5"
+      notice: "Successfully registered your recorder token."
 
   rescue ActiveRecord::RecordInvalid, ApiToken::ApiTokenTakenError
     render :error
   end
 
+  def destroy
+    api_token = ApiToken.find(params[:id])
+    authorize api_token
+    api_token.revoke!
+    redirect_to edit_user_path, notice: "Token revoked."
+  end
+
 end
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1d6c162..f316946 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -15,18 +15,18 @@ class UsersController < ApplicationController
   end
 
   def edit
-    @user = current_user
-    authorize @user
+    authorize current_user
+    render locals: { page: UserEditPagePresenter.new(current_user) }
   end
 
   def update
-    @user = User.find(current_user.id)
-    authorize @user
+    authorize current_user
+    user = User.find(current_user.id)
 
-    if @user.update_attributes(update_params)
-      redirect_to profile_path(@user), notice: 'Account settings saved.'
+    if user.update_attributes(update_params)
+      redirect_to profile_path(user), notice: 'Account settings saved.'
     else
-      render :edit, status: 422
+      render :edit, status: 422, locals: { page: UserEditPagePresenter.new(user) }
     end
   end
 
diff --git a/app/models/api_token.rb b/app/models/api_token.rb
index 976abf0..de6c2c6 100644
--- a/app/models/api_token.rb
+++ b/app/models/api_token.rb
@@ -7,6 +7,9 @@ class ApiToken < ActiveRecord::Base
   validates :user, :token, presence: true
   validates :token, uniqueness: true
 
+  scope :active, -> { where(revoked_at: nil) }
+  scope :revoked, -> { where('revoked_at IS NOT NULL') }
+
   def self.for_token(token)
     where(token: token).first
   end
@@ -27,6 +30,10 @@ class ApiToken < ActiveRecord::Base
     user.merge_to(target_user)
   end
 
+  def revoke!
+    update!(revoked_at: Time.now)
+  end
+
   private
 
   def taken?
diff --git a/app/models/user.rb b/app/models/user.rb
index 87c5565..aaee118 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -56,6 +56,14 @@ class User < ActiveRecord::Base
     new(temporary_username: 'anonymous')
   end
 
+  def active_api_tokens
+    api_tokens.active
+  end
+
+  def revoked_api_tokens
+    api_tokens.revoked
+  end
+
   def confirmed?
     email.present?
   end
diff --git a/app/policies/api_token_policy.rb b/app/policies/api_token_policy.rb
new file mode 100644
index 0000000..d5425e3
--- /dev/null
+++ b/app/policies/api_token_policy.rb
@@ -0,0 +1,9 @@
+class ApiTokenPolicy < ApplicationPolicy
+
+  def destroy?
+    return false unless user
+
+    user.admin? || record.user == user
+  end
+
+end
diff --git a/app/presenters/user_edit_page_presenter.rb b/app/presenters/user_edit_page_presenter.rb
new file mode 100644
index 0000000..9957887
--- /dev/null
+++ b/app/presenters/user_edit_page_presenter.rb
@@ -0,0 +1,27 @@
+class UserEditPagePresenter
+
+  attr_reader :user
+
+  def initialize(user)
+    @user = user
+  end
+
+  def active_tokens
+    sort(user.active_api_tokens)
+  end
+
+  def revoked_tokens
+    sort(user.revoked_api_tokens)
+  end
+
+  def show_tokens?
+    !active_tokens.empty? || !revoked_tokens.empty?
+  end
+
+  private
+
+  def sort(tokens)
+    tokens.sort_by { |token| token.created_at }.reverse
+  end
+
+end
diff --git a/app/views/docs/usage.html.md b/app/views/docs/usage.html.md
index 196bcdc..b2be245 100644
--- a/app/views/docs/usage.html.md
+++ b/app/views/docs/usage.html.md
@@ -66,9 +66,9 @@ publishing it on asciinema.org.
 
 ## `auth`
 
-__Assign local API token to asciinema.org account.__
+__Assign local recorder token to asciinema.org account.__
 
-On every machine you install asciinema recorder, you get a new, unique API
+On every machine you install asciinema recorder, you get a new, unique recorder
 token. This command connects this local token with your asciinema.org account,
 and links all asciicasts recorded on this machine with the account.
 
@@ -79,6 +79,6 @@ URL.
 NOTE: it is __necessary__ to do this if you want to __edit or delete__ your
 recordings on asciinema.org.
 
-You can synchronize your `~/.asciinema/config` file (which keeps the API
-token) across the machines but that's not necessary. You can assign new
+You can synchronize your `~/.asciinema/config` file (which keeps the token)
+across the machines but that's not necessary. You can assign new recorder
 tokens to your account from as many machines as you want.
diff --git a/app/views/users/edit.html.slim b/app/views/users/edit.html.slim
index 2ed8a44..3faf032 100644
--- a/app/views/users/edit.html.slim
+++ b/app/views/users/edit.html.slim
@@ -1,8 +1,8 @@
-.container
+.container.edit-page
   .row
     .col-md-9
-      = horizontal_form_for @user do |f|
-        legend Your settings
+      = horizontal_form_for page.user do |f|
+        legend Account settings
 
         = f.input :username
         = f.input :email, required: true
@@ -11,3 +11,40 @@
         = f.buttons do
           = f.button :submit, 'Save', class: 'btn-primary'
           = link_to 'Cancel', profile_path(current_user), class: 'btn'
+
+  .row
+    .col-md-12
+      legend Recorder tokens
+
+      - if page.show_tokens?
+        p The following recorder tokens have been associated with your account:
+
+        - unless page.active_tokens.empty?
+          ul
+            - page.active_tokens.each do |token|
+              li
+                = token.token
+                '  registered
+                = time_ago_tag token.created_at
+                '  -
+                = link_to 'Revoke', api_token_path(token), method: :delete
+
+        - unless page.revoked_tokens.empty?
+          ul
+            - page.revoked_tokens.each do |token|
+              li.revoked-token
+                = token.token
+                '  registered
+                = time_ago_tag token.created_at
+                ' , revoked
+                = time_ago_tag token.revoked_at
+      - else
+        p
+          | If you want your recordings to be assigned to your profile
+            you have to register your local recorder token.
+
+        p
+          ' There is currently no recorder token associated with your account.
+            Run
+          code asciinema auth
+          |  in your terminal to register one.
diff --git a/config/routes.rb b/config/routes.rb
index faa3066..294107c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -33,6 +33,7 @@ Rails.application.routes.draw do
   get "/login/:token" => "sessions#create", as: :login_token
   get "/logout" => "sessions#destroy"
 
+  resources :api_tokens, only: [:create, :destroy]
   get "/connect/:api_token" => "api_tokens#create"
 
   resource :user
diff --git a/db/migrate/20150401161102_add_revoked_at_to_api_tokens.rb b/db/migrate/20150401161102_add_revoked_at_to_api_tokens.rb
new file mode 100644
index 0000000..e49ef65
--- /dev/null
+++ b/db/migrate/20150401161102_add_revoked_at_to_api_tokens.rb
@@ -0,0 +1,5 @@
+class AddRevokedAtToApiTokens < ActiveRecord::Migration
+  def change
+    add_column :api_tokens, :revoked_at, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index fc69348..c8575be 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150327171201) do
+ActiveRecord::Schema.define(version: 20150401161102) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -21,6 +21,7 @@ ActiveRecord::Schema.define(version: 20150327171201) do
     t.string   "token",      null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "revoked_at"
   end
 
   add_index "api_tokens", ["token"], name: "index_api_tokens_on_token", using: :btree
diff --git a/spec/features/tokens_spec.rb b/spec/features/tokens_spec.rb
new file mode 100644
index 0000000..ef27f07
--- /dev/null
+++ b/spec/features/tokens_spec.rb
@@ -0,0 +1,38 @@
+require 'rails_helper'
+
+feature "Recorder tokens management" do
+
+  let!(:user) { create(:user) }
+
+  scenario 'Listing tokens when user has none' do
+    login_as user
+    visit edit_user_path
+
+    expect(page).to have_content('asciinema auth')
+  end
+
+  scenario 'Listing tokens when user has some' do
+    api_token = create(:api_token, user: user)
+
+    login_as user
+    visit edit_user_path
+
+    expect(page).to have_content(api_token.token)
+    expect(page).to have_link('Revoke')
+    expect(page).to have_no_content('asciinema auth')
+  end
+
+  scenario 'Revoking a token' do
+    api_token = create(:api_token, user: user)
+
+    login_as user
+    visit edit_user_path
+
+    click_on "Revoke"
+
+    expect(page).to have_content(api_token.token)
+    expect(page).to have_no_link('Revoke')
+  end
+
+end
+
diff --git a/spec/policies/api_token_policy_spec.rb b/spec/policies/api_token_policy_spec.rb
new file mode 100644
index 0000000..b25c5d6
--- /dev/null
+++ b/spec/policies/api_token_policy_spec.rb
@@ -0,0 +1,27 @@
+require 'rails_helper'
+
+describe ApiTokenPolicy do
+
+  subject { described_class }
+
+  permissions :destroy? do
+    it "denies access if user is nil" do
+      expect(subject).not_to permit(nil, ApiToken.new)
+    end
+
+    it "grants access if user is admin" do
+      user = stub_model(User, admin?: true)
+      expect(subject).to permit(user, ApiToken.new)
+    end
+
+    it "grants access if user is the owner of the token" do
+      user = stub_model(User, admin?: false)
+      expect(subject).to permit(user, ApiToken.new(user: user))
+    end
+
+    it "denies access if user isn't the owner of the token" do
+      expect(subject).not_to permit(User.new, ApiToken.new(user: User.new))
+    end
+  end
+
+end