forgot to push the rest to #188 (#193)

* frickin validations

* unschema

* better collaborator logic

* buncha cleanup

* see #191
This commit is contained in:
nora 2026-03-02 22:20:19 -05:00 committed by GitHub
parent 9998147a4e
commit 663a8d1f19
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 114 additions and 63 deletions

View file

@ -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

View file

@ -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")

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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 <strong><%= activity.parameters[:cancelled_email] || activity.parameters["cancelled_email"] %></strong>.
<% else %>
cancelled an invitation.

View file

@ -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 <strong><%= activity.parameters[:invited_email] || activity.parameters["invited_email"] %></strong> to collaborate.
<% else %>
invited a new collaborator.

View file

@ -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 <strong><%= activity.parameters[:removed_email] || activity.parameters["removed_email"] %></strong>.
<% else %>
removed a collaborator.

15
db/schema.rb generated
View file

@ -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