From e8d0e9f40a2b76696e67e3bbdb4c61d6e8213f31 Mon Sep 17 00:00:00 2001 From: Mohamad Mortada Date: Mon, 2 Nov 2026 10:59:07 -0800 Subject: [PATCH 1/4] [v4] Assign a POC to Doorkeeper oauth applications --- .../oauth_applications_controller.rb | 13 ++++ .../doorkeeper/applications/_form.html.erb | 70 ++++++++++++++++++ .../doorkeeper/applications/index.html.erb | 46 ++++++++++++ .../doorkeeper/applications/show.html.erb | 72 +++++++++++++++++++ config/initializers/doorkeeper.rb | 2 + config/locales/doorkeeper.en.yml | 1 + config/routes.rb | 1 + ...2120000_add_owner_to_oauth_applications.rb | 10 +++ db/schema.rb | 5 +- dev-docs/v4-api/standards.md | 3 +- .../api/v4/donations_controller_spec.rb | 2 +- spec/requests/oauth_applications_spec.rb | 44 ++++++++++++ 12 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 app/controllers/oauth_applications_controller.rb create mode 100644 app/views/doorkeeper/applications/_form.html.erb create mode 100644 app/views/doorkeeper/applications/index.html.erb create mode 100644 app/views/doorkeeper/applications/show.html.erb create mode 100644 db/migrate/20261102120000_add_owner_to_oauth_applications.rb create mode 100644 spec/requests/oauth_applications_spec.rb diff --git a/app/controllers/oauth_applications_controller.rb b/app/controllers/oauth_applications_controller.rb new file mode 100644 index 0000000000..7cbff4fdaf --- /dev/null +++ b/app/controllers/oauth_applications_controller.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class OauthApplicationsController < Doorkeeper::ApplicationsController + skip_after_action :verify_authorized + + private + + def application_params + owner = User.find_by(email: params.dig(:doorkeeper_application, :owner_email)) + super.merge(owner:) + end + +end diff --git a/app/views/doorkeeper/applications/_form.html.erb b/app/views/doorkeeper/applications/_form.html.erb new file mode 100644 index 0000000000..284b52c0f3 --- /dev/null +++ b/app/views/doorkeeper/applications/_form.html.erb @@ -0,0 +1,70 @@ +<%= form_for application, url: doorkeeper_submit_path(application), as: :doorkeeper_application, html: { role: "form" } do |f| %> + <% if application.errors.any? %> +

<%= t("doorkeeper.applications.form.error") %>

+ <% end %> + +
+ <%= f.label :name, class: "col-sm-2 col-form-label font-weight-bold" %> +
+ <%= f.text_field :name, class: "form-control #{ 'is-invalid' if application.errors[:name].present? }", required: true %> + <%= doorkeeper_errors_for application, :name %> +
+
+ +
+ <%= f.label :owner_email, "Point of contact", class: "col-sm-2 col-form-label font-weight-bold" %> +
+ <%= f.email_field :owner_email, value: params.dig(:doorkeeper_application, :owner_email) || application.owner&.email, class: "form-control #{ 'is-invalid' if application.errors[:owner].present? }", required: true %> + <%= doorkeeper_errors_for application, :owner %> + + Email of the HCB user responsible for this application. + +
+
+ +
+ <%= f.label :redirect_uri, class: "col-sm-2 col-form-label font-weight-bold" %> +
+ <%= f.text_area :redirect_uri, class: "form-control #{ 'is-invalid' if application.errors[:redirect_uri].present? }" %> + <%= doorkeeper_errors_for application, :redirect_uri %> + + <%= t("doorkeeper.applications.help.redirect_uri") %> + + + <% if Doorkeeper.configuration.allow_blank_redirect_uri?(application) %> + + <%= t("doorkeeper.applications.help.blank_redirect_uri") %> + + <% end %> +
+
+ +
+ <%= f.label :confidential, class: "col-sm-2 form-check-label font-weight-bold" %> +
+ <%= f.check_box :confidential, class: "checkbox #{ 'is-invalid' if application.errors[:confidential].present? }" %> + <%= doorkeeper_errors_for application, :confidential %> + + <%= t("doorkeeper.applications.help.confidential") %> + +
+
+ +
+ <%= f.label :scopes, class: "col-sm-2 col-form-label font-weight-bold" %> +
+ <%= f.text_field :scopes, class: "form-control #{ 'has-error' if application.errors[:scopes].present? }" %> + <%= doorkeeper_errors_for application, :scopes %> + + <%= t("doorkeeper.applications.help.scopes") %> + +
+
+ +
+
+ <%= f.submit t("doorkeeper.applications.buttons.submit"), class: "btn btn-primary" %> + <%= link_to t("doorkeeper.applications.buttons.cancel"), oauth_applications_path, class: "btn btn-secondary" %> +
+
+<% end %> diff --git a/app/views/doorkeeper/applications/index.html.erb b/app/views/doorkeeper/applications/index.html.erb new file mode 100644 index 0000000000..978a553b71 --- /dev/null +++ b/app/views/doorkeeper/applications/index.html.erb @@ -0,0 +1,46 @@ +
+

