Make sure authentication/authorization actions are called before other action

While profiling Projects::MergeRequestsController#new we noticed that even for an unauthorized user (which returns a plain 404) filters like set_suggested_approvers are executed. This is because before_actions are executed in the order they're declared, and in the source code we have:

  # ...

  before_action :build_merge_request, only: [:new, :new_diffs]
  before_action :set_suggested_approvers, only: [:new, :new_diffs, :edit]

  # Allow read any merge_request
  before_action :authorize_read_merge_request!

  # Allow write(create) merge_request
  before_action :authorize_create_merge_request!, only: [:new, :create]

  # Allow modify merge_request
  before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :remove_wip, :sort]

  before_action :authenticate_user!, only: [:assign_related_issues]

  before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts]

  # ...

We can save some DB/git actions by making sure our authorize_*, authenticate_* are executed first. This can probably be checked/updated in one MR for all controllers.

/cc @stanhu

Edited Aug 14, 2020 by 🤖 GitLab Bot 🤖
Assignee Loading
Time tracking Loading