From 663a8d1f19a04591a2317ea78a1283c22e395fb4 Mon Sep 17 00:00:00 2001
From: nora <163450896+24c02@users.noreply.github.com>
Date: Mon, 2 Mar 2026 22:20:19 -0500
Subject: [PATCH] forgot to push the rest to #188 (#193)
* frickin validations
* unschema
* better collaborator logic
* buncha cleanup
* see #191
---
...app_collaborator_invitations_controller.rb | 40 +++++++----------
.../developer_app_collaborators_controller.rb | 43 +++++++++----------
app/controllers/developer_apps_controller.rb | 6 +++
app/models/concerns/auditable.rb | 4 +-
app/models/program.rb | 5 ---
app/models/program_collaborator.rb | 9 ++++
app/policies/program_collaborator_policy.rb | 36 ++++++++++++++++
app/policies/program_policy.rb | 13 ++++--
.../program/_collaborator_cancelled.html.erb | 2 +-
.../program/_collaborator_invited.html.erb | 2 +-
.../program/_collaborator_removed.html.erb | 2 +-
db/schema.rb | 15 ++++++-
12 files changed, 114 insertions(+), 63 deletions(-)
create mode 100644 app/policies/program_collaborator_policy.rb
diff --git a/app/controllers/developer_app_collaborator_invitations_controller.rb b/app/controllers/developer_app_collaborator_invitations_controller.rb
index 5b616fa..59390ae 100644
--- a/app/controllers/developer_app_collaborator_invitations_controller.rb
+++ b/app/controllers/developer_app_collaborator_invitations_controller.rb
@@ -1,47 +1,37 @@
class DeveloperAppCollaboratorInvitationsController < ApplicationController
include IdentityAuthorizable
- before_action :set_app
- skip_after_action :verify_authorized, only: %i[accept decline]
+ before_action :set_invitation
# Invitee accepts
def accept
- invitation = @app.program_collaborators.pending.where(
- "(identity_id = ? OR (identity_id IS NULL AND invited_email = ?))",
- current_identity.id,
- current_identity.primary_email
- ).find(params[:id])
- invitation.update!(identity: current_identity) if invitation.identity_id.nil?
- invitation.accept!
- @app.create_activity :collaborator_accepted, owner: current_identity
+ authorize @invitation
+ @invitation.update!(identity: current_identity) if @invitation.identity_id.nil?
+ @invitation.accept!
+ @invitation.program.create_activity :collaborator_accepted, owner: current_identity
redirect_to developer_apps_path, notice: t(".success")
end
# Invitee declines
def decline
- invitation = @app.program_collaborators.pending.where(
- "(identity_id = ? OR (identity_id IS NULL AND invited_email = ?))",
- current_identity.id,
- current_identity.primary_email
- ).find(params[:id])
- invitation.decline!
- @app.create_activity :collaborator_declined, owner: current_identity
+ authorize @invitation
+ @invitation.decline!
+ @invitation.program.create_activity :collaborator_declined, owner: current_identity
redirect_to developer_apps_path, notice: t(".success")
end
# Owner cancels
def cancel
- authorize @app, :manage_collaborators?
- invitation = @app.program_collaborators.pending.find(params[:id])
- email = invitation.invited_email
- invitation.cancel!
- @app.create_activity :collaborator_cancelled, owner: current_identity, parameters: { cancelled_email: email }
- redirect_to developer_app_path(@app), notice: t(".success")
+ authorize @invitation
+ email = @invitation.invited_email
+ @invitation.cancel!
+ @invitation.program.create_activity :collaborator_cancelled, owner: current_identity, parameters: { cancelled_email: email }
+ redirect_to developer_app_path(@invitation.program), notice: t(".success")
end
private
- def set_app
- @app = Program.find(params[:developer_app_id])
+ def set_invitation
+ @invitation = ProgramCollaborator.find(params[:id])
end
end
diff --git a/app/controllers/developer_app_collaborators_controller.rb b/app/controllers/developer_app_collaborators_controller.rb
index d31d204..7433afc 100644
--- a/app/controllers/developer_app_collaborators_controller.rb
+++ b/app/controllers/developer_app_collaborators_controller.rb
@@ -15,38 +15,35 @@ class DeveloperAppCollaboratorsController < ApplicationController
identity = Identity.find_by(primary_email: email)
- unless identity&.id == @app.owner_identity_id
- collaborator = @app.program_collaborators.find_or_create_by(invited_email: email) do |pc|
- pc.identity = identity
- end
+ collaborator = @app.program_collaborators.find_or_create_by(invited_email: email) do |pc|
+ pc.identity = identity
+ end
- unless collaborator.persisted?
- alert_message = collaborator.errors.full_messages.to_sentence.presence || t(".invalid_email")
- redirect_to developer_app_path(@app), alert: alert_message
- return
- end
-
- reinvited = collaborator.declined? || collaborator.cancelled?
- collaborator.update!(status: :pending, identity: identity) if reinvited
-
- if collaborator.previously_new_record? || reinvited
- @app.create_activity :collaborator_invited, owner: current_identity, parameters: { invited_email: email }
- redirect_to developer_app_path(@app), notice: t(".invited")
- else
- redirect_to developer_app_path(@app), alert: t(".already_invited")
- end
+ unless collaborator.persisted?
+ alert_message = collaborator.errors.full_messages.to_sentence.presence || t(".invalid_email")
+ redirect_to developer_app_path(@app), alert: alert_message
return
end
- redirect_to developer_app_path(@app), notice: t(".invited")
+ reinvitable = collaborator.may_reinvite?
+ if reinvitable
+ collaborator.identity = identity
+ collaborator.reinvite!
+ end
+
+ if collaborator.previously_new_record? || reinvitable
+ @app.create_activity :collaborator_invited, owner: current_identity, parameters: { invited_email: email }
+ redirect_to developer_app_path(@app), notice: t(".invited")
+ else
+ redirect_to developer_app_path(@app), alert: t(".already_invited")
+ end
end
def destroy
- authorize @app, :manage_collaborators?
-
collaborator = @app.program_collaborators.find(params[:id])
+ authorize collaborator, :remove?
email = collaborator.invited_email
- collaborator.destroy
+ collaborator.remove!
@app.create_activity :collaborator_removed, owner: current_identity, parameters: { removed_email: email }
redirect_to developer_app_path(@app), notice: t(".success")
diff --git a/app/controllers/developer_apps_controller.rb b/app/controllers/developer_apps_controller.rb
index e4fea29..398cd8c 100644
--- a/app/controllers/developer_apps_controller.rb
+++ b/app/controllers/developer_apps_controller.rb
@@ -37,6 +37,12 @@ class DeveloperAppsController < ApplicationController
.includes(:owner)
.order(created_at: :desc)
.limit(50)
+ .to_a
+
+ # Preload backend_user through the polymorphic owner to avoid N+1 in IdentityMention
+ identities = @activities.filter_map { |a| a.owner if a.owner_type == "Identity" }
+ ActiveRecord::Associations::Preloader.new(records: identities, associations: :backend_user).call if identities.any?
+
render layout: "htmx"
end
diff --git a/app/models/concerns/auditable.rb b/app/models/concerns/auditable.rb
index b4ce924..1bb6cd5 100644
--- a/app/models/concerns/auditable.rb
+++ b/app/models/concerns/auditable.rb
@@ -48,8 +48,8 @@ module Auditable
next if old_val == new_val
transform = config[:transform]
changes[name] = {
- from: transform&.call(old_val) || old_val,
- to: transform&.call(new_val) || new_val
+ from: transform ? transform.call(old_val) : old_val,
+ to: transform ? transform.call(new_val) : new_val
}
end
end
diff --git a/app/models/program.rb b/app/models/program.rb
index de2fbe6..c4ca21b 100644
--- a/app/models/program.rb
+++ b/app/models/program.rb
@@ -72,7 +72,6 @@ class Program < ApplicationRecord
validates :redirect_uri, presence: true
validates :scopes, presence: true
validate :validate_community_scopes
- validate :validate_developer_owned_apps
before_validation :generate_uid, on: :create
before_validation :generate_secret, on: :create
@@ -176,10 +175,6 @@ class Program < ApplicationRecord
end
end
- def validate_developer_owned_apps
- # No restrictions - admins can set developer apps to any trust level
- end
-
def generate_uid
self.uid = SecureRandom.hex(16) if uid.blank?
end
diff --git a/app/models/program_collaborator.rb b/app/models/program_collaborator.rb
index f523280..b6292fd 100644
--- a/app/models/program_collaborator.rb
+++ b/app/models/program_collaborator.rb
@@ -17,6 +17,7 @@ class ProgramCollaborator < ApplicationRecord
state :accepted
state :declined
state :cancelled
+ state :removed
event :accept do
transitions from: :pending, to: :accepted
@@ -29,5 +30,13 @@ class ProgramCollaborator < ApplicationRecord
event :cancel do
transitions from: :pending, to: :cancelled
end
+
+ event :remove do
+ transitions from: %i[pending accepted], to: :removed
+ end
+
+ event :reinvite do
+ transitions from: %i[declined cancelled removed], to: :pending
+ end
end
end
diff --git a/app/policies/program_collaborator_policy.rb b/app/policies/program_collaborator_policy.rb
new file mode 100644
index 0000000..c8566ba
--- /dev/null
+++ b/app/policies/program_collaborator_policy.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+class ProgramCollaboratorPolicy < ApplicationPolicy
+ def accept?
+ record.pending? && belongs_to_user?
+ end
+
+ def decline?
+ record.pending? && belongs_to_user?
+ end
+
+ def cancel?
+ manage_collaborators?
+ end
+
+ def remove?
+ manage_collaborators?
+ end
+
+ private
+
+ def belongs_to_user?
+ record.identity_id == user.id ||
+ (record.identity_id.nil? && record.invited_email == user.primary_email)
+ end
+
+ def manage_collaborators?
+ program = record.program
+ program.owner_identity_id == user.id || admin?
+ end
+
+ def admin?
+ backend_user = user.backend_user
+ backend_user&.program_manager? || backend_user&.super_admin?
+ end
+end
diff --git a/app/policies/program_policy.rb b/app/policies/program_policy.rb
index dfe6df7..cc1de80 100644
--- a/app/policies/program_policy.rb
+++ b/app/policies/program_policy.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
-# `user` here is always an Identity (via IdentityAuthorizable).
+# `user` is normally an Identity (via IdentityAuthorizable), but may be a
+# Backend::User when activity partials render in the backend context.
class ProgramPolicy < ApplicationPolicy
def index?
user.developer_mode? || admin?
@@ -114,12 +115,16 @@ class ProgramPolicy < ApplicationPolicy
record.is_a?(Class) ? false : record.collaborator?(user)
end
+ def resolve_backend_user
+ user.is_a?(Backend::User) ? user : user.backend_user
+ end
+
def admin?
- backend_user = user.backend_user
- backend_user&.program_manager? || backend_user&.super_admin?
+ bu = resolve_backend_user
+ bu&.program_manager? || bu&.super_admin?
end
def super_admin?
- user.backend_user&.super_admin?
+ resolve_backend_user&.super_admin?
end
end
diff --git a/app/views/public_activity/program/_collaborator_cancelled.html.erb b/app/views/public_activity/program/_collaborator_cancelled.html.erb
index ebe839d..84c4f28 100644
--- a/app/views/public_activity/program/_collaborator_cancelled.html.erb
+++ b/app/views/public_activity/program/_collaborator_cancelled.html.erb
@@ -1,5 +1,5 @@
<%= render Components::PublicActivity::Snippet.new(activity, owner_component: Components::IdentityMention.new(activity.owner)) do %>
- <% if policy(activity.trackable).manage_collaborators? %>
+ <% if activity.trackable && policy(activity.trackable).manage_collaborators? rescue false %>
cancelled invitation for <%= activity.parameters[:cancelled_email] || activity.parameters["cancelled_email"] %>.
<% else %>
cancelled an invitation.
diff --git a/app/views/public_activity/program/_collaborator_invited.html.erb b/app/views/public_activity/program/_collaborator_invited.html.erb
index 3185bb4..921db47 100644
--- a/app/views/public_activity/program/_collaborator_invited.html.erb
+++ b/app/views/public_activity/program/_collaborator_invited.html.erb
@@ -1,5 +1,5 @@
<%= render Components::PublicActivity::Snippet.new(activity, owner_component: Components::IdentityMention.new(activity.owner)) do %>
- <% if policy(activity.trackable).manage_collaborators? %>
+ <% if activity.trackable && policy(activity.trackable).manage_collaborators? rescue false %>
invited <%= activity.parameters[:invited_email] || activity.parameters["invited_email"] %> to collaborate.
<% else %>
invited a new collaborator.
diff --git a/app/views/public_activity/program/_collaborator_removed.html.erb b/app/views/public_activity/program/_collaborator_removed.html.erb
index e3f5a28..e75c3ad 100644
--- a/app/views/public_activity/program/_collaborator_removed.html.erb
+++ b/app/views/public_activity/program/_collaborator_removed.html.erb
@@ -1,5 +1,5 @@
<%= render Components::PublicActivity::Snippet.new(activity, owner_component: Components::IdentityMention.new(activity.owner)) do %>
- <% if policy(activity.trackable).manage_collaborators? %>
+ <% if activity.trackable && policy(activity.trackable).manage_collaborators? rescue false %>
removed <%= activity.parameters[:removed_email] || activity.parameters["removed_email"] %>.
<% else %>
removed a collaborator.
diff --git a/db/schema.rb b/db/schema.rb
index 25bf43c..1b74f52 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -301,6 +301,7 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_02_000002) do
t.boolean "saml_debug"
t.boolean "is_in_workspace", default: false, null: false
t.string "slack_dm_channel_id"
+ t.string "webauthn_id"
t.boolean "is_alum", default: false
t.boolean "can_hq_officialize", default: false, null: false
t.index "lower((primary_email)::text)", name: "idx_identities_unique_primary_email", unique: true, where: "(deleted_at IS NULL)"
@@ -446,7 +447,6 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_02_000002) do
t.integer "sign_count"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
- t.datetime "compromised_at"
t.index ["external_id"], name: "index_identity_webauthn_credentials_on_external_id", unique: true
t.index ["identity_id"], name: "index_identity_webauthn_credentials_on_identity_id"
end
@@ -602,6 +602,18 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_02_000002) do
t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id"
end
+ create_table "webauthn_credentials", force: :cascade do |t|
+ t.bigint "identity_id", null: false
+ t.string "external_id", null: false
+ t.string "public_key", null: false
+ t.string "nickname", null: false
+ t.integer "sign_count", default: 0, null: false
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.index ["external_id"], name: "index_webauthn_credentials_on_external_id", unique: true
+ t.index ["identity_id"], name: "index_webauthn_credentials_on_identity_id"
+ end
+
add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
add_foreign_key "addresses", "identities"
@@ -636,4 +648,5 @@ ActiveRecord::Schema[8.0].define(version: 2026_03_02_000002) do
add_foreign_key "verifications", "identities"
add_foreign_key "verifications", "identity_aadhaar_records", column: "aadhaar_record_id"
add_foreign_key "verifications", "identity_documents"
+ add_foreign_key "webauthn_credentials", "identities"
end