Use Neopets username as base name for new NeoPass accounts, if possible

Yay, we got the API endpoint for this! The `linkage` scope is the key.

Rather than pulling back the specific fallback behavior we had wrote
for usernames before, which was slightly different and involved
appending `neopass` in there too (e.g. `matchu-neopass-1234`), I
figured let's just use a lot of the same logic, and just use the
preferred name as the base name. (I figure the `neopass` suffix isn't
that useful anyway, `matchu-1234` kinda looks better tbh! And it's all
fallback stuff that I expect serious users to replace, anyway.)

app/controllers/auth_users_controller.rb

@@ -14,7 +14,9 @@ class AuthUsersController < ApplicationController
 	end
 
 	def edit
-		@auth_user = current_auth_user
+		# For the edit form, the auth user *is* the persisted auth user.
+		@persisted_auth_user = current_auth_user
+		@auth_user = @persisted_auth_user
 	end
 
 	def new
@@ -22,15 +24,18 @@ class AuthUsersController < ApplicationController
 	end
 
 	def update
+		# When updating, we hold onto the original `@persisted_auth_user`, then
+		# make our changes to `@auth_user`. That way, the form can check the *live*
+		# value of `uses_password?` to decide whether to show the "Current
+		# password" field, instead of getting thrown off if the password changed
+		# but the record didn't get saved.
+		#
+		# HACK: Is there a way to get the kind of copy we want for real? `dup`
+		#       actually returns a *new* unsaved record with the same attributes.
 		@auth_user = load_auth_user
+		@persisted_auth_user = @auth_user.dup
 
-		# If the user has a password, then the `current_password` field is required
-		# when updating. If not, then it's not!
-		success = @auth_user.uses_password? ?
-			@auth_user.update_with_password(auth_user_params) :
-			@auth_user.update(auth_user_params)
-
-		if success
+		if @auth_user.update_with_password(auth_user_params)
 			# NOTE: Changing the password will sign you out, so make sure we stay
 			# signed in!
 			bypass_sign_in @auth_user, scope: :auth_user

app/controllers/neopass_connections_controller.rb

@@ -1,4 +1,4 @@
-class NeopassConnectionsController < ApplicationController
+class NeoPassConnectionsController < ApplicationController
 	def destroy
 		@user = load_user
 

app/helpers/application_helper.rb

@@ -69,8 +69,17 @@ module ApplicationHelper
     end
   end
 
+  # Add ?neopass=1 to the URL, or set the `neopass_access_secret=1` cookie, to
+  # view NeoPass features. (NOTE: In production, this is a secret value
+  # instead!)
+  #
+  # NOTE: We intentionally don't e.g. set the cookie just because you went to
+  #       the secret URL once, to avoid demo users getting confused about
+  #       whether NeoPass is publicly available: if they go to the public page,
+  #       they should NOT see NeoPass anymore, rather than think it's live!
   def can_use_neopass
-    params[:neopass] == Rails.configuration.neopass_access_secret
+    params[:neopass] == Rails.configuration.neopass_access_secret ||
+      cookies[:neopass_access_secret] == Rails.configuration.neopass_access_secret
   end
 
   def contact_email

app/models/auth_user.rb

@@ -10,6 +10,8 @@ class AuthUser < AuthRecord
 
   validates :uid, uniqueness: {scope: :provider, allow_nil: true}
 
+  validate :has_at_least_one_login_method
+  
   has_one :user, foreign_key: :remote_id, inverse_of: :auth_user
 
   # If the email is blank, ensure that it's `nil` rather than an empty string,
@@ -81,6 +83,23 @@ class AuthUser < AuthRecord
     encrypted_password?
   end
 
+  def update_with_password(params)
+    # If this account already uses passwords, use Devise's default behavior.
+    return super(params) if uses_password?
+
+    # Otherwise, we implement similar logic, but skipping the check for
+    # `current_password`: Bulk-assign most attributes, but only set the
+    # password if it's non-empty.
+    self.attributes = params.except(:password, :password_confirmation,
+      :current_password)
+    if params[:password].present?
+      self.password = params[:password]
+      self.password_confirmation = params[:password_confirmation]
+    end
+
+    self.save
+  end
+
   def connect_omniauth!(auth)
     raise MissingAuthInfoError, "Email missing" if auth.info.email.blank?
 