<%= t(".title") %>

+
+ +

<%= link_to t(".new"), new_oauth_application_path, class: "btn btn-success" %>

+ + + + + + + + + + + + + + <% @applications.each do |application| %> + + + + + + + + + <% end %> + +
<%= t(".name") %>Point of contact<%= t(".callback_url") %><%= t(".confidential") %><%= t(".actions") %>
+ <%= link_to application.name, oauth_application_path(application) %> + + <% if application.owner %> + <%= link_to application.owner.name, edit_user_path(application.owner) %> (<%= application.owner.email %>) + <% else %> + Unassigned + <% end %> + + <%= simple_format(application.redirect_uri) %> + + <%= application.confidential? ? t("doorkeeper.applications.index.confidentiality.yes") : t("doorkeeper.applications.index.confidentiality.no") %> + + <%= link_to t("doorkeeper.applications.buttons.edit"), edit_oauth_application_path(application), class: "btn btn-link" %> + + <%= render "delete_form", application: application %> +
diff --git a/app/views/doorkeeper/applications/show.html.erb b/app/views/doorkeeper/applications/show.html.erb new file mode 100644 index 0000000000..2c72cca2f4 --- /dev/null +++ b/app/views/doorkeeper/applications/show.html.erb @@ -0,0 +1,72 @@ +
+

<%= t(".title", name: @application.name) %>

+
+ +
+
+

Point of contact:

+

+ <% if @application.owner %> + <%= link_to @application.owner.name, edit_user_path(@application.owner) %> (<%= @application.owner.email %>) + <% else %> + Unassigned + <% end %> +

+ +

<%= t(".application_id") %>:

+

<%= @application.uid %>

+ +

<%= t(".secret") %>:

+

+ + <% secret = flash[:application_secret].presence || @application.plaintext_secret %> + <% if secret.blank? && Doorkeeper.config.application_secret_hashed? %> + <%= t(".secret_hashed") %> + <% else %> + <%= secret %> + <% end %> + +

+ +

<%= t(".scopes") %>:

+

+ + <% if @application.scopes.present? %> + <%= @application.scopes %> + <% else %> + <%= t(".not_defined") %> + <% end %> + +

+ +

<%= t(".confidential") %>:

+

<%= @application.confidential? %>

+ +

<%= t(".callback_urls") %>:

+ + <% if @application.redirect_uri.present? %> + + <% @application.redirect_uri.split.each do |uri| %> + + + + + <% end %> +
+ <%= uri %> + + <%= link_to t("doorkeeper.applications.buttons.authorize"), oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: "code", scope: @application.scopes), class: "btn btn-success", target: "_blank" %> +
+ <% else %> + <%= t(".not_defined") %> + <% end %> +
+ +
+

<%= t(".actions") %>

+ +

<%= link_to t("doorkeeper.applications.buttons.edit"), edit_oauth_application_path(@application), class: "btn btn-primary" %>

+ +

