From 644b181ed00bccc48b57f7776f43b47794b6e2c1 Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Tue, 9 Apr 2024 07:48:13 -0700 Subject: [PATCH] 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.) --- .../neopass_connections_controller.rb | 2 +- app/models/auth_user.rb | 46 +++++++++--- app/services/neopass.rb | 73 +++++++++++++++++++ config/initializers/devise.rb | 2 +- config/initializers/inflections.rb | 3 + 5 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 app/services/neopass.rb diff --git a/app/controllers/neopass_connections_controller.rb b/app/controllers/neopass_connections_controller.rb index 4bab4a6a..cf53a0a2 100644 --- a/app/controllers/neopass_connections_controller.rb +++ b/app/controllers/neopass_connections_controller.rb @@ -1,4 +1,4 @@ -class NeopassConnectionsController < ApplicationController +class NeoPassConnectionsController < ApplicationController def destroy @user = load_user diff --git a/app/models/auth_user.rb b/app/models/auth_user.rb index e5b47fcc..a326b8eb 100644 --- a/app/models/auth_user.rb +++ b/app/models/auth_user.rb @@ -156,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 @@ -183,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? diff --git a/app/services/neopass.rb b/app/services/neopass.rb new file mode 100644 index 00000000..c55bf127 --- /dev/null +++ b/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 diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 347b7779..f1a32c89 100644 --- a/config/initializers/devise.rb +++ b/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, diff --git a/config/initializers/inflections.rb b/config/initializers/inflections.rb index 620da4d8..753d5173 100644 --- a/config/initializers/inflections.rb +++ b/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