Compare commits

...

9 commits

Author SHA1 Message Date
644b181ed0 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.)
2024-04-09 07:48:13 -07:00
9ed34fa042 Add User-Agent header to our OwlsValueGuide requests
Note: I validated this was working by temporarily changing the URI to
`https://echo.free.beeceptor.com`, which echoes the headers back, then
called `OwlsValueGuide.load_itemdata` directly.
2024-04-09 06:59:44 -07:00
eb5f2a020c Add User-Agent header to our NeopetsMediaArchive requests
Note: I validated this was working by temporarily changing the URI to
`https://echo.free.beeceptor.com`, which echoes the headers back, then
called `NeopetsMediaArchive.load_file_from_origin` directly.
2024-04-09 06:58:03 -07:00
d50672fd73 Add User-Agent header to our AMFPHP requests
Oh right, I never did catch this when setting up User-Agent in the app!

(I noticed this because I'm making a new request now, and went to look
how we set it in previous stuff, and was like. Oh. We don't anywhere
right now. Interesting LOL)
2024-04-09 06:55:41 -07:00
58d86cf3ac Prevent user from removing all their login methods
Oh right, if you can remove your email, there's a way to fully lock out
your account:

1. Create account via NeoPass, so no password is set.
2. Ensure you have an email saved, then disconnect NeoPass.
3. Remove the email.
4. Now you have no NeoPass, no email, and no password!

In this change, we add a validation that requires an account to always
have at least one login method. This works well for the case described
above, and also helps offer server-side validation to the "can't
disconnect NeoPass until you have an email and password" stuff that
previously was only enforced by disabling the button.

That is, the following procedure could also lock you out before,
whereas now it raises the "Whoops, there was an error disconnecting
your NeoPass from your account, sorry." message:

1. Create account via NeoPass, so no password is set.
2. Ensure you have an email saved, so "Disconnect" button is enabled.
3. Open a new browser tab, and remove the email.
4. In the original browser tab, click "Disconnect".
2024-04-09 06:40:56 -07:00
9384fd2aa7 Add additional cookie method to view hidden NeoPass features
This is gonna help me in development, to stop having to add stuff to
the URL all the time!! I also considered just always making it
available in development, but I wanted to match production behavior to
help us ensure the hiding behavior is working, to avoid leaking NeoPass
without realizing.
2024-04-09 06:36:44 -07:00
95c1a4f391 Fix bugs in Settings page when changes to the model are incomplete
Ahh okay tricky lil thing: if you show the settings page with a partial
change to `AuthUser` that didn't get saved, it can throw off the state
of some stuff. For example, if you don't have a password yet, then
enter a new password but leave the confirmation box blank, then you'll
correctly see "Password confirmation can't be blank", but you'll *also*
then be prompted for your "Current password", even though you don't
have one yet, because `@auth_user.uses_password?` is true now.

In this change, we extend the Settings form to use two copies of the
`AuthUser`. One is the copy with changes on it, and the other is the
"persisted" copy, which we check for parts of the UI that care about
what's actually saved, vs form state.
2024-04-09 06:34:06 -07:00
f450937952 Oops, fix error when saving user settings with no password set
Ah okay, if you leave the password field blank but don't have one set,
our simple `update` method gets annoyed that you left it blank.

In this change, we simplify the model API by just overriding
`update_with_password` with our own special behavior for the
no-password case.
2024-04-09 06:20:13 -07:00
d10c11e261 Oops, fix tracking neopass_email on account creation.
My bad!
2024-04-09 05:45:39 -07:00
11 changed files with 193 additions and 39 deletions

View file

@ -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

View file

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

View file

@ -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

View file

@ -9,6 +9,8 @@ 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
@ -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?

View file

@ -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

73
app/services/neopass.rb Normal file
View file

@ -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

View file

@ -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})"

View file

@ -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

View file

@ -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"),

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

View file

@ -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