Fix gitlab_username_claim support for OmniAuth providers
What does this MR do and why?
Fix gitlab_username_claim support for OmniAuth providers
Prior to this fix gitlab_username_claim was not always working correctly. When loading the provider config, the auth hash class used an ivar which may not yet be initialized and would cause false negative config lookup. This fix replaces ivar usage with the method that will correctly get the provider name and allow config to be found.
Tests didn't catch this problem because we were stubbing the provider config loading mechanism. This change also updates tests to instead stub config at a lower level, ensuring config loading works as expected.
This bug was discovered when troubleshooting a customer issue with gitlab_username_claim
configuration for an OmniAuth identity provider.
Given an IdP configuration such as the following:
- {
name: "saml",
args: {
assertion_consumer_service_url: "https://gdk.test:3443/users/auth/saml/callback",
idp_cert_fingerprint: "BB:9B:AA:66:50:91:AA:FC:AC:BE:D8:2F:BB:D6:F9:E5:84:77:0D:B7",
idp_sso_target_url: "https://foo.example.com",
issuer: "https://gdk.test:3443",
name_identifier_format: "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
attribute_statements: { nickname: ['firstname'] },
gitlab_username_claim: 'uid'
}
And generating an OmniAuth Auth Hash as follows:
omni_authhash = OmniAuth::AuthHash.new(credentials: OmniAuth::AuthHash.new, extra: OmniAuth::AuthHash.new(raw_info: OmniAuth::AuthHash.new), info: OmniAuth::AuthHash::InfoHash.new(email: 'DREW@EXAMPLE.COM', name: 'Drew Blessing'), provider: 'saml', uid: 'dblessing')
Current Behavior
pry(main)> auth_hash = Gitlab::Auth::OAuth::AuthHash.new(omni_authhash)
=> #<Gitlab::Auth::OAuth::AuthHash:0x000000014010e938 @auth_hash={"credentials"=>{}, "extra"=>{"raw_info"=>{}}, "info"=>{"email"=>"DREW@EXAMPLE.COM", "name"=>"Drew Blessing"}, "provider"=>"saml", "uid"=>"dblessing"}>
pry(main)> auth_hash.username
=> "DREW"
Expected Behavior
pry(main)> auth_hash = Gitlab::Auth::OAuth::AuthHash.new(omni_authhash)
=> #<Gitlab::Auth::OAuth::AuthHash:0x000000014010e938 @auth_hash={"credentials"=>{}, "extra"=>{"raw_info"=>{}}, "info"=>{"email"=>"DREW@EXAMPLE.COM", "name"=>"Drew Blessing"}, "provider"=>"saml", "uid"=>"dblessing"}>
pry(main)> auth_hash.username
=> "dblessing"
Cause
Username claim relies on looking up the provider config, which is done by calling AuthHash#provider_config
which has the code Gitlab::Auth::OAuth::Provider.config_for(@provider) || {}
. Unless something previously called the AuthHash#provider
method, @provider
ivar is not initialized.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #428058 (closed)