@@ -119,6 +138,13 @@ class AuthUser < AuthRecord
     end
   end
 
+  def has_at_least_one_login_method
+    if !uses_password? && !email? && !uses_neopass?
+      errors.add(:base, "You must have either a password, an email, or a " +
+        "NeoPass. Otherwise, you can't log in!")
+    end
+  end
+
   def self.find_by_omniauth(auth)
     find_by(provider: auth.provider, uid: auth.uid)
   end
@@ -130,9 +156,26 @@ class AuthUser < AuthRecord
       find_or_create_by!(provider: auth.provider, uid: auth.uid) do |user|
         # This account is new! Let's do the initial setup.
 
-        # TODO: Can we somehow get the Neopets username if one exists, instead
-        # of just using total randomness?
-        user.name = build_unique_username
+        # First, get the user's Neopets username if possible, as a base for the
+        # name we create for them. (This is an async task under the hood, which
+        # means we can wrap it in a `with_timeout` block!)
+        neopets_username = Sync do |task|
+          task.with_timeout(5) do
+            NeoPass.load_main_neopets_username(auth.credentials.token)
+          end
+        rescue Async::TimeoutError
+          nil # If the request times out, just move on!
+        rescue => error
+          # If the request fails, log it and move on. (Could be a service
+          # outage, or a change on NeoPass's end that we're not in sync with.)
+          Rails.logger.error error
+          Sentry.capture_exception error
+          nil
+        end
+
+        # Generate a unique username for this user, based on their Neopets
+        # username, if they have one.
+        user.name = build_unique_username(neopets_username)
 
         # Copy the email address from their Neopets account to their DTI
         # account, unless they already have a DTI account with this email, in
@@ -140,14 +183,16 @@ class AuthUser < AuthRecord
         # password recovery!)
         email_exists = AuthUser.where(email: auth.info.email).exists?
         user.email = auth.info.email unless email_exists
+
+        # Additionally, regardless of whether we save it as `email`, we also
+        # save the email address as `neopass_email`, to use in the Settings UI
+        # to indicate what NeoPass you're linked to.
+        user.neopass_email = auth.info.email
       end.tap do |user|
         # If this account already existed, make sure we've saved the latest
-        # email to `neopass_email`.
-        #
-        # We track this separately from `email`, which the user can edit, to
-        # use in the Settings UI to indicate what NeoPass you're linked to. (In
-        # practice, this *shouldn't* ever change after initial setup, because
-        # NeoPass emails are immutable? But why not be resilient!)
+        # email to `neopass_email`. (In practice, this *shouldn't* ever change
+        # after initial setup, because NeoPass emails are immutable? But why
+        # not be resilient!)
         unless user.previously_new_record?
           user.update!(neopass_email: auth.info.email)
         end
@@ -155,17 +200,28 @@ class AuthUser < AuthRecord
     end
   end
 
-  def self.build_unique_username
-    # Start with a base name like "neopass-kougra-".
-    random_species_name = Species.all.pluck(:name).sample
-    base_name = "neopass-#{random_species_name}"
+  def self.build_unique_username(preferred_name = nil)
+    # If there's no preferred name provided (ideally the user's Neopets
+    # username), start with a base name like `neopass-kougra`.
+    if preferred_name.present?
+      base_name = preferred_name
+    else
+      random_species_name = Species.all.pluck(:name).sample
+      base_name = "neopass-#{random_species_name}"
+    end
 
     # Fetch the list of names that already start with that.
     name_query = sanitize_sql_like(base_name) + "%"
     similar_names = where("name LIKE ?", name_query).pluck(:name).to_set
 
