Move to reading/writing from the scopes column

app/avo/resources/api_key_resource.rb

@@ -18,7 +18,7 @@ class ExpiredFilter < ScopeBooleanFilter; end
   field :soft_deleted_rubygem_name, as: :text
   field :expires_at, as: :date_time
 
-  field :enabled_scopes, as: :tags
+  field :scopes, as: :tags
 
   sidebar do
     heading "Permissions"

app/controllers/api/v1/api_keys_controller.rb

@@ -35,7 +35,7 @@ def update
       check_mfa(user) do
         api_key = user.api_keys.find_by!(hashed_key: hashed_key(key_param))
 
-        if api_key.update(api_key_update_params)
+        if api_key.update(api_key_update_params(api_key))
           respond_with "Scopes for the API key #{api_key.name} updated"
         else
           errors = api_key.errors.full_messages
@@ -87,10 +87,10 @@ def key_param
   end
 
   def api_key_create_params
-    params.permit(:name, *ApiKey::API_SCOPES, :mfa, :rubygem_name)
+    ApiKeysHelper.api_key_params(params.permit(:name, *ApiKey::API_SCOPES, :mfa, :rubygem_name, scopes: [ApiKey::API_SCOPES]))
   end
 
-  def api_key_update_params
-    params.permit(*ApiKey::API_SCOPES, :mfa)
+  def api_key_update_params(key)
+    ApiKeysHelper.api_key_params(params.permit(*ApiKey::API_SCOPES, :mfa, scopes: [ApiKey::API_SCOPES]), key)
   end
 end

app/controllers/api/v1/github_secret_scanning_controller.rb

@@ -51,7 +51,7 @@ def revoke
 
   def schedule_revoke_email(api_key, url)
     return unless api_key.user?
-    Mailer.api_key_revoked(api_key.owner_id, api_key.name, api_key.enabled_scopes.join(", "), url).deliver_later
+    Mailer.api_key_revoked(api_key.owner_id, api_key.name, api_key.scopes.join(", "), url).deliver_later
   end
 
   def secret_scanning_key(key_id)

app/controllers/api/v1/oidc/api_key_roles_controller.rb

@@ -57,7 +57,7 @@ def assume_role
     render json: {
       rubygems_api_key: key,
       name: api_key.name,
-      scopes: api_key.enabled_scopes,
+      scopes: api_key.scopes,
       gem: api_key.rubygem,
       expires_at: api_key.expires_at
     }.compact, status: :created

app/controllers/api/v1/oidc/trusted_publisher_controller.rb

@@ -23,14 +23,14 @@ def exchange_token
     api_key = @trusted_publisher.api_keys.create!(
       hashed_key: hashed_key(key),
       name: "#{@trusted_publisher.name} #{iat.iso8601}",
-      push_rubygem: true,
+      scopes: %i[push_rubygem],
       expires_at: 15.minutes.from_now
     )
 
     render json: {
       rubygems_api_key: key,
       name: api_key.name,
-      scopes: api_key.enabled_scopes,
+      scopes: api_key.scopes,
       gem: api_key.rubygem,
       expires_at: api_key.expires_at
     }.compact, status: :created

app/controllers/api_keys_controller.rb

@@ -46,7 +46,7 @@ def create
 
   def update
     @api_key = current_user.api_keys.find(params.permit(:id).require(:id))
-    @api_key.assign_attributes(api_key_params)
+    @api_key.assign_attributes(api_key_params(@api_key))
 
     if @api_key.errors.present?
       flash.now[:error] = @api_key.errors.full_messages.to_sentence
@@ -96,9 +96,9 @@ def verify_session_redirect_path
     end
   end
 
-  def api_key_params
-    params.permit(api_key: PERMITTED_API_KEY_PARAMS).require(:api_key)
+  def api_key_params(existing_api_key = nil)
+    ApiKeysHelper.api_key_params(params.permit(api_key: PERMITTED_API_KEY_PARAMS).require(:api_key), existing_api_key)
   end
 
-  PERMITTED_API_KEY_PARAMS = [:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id].freeze
+  PERMITTED_API_KEY_PARAMS = [:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id, { scopes: [ApiKey::API_SCOPES] }].freeze
 end

app/controllers/concerns/api_keyable.rb

@@ -19,7 +19,7 @@ def generate_rubygems_key
   end
 
   def legacy_key_defaults