<%= render "delete_form", application: @application, submit_btn_css: "btn btn-danger" %>

+
+
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 02405213ad..c5618b81fe 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -169,6 +169,8 @@ use_refresh_token + enable_application_owner confirmation: true + # Provide support for an owner to be assigned to each registered application (disabled by default) # Optional parameter confirmation: true (default: false) if you want to enforce ownership of # a registered application diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 99fa3d4d89..5c7a15b7a9 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -4,6 +4,7 @@ en: doorkeeper/application: name: 'Name' redirect_uri: 'Redirect URI' + owner: 'Point of contact' errors: models: doorkeeper/application: diff --git a/config/routes.rb b/config/routes.rb index 69f7c6760c..4d47e876f3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -711,6 +711,7 @@ use_doorkeeper scope: "api/v4/oauth" do skip_controllers :authorized_applications + controllers applications: "oauth_applications" end use_doorkeeper_device_authorization_grant scope: "api/v4/oauth" diff --git a/db/migrate/20261102120000_add_owner_to_oauth_applications.rb b/db/migrate/20261102120000_add_owner_to_oauth_applications.rb new file mode 100644 index 0000000000..1e78503d98 --- /dev/null +++ b/db/migrate/20261102120000_add_owner_to_oauth_applications.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddOwnerToOauthApplications < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def change + add_reference :oauth_applications, :owner, polymorphic: true, index: { algorithm: :concurrently } + end + +end diff --git a/db/schema.rb b/db/schema.rb index 15facd4939..8001d9298d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_09_24_140000) do +ActiveRecord::Schema[8.1].define(version: 2026_11_02_120000) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" @@ -1824,12 +1824,15 @@ t.boolean "confidential", default: true, null: false t.datetime "created_at", null: false t.string "name", null: false + t.bigint "owner_id" + t.string "owner_type" t.text "redirect_uri", null: false t.string "scopes", default: "", null: false t.string "secret", null: false t.boolean "trusted", default: false, null: false t.string "uid", null: false t.datetime "updated_at", null: false + t.index ["owner_type", "owner_id"], name: "index_oauth_applications_on_owner" t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true end diff --git a/dev-docs/v4-api/standards.md b/dev-docs/v4-api/standards.md index a1a322858f..b1e372d04e 100644 --- a/dev-docs/v4-api/standards.md +++ b/dev-docs/v4-api/standards.md @@ -57,7 +57,8 @@ app = Doorkeeper::Application.create( name: "tester", redirect_uri: "http://localhost:3000/", scopes: ["read", "write"], - confidential: false + confidential: false, + owner: User.find_by(email: "you@example.com") # point of contact, required ) ``` diff --git a/spec/controllers/api/v4/donations_controller_spec.rb b/spec/controllers/api/v4/donations_controller_spec.rb index 1ddb99f7ab..fc21151d52 100644 --- a/spec/controllers/api/v4/donations_controller_spec.rb +++ b/spec/controllers/api/v4/donations_controller_spec.rb @@ -18,7 +18,7 @@ event = create(:event) create(:organizer_position, user:, event:) - trusted_app = Doorkeeper::Application.create!(name: "Trusted App", redirect_uri: "https://hcb.hackclub.com", trusted: true) + trusted_app = Doorkeeper::Application.create!(name: "Trusted App", redirect_uri: "https://hcb.hackclub.com", trusted: true, owner: user) token = create(:api_token, user:, application: trusted_app) request.headers["Authorization"] = "Bearer #{token.token}" diff --git a/spec/requests/oauth_applications_spec.rb b/spec/requests/oauth_applications_spec.rb new file mode 100644 index 0000000000..3141674829 --- /dev/null +++ b/spec/requests/oauth_applications_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "OAuth applications", type: :request do + let(:admin) { create(:user, :make_admin) } + let(:poc) { create(:user, email: "poc@hackclub.com") } + + before do + admin_session = create(:user_session, user: admin, verified: true, expiration_at: 1.hour.from_now) + + allow_any_instance_of(SessionsHelper) + .to receive(:find_current_session) + .and_return(admin_session) + end + + def create_application(owner_email:) + post "/api/v4/oauth/applications", params: { + doorkeeper_application: { name: "Test App", redirect_uri: "https://example.com/callback", owner_email: } + } + end + + it "assigns the point of contact by email" do + create_application(owner_email: poc.email) + + expect(Doorkeeper::Application.sole.owner).to eq(poc) + end + + it "requires a point of contact" do + create_application(owner_email: "nobody@example.com") + + expect(Doorkeeper::Application.count).to eq(0) + expect(response.body).to include("is-invalid") + end + + it "lists the point of contact" do + Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: poc) + + get "/api/v4/oauth/applications" + + expect(response).to have_http_status(:ok) + expect(response.body).to include(poc.email) + end +end From cfef9168e2d113cd0e796d2cd13ea276dc2a03c0 Mon Sep 17 00:00:00 2001 From: Mohamad Mortada Date: Mon, 2 Nov 2026 11:07:53 -0800 Subject: [PATCH 2/4] Fix CI: bump schema version and qualify delete_form partial path --- app/views/doorkeeper/applications/index.html.erb | 2 +- app/views/doorkeeper/applications/show.html.erb | 2 +- db/schema.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/doorkeeper/applications/index.html.erb b/app/views/doorkeeper/applications/index.html.erb index 978a553b71..3604ab7309 100644 --- a/app/views/doorkeeper/applications/index.html.erb +++ b/app/views/doorkeeper/applications/index.html.erb @@ -38,7 +38,7 @@ <%= link_to t("doorkeeper.applications.buttons.edit"), edit_oauth_application_path(application), class: "btn btn-link" %> - <%= render "delete_form", application: application %> + <%= render "doorkeeper/applications/delete_form", application: application %> <% end %> diff --git a/app/views/doorkeeper/applications/show.html.erb b/app/views/doorkeeper/applications/show.html.erb index 2c72cca2f4..7d5e3e418b 100644 --- a/app/views/doorkeeper/applications/show.html.erb +++ b/app/views/doorkeeper/applications/show.html.erb @@ -67,6 +67,6 @@