-    # Shuffle the list of four-digit numbers to create 10000 possible names,
-    # then use the first one that's not already claimed.
+    # If this was the user's preferred name (rather than a random one), try
+    # using the preferred name itself, if not already taken.
+    if preferred_name.present? && !similar_names.include?(preferred_name)
+      return preferred_name
+    end
+
+    # Otherwise, shuffle the list of four-digit numbers to create 10000
+    # possible names, 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?

app/models/pet.rb

@@ -158,7 +158,9 @@ class Pet < ApplicationRecord
   # Return the response body as a `HashWithIndifferentAccess`.
   def self.send_amfphp_request(request, timeout: 10)
     begin
-      response = request.post(timeout: timeout)
+      response = request.post(timeout: timeout, headers: {
+        "User-Agent" => Rails.configuration.user_agent_for_neopets,
+      })
     rescue RocketAMFExtensions::RemoteGateway::AMFError => e
       raise DownloadError, e.message
     rescue RocketAMFExtensions::RemoteGateway::ConnectionError => e

app/services/neopass.rb

@@ -0,0 +1,73 @@
+require "async/http/internet/instance"
+
+# While most of our NeoPass logic is built into Devise -> OmniAuth -> OIDC
+# OmniAuth plugin, NeoPass also offers some supplemental APIs that we use here.
+module NeoPass
+	# Share a pool of persistent connections, rather than reconnecting on
+	# each request. (This library does that automatically!)
+	INTERNET = Async::HTTP::Internet.instance
+
+	def self.load_main_neopets_username(access_token)
+		linkages = load_linkages(access_token)
+
+		begin
+			# Get the Neopets.com linkages, sorted with your "main" account to the
+			# front. (In theory, if there are any Neopets.com linkages, then one
+			# should be main—but let's be resilient and be prepared for none of them
+			# to be marked as such!)
+			neopets_linkages = linkages.
+				select { |l| l.fetch("client_name") == "Neopets.com" }.
+				sort_by { |l| l.fetch("extra", {}).fetch("main") == true ? 0 : 1 }
+
+			# Read the username from that linkage, if any.
+			neopets_linkages.first.try { |l| l.fetch("user_identifier") }
+		rescue KeyError => error
+			raise UnexpectedResponseFormat,
+				"missing field #{error.key} in NeoPass linkage response"
+		end
+	end
+
+	private
+
+	LINKAGE_URL = "https://oidc.neopets.com/linkage/all"
+	def self.load_linkages(access_token)
+		response = Sync do
+			response = INTERNET.get(LINKAGE_URL, [
+				["User-Agent", Rails.configuration.user_agent_for_neopets],
+				["Authorization", "Bearer #{access_token}"],
+			])
+		end
+
+		if response.status != 200
+			raise ResponseNotOK.new(response.status),
+				"expected status 200 but got #{response.status} (#{LINKAGE_URL})"
+		end
+
+		linkages_str = response.body.read
+
+		begin
+			linkages = JSON.parse(linkages_str)
+		rescue JSON::ParserError
+			Rails.logger.debug "Unexpected NeoPass linkage response:\n#{linkages_str}"
+			raise UnexpectedResponseFormat,
+				"failed to parse NeoPass linkage response as JSON"
+		end
+
+		if !linkages.is_a? Array
+			Rails.logger.debug "Unexpected NeoPass linkage response:\n#{linkages_str}"
+			raise UnexpectedResponseFormat,
+				"NeoPass linkage response was not an array of linkages"
+		end
+
+		linkages
+	end
+
+	class ResponseNotOK < StandardError
+		attr_reader :status
+		def initialize(status)
+			super
+			@status = status
+		end
+	end
+	class UnexpectedResponseFormat < StandardError;end
+end

app/services/neopets_media_archive.rb

@@ -72,7 +72,9 @@ module NeopetsMediaArchive
     # We use this in the `swf_assets:manifests:load` task to perform many
     # requests in parallel!
     Sync do
-      response = INTERNET.get(uri)
+      response = INTERNET.get(uri, [
+        ["User-Agent", Rails.configuration.user_agent_for_neopets],
+      ])
       if response.status != 200
         raise ResponseNotOK.new(response.status),
           "expected status 200 but got #{response.status} (#{uri})"

