Collapse duplicate OAuth authorisations

Only show each application once, even if it has multiple tokens, and
revoke all the relevant tokens when asked.

app/controllers/oauth_controller.rb

@@ -27,12 +27,23 @@ def user_authorizes_token?
   end
 
   def revoke
-    @token = current_user.oauth_tokens.find_by :token => params[:token]
-    if @token
-      @token.invalidate!
-      flash[:notice] = t("oauth.revoke.flash", :application => @token.client_application.name)
+    if params[:token]
+      tokens = current_user.oauth_tokens.where(:token => params[:token])
+    elsif params[:application]
+      tokens = current_user.oauth_tokens.where(:client_application => params[:application])
+    end
+
+    if tokens.nil?
+      render :text => "", :status => :bad_request
+    elsif tokens.empty?
+      render :text => "", :status => :not_found
+    else
+      tokens.each(&:invalidate!)
+
+      flash[:notice] = t("oauth.revoke.flash", :application => tokens.first.client_application.name)
+
+      redirect_to oauth_clients_url(:display_name => current_user.display_name)
     end
-    redirect_to oauth_clients_url(:display_name => @token.user.display_name)
   end
 
   protected

app/views/oauth_clients/index.html.erb

@@ -3,34 +3,42 @@
 <% end %>
 
 <% unless @tokens.empty? %>
-<h3><%= t'oauth_clients.index.my_tokens' %></h3>
-<p><%= t'oauth_clients.index.list_tokens' %></p>
-<table>
-  <tr><th><%= t'oauth_clients.index.application' %></th>
-    <th><%= t'oauth_clients.index.issued_at' %></th><th>&nbsp;</th></tr>
-  <% @tokens.each do |token|%>
-    <%= content_tag_for :tr, token do %>
-      <td><%= link_to token.client_application.name, token.client_application.url %></td>
-      <td><%= token.authorized_at %></td>
+<div id="authorised_applications">
+  <h3><%= t'oauth_clients.index.my_tokens' %></h3>
+  <p><%= t'oauth_clients.index.list_tokens' %></p>
+  <table>
+    <tr>
+      <th><%= t'oauth_clients.index.application' %></th>
+      <th><%= t'oauth_clients.index.first_authorised' %></th>
+      <th>&nbsp;</th>
+    </tr>
+    <% @tokens.group_by { |t| t.client_application }.each do |client,tokens|%>
+    <%= content_tag_for :tr, client do %>
+      <td><%= link_to client.name, client.url %></td>
+      <td><%= tokens.min_by { |t| t.authorized_at }.authorized_at %></td>
       <td>
-	<%= form_tag :controller => 'oauth', :action => 'revoke' do %>
-	<%= hidden_field_tag 'token', token.token %>
-	<%= submit_tag t('oauth_clients.index.revoke') %>
-	<% end %>
+        <%= form_tag :controller => 'oauth', :action => 'revoke' do %>
+        <%= hidden_field_tag 'application', client.id %>
+        <%= submit_tag t('oauth_clients.index.revoke') %>
+        <% end %>
       </td>
     <% end %>
-  <% end %>
-</table>
+    <% end %>
+  </table>
+</div>
 <% end %>
-<h3><%= t'oauth_clients.index.my_apps' %></h3>
-<% if @client_applications.empty? %>
-<p><%= raw(t('oauth_clients.index.no_apps', :oauth => "<a href=\"http://oauth.net\">OAuth</a>")) %></p>
-<% else %>
-<p><%= t'oauth_clients.index.registered_apps' %></p>
-<% @client_applications.each do |client|%>
-  <%= div_for client do %>
-    <%= link_to client.name, :action => :show, :id => client.id %>
+
+<div id="client_applications">
+  <h3><%= t'oauth_clients.index.my_apps' %></h3>
+  <% if @client_applications.empty? %>
+  <p><%= raw(t('oauth_clients.index.no_apps', :oauth => "<a href=\"http://oauth.net\">OAuth</a>")) %></p>
+  <% else %>
+  <p><%= t'oauth_clients.index.registered_apps' %></p>
+  <% @client_applications.each do |client|%>
+    <%= div_for client do %>
+      <%= link_to client.name, :action => :show, :id => client.id %>
+    <% end %>
   <% end %>
