From 470275c9b7772488ece7dd2a71dee66913e04ba2 Mon Sep 17 00:00:00 2001 From: Echo Date: Tue, 4 Aug 2026 17:17:36 -0400 Subject: [PATCH 1/2] fix(oauth): actually revoke all tokens --- .../authorized_applications_controller.rb | 46 +++++++++---- .../_authorized_application.html.erb | 24 +++---- .../authorized_applications/index.html.erb | 4 +- spec/requests/authorized_applications_spec.rb | 68 +++++++++++++++++++ 4 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 spec/requests/authorized_applications_spec.rb diff --git a/app/controllers/authorized_applications_controller.rb b/app/controllers/authorized_applications_controller.rb index 085c6354..09a6d334 100644 --- a/app/controllers/authorized_applications_controller.rb +++ b/app/controllers/authorized_applications_controller.rb @@ -2,24 +2,23 @@ class AuthorizedApplicationsController < ApplicationController include AhoyAnalytics def index - @access_tokens = current_identity.access_tokens - .includes(:application) - .order(created_at: :desc) - - render layout: request.headers["HX-Request"] ? "htmx" : false + @authorized_apps = load_apps + render layout: htmx? ? "htmx" : false end def destroy - token = current_identity.access_tokens.find(params[:id]) - track_event("oauth.revoked", program_name: token.application&.name, program_id: token.application&.id, scenario: token.application&.onboarding_scenario) - token.revoke - token.create_activity :revoke, owner: current_identity, recipient: current_identity + t = current_identity.access_tokens.find(params[:id]) + app = t.application + + track_event("oauth.revoked", program_name: app&.name, program_id: app&.id, scenario: app&.onboarding_scenario) - if request.headers["HX-Request"] - @access_tokens = current_identity.access_tokens - .includes(:application) - .order(created_at: :desc) + n = current_identity.all_access_tokens.where(application_id: app.id, revoked_at: nil).count + Program.revoke_tokens_and_grants_for(app.id, current_identity) + t.create_activity :revoke, owner: current_identity, recipient: current_identity, + parameters: { application_id: app.id, tokens_revoked: n } + if htmx? + @authorized_apps = load_apps flash.now[:success] = "Application access revoked successfully" render :index, layout: "htmx" else @@ -27,4 +26,25 @@ def destroy redirect_to security_path end end + + private + + def htmx? = request.headers["HX-Request"].present? + + def load_apps + current_identity.access_tokens.includes(:application).order(created_at: :desc).to_a + .group_by(&:application_id) + .filter_map { |_, toks| + tok = toks.first + next unless tok&.application + { + token: tok, + application: tok.application, + authorized_at: toks.map(&:created_at).min, + expires_at: toks.filter_map(&:expires_at).min, + scopes: toks.flat_map { |x| x.scopes.to_a }.uniq + } + } + .sort_by { |e| e[:authorized_at] }.reverse + end end diff --git a/app/views/authorized_applications/_authorized_application.html.erb b/app/views/authorized_applications/_authorized_application.html.erb index cd484695..62a25d54 100644 --- a/app/views/authorized_applications/_authorized_application.html.erb +++ b/app/views/authorized_applications/_authorized_application.html.erb @@ -1,41 +1,39 @@ +<% app, tok, scopes = e.values_at(:application, :token, :scopes) %>
<%= inline_icon("private", size: 20) %>
-
- <%= access_token.application.name %> -
+
<%= app.name %>
<%= t "authorized_applications.created_at" %> - <%= access_token.created_at.strftime("%b %d, %Y at %l:%M %p") %> + <%= e[:authorized_at].strftime("%b %d, %Y at %l:%M %p") %>
- <% if access_token.expires_at %> + <% if e[:expires_at] %>
<%= t "authorized_applications.expires_at" %> - <%= access_token.expires_at.strftime("%b %d, %Y") %> + <%= e[:expires_at].strftime("%b %d, %Y") %>
<% end %> - <% unless access_token.scopes.empty? %> + <% if scopes.present? %>
<%= t "authorized_applications.scopes" %> - <% scope_descriptions = access_token.scopes.map { |scope| - Program::AVAILABLE_SCOPES.find { |s| s[:name] == scope.to_s }&.dig(:description) || scope.to_s - } %> - <%= scope_descriptions.join(", ") %> + <%= scopes.map { |scope| + Program::AVAILABLE_SCOPES.find { |x| x[:name] == scope.to_s }&.dig(:description) || scope.to_s + }.join(", ") %>
<% end %>
diff --git a/app/views/authorized_applications/index.html.erb b/app/views/authorized_applications/index.html.erb index 555aa3b9..e79ed19d 100644 --- a/app/views/authorized_applications/index.html.erb +++ b/app/views/authorized_applications/index.html.erb @@ -1,9 +1,9 @@
- <% if @access_tokens.empty? %> + <% if @authorized_apps.blank? %>

<%= t "authorized_applications.none" %>