-    legacy_scopes = ApiKey::API_SCOPES.each_with_object({}) { |k, h| h[k] = true unless k == :show_dashboard }
-    legacy_scopes.merge(name: "legacy-key")
+    legacy_scopes = ApiKey::API_SCOPES - ApiKey::EXCLUSIVE_SCOPES
+    { scopes: legacy_scopes, name: "legacy-key" }
   end
 end

app/helpers/api_keys_helper.rb

@@ -17,6 +17,22 @@ def api_key_checkbox(form, api_scope)
     form.check_box api_scope, html_options, "true", "false"
   end
 
+  def self.api_key_params(params, existing_api_key = nil)
+    scopes = params.fetch(:scopes, existing_api_key&.scopes || []).to_set
+    boolean = ActiveRecord::Type::Boolean.new
+    ApiKey::API_SCOPES.each do |scope|
+      next unless params.key?(scope)
+
+      if boolean.cast(params.delete(scope))
+        scopes << scope
+      else
+        scopes.delete(scope)
+      end
+    end
+    params[:scopes] = scopes.sort
+    params
+  end
+
   private
 
   def invalid_gem_tooltip(name)

app/models/api_key.rb

@@ -11,7 +11,7 @@ class ApiKey < ApplicationRecord
   has_one :oidc_api_key_role, class_name: "OIDC::ApiKeyRole", through: :oidc_id_token, source: :api_key_role, inverse_of: :api_keys
   has_many :pushed_versions, class_name: "Version", inverse_of: :pusher_api_key, foreign_key: :pusher_api_key_id, dependent: :nullify
 
-  before_validation :set_scopes
+  before_validation :set_legacy_scope_columns
   before_validation :set_owner_from_user
   after_create :record_create_event
   after_update :record_expire_event, if: :saved_change_to_expires_at?
@@ -39,18 +39,18 @@ def self.expire_all!
     end
   end
 
-  def enabled_scopes
-    API_SCOPES.filter_map { |scope| scope if send(scope) }
-  end
-
   API_SCOPES.each do |scope|
     define_method(:"can_#{scope}?") do
-      scope_enabled = send(scope)
+      scope_enabled = scopes.include?(scope)
       return scope_enabled if !scope_enabled || new_record?
       touch :last_accessed_at
     end
   end
 
+  def scopes
+    super&.map(&:to_sym) || []
+  end
+
   def user
     owner if user?
   end
@@ -59,10 +59,6 @@ def user?
     owner_type == "User"
   end
 
-  def scopes
-    super&.map(&:to_sym)
-  end
-
   delegate :mfa_required_not_yet_enabled?, :mfa_required_weak_level_enabled?,
     :mfa_recommended_not_yet_enabled?, :mfa_recommended_weak_level_enabled?,
     to: :user, allow_nil: true
@@ -122,15 +118,15 @@ def exclusive_show_dashboard_scope
   end
 
   def other_enabled_scopes?
-    enabled_scopes.tap { |scope| scope.delete(:show_dashboard) }.any?
+    scopes.-(%i[show_dashboard]).any?
   end
 
   def scope_presence
-    errors.add :base, "Please enable at least one scope" unless enabled_scopes.any?
+    errors.add :base, "Please enable at least one scope" if scopes.blank?
   end
 
   def rubygem_scope_definition
-    return if APPLICABLE_GEM_API_SCOPES.intersect?(enabled_scopes)
+    return if APPLICABLE_GEM_API_SCOPES.intersect?(scopes)
     errors.add :rubygem, "scope can only be set for push/yank rubygem, and add/remove owner scopes"
   end
 
@@ -151,15 +147,18 @@ def set_owner_from_user
     self.owner ||= user
   end
 
-  def set_scopes
-    self.scopes = enabled_scopes
+  def set_legacy_scope_columns
+    scopes = self.scopes
+    API_SCOPES.each do |scope|
+      self[scope] = scopes.include?(scope)
+    end
   end
 
   def record_create_event
     case owner
     when User
       user.record_event!(Events::UserEvent::API_KEY_CREATED,
-          name:, scopes: enabled_scopes, gem: rubygem&.name, mfa:, api_key_gid: to_gid)
+          name:, scopes:, gem: rubygem&.name, mfa:, api_key_gid: to_gid)
     end
   end
 

app/models/oidc/api_key_permissions.rb

