diff --git a/adminapp/src/App.jsx b/adminapp/src/App.jsx index 4ebce9cbf..8ee4dc125 100644 --- a/adminapp/src/App.jsx +++ b/adminapp/src/App.jsx @@ -110,6 +110,7 @@ import StaticStringCreatePage from "./pages/StaticStringCreatePage"; import StaticStringsNamespacePage from "./pages/StaticStringsNamespacePage"; import StaticStringsPage from "./pages/StaticStringsPage"; import VendorAccountDetailPage from "./pages/VendorAccountDetailPage"; +import VendorAccountEditPage from "./pages/VendorAccountEditPage"; import VendorAccountListPage from "./pages/VendorAccountListPage"; import VendorConfigurationDetailPage from "./pages/VendorConfigurationDetailPage"; import VendorConfigurationEditPage from "./pages/VendorConfigurationEditPage"; @@ -577,6 +578,11 @@ function PageSwitch() { VendorAccountDetailPage )} /> + get(`/adminapi/v1/anon_proxy_vendor_accounts/${id}`, data, ...args), + updateVendorAccount: ({ id, ...data }, ...args) => + post(`/adminapi/v1/anon_proxy_vendor_accounts/${id}`, data, ...args), destroyVendorAccount: ({ id, ...data }, ...args) => post(`/adminapi/v1/anon_proxy_vendor_accounts/${id}/destroy`, data, ...args), diff --git a/adminapp/src/components/AdminActions.jsx b/adminapp/src/components/AdminActions.jsx index b5c202e47..ba44e440f 100644 --- a/adminapp/src/components/AdminActions.jsx +++ b/adminapp/src/components/AdminActions.jsx @@ -1,28 +1,57 @@ import api from "../api"; import useErrorSnackbar from "../hooks/useErrorSnackbar"; -import { Button, Card, CardContent, CircularProgress, Stack } from "@mui/material"; +import { + Button, + Card, + CardContent, + CircularProgress, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + Stack, +} from "@mui/material"; import Typography from "@mui/material/Typography"; +import size from "lodash/size"; import React from "react"; export default function AdminActions({ adminActions, updateModel }) { const { enqueueErrorSnackbar } = useErrorSnackbar(); const [lastResponse, setLastResponse] = React.useState(null); const [actionLoadings, setActionLoadings] = React.useState({}); + const [confirmingAction, setConfirmingAction] = React.useState(null); - function handleClick(e, url, params) { - e.preventDefault(); - setActionLoadings({ ...actionLoadings, [url]: true }); + if (!size(adminActions)) { + return null; + } + + function submitAction(e, action) { + setActionLoadings({ ...actionLoadings, [action.url]: true }); api - .post(url, params) + .post(action.url, action.params) .then((r) => { if (r.headers["admin-action-handler"] === "update") { updateModel(r.data); } else { setLastResponse(r.data); } + setConfirmingAction(null); // In case this came from the confirmation modal }) .catch(enqueueErrorSnackbar) - .finally(() => setActionLoadings({ ...actionLoadings, [url]: false })); + .finally(() => setActionLoadings({ ...actionLoadings, [action.url]: false })); + } + + /** + * @param {MouseEvent} e + * @param {AdminAction} action + */ + function handleClick(e, action) { + e.preventDefault(); + if (action.confirmationPrompt) { + setConfirmingAction(action); + } else { + submitAction(e, action); + } } return ( @@ -32,11 +61,11 @@ export default function AdminActions({ adminActions, updateModel }) { Actions - {adminActions.map(({ label, url, params }) => ( + {adminActions.map(({ label, url, ...rest }) => ( + + + ); } diff --git a/adminapp/src/pages/VendorAccountDetailPage.jsx b/adminapp/src/pages/VendorAccountDetailPage.jsx index 8afdff37f..ba105d358 100644 --- a/adminapp/src/pages/VendorAccountDetailPage.jsx +++ b/adminapp/src/pages/VendorAccountDetailPage.jsx @@ -1,4 +1,5 @@ import api from "../api"; +import AdminActions from "../components/AdminActions"; import AdminLink from "../components/AdminLink"; import BoolCheckmark from "../components/BoolCheckmark"; import DetailGrid from "../components/DetailGrid"; @@ -15,6 +16,7 @@ export default function VendorAccountDetailPage() { resource="vendor_account" apiGet={api.getVendorAccount} apiDelete={api.destroyVendorAccount} + canEdit properties={(model) => [ { label: "ID", value: model.id }, { label: "Created At", value: dayjs(model.createdAt) }, @@ -39,9 +41,13 @@ export default function VendorAccountDetailPage() { label: "Latest Access Code Set At", value: formatDate(model.latestAccessCodeSetAt), }, + { + label: "Pending Closure", + value: model.pendingClosure, + }, ]} > - {(model) => [ + {(model, setModel) => [ }, + { label: "Created At", value: formatDate(model.contact.createdAt) }, { label: "Address", value: model.contact.formattedAddress }, { label: "Relay Key", value: model.contact.relayKey }, ]} /> ), + , + ); +} diff --git a/adminapp/src/pages/VendorAccountForm.jsx b/adminapp/src/pages/VendorAccountForm.jsx new file mode 100644 index 000000000..2149c2fb9 --- /dev/null +++ b/adminapp/src/pages/VendorAccountForm.jsx @@ -0,0 +1,66 @@ +import FormLayout from "../components/FormLayout"; +import ResponsiveStack from "../components/ResponsiveStack"; +import SafeDateTimePicker from "../components/SafeDateTimePicker"; +import { formatOrNull } from "../modules/dayConfig"; +import { FormControlLabel, Stack, Switch, TextField, Typography } from "@mui/material"; +import React from "react"; + +export default function VendorAccountForm({ + isCreate, + resource, + setField, + setFieldFromInput, + register, + isBusy, + onSubmit, +}) { + return ( + + + Be careful when changing these values. For technical use only. + + + + + + setField("latestAccessCodeSetAt", formatOrNull(v))} + sx={{ width: { xs: "100%", sm: "50%" } }} + /> + setField("latestAccessCodeRequestedAt", formatOrNull(v))} + sx={{ width: { xs: "100%", sm: "50%" } }} + /> + + } + label="Pending Closure" + name="pendingClosure" + checked={resource.pendingClosure} + onChange={setFieldFromInput} + /> + + + ); +} diff --git a/db/migrations/110_multiple_contacts_per_vendor_account.rb b/db/migrations/110_multiple_contacts_per_vendor_account.rb new file mode 100644 index 000000000..263781b74 --- /dev/null +++ b/db/migrations/110_multiple_contacts_per_vendor_account.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +Sequel.migration do + tbl = :anon_proxy_member_contacts + up do + # Coming from the previous migration, we need to drop the constraint (since the index depends on it), + # but using add_index in the DOWN, then coming back UP, we don't have the constraint, + # we have to drop the index. I am not really sure why. + run "ALTER TABLE #{tbl} DROP CONSTRAINT IF EXISTS anon_proxy_member_contacts_email_relay_key_key" + run "ALTER TABLE #{tbl} DROP CONSTRAINT IF EXISTS anon_proxy_member_contacts_phone_relay_key_key" + alter_table tbl do + drop_index([], name: :anon_proxy_member_contacts_email_relay_key_key, if_exists: true) + drop_index([], name: :anon_proxy_member_contacts_phone_relay_key_key, if_exists: true) + add_index :email + add_index :phone + end + end + + down do + alter_table tbl do + drop_index :email + drop_index :phone + add_index [:email, :relay_key], unique: true, name: :anon_proxy_member_contacts_email_relay_key_key + add_index [:phone, :relay_key], unique: true, name: :anon_proxy_member_contacts_phone_relay_key_key + end + end +end diff --git a/lib/suma/admin_actions.rb b/lib/suma/admin_actions.rb index 811bd04b8..355400772 100644 --- a/lib/suma/admin_actions.rb +++ b/lib/suma/admin_actions.rb @@ -6,13 +6,13 @@ # The response value of the action is displayed in the admin UI. module Suma::AdminActions class Action < Suma::TypedStruct - attr_reader :label, :url, :params + attr_reader :label, :url, :params, :confirmation_prompt - def _defaults = {params: {}} + def _defaults = {params: {}, confirmation_prompt: ""} end def admin_actions = _admin_actions_self.select(&:itself) # @return [Array] def _admin_actions_self = raise NotImplementedError - def _admin_action(label, url, params: {}) = Action.new(label:, url:, params:) + def _admin_action(label, url, params: {}, **) = Action.new(label:, url:, params:, **) end diff --git a/lib/suma/admin_api/anon_proxy_vendor_accounts.rb b/lib/suma/admin_api/anon_proxy_vendor_accounts.rb index 3e8ea8e8e..da0eb8927 100644 --- a/lib/suma/admin_api/anon_proxy_vendor_accounts.rb +++ b/lib/suma/admin_api/anon_proxy_vendor_accounts.rb @@ -22,6 +22,7 @@ class DetailedVendorAccountEntity < AnonProxyVendorAccountEntity expose :latest_access_code_set_at expose :latest_access_code_requested_at expose :latest_access_code_magic_link + expose :pending_closure expose :contact, with: AnonProxyMemberContactEntity expose :registrations, with: VendorAccountRegistrationEntity end @@ -37,10 +38,63 @@ class DetailedVendorAccountEntity < AnonProxyVendorAccountEntity Suma::AnonProxy::VendorAccount, DetailedVendorAccountEntity, ) + Suma::AdminAPI::CommonEndpoints.update( + self, + Suma::AnonProxy::VendorAccount, + DetailedVendorAccountEntity, + ) do + params do + optional :latest_access_code, type: String + optional :latest_access_code_magic_link, type: String + optional :latest_access_code_set_at, type: Time + optional :latest_access_code_requested_at, type: Time + optional :pending_closure, type: Boolean + end + end + Suma::AdminAPI::CommonEndpoints.destroy( self, Suma::AnonProxy::VendorAccount, DetailedVendorAccountEntity, ) + + route_param :id, type: Integer do + helpers do + def lookup!(rw) + check_admin_role_access!(rw, Suma::AnonProxy::VendorAccount) + (m = Suma::AnonProxy::VendorAccount[params[:id]]) or forbidden! + return m + end + end + + post :revoke_lime_login do + a = lookup!(:write) + a.member.audit_activity("revokelime", action: a) + Suma::Program::ServiceRevoker.new(dry_run: false).close_lime_account(a) + created_resource_headers(a.id, a.admin_link) + admin_action_handler :update + status 200 + present a, with: DetailedVendorAccountEntity + end + + post "revoke_lime_login/finish" do + a = lookup!(:write) + a.update(pending_closure: false, contact: nil) + created_resource_headers(a.id, a.admin_link) + admin_action_handler :update + status 200 + present a, with: DetailedVendorAccountEntity + end + + post :revoke_lyft_pass do + a = lookup!(:write) + a.member.audit_activity("revokelyft", action: a) + Suma::Program::ServiceRevoker.new(dry_run: false).revoke_lyft_passes(a.registrations) + created_resource_headers(a.id, a.admin_link) + admin_action_handler :update + status 200 + present a, with: DetailedVendorAccountEntity + end + end end end diff --git a/lib/suma/anon_proxy/auth_to_vendor.rb b/lib/suma/anon_proxy/auth_to_vendor.rb index 2c2c47290..4b9554fba 100644 --- a/lib/suma/anon_proxy/auth_to_vendor.rb +++ b/lib/suma/anon_proxy/auth_to_vendor.rb @@ -6,13 +6,13 @@ class Suma::AnonProxy::AuthToVendor extend Suma::SimpleRegistry require_relative "auth_to_vendor/fake" - register(:fake, Fake) + register(Fake) require_relative "auth_to_vendor/lime" - register(:lime, Lime) + register(Lime) require_relative "auth_to_vendor/lyft_pass" - register(:lyft_pass, LyftPass) + register(LyftPass) # @return [Suma::AnonProxy::Provision] def self.create!(key, vendor_account:) diff --git a/lib/suma/anon_proxy/auth_to_vendor/fake.rb b/lib/suma/anon_proxy/auth_to_vendor/fake.rb index 5f5c6d6c3..e9e364916 100644 --- a/lib/suma/anon_proxy/auth_to_vendor/fake.rb +++ b/lib/suma/anon_proxy/auth_to_vendor/fake.rb @@ -2,6 +2,8 @@ class Suma::AnonProxy::AuthToVendor::Fake < Suma::AnonProxy::AuthToVendor class << self + def key = :fake + attr_accessor :calls, :auth, :needs_polling def reset diff --git a/lib/suma/anon_proxy/auth_to_vendor/lime.rb b/lib/suma/anon_proxy/auth_to_vendor/lime.rb index 2e458926a..8afbcc2c7 100644 --- a/lib/suma/anon_proxy/auth_to_vendor/lime.rb +++ b/lib/suma/anon_proxy/auth_to_vendor/lime.rb @@ -8,6 +8,8 @@ class NoToken < Suma::Http::Error; end USER_AGENT = "Android Lime/3.219.0; (com.limebike; build:3.219.0; Android 33) 4.12.0" APP_VERSION = "3.219.0" + def self.key = :lime + def agreement_params return {user_agreement_version: Suma::Lime.user_agreement_version, user_agreement_country_code: "US"} end diff --git a/lib/suma/anon_proxy/auth_to_vendor/lyft_pass.rb b/lib/suma/anon_proxy/auth_to_vendor/lyft_pass.rb index 518355e1e..3434ab8e5 100644 --- a/lib/suma/anon_proxy/auth_to_vendor/lyft_pass.rb +++ b/lib/suma/anon_proxy/auth_to_vendor/lyft_pass.rb @@ -3,6 +3,8 @@ require "suma/lyft/pass" class Suma::AnonProxy::AuthToVendor::LyftPass < Suma::AnonProxy::AuthToVendor + def self.key = :lyft_pass + def auth(now:) programs = self.programs_requiring_attention(now:).to_a return unless programs.any? diff --git a/lib/suma/anon_proxy/message_handler/lime.rb b/lib/suma/anon_proxy/message_handler/lime.rb index 4ebfb0539..176e83e64 100644 --- a/lib/suma/anon_proxy/message_handler/lime.rb +++ b/lib/suma/anon_proxy/message_handler/lime.rb @@ -35,18 +35,17 @@ def handle(vendor_account_message) vendor_account = vendor_account_message.vendor_account if vendor_account.pending_closure - lime_atv = Suma::AnonProxy::AuthToVendor::Lime.new(vendor_account) - # This will log the user out of their own device, which is good enough. - begin - lime_atv.exchange_magic_link_token(token) - rescue Suma::AnonProxy::AuthToVendor::Lime::NoToken - # If when we log in, we're served with a user agreement rather than an auth token, - # that is good enough (we think). - nil - end - # This email is now trashed for 90 days; clear it out so the next time - # the account is linked, a new contact is generated. - vendor_account.update(pending_closure: false, contact: nil) + # As of sometime in early 2026, we can no longer exchange the magic link token; + # instead, it's supposed to be followed to a Cloudflare page, + # then a link with a new token is issued, and users follow that. + # So we need to log people out manually. + # Update the magic link, so an admin can follow the link to login manually + # (which will THEN trash the user's existing access). + # Then the admin should use the external action to mark the account + # as no longer pending closure, and removing the contact association, + # which is what used to happen here. + vendor_account.replace_access_code(token, magic_link) + vendor_account.save_changes result.handled = true return result elsif Suma::Payment.service_usage_prohibited_reason(vendor_account.member.payment_account) diff --git a/lib/suma/anon_proxy/vendor_account.rb b/lib/suma/anon_proxy/vendor_account.rb index 1918b058a..f6d6a0bd3 100644 --- a/lib/suma/anon_proxy/vendor_account.rb +++ b/lib/suma/anon_proxy/vendor_account.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true +require "suma/admin_actions" require "suma/admin_linked" require "suma/anon_proxy" require "suma/postgres" class Suma::AnonProxy::VendorAccount < Suma::Postgres::Model(:anon_proxy_vendor_accounts) include Suma::Postgres::HybridSearch + include Suma::AdminActions include Suma::AdminLinked RECENT_ACCESS_CODE_CUTOFF = 10.minutes @@ -199,6 +201,35 @@ class UIStateV1 < Suma::TypedStruct def prompt_for_payment_method = self.requires_payment_method && !self.has_payment_method end + def _admin_actions_self + r = [] + if self.auth_to_vendor.class.key == :lime && self.latest_access_code_set_at + r << if self.pending_closure + self._admin_action( + "Finish Lime Revocation", + "/adminapi/v1/anon_proxy_vendor_accounts/#{self.id}/revoke_lime_login/finish", + confirmation_prompt: "Did you log in with the Lime magic link?", + ) + else + prompt = "This will start logging the user out of Lime. You need to refresh the page until " \ + "a Magic Link is present, then log in with it, then use Finish Lime Revocation." + self._admin_action( + "Revoke Lime Login", + "/adminapi/v1/anon_proxy_vendor_accounts/#{self.id}/revoke_lime_login", + confirmation_prompt: prompt, + ) + end + elsif self.auth_to_vendor.class.key == :lyft_pass && self.registrations.any? + a = self._admin_action( + "Revoke Lyft Pass", + "/adminapi/v1/anon_proxy_vendor_accounts/#{self.id}/revoke_lyft_pass", + confirmation_prompt: "This will remove the user from any associated Lyft Passes. Are you sure?", + ) + r << a + end + return r + end + def rel_admin_link = "/vendor-account/#{self.id}" def hybrid_search_fields diff --git a/lib/suma/async/deprecated_jobs.rb b/lib/suma/async/deprecated_jobs.rb index 8eaede45d..c72da0594 100644 --- a/lib/suma/async/deprecated_jobs.rb +++ b/lib/suma/async/deprecated_jobs.rb @@ -12,10 +12,12 @@ # Then it can be deleted later. "Async::AutomationTriggerRunner", "Async::Emailer", + "Async::EnrollmentRemovalRunner", "Async::EnsureDefaultMemberLedgersOnCreate", "Async::MembershipVerifiedVerifyOnboarding", "Async::PaymentInstrumentChargeBalance", "Async::UpsertFrontappContact", + "Async::ServiceRevokerRunner", "Async::SyncLimeFreeBikeStatusGbfs", "Async::SyncLimeGeofencingZonesGbfs", "Async::SyncLyftFreeBikeStatusGbfs", diff --git a/lib/suma/async/enrollment_removal_runner.rb b/lib/suma/async/enrollment_removal_runner.rb deleted file mode 100644 index 255eed91d..000000000 --- a/lib/suma/async/enrollment_removal_runner.rb +++ /dev/null @@ -1,76 +0,0 @@ -# frozen_string_literal: true - -require "amigo/job" - -class Suma::Async::EnrollmentRemovalRunner - extend Amigo::Job - - on Regexp.new('^suma\.(' \ - '(program\.enrollment\.updated)' \ - '|(organization\.membership\.updated)' \ - '|(member\.role\.removed)' \ - '|(organization\.role\.removed)' \ - ")$") - - class << self - attr_accessor :testing_last_ran_removers - end - - def _perform(event) - case event.name - when "suma.program.enrollment.updated" - enrollment = self.lookup_model(Suma::Program::Enrollment, event) - case event.payload[1] - when changed(:unenrolled_at, from: nil) - removers = self.handle_direct_enrollment_unenrolled(enrollment) - else - return - end - when "suma.organization.membership.updated" - membership = self.lookup_model(Suma::Organization::Membership, event) - case event.payload[1] - when changed(:verified_organization_id, to: nil) - removers = [ - Suma::Program::EnrollmentRemover.new(membership.member).reenroll do - membership.this.update( - unverified_organization_name: nil, - verified_organization_id: membership.former_organization_id, - former_organization_id: nil, - formerly_in_organization_at: nil, - ) - end, - ] - else - return - end - when "suma.member.role.removed" - member = self.lookup_model(Suma::Member, event.payload[0]) - role = self.lookup_model(Suma::Role, event.payload[1]) - removers = [ - Suma::Program::EnrollmentRemover.new(member).reenroll do - member.ensure_role(role) - end, - ] - when "suma.organization.role.removed" - organization = self.lookup_model(Suma::Organization, event.payload[0]) - role = self.lookup_model(Suma::Role, event.payload[1]) - removers = organization.memberships.map do |m| - Suma::Program::EnrollmentRemover.new(m.member).reenroll do - organization.ensure_role(role) - end - end - else - raise NotImplementedError, "unhandled event: #{event.name}" - end - self.class.testing_last_ran_removers = removers if Suma::RACK_ENV == "test" - return if removers.nil? - removers.each(&:process) - end - - def handle_direct_enrollment_unenrolled(enrollment) - removers = enrollment.members.map do |m| - Suma::Program::EnrollmentRemover.new(m).reenroll { enrollment.update(unenrolled_at: nil) } - end - return removers - end -end diff --git a/lib/suma/async/service_revoker_runner.rb b/lib/suma/async/service_revoker_runner.rb deleted file mode 100644 index 3a1764319..000000000 --- a/lib/suma/async/service_revoker_runner.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require "amigo/job" - -# Run the service revoker on the originating ledger -# any time a book transaction is created. -class Suma::Async::ServiceRevokerRunner - extend Amigo::Job - - on "suma.payment.booktransaction.created" - - def _perform(event) - bx = self.lookup_model(Suma::Payment::BookTransaction, event) - Suma::Program::ServiceRevoker.run_for(bx.originating_ledger) - end -end diff --git a/lib/suma/async/service_revoker_scheduler.rb b/lib/suma/async/service_revoker_scheduler.rb index 37579fe21..1be8732b3 100644 --- a/lib/suma/async/service_revoker_scheduler.rb +++ b/lib/suma/async/service_revoker_scheduler.rb @@ -9,10 +9,10 @@ class Suma::Async::ServiceRevokerScheduler extend Amigo::ScheduledJob sidekiq_options(Suma::Async.cron_job_options) - cron "4,34 * * * *" # Twice an hour + cron "34 12 * * *" splay 60.seconds def _perform - Suma::Program::ServiceRevoker.run + Suma::Program::ServiceRevoker.new.run end end diff --git a/lib/suma/program.rb b/lib/suma/program.rb index c2a07fcfc..7f3330981 100644 --- a/lib/suma/program.rb +++ b/lib/suma/program.rb @@ -85,7 +85,7 @@ def hybrid_search_fields # @return [String] end -require "suma/program/enrollment_remover" +require "suma/program/service_revoker" require "suma/program/has" # Table: programs diff --git a/lib/suma/program/enrollment_remover.rb b/lib/suma/program/enrollment_remover.rb deleted file mode 100644 index e3d77e205..000000000 --- a/lib/suma/program/enrollment_remover.rb +++ /dev/null @@ -1,83 +0,0 @@ -# frozen_string_literal: true - -require "suma/program/service_revoker" - -# Logic that runs when a member sees reduction in program access; -# for example, losing a role, getting an expired organization membership, -# having a direct enrollment removed, etc. -# -# Each removal calculation can be somewhat slow, so the removal is done asynchronously- -# users will lose the ability to see a program immediately, but secondary effects -# like closing a vendor account will happen afterwards. -# -# We haven't figured out a generic way to process un-enrollments, -# since there is a lot of nuance with how resources can be shared. -# -# For example, we don't want to remove someone from a Lyft Pass program -# if they have access to any program using it; we don't want to close -# someone's Lime account if any program gives them access to a Lime vendor service. -# -# So this logic is closely coupled to knowing about what external resources -# may need revocation. -class Suma::Program::EnrollmentRemover - attr_accessor :before_enrollments, :after_enrollments, :removed_enrollments - - def initialize(member) - @member = member - end - - def reenroll(&block) - @reenroll_block = block - return self - end - - def process - raise LocalJumpError, "must first call reenroll with a block" unless @reenroll_block - Suma::Program::ServiceRevoker.check_transaction! - @member.db.transaction(rollback: :always) do - m2 = Suma::Member[@member.id] - @reenroll_block.call(m2) - @before_enrollments = m2.combined_program_enrollments.select(&:enrolled?) - end - @after_enrollments = @member.combined_program_enrollments.select(&:enrolled?) - @removed_enrollments = @before_enrollments - @after_enrollments - - @before_configs = @before_enrollments.map(&:program).flat_map(&:anon_proxy_vendor_configurations).uniq - @after_configs = @after_enrollments.map(&:program).flat_map(&:anon_proxy_vendor_configurations).uniq - @removed_configs = @before_configs - @after_configs - self._close_lime_account - self._revoke_lyft_pass - return self - end - - protected def _close_lime_account - was_in_lime = @before_configs.any? { |vc| vc.auth_to_vendor_key == "lime" } - still_in_lime = @after_configs.any? { |vc| vc.auth_to_vendor_key == "lime" } - return unless was_in_lime && !still_in_lime - Suma::Program::ServiceRevoker.close_lime_accounts(@member) - end - - # Revoking lyft pass is complex for a couple reasons: - # - # First, we can't just revoke access to any unenrolled programs; - # we need to make sure the member cannot still access that lyft pass program from a different suma program. - # We only revoke access to lyft passes the member can no longer access at all. - # - # Second, and more confusingly, we need to update the VendorAccount tied to the LyftPass vendor configuration, - # to un-mark the member as registered in that lyft pass program. This way, if the member gains new access - # to this lyft pass program id, the correct code paths (prompting them to 'link' their account, - # and thus regain access to Lyft Pass) will run. - def _revoke_lyft_pass - previous_pass_ids = @before_enrollments.map { |e| e.program.lyft_pass_program_id }.select(&:present?) - current_pass_ids = @after_enrollments.map { |e| e.program.lyft_pass_program_id }.select(&:present?) - removed_pass_ids = previous_pass_ids - current_pass_ids - return if removed_pass_ids.empty? - registrations = Suma::AnonProxy::VendorAccountRegistration.where( - external_program_id: removed_pass_ids, - account: @member.anon_proxy_vendor_accounts_dataset.where( - configuration: Suma::AnonProxy::VendorConfiguration.where(auth_to_vendor_key: "lyft_pass"), - ), - ).all - Suma::Program::ServiceRevoker.revoke_lyft_passes(registrations) - end -end diff --git a/lib/suma/program/service_revoker.rb b/lib/suma/program/service_revoker.rb index f8cfeeb98..7c1921828 100644 --- a/lib/suma/program/service_revoker.rb +++ b/lib/suma/program/service_revoker.rb @@ -10,26 +10,36 @@ class Suma::Program::ServiceRevoker include Appydays::Loggable - def self.check_transaction! + private def check_transaction! Suma::Postgres.check_transaction( Suma::Member.db, "Service revocation has side effects, and should be idempotent, so can/should not use a transaction.", ) end + def initialize(dry_run: Suma::Program.service_revoker_dry_run) + @dry_run = dry_run + end + + def dry_run? = @dry_run + def do_dry_run(event, kw) = self.logger.warn(event, **kw) + # Run service revocation for each member that cannot use services. # We apply a number of heuristics to avoid querying members who are not eligible # for revocation, and ensure we only attempt to revoke when there is # some actionable reason. # In the worst case, revocation is idempotent, so calling it multiple times # won't have a negative impact. - def self.run + def run check_transaction! balances = Suma::Payment::Ledger::Balance # We only care about cash balances, balances = balances.where(ledger_name: "Cash") - # with a negative balance, or one lower than the minimal balance (which can be positive), - ok_balance = [Suma::Payment.minimum_cash_balance_for_services_cents, 0].max + # with a negative (grace) balance, or one lower than the minimal balance (which can be positive), + ok_balance = [ + Suma::Payment.minimum_cash_balance_grace_cents, + Suma::Payment.minimum_cash_balance_for_services_cents, + ].max balances = balances.where { balance_cents < ok_balance } # which have changed recently (assume we've acted on things older than this) balances = balances.where { latest_transaction_at > (Time.now - Suma::Program.service_revoker_lookback) } @@ -39,11 +49,12 @@ def self.run end end - def self.run_for(ledger) - return unless ledger.name == "Cash" + # @param ledger [Suma::Payment::Ledger] + def run_for(ledger) + raise Suma::InvalidPrecondition, "only run for cash ledger" unless ledger.name == "Cash" member = ledger.account.member return unless member - if Suma::Program.service_revoker_dry_run + if self.dry_run? self._revoke_if_cannot_use(member) else idem_key = "service-revoker-#{member.id}-#{ledger.balance_view.latest_transaction_at}" @@ -53,12 +64,12 @@ def self.run_for(ledger) end end - def self._revoke_if_cannot_use(member) + protected def _revoke_if_cannot_use(member) return if Suma::Payment.can_use_services?(member.payment_account) self.revoke_member_service_access(member) end - def self.revoke_member_service_access(member) + def revoke_member_service_access(member) self.close_lime_accounts(member) self.revoke_all_lyft_passes(member) end @@ -77,37 +88,37 @@ def self.revoke_member_service_access(member) # - vendor accounts pending closure re-request a magic link (processing this is also idempotent); # - vendor accounts already closed no longer have a member contact so are skipped. # - def self.close_lime_accounts(member) + def close_lime_accounts(member) check_transaction! lime_configs = Suma::AnonProxy::VendorConfiguration.where(auth_to_vendor_key: "lime").all vendor_accounts = member.anon_proxy_vendor_accounts_dataset. where(configuration: lime_configs). exclude(contact_id: nil). all - return if vendor_accounts.empty? - if Suma::Program.service_revoker_dry_run - vendor_accounts.each do |va| - self.logger.warn( - "service_revoker_dry_run", - action: "close_lime", - vendor_account_id: va.id, - member_name: va.member.name, - ) - end + vendor_accounts.each do |va| + self.close_lime_account(va) + end + end + + def close_lime_account(va) + if self.dry_run? + self.do_dry_run( + "service_revoker_dry_run", + action: "close_lime", + vendor_account_id: va.id, + member_name: va.member.name, + ) return end - # Update accounts by ID to make sure we're looking at a consistent set of rows. - Suma::AnonProxy::VendorAccount.where(id: vendor_accounts.map(&:id)).update(pending_closure: true) # Request a new magic link for each account. See method doc for explanation. - vendor_accounts.each do |va| - Suma::AnonProxy::AuthToVendor::Lime.new(va).auth - end + va.replace_access_code(nil, nil, at: nil).update(pending_closure: true) + Suma::AnonProxy::AuthToVendor::Lime.new(va).auth end # Revoke access to all Lyft Pass programs. # Generally this is only used for global changes, # like when a user loses access to services due to a negative ledger. - def self.revoke_all_lyft_passes(member) + def revoke_all_lyft_passes(member) registrations = Suma::AnonProxy::VendorAccountRegistration.where( account: member.anon_proxy_vendor_accounts_dataset.where( configuration: Suma::AnonProxy::VendorConfiguration.where(auth_to_vendor_key: "lyft_pass"), @@ -125,7 +136,7 @@ def self.revoke_all_lyft_passes(member) # At this point, we won't try to re-revoke it. # # @param [Array] registrations - def self.revoke_lyft_passes(registrations) + def revoke_lyft_passes(registrations) check_transaction! return if registrations.empty? lp = Suma::Lyft::Pass.from_config @@ -134,8 +145,8 @@ def self.revoke_lyft_passes(registrations) member = r.account.member # If we're running this after member deletion, use their previous phone as what was previous valid. phone = member.soft_deleted? ? member.previous_phones.first : member.phone - if Suma::Program.service_revoker_dry_run - self.logger.warn( + if self.dry_run? + self.do_dry_run( "service_revoker_dry_run", action: "revoke_lyft", phone:, diff --git a/lib/suma/simple_registry.rb b/lib/suma/simple_registry.rb index 36675cdfa..c373bdad6 100644 --- a/lib/suma/simple_registry.rb +++ b/lib/suma/simple_registry.rb @@ -9,7 +9,13 @@ def registered_keys = self.registry.keys # Override the registry key used in +registry_lookup!+. attr_accessor :registry_override - def register(key, value, *args, **kwargs) + def register(key, value=nil, *args, **kwargs) + if value.nil? + raise ArgumentError, "if value is not provided, key '#{key.inspect}' must respond to :key" unless + key.respond_to?(:key) + value = key + key = key.key + end self.registry[key.to_s] = [value, args, kwargs] end diff --git a/spec/suma/admin_api/anon_proxy_vendor_accounts_spec.rb b/spec/suma/admin_api/anon_proxy_vendor_accounts_spec.rb index 43bc76f6a..293ad5a26 100644 --- a/spec/suma/admin_api/anon_proxy_vendor_accounts_spec.rb +++ b/spec/suma/admin_api/anon_proxy_vendor_accounts_spec.rb @@ -79,6 +79,17 @@ def make_item(_i) end end + describe "POST /v1/anon_proxy_vendor_accounts/:id" do + it "updates the object" do + o = Suma::Fixtures.anon_proxy_vendor_account.create + + post "/v1/anon_proxy_vendor_accounts/#{o.id}", pending_closure: true + + expect(last_response).to have_status(200) + expect(o.refresh).to have_attributes(pending_closure: true) + end + end + describe "POST /v1/anon_proxy_vendor_accounts/:id/destroy" do it "destroys the resource" do m = Suma::Fixtures.anon_proxy_vendor_account.create @@ -90,4 +101,65 @@ def make_item(_i) expect(m).to be_destroyed end end + + describe "POST /v1/anon_proxy_vendor_accounts/:id/revoke_lime_login" do + it "uses the service revoker" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") + acct = Suma::Fixtures.anon_proxy_vendor_account(configuration: vc).create + acct.replace_access_code("x", "https://link").save_changes + req = stub_request(:post, "https://web-production.lime.bike/api/rider/v2/onboarding/magic-link"). + to_return(json_response({})) + + post "/v1/anon_proxy_vendor_accounts/#{acct.id}/revoke_lime_login" + + expect(last_response).to have_status(200) + expect(last_response).to have_json_body.that_includes(id: acct.id) + expect(req).to have_been_made + end + end + + describe "POST /v1/anon_proxy_vendor_accounts/:id/revoke_lime_login/finish" do + it "updates the account" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") + acct = Suma::Fixtures.anon_proxy_vendor_account(configuration: vc).create(pending_closure: true) + acct.replace_access_code("x", "https://link").save_changes + + post "/v1/anon_proxy_vendor_accounts/#{acct.id}/revoke_lime_login/finish" + + expect(last_response).to have_status(200) + expect(last_response).to have_json_body.that_includes(id: acct.id) + expect(acct.refresh).to have_attributes(contact: nil, pending_closure: false) + end + end + + describe "POST /v1/anon_proxy_vendor_accounts/:id/revoke_lyft_pass", no_transaction_check: true do + before(:each) do + Suma::Lyft.reset_configuration + + Suma::ExternalCredential.create( + service: "lyft-pass-access-token", + expires_at: 5.hours.from_now, + data: {body: {}, cookies: {}}.to_json, + ) + + Suma::Lyft.pass_authorization = "Basic xyz" + Suma::Lyft.pass_email = "a@b.c" + Suma::Lyft.pass_org_id = "1234" + end + + it "revokes registered passes" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lyft_pass") + acct = Suma::Fixtures.anon_proxy_vendor_account.create(configuration: vc) + acct.add_registration(external_program_id: "111") + + req = stub_request(:post, "https://www.lyft.com/api/rideprograms/enrollment/revoke"). + to_return(status: 200) + + post "/v1/anon_proxy_vendor_accounts/#{acct.id}/revoke_lyft_pass" + + expect(last_response).to have_status(200) + expect(last_response).to have_json_body.that_includes(id: acct.id) + expect(req).to have_been_made + end + end end diff --git a/spec/suma/anon_proxy/message_handler_spec.rb b/spec/suma/anon_proxy/message_handler_spec.rb index 21e2520ed..de09c78a2 100644 --- a/spec/suma/anon_proxy/message_handler_spec.rb +++ b/spec/suma/anon_proxy/message_handler_spec.rb @@ -216,43 +216,20 @@ def create_message(file) vendor_account.update(pending_closure: true) end - it "logs in the user and deletes the contact" do + it "sets the magic link fields" do expect(vendor_account).to have_attributes(contact: be_present) - req = stub_request(:post, "https://web-production.lime.bike/api/rider/v2/onboarding/login"). - with( - body: { - "has_virtual_card" => "false", - "magic_link_token" => "M1ZgpMepjL5kW9XgzCmnsBKQ", - "user_agreement_country_code" => "US", "user_agreement_version" => "5", - }, - ).to_return(fixture_response("lime/app_post_magic_link")) - got = Suma::AnonProxy::MessageHandler.handle( Suma::AnonProxy::Relay.create!("fake-email-relay"), signin_message, ) - expect(req).to have_been_made expect(got).to have_attributes(vendor_account:, outbound_delivery: nil) expect(vendor_account.refresh).to have_attributes( - latest_access_code: nil, - pending_closure: false, - contact: nil, - ) - end - - it "ignores NoToken errors" do - req = stub_request(:post, "https://web-production.lime.bike/api/rider/v2/onboarding/login"). - to_return(fixture_response("lime/app_post_sign_terms")) - - Suma::AnonProxy::MessageHandler.handle( - Suma::AnonProxy::Relay.create!("fake-email-relay"), - signin_message, + latest_access_code: "M1ZgpMepjL5kW9XgzCmnsBKQ", + pending_closure: true, + contact: be_present, ) - - expect(req).to have_been_made - expect(vendor_account.refresh).to have_attributes(pending_closure: false) end end diff --git a/spec/suma/anon_proxy/vendor_account_spec.rb b/spec/suma/anon_proxy/vendor_account_spec.rb index f74309ce0..1a1757609 100644 --- a/spec/suma/anon_proxy/vendor_account_spec.rb +++ b/spec/suma/anon_proxy/vendor_account_spec.rb @@ -133,6 +133,31 @@ end end + describe "admin actions" do + it "can revoke lime access" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") + acct = Suma::Fixtures.anon_proxy_vendor_account(configuration: vc).create + expect(acct.admin_actions).to be_empty + acct.replace_access_code("x", "https://link") + expect(acct.admin_actions).to contain_exactly(have_attributes(label: "Revoke Lime Login")) + end + + it "can finish revoking lime access" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") + acct = Suma::Fixtures.anon_proxy_vendor_account(configuration: vc).create(pending_closure: true) + acct.replace_access_code("x", "https://link") + expect(acct.admin_actions).to contain_exactly(have_attributes(label: "Finish Lime Revocation")) + end + + it "can revoke lyft access" do + vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lyft_pass") + acct = Suma::Fixtures.anon_proxy_vendor_account.create(configuration: vc) + expect(acct.admin_actions).to be_empty + acct.add_registration(external_program_id: "111") + expect(acct.admin_actions).to contain_exactly(have_attributes(label: "Revoke Lyft Pass")) + end + end + describe "ui state helpers" do let(:member) { Suma::Fixtures.member.create } let(:vendor) { Suma::Fixtures.vendor.create } diff --git a/spec/suma/async/jobs_spec.rb b/spec/suma/async/jobs_spec.rb index 53083b533..1f3f37188 100644 --- a/spec/suma/async/jobs_spec.rb +++ b/spec/suma/async/jobs_spec.rb @@ -76,181 +76,6 @@ end end - describe "EnrollmentRemovalRunner", skip: "eligibility rewrite" do - let(:jobclass) { Suma::Async::EnrollmentRemovalRunner } - let(:member) { Suma::Fixtures.member.create } - - before(:each) do - jobclass.testing_last_ran_removers = [] - end - - context "runs the enrollment remover when" do - specify "a direct program enrollment is unenrolled" do - e = Suma::Fixtures.program_enrollment.create(member:) - expect do - e.update(unenrolled: true) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "an organization program enrollment is unenrolled" do - organization = Suma::Fixtures.organization.create - Suma::Fixtures.organization_membership.verified(organization).create(member:) - Suma::Fixtures.organization_membership.former(organization).create - e = Suma::Fixtures.program_enrollment.create(organization:) - expect do - e.update(unenrolled: true) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "an organization role program enrollment is unenrolled" do - role = Suma::Fixtures.role.create - organization = Suma::Fixtures.organization.create - organization.add_role(role) - Suma::Fixtures.organization_membership.verified(organization).create(member:) - e = Suma::Fixtures.program_enrollment.create(role:) - expect do - e.update(unenrolled: true) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "a member role program enrollment is unenrolled" do - role = Suma::Fixtures.role.create - member.add_role(role) - e = Suma::Fixtures.program_enrollment.create(role:) - expect do - e.update(unenrolled: true) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "a member is removed from an organization" do - organization = Suma::Fixtures.organization.create - m = Suma::Fixtures.organization_membership.verified(organization).create(member:) - Suma::Fixtures.program_enrollment.create(organization:) - expect do - m.remove_from_organization - m.save_changes - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "a role is removed from a member" do - role = Suma::Fixtures.role.create - member.add_role(role) - Suma::Fixtures.program_enrollment.create(role:) - expect do - member.remove_role(role) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - - specify "a role is removed from an organization" do - role = Suma::Fixtures.role.create - organization = Suma::Fixtures.organization.create - organization.add_role(role) - Suma::Fixtures.organization_membership.verified(organization).create(member:) - Suma::Fixtures.program_enrollment.create(role:) - expect do - organization.remove_role(role) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: be_empty, - ), - ) - end - end - - context "noops when" do - specify "an organization membership changes other than to verified" do - organization = Suma::Fixtures.organization.create - m = Suma::Fixtures.organization_membership.unverified.create(member:) - Suma::Fixtures.program_enrollment.create(organization:) - expect do - m.verified_organization = organization - m.save_changes - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to be_empty - end - - specify "a member role is removed, but during processing the member has regained that role" do - role = Suma::Fixtures.role.create - member.add_role(role) - Suma::Fixtures.program_enrollment.create(role:) - expect do - member.publish_immediate("role.removed", member.id, role.id) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: have_length(1), - ), - ) - end - - specify "an organization role is removed, but during processing the organization has regained that role" do - role = Suma::Fixtures.role.create - organization = Suma::Fixtures.organization.create - organization.add_role(role) - Suma::Fixtures.organization_membership.verified(organization).create(member:) - Suma::Fixtures.program_enrollment.create(role:) - expect do - organization.publish_immediate("role.removed", organization.id, role.id) - end.to perform_async_job(jobclass) - expect(jobclass.testing_last_ran_removers).to contain_exactly( - have_attributes( - before_enrollments: have_length(1), - after_enrollments: have_length(1), - ), - ) - end - end - - it "errors if somehow an unhandled event is captured by the regex but unhandled" do - expect(jobclass).to receive(:pattern).and_return("*").at_least(:once) - - expect do - expect do - Suma::Fixtures.legal_entity.create - end.to perform_async_job(jobclass) - end.to raise_error(NotImplementedError, "unhandled event: suma.legalentity.created") - end - end - describe "ForwardMessages" do before(:each) do Suma::Message::Forwarder.reset_configuration @@ -838,21 +663,12 @@ def prepare_stripe_req describe "ServiceRevokerScheduler" do it "runs the revoker" do - expect(Suma::Program::ServiceRevoker).to receive(:run) + sr = Suma::Program::ServiceRevoker.new + expect(Suma::Program::ServiceRevoker).to receive(:new).and_return(sr) Suma::Async::ServiceRevokerScheduler.new.perform(true) end end - describe "ServiceRevokerRunner" do - it "runs the revoker on the originating ledger when a book transaction is created" do - orig = Suma::Fixtures.ledger.create(name: "cash") - expect(Suma::Program::ServiceRevoker).to receive(:run_for).with(orig) - expect do - Suma::Fixtures.book_transaction.from(orig).create - end.to perform_async_job(Suma::Async::ServiceRevokerRunner) - end - end - describe "SignalwireProcessOptouts", reset_configuration: Suma::Signalwire do it "syncs refunds" do import_localized_message_seeds diff --git a/spec/suma/program/enrollment_remover_spec.rb b/spec/suma/program/enrollment_remover_spec.rb deleted file mode 100644 index 7b95c0eb8..000000000 --- a/spec/suma/program/enrollment_remover_spec.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Suma::Program::EnrollmentRemover, :db, :no_transaction_check, skip: "eligibility rewrite" do - let(:member) { Suma::Fixtures.member.create } - let(:instance) { described_class.new(member) } - - before(:each) do - Suma::Program.service_revoker_dry_run = false - end - - it "runs the preprocess block to capture before and after enrollments on the member and rolls back changes" do - p1 = Suma::Fixtures.program.create - p2 = Suma::Fixtures.program.create - p1enroll = Suma::Fixtures.program_enrollment(member:).in(p1).create - instance.reenroll do |m| - # Refer to the same row, but NOT the same object in memory - expect(m).to be === member - expect(m.object_id).to_not eql(member.object_id) - role = Suma::Role.create(name: "enrollmenttestrole") - m.add_role(role) - Suma::Fixtures.program_enrollment(role:).in(p2).create - end - instance.process - # We added the 'removed' enrollment - expect(instance.before_enrollments).to contain_exactly( - have_attributes(program: p1), have_attributes(program: p2), - ) - # We found only the existing enrollment - expect(instance.after_enrollments).to contain_exactly(have_attributes(program: p1)) - # We calculated the removed enrollments correctly - expect(instance.removed_enrollments).to contain_exactly(have_attributes(program: p2)) - # We rolled back any database changes - expect(Suma::Program::Enrollment.all).to have_same_ids_as(p1enroll) - end - - it "excludes not-currently-enrolled enrollments from all calculations" do - e1 = Suma::Fixtures.program_enrollment(member:).create - e2 = Suma::Fixtures.program_enrollment(member:).unenrolled.create - e3 = Suma::Fixtures.program_enrollment(member:).unenrolled.create - e4 = Suma::Fixtures.program_enrollment(member:).unapproved.create - instance.reenroll do - e2.update(unenrolled: false) - end - instance.process - expect(instance.before_enrollments).to have_same_ids_as(e1, e2) - expect(instance.after_enrollments).to have_same_ids_as(e1) - end - - it "errors if in a transaction", no_transaction_check: false do - instance.reenroll {} - expect { instance.process }.to raise_error(Suma::Postgres::InTransaction) - end - - describe "lyft pass" do - before(:each) do - Suma::Lyft.reset_configuration - - Suma::ExternalCredential.create( - service: "lyft-pass-access-token", - expires_at: 5.hours.from_now, - data: {body: {}, cookies: {}}.to_json, - ) - - Suma::Lyft.pass_authorization = "Basic xyz" - Suma::Lyft.pass_email = "a@b.c" - Suma::Lyft.pass_org_id = "1234" - end - - it "revokes lyft pass and destroys registrations for lyft pass program ids the member no longer can access" do - lyft_pass_config = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lyft_pass") - lyft_pass_vendor_acct = Suma::Fixtures.anon_proxy_vendor_account. - create(configuration: lyft_pass_config, member: member) - reg1 = lyft_pass_vendor_acct.add_registration(external_program_id: "111") - reg2 = lyft_pass_vendor_acct.add_registration(external_program_id: "222") - - p1_lp1 = Suma::Fixtures.program.create(lyft_pass_program_id: "111") - p2_lp1 = Suma::Fixtures.program.create(lyft_pass_program_id: "111") - p3_lp2 = Suma::Fixtures.program.create(lyft_pass_program_id: "222") - - p1enroll = Suma::Fixtures.program_enrollment(member:).in(p1_lp1).create - instance.reenroll do - Suma::Fixtures.program_enrollment(member:).in(p2_lp1).create - Suma::Fixtures.program_enrollment(member:).in(p3_lp2).create - end - - req = stub_request(:post, "https://www.lyft.com/api/rideprograms/enrollment/revoke"). - with(body: hash_including("ride_program_id" => "222")).to_return(status: 200) - - instance.process - - expect(req).to have_been_made - expect(reg1).to_not be_destroyed - expect(reg2).to be_destroyed - end - end - - describe "lime" do - it "starts the account close process" do - program = Suma::Fixtures.program.create - vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") - program.add_anon_proxy_vendor_configuration(vc) - contact = Suma::Fixtures.anon_proxy_member_contact.email("a@example.com").create(member:) - lime_va = Suma::Fixtures.anon_proxy_vendor_account(member:, configuration: vc, contact:).create - instance.reenroll do - Suma::Fixtures.program_enrollment(member:, program:).create - end - req = stub_request(:post, "https://web-production.lime.bike/api/rider/v2/onboarding/magic-link"). - to_return(json_response({})) - instance.process - expect(req).to have_been_made - lime_va.refresh - expect(lime_va).to have_attributes(pending_closure: true) - end - end -end diff --git a/spec/suma/program/service_revoker_spec.rb b/spec/suma/program/service_revoker_spec.rb index 1021f1060..a91de27ac 100644 --- a/spec/suma/program/service_revoker_spec.rb +++ b/spec/suma/program/service_revoker_spec.rb @@ -8,7 +8,7 @@ end it "errors if in a transaction", no_transaction_check: false do - expect { described_class.run }.to raise_error(Suma::Postgres::InTransaction) + expect { described_class.new.run }.to raise_error(Suma::Postgres::InTransaction) end def create_cash_ledger @@ -35,20 +35,20 @@ def create_cash_ledger create(amount: money("$500"), apply_at: 2.days.ago) member = ledger_to_revoke.account.member - expect(described_class).to receive(:close_lime_accounts).with(member) - expect(described_class).to receive(:revoke_all_lyft_passes).with(member) - described_class.run + sr = described_class.new + expect(sr).to receive(:close_lime_accounts).with(member) + expect(sr).to receive(:revoke_all_lyft_passes).with(member) + sr.run end it "skips idempotency in dry run" do - Suma::Program.service_revoker_dry_run = true - ledger = create_cash_ledger Suma::Fixtures.book_transaction.from(ledger).create(amount: money("$500"), apply_at: 2.days.ago) member = ledger.account.member - expect(described_class).to receive(:close_lime_accounts).with(member) - expect(described_class).to receive(:revoke_all_lyft_passes).with(member) - described_class.run_for(ledger) + sr = described_class.new(dry_run: true) + expect(sr).to receive(:close_lime_accounts).with(member) + expect(sr).to receive(:revoke_all_lyft_passes).with(member) + sr.run_for(ledger) expect(Suma::Idempotency.all).to be_empty end @@ -78,7 +78,7 @@ def create_cash_ledger req2 = stub_request(:post, "https://www.lyft.com/api/rideprograms/enrollment/revoke"). with(body: hash_including("ride_program_id" => "222")).to_return(status: 200) - described_class.revoke_all_lyft_passes(member) + described_class.new.revoke_all_lyft_passes(member) expect(req1).to have_been_made expect(req2).to have_been_made @@ -87,14 +87,12 @@ def create_cash_ledger end it "logs and skips idempotency with dry run" do - Suma::Program.service_revoker_dry_run = true - lyft_pass_config = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lyft_pass") lyft_pass_vendor_acct = Suma::Fixtures.anon_proxy_vendor_account.create(configuration: lyft_pass_config, member:) reg1 = lyft_pass_vendor_acct.add_registration(external_program_id: "111") logs = capture_logs_from(described_class.logger) do - described_class.revoke_all_lyft_passes(member) + described_class.new(dry_run: true).revoke_all_lyft_passes(member) end expect(logs).to have_a_line_matching(/service_revoker_dry_run/) expect(reg1).to_not be_destroyed @@ -115,7 +113,7 @@ def create_cash_ledger "user_identifier" => {"phone_number" => "+15553334444"}, )).to_return(status: 200) - described_class.revoke_all_lyft_passes(member) + described_class.new.revoke_all_lyft_passes(member) expect(req).to have_been_made expect(reg).to be_destroyed @@ -128,7 +126,7 @@ def create_cash_ledger create(configuration: lyft_pass_config, member:) reg = lyft_pass_vendor_acct.add_registration(external_program_id: "111") - described_class.revoke_all_lyft_passes(member) + described_class.new.revoke_all_lyft_passes(member) expect(reg).to be_destroyed end @@ -139,32 +137,31 @@ def create_cash_ledger it "noops if there are no accounts with contacts in any lime programs" do vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") Suma::Fixtures.anon_proxy_vendor_account(member:, configuration: vc).create - expect { described_class.close_lime_accounts(member) }.to_not raise_error + expect { described_class.new.close_lime_accounts(member) }.to_not raise_error end it "starts the account close process" do vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") contact = Suma::Fixtures.anon_proxy_member_contact.email.create(member:) lime_va = Suma::Fixtures.anon_proxy_vendor_account(member:, configuration: vc, contact:).create + lime_va.replace_access_code("x", "https://link").save_changes other_va = Suma::Fixtures.anon_proxy_vendor_account(member:).create req = stub_request(:post, "https://web-production.lime.bike/api/rider/v2/onboarding/magic-link"). to_return(json_response({})) - described_class.close_lime_accounts(member) + described_class.new.close_lime_accounts(member) expect(req).to have_been_made lime_va.refresh - expect(lime_va).to have_attributes(pending_closure: true) + expect(lime_va).to have_attributes(pending_closure: true, latest_access_code: nil) expect(other_va.refresh).to have_attributes(pending_closure: false) end it "logs and skips idempotency with dry run" do - Suma::Program.service_revoker_dry_run = true - vc = Suma::Fixtures.anon_proxy_vendor_configuration.create(auth_to_vendor_key: "lime") contact = Suma::Fixtures.anon_proxy_member_contact.email.create(member:) lime_va = Suma::Fixtures.anon_proxy_vendor_account(member:, configuration: vc, contact:).create logs = capture_logs_from(described_class.logger) do - described_class.close_lime_accounts(member) + described_class.new(dry_run: true).close_lime_accounts(member) end expect(logs).to have_a_line_matching(/service_revoker_dry_run/) expect(lime_va.refresh).to have_attributes(pending_closure: false) diff --git a/spec/suma/simple_registry_spec.rb b/spec/suma/simple_registry_spec.rb index 5f0326076..cc8c81f02 100644 --- a/spec/suma/simple_registry_spec.rb +++ b/spec/suma/simple_registry_spec.rb @@ -28,6 +28,19 @@ def initialize(x, y:) expect { base_cls.registry_lookup!(" ") }.to raise_error(described_class::Unregistered, /key cannot be blank/) end + it "can register a class with a 'key' method" do + cls2 = Class.new do + extend Suma::SimpleRegistry + + def self.key = "xyz" + end + + base_cls.register(cls2) + expect(base_cls.registry_lookup!(:xyz)).to eq(cls2) + expect(base_cls.registry_create!(:xyz)).to be_a(cls2) + expect { base_cls.register(5) }.to raise_error(ArgumentError, /if value is not provided/) + end + it "initializes classes with registered arguments" do base_cls.register(:sub, subclsargs, "xval", y: "yval") expect(base_cls.registry_lookup!(:sub)).to eq(subclsargs)