Merge branch '44332-openid-scope-error' into 'master'

Fix #44332 - Add support for profile and email

Closes #44332

See merge request gitlab-org/gitlab-ce!24335

changelogs/unreleased/44332-add-openid-profile-scopes.yml

+---
+title: GitLab now supports the profile and email scopes from OpenID Connect
+merge_request: 24335
+author: Goten Xiao
+type: added

config/initializers/doorkeeper_openid_connect.rb

@@ -31,8 +31,27 @@ Doorkeeper::OpenidConnect.configure do
 
       o.claim(:name)           { |user| user.name }
       o.claim(:nickname)       { |user| user.username }
-      o.claim(:email)          { |user| user.public_email }
-      o.claim(:email_verified) { |user| true if user.public_email? }
+
+      # Check whether the application has access to the email scope, and grant
+      # access to the user's primary email address if so, otherwise their
+      # public email address (if present)
+      # This allows existing solutions built for GitLab's old behavior to keep
+      # working without modification.
+      o.claim(:email) do |user, scopes|
+        scopes.exists?(:email) ? user.email : user.public_email
+      end
+      o.claim(:email_verified) do |user, scopes|
+        if scopes.exists?(:email)
+          user.primary_email_verified?
+        elsif user.public_email?
+          user.verified_email?(user.public_email)
+        else
+          # If there is no public email set, tell doorkicker-openid-connect to
+          # exclude the email_verified claim by returning nil.
+          nil
+        end
+      end
+
       o.claim(:website)        { |user| user.full_website_url if user.website_url? }
       o.claim(:profile)        { |user| Gitlab::Routing.url_helpers.user_url user }
       o.claim(:picture)        { |user| user.avatar_url(only_path: false) }

config/locales/doorkeeper.en.yml

@@ -64,6 +64,8 @@ en:
       read_registry: Grants permission to read container registry images
       openid: Authenticate using OpenID Connect
       sudo: Perform API actions as any user in the system
+      profile: Allows read-only access to the user's personal information using OpenID Connect
+      email: Allows read-only access to the user's primary email address using OpenID Connect
     scope_desc:
       api:
         Grants complete read/write access to the API, including all groups and projects.
@@ -77,6 +79,10 @@ en:
         Grants permission to authenticate with GitLab using OpenID Connect. Also gives read-only access to the user's profile and group memberships.
       sudo:
         Grants permission to perform API actions as any user in the system, when authenticated as an admin user.
+      profile:
+        Grants read-only access to the user's profile data using OpenID Connect.
+      email:
+        Grants read-only access to the user's primary email address using OpenID Connect.
     flash:
       applications:
         create:

lib/gitlab/auth.rb

@@ -12,6 +12,9 @@ module Gitlab
     # Scopes used for OpenID Connect
     OPENID_SCOPES = [:openid].freeze
 
+    # OpenID Connect profile scopes
+    PROFILE_SCOPES = [:profile, :email].freeze
+
     # Default scopes for OAuth applications that don't define their own
     DEFAULT_SCOPES = [:api].freeze
 
@@ -284,7 +287,7 @@ module Gitlab
 
       # Other available scopes
       def optional_scopes
-        available_scopes + OPENID_SCOPES - DEFAULT_SCOPES
+        available_scopes + OPENID_SCOPES + PROFILE_SCOPES - DEFAULT_SCOPES
       end
 
       def registry_scopes

spec/lib/gitlab/auth_spec.rb

@@ -19,7 +19,7 @@ describe Gitlab::Auth do
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid]
+      expect(subject.optional_scopes).to eq %i[read_user sudo read_repository read_registry openid profile email]
     end
 
     context 'registry_scopes' do

spec/requests/openid_connect_spec.rb

@@ -35,7 +35,7 @@ describe 'OpenID Connect requests' do
       'name'           => 'Alice',
       'nickname'       => 'alice',
       'email'          => 'public@example.com',
-      'email_verified' => true,
+      'email_verified' => false,
       'website'        => 'https://example.com',
       'profile'        => 'http://localhost/alice',
       'picture'        => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
@@ -111,6 +111,18 @@ describe 'OpenID Connect requests' do
       it 'does not include any unknown claims' do
         expect(json_response.keys).to eq %w[sub sub_legacy] + user_info_claims.keys
       end
+
+      it 'includes email and email_verified claims' do
+        expect(json_response.keys).to include('email', 'email_verified')
+      end
+
+      it 'has public email in email claim' do
+        expect(json_response['email']).to eq(user.public_email)
+      end
+
+      it 'has false in email_verified claim' do
+        expect(json_response['email_verified']).to eq(false)
+      end
     end
 
     context 'ID token payload' do
@@ -175,7 +187,35 @@ describe 'OpenID Connect requests' do
       expect(response).to have_gitlab_http_status(200)
       expect(json_response['issuer']).to eq('http://localhost')
       expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys')
-      expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid])
+      expect(json_response['scopes_supported']).to eq(%w[api read_user sudo read_repository openid profile email])
+    end
+  end
+
+  context 'Application with OpenID and email scopes' do
+    let(:application) { create :oauth_application, scopes: 'openid email' }
+
+    it 'token response includes an ID token' do
+      request_access_token!
+
+      expect(json_response).to include 'id_token'
+    end
+
+    context 'UserInfo payload' do
+      before do
+        request_user_info!
+      end
+
+      it 'includes the email and email_verified claims' do
+        expect(json_response.keys).to include('email', 'email_verified')
+      end
+
+      it 'has private email in email claim' do
+        expect(json_response['email']).to eq(user.email)
+      end
+
+      it 'has true in email_verified claim' do
+        expect(json_response['email_verified']).to eq(true)
+      end
     end
   end
 end