<% else %>
- <%= render partial: "authorized_application", collection: @access_tokens, as: :access_token %> + <%= render partial: "authorized_application", collection: @authorized_apps, as: :e %>
<% end %>
diff --git a/spec/requests/authorized_applications_spec.rb b/spec/requests/authorized_applications_spec.rb new file mode 100644 index 00000000..eca4ded1 --- /dev/null +++ b/spec/requests/authorized_applications_spec.rb @@ -0,0 +1,68 @@ +require "rails_helper" + +RSpec.describe "AuthorizedApplications", type: :request do + let(:identity) { create(:identity) } + let(:program) { create(:program, :with_all_scopes) } + + def tok(scopes:, app: program) + create(:oauth_token, resource_owner: identity, application: app, scopes:) + end + + before do + allow_any_instance_of(ApplicationController).to receive(:current_identity).and_return(identity) + end + + describe "GET /authorized_applications" do + it "lists one entry per app even with multiple tokens" do + tok(scopes: "basic_info") + tok(scopes: "address phone") + + get authorized_applications_path + expect(response).to have_http_status(:ok) + expect(response.body.scan(program.name).size).to eq(1) + end + end + + describe "DELETE /authorized_applications/:id" do + it "revokes all tokens for the app, not only the clicked one" do + a = tok(scopes: "basic_info") + b = tok(scopes: "address") + other = tok(scopes: "email", app: create(:program)) + + delete authorized_application_path(a) + + expect(response).to redirect_to(security_path) + expect(a.reload.revoked_at).to be_present + expect(b.reload.revoked_at).to be_present + expect(other.reload.revoked_at).to be_nil + end + + it "revokes access grants for the app" do + t = tok(scopes: "basic_info") + g = Doorkeeper::AccessGrant.create!( + resource_owner: identity, + application: program, + token: SecureRandom.hex(32), + expires_in: 600, + redirect_uri: program.redirect_uri, + scopes: "basic_info" + ) + + delete authorized_application_path(t) + expect(g.reload.revoked_at).to be_present + end + + it "rejects /api/v1/me for every former token of that app" do + a = tok(scopes: "basic_info address") + b = tok(scopes: "phone") + + delete authorized_application_path(a) + + get "/api/v1/me", headers: { "Authorization" => "Bearer #{a.token}" } + expect(response).to have_http_status(:unauthorized) + + get "/api/v1/me", headers: { "Authorization" => "Bearer #{b.token}" } + expect(response).to have_http_status(:unauthorized) + end + end +end From be2c12989e29848a5af256d4d01c9a3effdaaf26 Mon Sep 17 00:00:00 2001 From: Echo Date: Tue, 4 Aug 2026 17:27:49 -0400 Subject: [PATCH 2/2] rubocop --- app/controllers/api/v1/application_controller.rb | 2 +- app/controllers/identity_backup_codes_controller.rb | 2 +- app/controllers/identity_totps_controller.rb | 2 +- app/controllers/identity_webauthn_credentials_controller.rb | 2 +- app/controllers/step_up_controller.rb | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/v1/application_controller.rb b/app/controllers/api/v1/application_controller.rb index 9e70a588..83e54646 100644 --- a/app/controllers/api/v1/application_controller.rb +++ b/app/controllers/api/v1/application_controller.rb @@ -44,7 +44,7 @@ def authenticate! @current_program = @current_token.application @current_scopes = @current_token.scopes unless @current_program&.active? - return render json: { error: "invalid_auth" }, status: :unauthorized + render json: { error: "invalid_auth" }, status: :unauthorized end else unless @current_token.hq_official? diff --git a/app/controllers/identity_backup_codes_controller.rb b/app/controllers/identity_backup_codes_controller.rb index d1a06052..f8458a78 100644 --- a/app/controllers/identity_backup_codes_controller.rb +++ b/app/controllers/identity_backup_codes_controller.rb @@ -1,5 +1,5 @@ class IdentityBackupCodesController < ApplicationController - before_action -> { require_step_up("regenerate_backup_codes", return_to: identity_backup_codes_path) }, only: [:create, :confirm] + before_action -> { require_step_up("regenerate_backup_codes", return_to: identity_backup_codes_path) }, only: [ :create, :confirm ] def index @backup_codes = current_identity.backup_codes.active.order(created_at: :desc) diff --git a/app/controllers/identity_totps_controller.rb b/app/controllers/identity_totps_controller.rb index 8958360b..bd19824d 100644 --- a/app/controllers/identity_totps_controller.rb +++ b/app/controllers/identity_totps_controller.rb @@ -1,5 +1,5 @@ class IdentityTotpsController < ApplicationController - before_action -> { require_step_up("add_totp", return_to: security_path) }, only: [:new, :verify] + before_action -> { require_step_up("add_totp", return_to: security_path) }, only: [ :new, :verify ] def index @totp = current_identity.totp diff --git a/app/controllers/identity_webauthn_credentials_controller.rb b/app/controllers/identity_webauthn_credentials_controller.rb index b378b895..3fc3a066 100644 --- a/app/controllers/identity_webauthn_credentials_controller.rb +++ b/app/controllers/identity_webauthn_credentials_controller.rb @@ -1,7 +1,7 @@ class IdentityWebauthnCredentialsController < ApplicationController include WebauthnAuthenticatable - before_action -> { require_step_up("add_passkey", return_to: security_path) }, only: [:new, :options, :create] + before_action -> { require_step_up("add_passkey", return_to: security_path) }, only: [ :new, :options, :create ] def index @webauthn_credentials = current_identity.webauthn_credentials.order(created_at: :desc) diff --git a/app/controllers/step_up_controller.rb b/app/controllers/step_up_controller.rb index 0042bd36..6e9089a6 100644 --- a/app/controllers/step_up_controller.rb +++ b/app/controllers/step_up_controller.rb @@ -7,8 +7,8 @@ class StepUpController < ApplicationController VALID_ACTIONS = %w[remove_totp disable_2fa oidc_reauth email_change remove_passkey add_passkey regenerate_backup_codes add_totp].freeze ACTIONS_WITHOUT_EMAIL_FALLBACK = %w[email_change disable_2fa remove_passkey].freeze - before_action :validate_action_type, except: [:webauthn_options] - before_action :require_pending_step_up, only: [:send_email_code, :verify, :resend_email] + before_action :validate_action_type, except: [ :webauthn_options ] + before_action :require_pending_step_up, only: [ :send_email_code, :verify, :resend_email ] def new session[:pending_step_up_action] = params[:action_type]