Compare commits

..

No commits in common. "644b181ed00bccc48b57f7776f43b47794b6e2c1" and "0a046ed9c1aaa68656083d506dd95f55e091403f" have entirely different histories.

11 changed files with 39 additions and 193 deletions

View file

@ -14,9 +14,7 @@ class AuthUsersController < ApplicationController
end
def edit
# For the edit form, the auth user *is* the persisted auth user.
@persisted_auth_user = current_auth_user
@auth_user = @persisted_auth_user
@auth_user = current_auth_user
end
def new
@ -24,18 +22,15 @@ 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 @auth_user.update_with_password(auth_user_params)
# 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
# NOTE: Changing the password will sign you out, so make sure we stay
# signed in!
bypass_sign_in @auth_user, scope: :auth_user

View file

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

View file

@ -69,17 +69,8 @@ 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 ||
cookies[:neopass_access_secret] == Rails.configuration.neopass_access_secret
params[:neopass] == Rails.configuration.neopass_access_secret
end
def contact_email

View file

@ -9,8 +9,6 @@ class AuthUser < AuthRecord
length: {maximum: 30}
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
@ -83,23 +81,6 @@ 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?
@ -138,13 +119,6 @@ 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
@ -156,26 +130,9 @@ 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.
# 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)
# TODO: Can we somehow get the Neopets username if one exists, instead
# of just using total randomness?
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
@ -183,16 +140,14 @@ 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`. (In practice, this *shouldn't* ever change
# after initial setup, because NeoPass emails are immutable? But why
# not be resilient!)
# 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!)
unless user.previously_new_record?
user.update!(neopass_email: auth.info.email)
end
@ -200,28 +155,17 @@ class AuthUser < AuthRecord
end
end
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
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}"
# 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
# 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.
# 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?

View file

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

View file

@ -1,73 +0,0 @@
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

View file

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

View file

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

View file

@ -39,7 +39,7 @@
</fieldset>
<%# Current password is only required if you have one! %>
<% if @persisted_auth_user.uses_password? %>
<% if @auth_user.uses_password? %>
<fieldset>
<div class="field">
<%= f.label :current_password %>
@ -55,14 +55,13 @@
</div>
<% end %>
<% if @persisted_auth_user.uses_neopass? %>
<% if @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 " +
"\"#{@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!"
"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!"
} do |form|
%>
<h2>Your NeoPass</h2>
@ -70,7 +69,7 @@
<strong>
NeoPass ID:
</strong>
<%= @persisted_auth_user.neopass_friendly_id %>
<%= @auth_user.neopass_friendly_id %>
</section>
<section class="neopass-explanation">
<p>
@ -79,24 +78,23 @@
you can still use "Forgot your password?" to recover your Dress to
Impress account, using the Email saved in "Your info".
</p>
<% if !@persisted_auth_user.uses_password? && !@persisted_auth_user.email? %>
<% if !@auth_user.uses_password? && !@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 !@persisted_auth_user.uses_password? %>
<% elsif !@auth_user.uses_password? %>
<p>
Be extra careful here! Your account doesn't have a password set.
</p>
<% elsif !@persisted_auth_user.email? %>
<% elsif !@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: !@persisted_auth_user.uses_password? &&
!@persisted_auth_user.email? %>
disabled: !@auth_user.uses_password? && !@auth_user.email? %>
<% end %>
<% elsif can_use_neopass %>
<%= form_with url: auth_user_neopass_omniauth_authorize_path(intent: "connect"),

View file

@ -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, :linkage],
scope: [:openid, :email],
response_type: :code,
issuer: Rails.configuration.neopass_origin,
discovery: true,

View file

@ -18,7 +18,4 @@
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