-<% end %>
-<% end %>
-<h3><%= link_to t('oauth_clients.index.register_new'), :action => :new %></h3>
+  <% end %>
+  <h3><%= link_to t('oauth_clients.index.register_new'), :action => :new %></h3>
+</div>

config/locales/en.yml

@@ -1682,7 +1682,7 @@ en:
       my_tokens: "My Authorised Applications"
       list_tokens: "The following tokens have been issued to applications in your name:"
       application: "Application Name"
-      issued_at: "Issued At"
+      first_authorised: "First Authorised"
       revoke: "Revoke!"
       my_apps: "My Client Applications"
       no_apps: "Do you have an application you would like to register for use with us using the %{oauth} standard? You must register your web application before it can make OAuth requests to this service."

test/controllers/oauth_clients_controller_test.rb

@@ -36,7 +36,9 @@ def test_routes
 
   def test_index
     user = create(:user)
-    create_list(:client_application, 2, :user => user)
+    applications = create_list(:client_application, 3, :user => user)
+    create_list(:oauth_token, 1, :user => user, :client_application => applications.first, :authorized_at => Time.now.utc)
+    create_list(:oauth_token, 3, :user => user, :client_application => applications.second, :authorized_at => Time.now.utc)
 
     get :index, :display_name => user.display_name
     assert_response :redirect
@@ -45,7 +47,12 @@ def test_index
     get :index, { :display_name => user.display_name }, { :user => user }
     assert_response :success
     assert_template "index"
-    assert_select "div.client_application", 2
+    assert_select "div#authorised_applications", 1 do
+      assert_select "tr.client_application", 2
+    end
+    assert_select "div#client_applications", 1 do
+      assert_select "div.client_application", 3
+    end
   end
 
   def test_new

test/controllers/oauth_controller_test.rb

@@ -45,4 +45,58 @@ def test_routes
       { :controller => "oauth", :action => "test_request" }
     )
   end
+
+  ##
+  # test revoking a specific token
+  def test_revoke_by_token
+    user = create(:user)
+    applications = create_list(:client_application, 2, :user => user)
+    tokens = create_list(:oauth_token, 2, :user => user, :client_application => applications.first, :authorized_at => Time.now.utc)
+    create_list(:oauth_token, 2, :user => user, :client_application => applications.second, :authorized_at => Time.now.utc)
+
+    assert_no_difference "OauthToken.authorized.count" do
+      post :revoke, :token => tokens.first.token
+      assert_response :forbidden
+    end
+
+    assert_no_difference "OauthToken.authorized.count" do
+      post :revoke, { :token => "dummy" }, { :user => user }
+      assert_response :not_found
+    end
+
+    assert_difference "OauthToken.authorized.count", -1 do
+      post :revoke, { :token => tokens.first.token }, { :user => user }
+      assert_response :redirect
+      assert_redirected_to oauth_clients_url(:display_name => user.display_name)
+      assert tokens.first.reload.invalidated?
+      assert !tokens.second.reload.invalidated?
+    end
+  end
+
+  ##
+  # test revoking all tokens for an application
+  def text_revoke_by_application
+    user = create(:user)
+    applications = create_list(:client_application, 2, :user => user)
+    tokens = create_list(:oauth_token, 2, :user => user, :client_application => applications.first, :authorized_at => Time.now.utc)
+    create_list(:oauth_token, 2, :user => user, :client_application => applications.second, :authorized_at => Time.now.utc)
+
+    assert_no_difference "OauthToken.authorized.count" do
+      post :revoke, :application => applications.first.id
+      assert_response :forbidden
+    end
+
+    assert_no_difference "OauthToken.authorized.count" do
+      post :revoke, { :application => 0 }, { :user => user }
+      assert_response :not_found
+    end
+
+    assert_difference "OauthToken.authorized.count", -2 do
+      post :revoke, { :application => applications.first.id }, { :user => user }
+      assert_response :redirect
+      assert_redirected_to oauth_clients_url(:display_name => user.display_name)
+      assert tokens.first.reload.invalidated?
+      assert tokens.second.reload.invalidated?
+    end
+  end
 end

test/factories/oauth_tokens.rb

@@ -0,0 +1,6 @@
+FactoryGirl.define do
+  factory :oauth_token do
+    user
+    client_application
+  end
+end