@@ -1,6 +1,6 @@
 class OIDC::ApiKeyPermissions < ApplicationModel
   def create_params(user)
-    params = scopes.map(&:to_sym).index_with(true)
+    params = { scopes: scopes }
     params[:ownership] = gems&.first&.then { user.ownerships.joins(:rubygem).find_by!(rubygem: { name: _1 }) }
     params[:expires_at] = DateTime.now.utc + valid_for
     params

app/views/api_keys/index.html.erb

@@ -44,7 +44,7 @@
           <%= api_key.name %>
         </td>
         <td class="owners__cell" data-title="Scopes">
-          <% api_key.enabled_scopes.each do |scope| %>
+          <% api_key.scopes.each do |scope| %>
             <ul class="scopes__list"><%= t(".#{scope}") %></ul>
           <% end %>
         </td>

app/views/mailer/api_key_created.html.erb

@@ -15,7 +15,7 @@
         <p>
           Name: <strong><%= @api_key.name %></strong>
           <br>
-          Scope: <strong><%= @api_key.enabled_scopes.join(", ") %></strong>
+          Scope: <strong><%= @api_key.scopes.to_sentence %></strong>
           <br>
           Created at: <strong><%= @api_key.created_at.to_formatted_s(:rfc822) %></strong>
           <% if @api_key.oidc_id_token.present? %>

db/migrate/20240506180700_add_non_null_constraint_to_api_key_scopes.rb

@@ -0,0 +1,5 @@
+class AddNonNullConstraintToApiKeyScopes < ActiveRecord::Migration[7.1]
+  def change
+    add_check_constraint :api_keys, "scopes IS NOT NULL", name: "api_keys_scopes_null", validate: false
+  end
+end

db/migrate/20240506180817_validate_non_null_constraint_to_api_key_scopes.rb

@@ -0,0 +1,5 @@
+class ValidateNonNullConstraintToApiKeyScopes < ActiveRecord::Migration[7.1]
+  def change
+    validate_check_constraint :api_keys, name: "api_keys_scopes_null"
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_04_25_190438) do
+ActiveRecord::Schema[7.1].define(version: 2024_05_06_180817) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "hstore"
   enable_extension "pgcrypto"
@@ -60,6 +60,7 @@
     t.index ["owner_type", "owner_id"], name: "index_api_keys_on_owner"
     t.check_constraint "owner_id IS NOT NULL", name: "api_keys_owner_id_null"
     t.check_constraint "owner_type IS NOT NULL", name: "api_keys_owner_type_null"
+    t.check_constraint "scopes IS NOT NULL", name: "api_keys_scopes_null"
   end
 
   create_table "audits", force: :cascade do |t|

db/seeds.rb

@@ -103,7 +103,7 @@
 user.web_hooks.find_or_create_by!(url: "https://example.com/rubygem0", rubygem: rubygem0)
 user.web_hooks.find_or_create_by!(url: "http://example.com/all", rubygem: nil)
 
