Skip to content
Snippets Groups Projects

Free up Usernames after deny/decline

Merged Marie requested to merge feat/declineandfreeuser into develop

What does this PR do?

Adds a new mod log type for decline, new Endpoint for declining which frees up the username from the used username table followed by it deleting the user.

This is essentially just a copied approve-user endpoint with the email text changed out and instead of marking the user approved it deletes the user and also frees up the username

Closes #752 (closed), #759 (closed)

Contribution Guidelines By submitting this merge request, you agree to follow our Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines
  • I have made sure to test this pull request
Edited by Marie

Merge request reports

Merge request pipeline #1558 passed

Merge request pipeline passed for ed064b21

Merged by MarieMarie 4 months ago (Oct 18, 2024 6:13pm UTC)

Loading

Pipeline #1561 passed

Pipeline passed for d4ef030f on develop

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Hazelnoot
  • Marie added 1 commit

    added 1 commit

    Compare with previous version

  • Marie resolved all threads

    resolved all threads

  • Marie requested review from @fEmber

    requested review from @fEmber

  • added enhancement label

  • Hazelnoot approved this merge request

    approved this merge request

  • Marie mentioned in issue #759 (closed)

    mentioned in issue #759 (closed)

  • Marie added 1 commit

    added 1 commit

    • ed064b21 - upd: remove type username to confirm dialog

    Compare with previous version

  • Marie reset approvals from @fEmber by pushing to the branch

    reset approvals from @fEmber by pushing to the branch

  • Marie changed the description

    changed the description

  • Hazelnoot approved this merge request

    approved this merge request

  • Marie enabled an automatic merge when all merge checks for ed064b21 pass

    enabled an automatic merge when all merge checks for ed064b21 pass

    • Resolved by dakkar

      I am still uneasy about this.

      If user foo signs up, then is rejected, then another user foo signs up and is accepted, we'll have two rows in the user table with the same usernameLower and null host.

      I suspect that this will confuse every user lookup: I can't find any place that passes isDeleted:false to the various query methods on UserRepository, so things like fetching a user's profile, or even logging in, may randomly pick up the deleted row instead of the "real" one.

      I would feel much better if, in addition to setting isDeleted=true, we mangled the username (my proposal was something like ${username}\x01deleted\x01${Date.now()}), using characters that are not normally allowed in usernames (but not the dot) so there's no chance of collisions.

      Also: DeleteAccountService will send out a bunch of AP activities that are completely useless, since no remote instance could have known about a non-approved account. Should be sufficient to do:

      await this.usersRepository.update(user.id, {
        isDeleted: true,
        username: mangledName,
        usernameLower: mangledName.toLowerCase(),
      });
      
      this.globalEventService.publishInternalEvent('userChangeDeletedState', { id: user.id, isDeleted: true });
  • dakkar resolved all threads

    resolved all threads

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading