Skip to content

Support both username and User object for PersonalProjectFinder.new method

What does this MR do and why?

This solves an issue:

The user argument of PersonalProjectFinder is in some cases a String object and not a User. An can? method call failed because of this.

This MR will fix that by requesting a User object if the user is a string. Using Kibana, I found that the user is actually a username.

On master, a url like /groups/flightjs/-/children.json?user=someuser will result in Error 500. On this branch, it will lookup the user argument and no error is raised

This fixes gitlab-com/gl-infra/production#18362 (closed) We have around 200 errors per week due to this issue. Internal Kibana

Database review

This MR requires a database review because of the change in PersonalProjectFinder: if the parameter user is a string, we lookup the user.

  • If the parameter is a User object (almost all requests), we do nothing, no additional queries are. executed
  • If the parameter is a String object (~150-200 requests per week), we find the user using User.find_by_username
The sql that is being executed when calling `User.find_by_username('username'): (Query Plan)
SELECT "users"."id",
       "users"."email",
       "users"."encrypted_password",
       "users"."reset_password_token",
       "users"."reset_password_sent_at",
       "users"."remember_created_at",
       "users"."sign_in_count",
       "users"."current_sign_in_at",
       "users"."last_sign_in_at",
       "users"."current_sign_in_ip",
       "users"."last_sign_in_ip",
       "users"."created_at",
       "users"."updated_at",
       "users"."name",
       "users"."admin",
       "users"."projects_limit",
       "users"."failed_attempts",
       "users"."locked_at",
       "users"."username",
       "users"."can_create_group",
       "users"."can_create_team",
       "users"."state",
       "users"."color_scheme_id",
       "users"."password_expires_at",
       "users"."created_by_id",
       "users"."last_credential_check_at",
       "users"."avatar",
       "users"."confirmation_token",
       "users"."confirmed_at",
       "users"."confirmation_sent_at",
       "users"."unconfirmed_email",
       "users"."hide_no_ssh_key",
       "users"."admin_email_unsubscribed_at",
       "users"."notification_email",
       "users"."hide_no_password",
       "users"."password_automatically_set",
       "users"."encrypted_otp_secret",
       "users"."encrypted_otp_secret_iv",
       "users"."encrypted_otp_secret_salt",
       "users"."otp_required_for_login",
       "users"."otp_backup_codes",
       "users"."public_email",
       "users"."dashboard",
       "users"."project_view",
       "users"."consumed_timestep",
       "users"."layout",
       "users"."hide_project_limit",
       "users"."note",
       "users"."unlock_token",
       "users"."otp_grace_period_started_at",
       "users"."external",
       "users"."incoming_email_token",
       "users"."auditor",
       "users"."require_two_factor_authentication_from_group",
       "users"."two_factor_grace_period",
       "users"."last_activity_on",
       "users"."notified_of_own_activity",
       "users"."preferred_language",
       "users"."theme_id",
       "users"."accepted_term_id",
       "users"."feed_token",
       "users"."private_profile",
       "users"."roadmap_layout",
       "users"."include_private_contributions",
       "users"."commit_email",
       "users"."group_view",
       "users"."managing_group_id",
       "users"."first_name",
       "users"."last_name",
       "users"."static_object_token",
       "users"."role",
       "users"."user_type",
       "users"."static_object_token_encrypted",
       "users"."otp_secret_expires_at",
       "users"."onboarding_in_progress",
       "users"."color_mode_id",
       "users"."composite_identity_enforced"
FROM   "users"
WHERE  ( Lower("users"."username") IN ( Lower('username') ) )
LIMIT  1 

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

Using GDK, call curl <your gdk url>/groups/flightjs/-/children.json?user=someuser. On master it will fail. On this branch, it should pass

Edited by Rutger Wessels

Merge request reports

Loading