Tweak NeoPass login success messages, to focus less on random username

Until we have more figured out about the username situation here, let's
not greet the user by the name we *generated* for them.

app/controllers/devise/omniauth_callbacks_controller.rb

@@ -7,11 +7,12 @@ class Devise::OmniauthCallbacksController < ApplicationController
 
 		if @auth_user.previously_new_record?
 			flash[:notice] =
-				"Welcome to Dress to Impress, #{@auth_user.name}! We've set up a DTI " +
+				"Welcome to Dress to Impress! We've set up a DTI account for you, " +
-				"account for you. Click Settings in the top right to learn more, or " +
+				"attached to your NeoPass. Click Settings in the top right to learn " +
-				"just go ahead and get started!"
+				"more, or just go ahead and get started!"
 		else
-			flash[:notice] = "Welcome back, #{@auth_user.name}! 💖"
+			flash[:notice] =
+				"Welcome back, #{@auth_user.name}! You are now logged in. 💖"
 		end
 
 		sign_in_and_redirect @auth_user, event: :authentication

app/models/auth_user.rb

@@ -6,7 +6,7 @@ class AuthUser < AuthRecord
     omniauth_providers: [:neopass]
 
   validates :name, presence: true, uniqueness: {case_sensitive: false},
-    length: {maximum: 20}
+    length: {maximum: 30}
   
   has_one :user, foreign_key: :remote_id, inverse_of: :auth_user
   
@@ -41,14 +41,13 @@ class AuthUser < AuthRecord
   end
 
   def self.from_omniauth(auth)
-    raise MissingAuthInfoError, "Username missing" if auth.uid.blank?
     raise MissingAuthInfoError, "Email missing" if auth.info.email.blank?
 
     transaction do
       find_or_create_by!(provider: auth.provider, uid: auth.uid) do |user|
-        # Use the Neopets username if possible, or a unique username if not.
+        # TODO: Can we somehow get the Neopets username if one exists, instead
-        dti_username = build_unique_username_like(auth.uid)
+        # of just using total randomness?
-        user.name = dti_username
+        user.name = build_unique_username
 
         # Copy the email address from their Neopets account to their DTI
         # account, unless they already have a DTI account with this email, in
@@ -60,25 +59,32 @@ class AuthUser < AuthRecord
     end
   end
 
-  def self.build_unique_username_like(name)
+  def self.build_unique_username
-    name_query = sanitize_sql_like(name) + "%"
+    # Start with a base name like "neopass-kougra-".
-    similar_names = where("name LIKE ?", name_query).pluck(:name)
+    random_species_name = Species.all.pluck(:name).sample
+    base_name = "neopass-#{random_species_name}"
 
-    # Use the given name itself, if we can.
+    # Fetch the list of names that already start with that.
-    return name unless similar_names.include?(name)
+    name_query = sanitize_sql_like(base_name) + "%"
+    similar_names = where("name LIKE ?", name_query).pluck(:name).to_set
 
-    # If not, try appending "-neopass".
+    # Shuffle the list of four-digit numbers to create 10000 possible names,
-    return "#{name}-neopass" unless similar_names.include?("#{name}-neopass")
+    # then use the first one that's not already claimed.
+    potential_names = (0..9999).map { |n| "#{base_name}-#{n}" }.shuffle
+    name = potential_names.find { |name| !similar_names.include?(name) }
+    return name unless name.nil?
 
-    # After that, try appending "-neopass-1", "-neopass-2", etc, until a
+    # If that failed, try again but with six digits.
-    # unique name arises. (We don't expect this to happen basically ever, but
+    potential_names = (0..999999).map { |n| "#{base_name}-#{n}" }.shuffle
-    # it's nice to have a guarantee!)
+    name = potential_names.find { |name| !similar_names.include?(name) }
-    max = similar_names.size + 1
+    return name unless name.nil?
-    candidates = (1..max).map { |n| "#{name}-neopass-#{n}"}
-    numerical_name = candidates.find { |name| !similar_names.include?(name) }
-    return numerical_name unless numerical_name.nil?
 
