Skip to content

Fix OAuth bug related to openid_connect

Drew Blessing requested to merge dblessing_fix_oauth_provider_startup_error into master

What does this MR do and why?

Related to #424800 (closed)

The bug was introduced by !95308 (merged) but was only uncovered once !95308 (merged) was merged recently because that MR added a boot-time call to the buggy method. Previously the bug would have only been uncovered in very specific runtime circumstances (using openid_connect and attempting to load configuration for an otherwise unconfigured provider).

This change ensures the method fails safely and also updates tests to ensure the issue is caught in the future. I fixed this ASAP because it caused me issues on GDK upgrade and I imagine it will affect others, too.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. Configure openid_connect in gitlab.yml. Be sure not to configure a name under args:
    omniauth: 
      providers:
      - { name: 'openid_connect' }
  2. Start GDK ensuring the latest updates from master to include !95308 (merged)
  3. Observe the following error on boot:
    GitlabSettings::MissingSetting: option 'name' not defined
    /Users/dblessing/development/gdk/gitlab/lib/gitlab_settings/options.rb:159:in `method_missing'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/auth/o_auth/provider.rb:72:in `block in config_for'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/auth/o_auth/provider.rb:71:in `each'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/auth/o_auth/provider.rb:71:in `find'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/auth/o_auth/provider.rb:71:in `config_for'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/github_import/markdown_text.rb:36:in `github_url'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/github_import/attachments_downloader.rb:15:in `<class:AttachmentsDownloader>'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/github_import/attachments_downloader.rb:5:in `<module:GithubImport>'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/github_import/attachments_downloader.rb:4:in `<module:Gitlab>'
    /Users/dblessing/development/gdk/gitlab/lib/gitlab/github_import/attachments_downloader.rb:3:in `<main>'
    /Users/dblessing/development/gdk/gitlab/lib/tasks/gitlab/db.rake:505:in `block (4 levels) in <main>'
    /Users/dblessing/development/gdk/gitlab/lib/tasks/gitlab/db.rake:586:in `block (4 levels) in <main>'

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Drew Blessing

Merge request reports

Loading