diff --git a/db/migrations/011_users_choose_username.rb b/db/migrations/011_users_choose_username.rb new file mode 100644 index 0000000..d9c98ee --- /dev/null +++ b/db/migrations/011_users_choose_username.rb @@ -0,0 +1,7 @@ +Sequel.migration do + change do + # Created as "invite-"; the invitee picks the real username when + # accepting the invite. + add_column :users, :choose_username, TrueClass, null: false, default: false + end +end diff --git a/lib/users_admin.rb b/lib/users_admin.rb index 10025fc..dac0423 100644 --- a/lib/users_admin.rb +++ b/lib/users_admin.rb @@ -84,6 +84,10 @@ module UsersAdmin app.post '/admin/users' do @user = User.new(username: params[:username].to_s.strip.downcase, name: params[:name].to_s.strip, email: params[:email].to_s.strip.downcase.then { |e| e.empty? ? nil : e }) + if @user.username.empty? # invitee chooses it on the invite page + @user.username = "invite-#{SecureRandom.hex(4)}" + @user.choose_username = true + end @user.superuser = params[:superuser] == '1' if current_user.superuser unless @user.valid? && !User[username: @user.username] && !(@user.email && User[email: @user.email]) @error = @user.errors.full_messages.join('; ').then { |m| m.empty? ? 'Username or email already in use.' : m } @@ -188,6 +192,19 @@ module UsersAdmin return erb :'admin/set_password' end user = @token.user + if user.choose_username + name = params[:username].to_s.strip.downcase + @error = + if !name.match?(User::NAME_RE) || name.start_with?('invite-') then 'Username: lowercase letters, digits, . _ @ - (2–64).' + elsif User.exclude(id: user.id).where(username: name).any? then 'That username is taken.' + end + if @error + status 422 + return erb :'admin/set_password' + end + user.username = name + user.choose_username = false + end user.password = params[:password] user.must_change_password = false user.session_version += 1 diff --git a/test/users_admin_test.rb b/test/users_admin_test.rb index 28ca727..a5bd672 100644 --- a/test/users_admin_test.rb +++ b/test/users_admin_test.rb @@ -47,6 +47,31 @@ class UsersAdminTest < Minitest::Test refute_includes last_response.body, '/admin/invite/' # shown once end + def test_invite_without_username_lets_invitee_choose + admin_login! + create_user(username: '', perm: { 'bingo' => { 'read' => '1' } }) + u = User.where(choose_username: true).first + assert_match(/\Ainvite-[0-9a-f]{8}\z/, u.username) + get '/admin/users' + link = last_response.body[%r{https?://\S+?/admin/invite/[\w-]+}] + assert link, 'invite link shown' + path = URI(link).path + clear_cookies + get path + assert_includes last_response.body, 'Choose a username' + tok = last_response.body[/name="authenticity_token" value="([^"]+)"/, 1] + post path, { authenticity_token: tok, username: ENV['ADMIN_USER'], password: 'kate-password-12', confirm: 'kate-password-12' } + assert_equal 422, last_response.status # taken + post path, { authenticity_token: tok, username: 'invite-x', password: 'kate-password-12', confirm: 'kate-password-12' } + assert_equal 422, last_response.status # reserved prefix + post path, { authenticity_token: tok, username: 'Kate', password: 'kate-password-12', confirm: 'kate-password-12' } + assert_equal 302, last_response.status + u.refresh + assert_equal 'kate', u.username + refute u.choose_username + assert u.can?('bingo', 'read') + end + def test_delete_implies_read admin_login! create_user(username: 'dave', perm: { 'bingo' => { 'delete' => '1' } }) diff --git a/views/admin/set_password.erb b/views/admin/set_password.erb index 870b6e4..96dadc8 100644 --- a/views/admin/set_password.erb +++ b/views/admin/set_password.erb @@ -1,13 +1,17 @@

<%= @purpose == 'invite' ? 'Welcome' : 'Reset password' %>

-

Set a password for <%= @token.user.username %>

+

<% if @token.user.choose_username %>Choose a username and password<% else %>Set a password for <%= @token.user.username %><% end %>

<% if @error %>
<%= @error %>
<% end %>
- + <% if @token.user.choose_username %> +
+ <% else %> + + <% end %>
diff --git a/views/admin/users/form.erb b/views/admin/users/form.erb index fd6a7e8..03d9ddd 100644 --- a/views/admin/users/form.erb +++ b/views/admin/users/form.erb @@ -14,7 +14,7 @@
>
-
/>
+
/>
diff --git a/views/admin/users/index.erb b/views/admin/users/index.erb index 2ea9f67..55443e4 100644 --- a/views/admin/users/index.erb +++ b/views/admin/users/index.erb @@ -25,7 +25,7 @@ <%= perm_summary(u) %> <% if !u.active %>disabled - <% elsif !u.password_digest %>invited + <% elsif !u.password_digest %>invited<% if u.choose_username %>
picks own username<% end %> <% else %>active<% end %> <%= u.last_login_at&.strftime('%Y-%m-%d %H:%M') || '—' %>