-    raise "Failed to build unique username (shouldn't be possible?)"
+    # If *that* failed, then golly gee, we have millions of NeoPass users
+    # running around using the default username. Good for us, I guess?? If so,
+    # uhh, let's cross that bridge when we come to it. (At time of writing,
+    # there are about 60k total registered DTI users at *all*.)
+    raise "Failed to build unique username (all million+ names starting with " +
+      "\"#{base_name}\" are taken??)"
   end
 
   class MissingAuthInfoError < ArgumentError;end

config/credentials.yml.enc

@@ -1 +1 @@
-p001YS2L7h/DJ9mWOUxcWD/wBNXQ41BOsN9vjnW9QWtECNNzMjRsPVZ8vkOXmRt5mHAOmvzAUhRbKmWE0PaUHMpJAVlct1mCpXlE2rHwKESuZ4S1tJFiSCfu0Uu97Nbw3kQScMJ9cB801QPpjDq5FxoCEz4XtuCEc8nuh23Ni8I77Jw9NG4VfiFFQHFhNk8xhAoAIiYekEoliRYJc1osmzSb8FcDlIV/GompPN4G2EaBcFvX1CMP2F2AYHR8EVvaeTmIv0GsdLL8NnEnsuqd+GZMljIr346aOSOzmUC76rgubpEdqG4mBZzzUjQxvXqnuawOmbQWAbDkDcSoZYwVhc2/QmR3VtFtV/YaBX27h2cp9Ef14FgpK7y1KO1vE7row8lxq4rwtZaGT0nbXf//4gpdDdrRKEOJRjwi5l+ydPgfa4aHQohFZ+4a7App3N5dovG1/c9W/a4T/7i5aBxYeiCKD6+byss74jJaqZ34eNxVQOZxoi+HfHPQwMRwsGVxKzGJaGLijwjuYq5iAdSBiMY2WouF17gNJKXgEILFvy/xAyHnUDM3K/traeC7ULRztdGGxHHIC2D26+s5mS5zI9OAM2lXI6ms4jKEui/BYKuQ/sHpB4Cg7wdk0JQ5SPW/8b35oW6Cuz31NrzUju5WXtXtGvlMPBKUWz8q8wIGEtM5Bc/5ASDS9Ag8J8seW2gprab2Ja7apUXPBUGuR5aU6JKLGNpAM/vV9lavgrm/rvXNlkPvgoULpBhtZI5EfuiSncke7NCuJUEw4fZr5KeRze5U7qyk1Jg+Fz2nQGvOn0T1PDvezN5yT+b/0YZnyTI9zmhur4KY4Z9OZgsMTG073qkdZF6y3qeWKHpWrhXKWc8i/CAVsUKrWQpSDWDhvzhhanR936OEoqqpKoGunsba1fh5oOdrTBiFnH+MfI/IAh7tjUjcQx6bu8/1rdaZ7omBdaeDx36SekoOqndCBgrMX0mri5hoG2LIVA==--VLKM05ugRRSrks6H--/nICajJes+PjNkh9lyRi0Q==
+nyb0iYp3ohic5yKU42wU1YFnL1YHMvOxyCZOz1Vs/fDgcSHFajsTaBM+smD+/+6xnhOlu9uaHIXCGUZiKnY6pjnJJRrO2w6h20p7tuulbPwJEKszljdnREMSII2ltatK0efb2X+UkqIU+fAr7L+6VglX6tYKSk06BC505HUKWhK+UHkHVx9UiQe6KJhX//+G/jE033kMnj/Krqn6QrJ3xwe1jWwdnAi/glJSLt2zGJDndPVPag/8UBeJqd3VtJOIDFTe0FxGvtc5KFUYrkZ1YJWWCIcGlDWpm0Mt3EEQ2yrqYC0VtSg6EedcDZBqcXF9uY2femXSriOmAw7hTHcF0YG0N0elweq8LCa3Q1sEW+BGibrPwIL14aogySDR7BYoq3LevW94ExnUsIwyGPRFpG+nDWA3HGb/BSySSIkLd70a3943S8a7klmo8fo1CArxfIt/8xyXrF0D6T+9FHnhvaj+pDSBOUxlcqZ3qvmLEWwMfCUBOLpTHo2ZlKpKlDJuJmAZ7/ah+Mg34WWPbsXU+7tUhKEXckvM/gaTvmByhxdBDE/7KsnP50qQods7ODjrDTaxHut9OBrl3ODc9Y14dp66OYIMy4X8sS/N5momi59E9MZaGj6bfaZor0ZxctRRJTjrEHFyvxO0CXYh5kkHLjs5BVB00wWHeNA9qgNEdsGVrYCa4v3RXZ6vc8JmtUXy1vwErC67uaemgWds38OV3/xViZFzbnZtqOiSWTzD8/myhKa75GLZ0vjk8+s96GDqX2+lHz6X+RA7NcCdGO+gW1f3Xm7M/Rp5nAUHSfrLG6Gv/RH8nkvGxkT6dcwZjSJ00+1ENL2qqN20008OX5TuWKeo1/TF4DFxJ+X1ipWqoZhypZOW38sRROqE5uGF1aBga8CbCLWF2p1E3HSi45vbSMnpWau4i+8KQvzfUksa7nk1O6qqEamdoJGb9OJuv//ZYcv1GyDfM/8iFIReIwsM7q/hlB0PjfFkoYsC5WRbRMncKEzzGs8fVpL4O4enDPe4ZopaYHecnPy1oDoxrpl6OOSSOjMxg+0=--+Jx8l9zhIw0wWX6f--2I4PF87PHuXTkmcBI3yhVA==

