diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..b7a02f3 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,74 @@ +# AGENTS.md + +Akkounts is a Rails 8 monolith for managing Kosmos/LDAP user accounts. +It authenticates against an LDAP directory via Devise and integrates with +ejabberd, Discourse, Mastodon, remoteStorage, Nostr, LNDHub, and BTCPay. + +## Development environment + +Development runs in Docker Compose — run all commands against the `web` container. +Start services: `docker compose up` (web, ldap, redis, minio, liquor-cabinet, strfry). +The `web` service runs `bin/dev` (foreman: Puma + Tailwind CSS watcher) and embeds +Solid Queue workers (`SOLID_QUEUE_IN_PUMA=true`). + +First-time LDAP setup (after creating the 389ds backend once): +`docker compose exec ldap dsconf localhost backend create --suffix="dc=kosmos,dc=org" --be_name="dev"` +then `docker compose run web bin/rails ldap:setup`. + +## Common commands (prefix with `docker compose exec web`) + +- `bin/rspec` — run the test suite (CI runs `bundle exec rspec`) +- `bin/rails db:prepare` — create/migrate DB (also `db:setup`, `db:migrate`) +- `bin/rails ldap:setup` — reset LDAP dir and seed dev entries +- `bin/rails css:build` — build Tailwind CSS (auto-watched in dev) +- `bin/rails lndhub:generate_wallets`, `invitations:generate_for_all_users[N]` +- `bin/jobs` — Solid Queue CLI + +Tests use the test env explicitly: `docker compose exec -e RAILS_ENV=test web bin/rspec`. +There is no enforced linter — a `bin/rubocop` binstub exists but no `.rubocop.yml` +(rubocop is a transitive gem, not in the Gemfile); CI does not lint. + +## Stack & conventions + +- **Assets/views:** ERB + ViewComponent (`app/components/`), Tailwind built via Bun + + importmap (`config/importmap.rb`); pin JS deps with `bin/importmap pin --download`. +- **Auth:** Devise + `devise_ldap_authenticatable`; `authentication_keys = [:cn]`, + custom SSHA512 password builder, admin bind. Admin-only mounts (`/jobs` MissionControl, + `/flipper` Flipper UI) are gated by `authenticate :user, ->(u) { u.is_admin? }`. +- **Services:** `app/services/` uses `ApplicationService.call(**args)` with namespaced + `*ManagerService` bases (e.g. `UserManagerService`) and verb-named operations + (`UserManager::CreateAccount`, `NostrManager::PublishEvent`, `BtcpayManager::*`). +- **Models:** `User` is central; `Setting` uses rails-settings-cached with one concern per + service in `app/models/concerns/settings/`. LNDHub models connect to a separate DB + (`LndhubBase` `establish_connection :lndhub`). LDAP is accessed via `LdapService`/ + `LdapManagerService`, not ActiveRecord. +- **Jobs:** `app/jobs/` inherit `ApplicationJob < ActiveJob::Base`; backend is **Solid Queue** + (`config.active_job.queue_adapter = :solid_queue`, separate `queue` DB in prod). + `config/recurring.yml` has no active scheduled jobs. Ignore `config/sidekiq.yml` — it's + stale (sidekiq is not in the Gemfile; the README mention is outdated). +- **Config:** env-var driven (`.env.example`/`.env.development`/`.env.test`); `SERVICES` + constant is loaded from `config/services.yml` in `config/initializers/service_details.rb`. + Custom LDAP schema attributes are applied from `schemas/ldap/*.ldif` by `lib/tasks/ldap.rake`. + +## Testing conventions (`spec/`) + +`spec/rails_helper.rb` includes FactoryBot, Devise `ControllerHelpers` (controller specs), +Warden `Test::Helpers`, DatabaseCleaner (transactional fixtures **off**), ViewComponent +`TestHelpers` + Capybara matchers, and `ActiveJob::TestHelper` for `type: :job`. + +- **Feature specs** (`spec/features/`, `type: :feature`): Capybara **rack-test** driver (no JS). + Log in with Warden: `login_as user, scope: :user`; stub admin with + `allow(Devise::LDAP::Adapter).to receive(:get_ldap_param).with(user.cn, :admin).and_return(["true"])`. +- **Services** (`spec/services/`) are tagged `type: :model`; exercise `#call`/`service.send(:private)` + and assert on DB rows / `enqueued_jobs`. +- **Request/component/mailer/job/model/helper** specs in matching `spec/` subdirs. + Fixtures live in `spec/fixtures/`; factories in `spec/factories/`. +- `spec_helper.rb` reloads routes before each `type: :controller` spec (Devise + Rails 8 workaround). + +## Notes & gotchas + +- Generators (`config/application.rb`): ERB templates, RSpec, no stylesheets, factory_bot + with `_factory` suffix dir `spec/factories`. +- `gitno/` and `extras/strfry/` are scratch/infra (Deno Nostr relay policies) — not app code; ignore. +- Don't add code comments unless asked (matches repo style); `frozen_string_literal` is inconsistent. +- Default dev login: username `admin` / password `admin is admin`. diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index e6504e4..2c0fade 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -12,7 +12,11 @@ class Admin::UsersController < Admin::BaseController @contributors = ldap.search_users(:memberStatus, :contributor, :cn) if @show_contributors @sustainers = ldap.search_users(:memberStatus, :sustainer, :cn) if @show_sustainers @admins = ldap.search_users(:admin, true, :cn) - @pagy, @users = pagy(User.where(ou: ou).order(cn: :asc)) + + @username = params[:username].presence + users_scope = User.where(ou: ou).order(cn: :asc) + users_scope = users_scope.where("cn LIKE ?", "%#{User.sanitize_sql_like(@username.downcase)}%") if @username + @pagy, @users = pagy(users_scope) @stats = { users_confirmed: User.where(ou: ou).confirmed.count, diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4eb4820..d2b0bc4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -39,7 +39,7 @@ class ApplicationController < ActionController::Base end def after_sign_in_path_for(user) - session[:user_return_to] || root_path + session.delete(:user_return_to) || root_path end def lndhub_authenticate(options={}) diff --git a/app/controllers/devise/passwords_controller.rb b/app/controllers/devise/passwords_controller.rb index ab99fc8..370645e 100644 --- a/app/controllers/devise/passwords_controller.rb +++ b/app/controllers/devise/passwords_controller.rb @@ -55,7 +55,12 @@ class Devise::PasswordsController < DeviseController protected def after_resetting_password_path_for(resource) - Devise.sign_in_after_reset_password ? after_sign_in_path_for(resource) : new_session_path(resource_name) + session.delete(:user_return_to) + if Devise.sign_in_after_reset_password + root_path + else + new_session_path(resource_name) + end end # The path used after sending reset password instructions diff --git a/app/controllers/rs/oauth_controller.rb b/app/controllers/rs/oauth_controller.rb index 2e2933f..360d50c 100644 --- a/app/controllers/rs/oauth_controller.rb +++ b/app/controllers/rs/oauth_controller.rb @@ -1,4 +1,5 @@ class Rs::OauthController < ApplicationController + prepend_before_action :assert_redirect_uri, only: [:new, :create] before_action :require_signed_in_with_username, only: :new before_action :authenticate_user!, only: :create @@ -16,8 +17,6 @@ class Rs::OauthController < ApplicationController ["In 1 month", 1.month.from_now], ["In 1 day", 1.day.from_now]] - http_status :bad_request and return unless @redirect_uri.present? - unless current_user == @user sign_out :user @@ -64,8 +63,6 @@ class Rs::OauthController < ApplicationController state = params[:state].presence expire_at = params[:expire_at].presence - http_status :bad_request and return unless redirect_uri.present? - if permissions.empty? redirect_to(url_with_state("#{redirect_uri}#error=invalid_scope", state), allow_other_host: true) and return @@ -97,6 +94,10 @@ class Rs::OauthController < ApplicationController private + def assert_redirect_uri + http_status :bad_request unless params[:redirect_uri].present? + end + def require_signed_in_with_username unless user_signed_in? session[:user_return_to] = request.url diff --git a/app/views/admin/users/index.html.erb b/app/views/admin/users/index.html.erb index 413ec2b..f965059 100644 --- a/app/views/admin/users/index.html.erb +++ b/app/views/admin/users/index.html.erb @@ -30,6 +30,11 @@ <% end %> +
+ <%= render partial: "admin/username_search_form", + locals: { path: admin_users_path } %> +
+
diff --git a/spec/controllers/rs/oauth_controller_spec.rb b/spec/controllers/rs/oauth_controller_spec.rb index 03f4750..78ea3d0 100644 --- a/spec/controllers/rs/oauth_controller_spec.rb +++ b/spec/controllers/rs/oauth_controller_spec.rb @@ -222,6 +222,19 @@ RSpec.describe Rs::OauthController, type: :controller do expect(response).to redirect_to(new_user_session_path(cn: user.cn, ou: user.ou)) end + + context "without a redirect_uri" do + it "returns a 400 without storing the return location" do + get :new, params: { + username: user.cn, + scope: "documents,photos", + client_id: "https://example.com" + } + + expect(response.response_code).to eq(400) + expect(session[:user_return_to]).to be_nil + end + end end describe "root access" do diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index c804823..62d6a2b 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -15,6 +15,8 @@ RSpec.describe "Admin: User management", type: :feature do .and_return({ uid: user.cn, mail: user.email, display_name: "Freddy" }) allow_any_instance_of(LdapManager::FetchAvatar).to receive(:call) .and_return(nil) + allow_any_instance_of(LdapService).to receive(:search_users) + .and_return([]) login_as admin, :scope => :user end @@ -43,6 +45,30 @@ RSpec.describe "Admin: User management", type: :feature do expect(user.invitations.count).to eq(5) end + describe "User index page" do + it "lists all users" do + visit admin_users_path + + expect(page).to have_link "jimmy" + expect(page).to have_link "alfred" + end + + scenario 'Filter users by username' do + visit admin_users_path + + fill_in "username", with: "alf" + click_button "Filter" + + expect(page).to have_link "alfred" + expect(page).to have_no_link "jimmy" + + click_link "Remove filter" + + expect(page).to have_link "jimmy" + expect(page).to have_link "alfred" + end + end + scenario 'Remove invitations from account' do 3.times { Invitation.create(user: user) } expect(user.invitations.count).to eq(3) diff --git a/spec/features/devise/password_reset.rb b/spec/features/devise/password_reset.rb index c12396c..874f39a 100644 --- a/spec/features/devise/password_reset.rb +++ b/spec/features/devise/password_reset.rb @@ -50,5 +50,26 @@ RSpec.describe 'Password reset', type: :feature do expect(page).to have_content 'Your password has been changed successfully' expect(user.reload.reset_password_token).to be_nil end + + scenario "Ignores a stale return location left by an rs/oauth request" do + expect(Devise::LDAP::Adapter).to receive(:update_password) + .with(user.cn, 'catch me if you can').and_return(true) + + # Simulate an earlier rs/oauth authorization request that stored a + # return URL in the session (the original cause of issue #235). + visit new_rs_oauth_path(user.cn, + redirect_uri: "https://example.com", + client_id: "https://example.com", + scope: "documents") + + visit edit_user_password_path(reset_password_token: token) + fill_in :user_password, with: 'catch me if you can' + fill_in :user_password_confirmation, with: 'catch me if you can' + click_button 'Change my password' + + expect(page).to have_content 'Your password has been changed successfully' + expect(page).to have_current_path(root_path) + expect(page).not_to have_content 'Bad request' + end end end diff --git a/spec/requests/devise/passwords_spec.rb b/spec/requests/devise/passwords_spec.rb new file mode 100644 index 0000000..bdfae93 --- /dev/null +++ b/spec/requests/devise/passwords_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe "Devise password reset", type: :request do + let(:user) { create :user } + + describe "PUT /users/password" do + let(:token) { user.send(:set_reset_password_token) } + + before do + allow(Devise::LDAP::Adapter).to receive(:update_password).and_return(true) + end + + context "with a stale stored return location from an rs/oauth request" do + before do + get new_rs_oauth_url(user.cn, + redirect_uri: "https://example.com", + client_id: "https://example.com", + scope: "documents") + expect(session[:user_return_to]).to be_present + end + + it "redirects to the dashboard instead of the stored return URL" do + put user_password_path, params: { + user: { + reset_password_token: token, + password: "a brand new password", + password_confirmation: "a brand new password" + } + } + + expect(response).to redirect_to(root_path) + expect(session[:user_return_to]).to be_nil + end + end + end +end