-author.api_keys.find_or_create_by!(hashed_key: "securehashedkey", name: "api key", push_rubygem: true)
+author.api_keys.find_or_create_by!(hashed_key: "securehashedkey", name: "api key", scopes: %i[push_rubygem])
 
 Admin::GitHubUser.create_with(
   is_admin: true,
@@ -219,7 +219,7 @@
 author_oidc_api_key_role.user.api_keys.create_with(
   hashed_key: "expiredhashedkey",
   ownership: rubygem0.ownerships.find_by!(user: author),
-  push_rubygem: true
+  scopes: %i[push_rubygem]
 ).find_or_create_by!(
   name: "push-rubygem-1-expired"
 ).tap do |api_key|
@@ -234,7 +234,7 @@
 author_oidc_api_key_role.user.api_keys.create_with(
   hashed_key: "unexpiredhashedkey",
   ownership: rubygem0.ownerships.find_by!(user: author),
-  push_rubygem: true,
+  scopes: %i[push_rubygem],
   expires_at: "2120-01-01T00:00:00Z"
 ).find_or_create_by!(
   name: "push-rubygem-1-unexpired"
@@ -249,7 +249,7 @@
 author.api_keys.find_or_create_by!(
   hashed_key: "unexpiredmanualhashedkey",
   name: "Manual",
-  push_rubygem: true
+  scopes: %i[push_rubygem]
 )
 
 SendgridEvent.create_with(
@@ -283,7 +283,7 @@
 trusted_publisher.rubygem_trusted_publishers.find_or_create_by!(rubygem: rubygem0).trusted_publisher.api_keys.find_or_create_by!(
   name: "GitHub Actions something",
   hashed_key: "securehashedkey-tp",
-  push_rubygem: true
+  scopes: %i[push_rubygem]
 ).pushed_versions.create_with(indexed: true, sha256: Digest::SHA2.base64digest("rubygem0-0.1.0.gem")).find_or_create_by!(
   rubygem: rubygem0, number: "0.1.0", platform: "ruby", gem_platform: "ruby"
 )

test/factories/api_key.rb

@@ -6,7 +6,7 @@
     name { "ci-key" }
 
     # enabled by default. disabled when show_dashboard is enabled.
-    index_rubygems { show_dashboard ? false : true }
+    scopes { %w[index_rubygems] }
 
     hashed_key { Digest::SHA256.hexdigest(key) }
   end

test/functional/api/v1/api_keys_controller_test.rb

@@ -158,7 +158,7 @@ def self.should_expect_otp_for_update
     context "with correct OTP" do
       setup do
         @request.env["HTTP_OTP"] = ROTP::TOTP.new(@user.totp_seed).now
-        @api_key = create(:api_key, owner: @user, key: "12345", push_rubygem: true)
+        @api_key = create(:api_key, owner: @user, key: "12345", scopes: %i[push_rubygem])
 
         put :update, params: { api_key: "12345", index_rubygems: "true" }
         @api_key.reload
@@ -545,7 +545,7 @@ def self.should_expect_otp_for_update
 
     context "with correct credentials" do
       setup do
-        @api_key = create(:api_key, owner: @user, key: "12345", push_rubygem: true)
+        @api_key = create(:api_key, owner: @user, key: "12345", scopes: %i[push_rubygem])
         authorize_with("#{@user.email}:#{@user.password}")
       end
 

test/functional/api/v1/deletions_controller_test.rb

@@ -5,7 +5,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase
 
   context "with yank rubygem api key scope" do
     setup do
-      @api_key = create(:api_key, key: "12345", yank_rubygem: true)
+      @api_key = create(:api_key, key: "12345", scopes: %i[yank_rubygem])
       @user = @api_key.user
       @request.env["HTTP_AUTHORIZATION"] = "12345"
     end
@@ -136,7 +136,7 @@ class Api::V1::DeletionsControllerTest < ActionController::TestCase
 
       context "with api key gem scoped" do
         setup do
-          @api_key = create(:api_key, name: "gem-scoped-delete-key", key: "123456", yank_rubygem: true, owner: @user, rubygem_id: @rubygem.id)
+          @api_key = create(:api_key, name: "gem-scoped-delete-key", key: "123456", scopes: %i[yank_rubygem], owner: @user, rubygem_id: @rubygem.id)
           @request.env["HTTP_AUTHORIZATION"] = "123456"
         end
 

test/functional/api/v1/owners_controller_test.rb

@@ -115,7 +115,7 @@ def self.should_respond_to(format)
         @second_user = create(:user)
         @third_user = create(:user)
         @ownership = create(:ownership, rubygem: @rubygem, user: @user)
-        @api_key = create(:api_key, key: "12334", add_owner: true, owner: @user)
+        @api_key = create(:api_key, key: "12334", scopes: %i[add_owner], owner: @user)
         @request.env["HTTP_AUTHORIZATION"] = "12334"
       end
 
@@ -557,7 +557,7 @@ def self.should_respond_to(format)
         @ownership = create(:ownership, rubygem: @rubygem, user: @user)
         @ownership = create(:ownership, rubygem: @rubygem, user: @second_user)
 
-        @api_key = create(:api_key, key: "12223", remove_owner: true, owner: @user)
+        @api_key = create(:api_key, key: "12223", scopes: %i[remove_owner], owner: @user)
         @request.env["HTTP_AUTHORIZATION"] = "12223"
       end
 
@@ -915,7 +915,7 @@ def self.should_respond_to(format)
   end
 
   should "return plain text 404 error" do
-    create(:api_key, key: "12223", add_owner: true)
+    create(:api_key, key: "12223", scopes: %i[add_owner])
     @request.env["HTTP_AUTHORIZATION"] = "12223"
     @request.accept = "*/*"
     post :create, params: { rubygem_id: "bananas" }