config/environments/development.rb

@@ -126,4 +126,38 @@ Rails.application.configure do
   # Set the NeoPass redirect callback URL.
   config.neopass_redirect_uri =
     "http://localhost:3000/users/auth/neopass/callback"
+
+  # If the "USE_LIVE_NEOPASS=1" environment variable is set, override the
+  # NeoPass config with the production values instead.
+  #
+  # Note that this does *not* allow you to just use NeoPass with the
+  # development server as one might like! Our `localhost:3000` redirect URL is
+  # not registered with live NeoPass, so we have to provide the production
+  # callback, or else NeoPass will reject the initial auth request altogether!
+  #
+  # Instead, you'll need to somehow intercept the flow:
+  #     1. Dress to Impress (development) sends you to NeoPass, with production
+  #        configuration in the request.
+  #     2. NeoPass redirects back to Dress to Impress (production).
+  #     3. Use some kind of tool to prevent the above redirect, and rewrite it
+  #        to `localhost:3000` instead.
+  #        - For me, it's convenient to do this via the Burp Suite's "Proxy"
+  #          tool: intercept the request, cancel it, and manually rewrite the
+  #          URL and navigate to it.
+  #        - Another way I've used for similar things in the past is to edit my
+  #          /etc/hosts file to temporarily point `impress.openneo.net` to
+  #          `127.0.0.1`. Then, when the request fails, manually rewrite the
+  #          URL and navigate to it.
+  #        - I suppose you could also have your browser's Network panel persist
+  #          logs, then you can see the `/users/auth/neopass/callback` request
+  #          that fails and redirects back to the production sign-in page, and
+  #          manually rewrite it? (The request should be safe to let through,
+  #          because production DTI will reject the callback, because it knows
+  #          from the `state` parameter that it didn't initiate this flow.)
+  if ENV["USE_LIVE_NEOPASS"].present?
+    puts "Using live NeoPass, instead of the development server."
+    config.neopass_origin = "https://oidc.neopets.com"
+    config.neopass_redirect_uri =
+      "https://impress.openneo.net/users/auth/neopass/callback"
+  end
 end

config/environments/production.rb

@@ -137,7 +137,7 @@ Rails.application.configure do
 
   # To see NeoPass features, add ?neopass=<SECRET> to relevant pages.
   config.neopass_access_secret =
-    Rails.application.credentials.neopass.access_secret
+    Rails.application.credentials.neopass.access_secret!
 
   # Use the live NeoPass production server.
   config.neopass_origin = "https://oidc.neopets.com"

config/initializers/devise.rb