<%= link_to t("doorkeeper.applications.buttons.edit"), edit_oauth_application_path(@application), class: "btn btn-primary" %>

-

<%= render "delete_form", application: @application, submit_btn_css: "btn btn-danger" %>

+

<%= render "doorkeeper/applications/delete_form", application: @application, submit_btn_css: "btn btn-danger" %>

diff --git a/db/schema.rb b/db/schema.rb index 080dc4b0d6..0028cc16b4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_10_30_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_11_02_120000) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_catalog.plpgsql" From fb94176177896224237a233a30cd2b3690525490 Mon Sep 17 00:00:00 2001 From: Mohamad Mortada Date: Mon, 2 Nov 2026 11:20:02 -0800 Subject: [PATCH 3/4] Assign owners to Doorkeeper apps in specs --- spec/models/user_spec.rb | 2 +- spec/requests/api/v4/oauth_locked_user_spec.rb | 2 +- spec/requests/doorkeeper_impersonation_spec.rb | 2 +- .../maintenance/revoke_locked_users_api_access_task_spec.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7c81e39019..9310ef8dbd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -277,7 +277,7 @@ end describe "#lock!" do - let(:application) { Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback") } + let(:application) { Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: create(:user)) } it "locks" do user = create(:user, locked_at: nil) diff --git a/spec/requests/api/v4/oauth_locked_user_spec.rb b/spec/requests/api/v4/oauth_locked_user_spec.rb index 8e3c13d44e..6a766a191a 100644 --- a/spec/requests/api/v4/oauth_locked_user_spec.rb +++ b/spec/requests/api/v4/oauth_locked_user_spec.rb @@ -5,7 +5,7 @@ RSpec.describe "OAuth tokens of locked users", type: :request do let(:user) { create(:user) } let(:application) do - Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", scopes: "read") + Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: create(:user), scopes: "read") end # Access tokens expire after two hours, so by the time staff lock an account # most of its OAuth tokens look like this: expired, unrevoked, and still diff --git a/spec/requests/doorkeeper_impersonation_spec.rb b/spec/requests/doorkeeper_impersonation_spec.rb index 279e9f25db..b175d3247f 100644 --- a/spec/requests/doorkeeper_impersonation_spec.rb +++ b/spec/requests/doorkeeper_impersonation_spec.rb @@ -6,7 +6,7 @@ let(:admin) { create(:user, :make_admin) } let(:user) { create(:user) } let(:application) do - Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", scopes: "read") + Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: create(:user), scopes: "read") end before do diff --git a/spec/tasks/maintenance/revoke_locked_users_api_access_task_spec.rb b/spec/tasks/maintenance/revoke_locked_users_api_access_task_spec.rb index 9bb003b980..4a5604e12f 100644 --- a/spec/tasks/maintenance/revoke_locked_users_api_access_task_spec.rb +++ b/spec/tasks/maintenance/revoke_locked_users_api_access_task_spec.rb @@ -14,7 +14,7 @@ end it "revokes a locked user's expired tokens that still hold a refresh token" do - application = Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback") + application = Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: create(:user)) survivor = create(:api_token, user: locked_user, application:, use_refresh_token: true, expires_in: 1, created_at: 3.hours.ago) described_class.process(locked_user) From 8f28156084969181188e9b9880ee1311e74186b8 Mon Sep 17 00:00:00 2001 From: Mohamad Mortada Date: Tue, 10 Nov 2026 15:52:05 -0800 Subject: [PATCH 4/4] Preserve OAuth app owner on update and clarify unknown POC error --- .../oauth_applications_controller.rb | 9 ++++++--- config/locales/doorkeeper.en.yml | 2 ++ spec/requests/oauth_applications_spec.rb | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/controllers/oauth_applications_controller.rb b/app/controllers/oauth_applications_controller.rb index 7cbff4fdaf..960e6f2728 100644 --- a/app/controllers/oauth_applications_controller.rb +++ b/app/controllers/oauth_applications_controller.rb @@ -1,13 +1,16 @@ # frozen_string_literal: true class OauthApplicationsController < Doorkeeper::ApplicationsController - skip_after_action :verify_authorized + skip_after_action :verify_authorized # access is gated by Doorkeeper's admin_authenticator private def application_params - owner = User.find_by(email: params.dig(:doorkeeper_application, :owner_email)) - super.merge(owner:) + owner_email = params.dig(:doorkeeper_application, :owner_email) + return super if owner_email.nil? + + @owner ||= User.find_by(email: owner_email) + super.merge(owner: @owner) end end diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 5c7a15b7a9..c4035e759e 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -9,6 +9,8 @@ en: models: doorkeeper/application: attributes: + owner: + blank: 'must be the email of an existing user.' redirect_uri: fragment_present: 'cannot contain a fragment.' invalid_uri: 'must be a valid URI.' diff --git a/spec/requests/oauth_applications_spec.rb b/spec/requests/oauth_applications_spec.rb index 3141674829..cc63d5c319 100644 --- a/spec/requests/oauth_applications_spec.rb +++ b/spec/requests/oauth_applications_spec.rb @@ -30,7 +30,24 @@ def create_application(owner_email:) create_application(owner_email: "nobody@example.com") expect(Doorkeeper::Application.count).to eq(0) - expect(response.body).to include("is-invalid") + expect(response.body).to include("Must be the email of an existing user.") + end + + it "keeps the point of contact when updating without an email" do + application = Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: poc) + + patch "/api/v4/oauth/applications/#{application.id}", params: { doorkeeper_application: { name: "Renamed" } } + + expect(application.reload).to have_attributes(name: "Renamed", owner: poc) + end + + it "shows the point of contact" do + application = Doorkeeper::Application.create!(name: "Test App", redirect_uri: "https://example.com/callback", owner: poc) + + get "/api/v4/oauth/applications/#{application.id}" + + expect(response).to have_http_status(:ok) + expect(response.body).to include(poc.email) end it "lists the point of contact" do