app/services/owls_value_guide.rb

@@ -36,7 +36,9 @@ module OwlsValueGuide
 
 		url = ITEMDATA_URL_TEMPLATE.expand(item_name: item_name)
 		begin
-			res = get(url)
+			res = get(url, headers: {
+				"User-Agent" => Rails.configuration.user_agent_for_neopets,
+			})
 		rescue StandardError => error
 			raise NetworkError, "Couldn't connect to Owls: #{error.message}"
 		end

app/views/auth_users/edit.html.erb

@@ -39,7 +39,7 @@
   </fieldset>
 
   <%# Current password is only required if you have one! %>
-  <% if @auth_user.uses_password? %>
+  <% if @persisted_auth_user.uses_password? %>
     <fieldset>
       <div class="field">
         <%= f.label :current_password %>
@@ -55,13 +55,14 @@
   </div>
 <% end %>
 
-<% if @auth_user.uses_neopass? %>
+<% if @persisted_auth_user.uses_neopass? %>
   <%= form_with url: user_neopass_connection_path(@auth_user.user),
       method: :delete, class: "settings-form", data: {
         turbo_confirm: "Are you sure? Without a NeoPass, you'll need to use " +
-          "your password or your recovery email \"#{@auth_user.email}\" to " +
-          "log in again.\n\nMake sure you have everything all set up first! " +
-          "Otherwise, you might be locked out of this account forever!"
+          "your password or your recovery email " +
+          "\"#{@persisted_auth_user.email}\" to log in again.\n\nMake sure " +
+          "you have everything all set up first! Otherwise, you might be " +
+          "locked out of this account forever!"
       } do |form|
   %>
     <h2>Your NeoPass</h2>
@@ -69,7 +70,7 @@
       <strong>
         NeoPass ID:
       </strong>
-      <%= @auth_user.neopass_friendly_id %>
+      <%= @persisted_auth_user.neopass_friendly_id %>
     </section>
     <section class="neopass-explanation">
       <p>
@@ -78,23 +79,24 @@
         you can still use "Forgot your password?" to recover your Dress to
         Impress account, using the Email saved in "Your info".
       </p>
-      <% if !@auth_user.uses_password? && !@auth_user.email %>
+      <% if !@persisted_auth_user.uses_password? && !@persisted_auth_user.email? %>
         <p>
           You can't remove this NeoPass yet, because you need to either set a
           password or a recovery email first. (Ideally both!)
         </p>
-      <% elsif !@auth_user.uses_password? %>
+      <% elsif !@persisted_auth_user.uses_password? %>
         <p>
           Be extra careful here! Your account doesn't have a password set.
         </p>
-      <% elsif !@auth_user.email? %>
+      <% elsif !@persisted_auth_user.email? %>
         <p>
           Be extra careful here! Your account doesn't have an email set.
         </p>
       <% end %>
     </section>
     <%= form.submit "Disconnect your NeoPass",
-        disabled: !@auth_user.uses_password? && !@auth_user.email? %>
+        disabled: !@persisted_auth_user.uses_password? &&
+          !@persisted_auth_user.email? %>
   <% end %>
 <% elsif can_use_neopass %>
   <%= form_with url: auth_user_neopass_omniauth_authorize_path(intent: "connect"),

config/initializers/devise.rb

@@ -278,7 +278,7 @@ Devise.setup do |config|
 
     # 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],
+    scope: [:openid, :email, :linkage],
     response_type: :code,
     issuer: Rails.configuration.neopass_origin,
     discovery: true,

config/initializers/inflections.rb

@@ -18,4 +18,7 @@
 ActiveSupport::Inflector.inflections(:en) do |inflect|
   # Teach Zeitwerk that `RocketAMF` is what to expect in `lib/rocketamf`.
   inflect.acronym "RocketAMF"
+
+  # Teach Zeitwerk that `NeoPass` is what to expect in `app/services/neopass.rb`.
+  inflect.acronym "NeoPass"
 end