@@ -275,19 +275,47 @@ Devise.setup do |config|
   # up on your models and hooks.
   config.omniauth :openid_connect, {
     name: :neopass,
-    scope: [:openid, :email, :profile],
+
+    # We'll request only basic info, and we'll "discover" most of the server's
+    # configuration by reading its `/.well-known/openid_configuation` endpoint.
+    scope: [:openid, :email],
     response_type: :code,
     issuer: Rails.configuration.neopass_origin,
     discovery: true,
+
+    # Here's the client info registered with the NeoPass team! They generated
+    # a Client ID and a Client Secret for us; and we provided them with a
+    # redirect URL, which must match the one used here, or the initial auth
+    # request will be rejected.
     client_options: {
-      identifier: "DTI-TODO",
+      identifier: "19ea1361-f0b1-48f2-9405-b570c655afd9",
-      secret: "DTI-TODO",
+      secret: Rails.application.credentials.dig(:neopass, :client_secret),
       redirect_uri: Rails.configuration.neopass_redirect_uri,
     },
+
+    # Specify that the client ID and secret should be passed as POST form data,
+    # rather than the default of an `Authorization` header. This is necessary
+    # to match the NeoPass specification; if we use the header instead, it will
+    # tell us their record of the Dress to Impress client isn't configured to
+    # allow this!
+    #
+    # NOTE: This isn't a documented supported value for `client_auth_method`
+    #       here. The `omniauth_openid_connect` docs say it must be `:basic` or
+    #       `:jwks`, but as far as I can tell, the value gets passed to
+    #       `Rack::OAuth2::Client#access_token!`, which uses an `Authorization`
+    #       header if the value is `:basic`, or POST form data if the value is
+    #       anything else. So this option isn't stable, but it works for now!
+    client_auth_method: :request_body,
   }
 
-  # Output OmniAuth debug info to the server logs in development
+  # Output OmniAuth debug info to the server logs.
-  OmniAuth.config.logger = Rails.logger if Rails.env.development?
+  #
+  # TODO: We should perhaps evaluate whether these logs contain sensitive
+  #       information in production? I wouldn't think so, and it will be useful
+  #       for debugging NeoPass, but let's keep an eye on that! Consider
+  #       setting this to only be true in development mode, if we're not
+  #       actively using it anymore.
+  OmniAuth.config.logger = Rails.logger
 
   # ==> Warden configuration
   # If you want to use other strategies, that are not supported by Devise, or

db/migrate/20240401124200_increase_username_length.rb

@@ -0,0 +1,13 @@
+class IncreaseUsernameLength < ActiveRecord::Migration[7.1]
+  def change
+    # NOTE: This is paired with a migration to the `openneo_id` database, too!
+    reversible do |direction|
+      direction.up {
+        change_column :users, :name, :string, limit: 30, null: false
+      }
+      direction.down {
+        change_column :users, :name, :string, limit: 20, null: false
+      }
+    end
+  end
+end

db/openneo_id_migrate/20240401124406_increase_username_length.rb

@@ -0,0 +1,14 @@
+class IncreaseUsernameLength < ActiveRecord::Migration[7.1]
+  def change
+    # NOTE: This is paired with a migration to the `openneo_impress` database,
+    #       too!
+    reversible do |direction|
+      direction.up {
+        change_column :users, :name, :string, limit: 30, null: false
+      }
+      direction.down {
+        change_column :users, :name, :string, limit: 20, null: false
+      }
+    end
+  end
+end

db/openneo_id_schema.rb

@@ -10,9 +10,9 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_03_15_020053) do
+ActiveRecord::Schema[7.1].define(version: 2024_04_01_124406) do
   create_table "users", id: { type: :integer, unsigned: true }, charset: "utf8mb3", collation: "utf8mb3_general_ci", force: :cascade do |t|
-    t.string "name", limit: 20, null: false
+    t.string "name", limit: 30, null: false
     t.string "encrypted_password", limit: 64
     t.string "email", limit: 50
     t.string "password_salt", limit: 32

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_03_23_234243) do
+ActiveRecord::Schema[7.1].define(version: 2024_04_01_124200) do
   create_table "alt_styles", charset: "utf8mb4", collation: "utf8mb4_unicode_520_ci", force: :cascade do |t|
     t.integer "species_id", null: false
     t.integer "color_id", null: false
@@ -254,7 +254,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_03_23_234243) do
   end
 
   create_table "users", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_520_ci", force: :cascade do |t|
-    t.string "name", limit: 20, null: false
+    t.string "name", limit: 30, null: false
     t.integer "auth_server_id", limit: 1, null: false
     t.integer "remote_id", null: false
     t.integer "points", default: 0, null: false