Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable additonal profile array response oauth2 flow (Github private email addresses) #8391

Closed
hauserkristof opened this issue Jan 22, 2025 · 2 comments
Assignees
Milestone

Comments

@hauserkristof
Copy link
Contributor

Current issue / limitation

As mentioned in #7949 users cannot authenticate if the /user endpoint of GitHub is returning None. This is not an issue of the user or administrator of pgAdmin.

GitHub has a feature that enables users to hide their email addresses, but it could be queried with an authenticated request (other providers can solve this).

The main issue is that the user/emails endpoint returns an array of objects, instead of a single object.

"Solutions" that do not work

I've tried to set the OAUTH2_USERINFO_ENDPOINT config value to user/emails with no luck

Also tried setting the OAUTH2_ADDITIONAL_CLAIMS to various values too, with no luck, email field is still None.

Debug log:

255.2.3.4 - - [22/Jan/2025:20:33:49 +0000] "GET /static/js/generated/vendor.others.js?ver=81400 HTTP/1.1" 200 0 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36"
2025-01-22 20:33:49,757: DEBUG	pgadmin:	Authentication initiated via source: oauth2
2025-01-22 20:33:49,758: DEBUG	pgadmin:	Authentication initiated via source: oauth2 is failed.
255.2.3.4 - - [22/Jan/2025:20:33:49 +0000] "POST /authenticate/login HTTP/1.1" 302 639 "https://pgadmin.xxx.yyy.dev/login?next=/" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36"
2025-01-22 20:33:50,758: ERROR	pgadmin:	'list' object has no attribute 'keys'
Traceback (most recent call last):
  File "/venv/lib/python3.12/site-packages/flask/app.py", line 917, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib/python3.12/site-packages/flask/app.py", line 902, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)  # type: ignore[no-any-return]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/pgadmin4/pgadmin/authenticate/oauth2.py", line 56, in oauth_authorize
    status, msg = auth_obj.login()
                  ^^^^^^^^^^^^^^^^
  File "/pgadmin4/pgadmin/authenticate/__init__.py", line 299, in login
    status, msg = self.source.login(self.form)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/pgadmin4/pgadmin/authenticate/oauth2.py", line 138, in login
    [value for value in self.email_keys if value in profile.keys()]
                                                    ^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'keys'

Full Config:

import os

clientId = os.path.expandvars('$OAUTH2_CLIENT_ID')
clientSecret = os.path.expandvars('$OAUTH2_CLIENT_SECRET')

AUTHENTICATION_SOURCES = ['oauth2', 'internal']

OAUTH2_AUTO_CREATE_USER = True
MASTER_PASSWORD = False
# PGADMIN_CONFIG_CONSOLE_LOG_LEVEL = 10

OAUTH2_CONFIG = [
    {
        # The name of the of the oauth provider, ex: github, google
        'OAUTH2_NAME': 'github',
        'OAUTH2_DISPLAY_NAME': 'Login with Github',
        'OAUTH2_CLIENT_ID': clientId,
        'OAUTH2_CLIENT_SECRET': clientSecret,
        # URL to generate a token,
        # Ex: https://github.com/login/oauth/access_token
        'OAUTH2_TOKEN_URL': 'https://github.com/login/oauth/access_token',
        # URL is used for authentication,
        # Ex: https://github.com/login/oauth/authorize
        'OAUTH2_AUTHORIZATION_URL': 'https://github.com/login/oauth/authorize',
        # Oauth base url, ex: https://api.github.com/
        'OAUTH2_API_BASE_URL': 'https://api.github.com/',
        # Name of the Endpoint, ex: user
        'OAUTH2_USERINFO_ENDPOINT': 'user/emails',
        # Oauth scope, ex: 'openid email profile'
        # Note that an 'email' claim is required in the resulting profile
        'OAUTH2_SCOPE': 'user',
        'OAUTH2_USERNAME_CLAIM': 'email',
        # Font-awesome icon, ex: fa-github
        'OAUTH2_ICON': 'fa-github',
        # UI button colour, ex: #0000ff
        'OAUTH2_BUTTON_COLOR': '#0000ff',
        
        'OAUTH2_ADDITIONAL_CLAIMS': 'email_info',
        'OAUTH2_SSL_CERT_VERIFICATION': True,
        'OAUTH2_LOGOUT_URL': None
    }
]

Describe the solution you'd like

Current code could be changed from in (pgadmin4/web/pgadmin/authenticate/oauth2.py):

def login(self, form):
    profile = self.get_user_profile()
    email_key = \
        [value for value in self.email_keys if value in profile.keys()]
    email = profile[email_key[0]] if (len(email_key) > 0) else None

To:

def get_profile_dict(profile):
    """
    Returns the dictionary from profile whether it's a list or dictionary.
    Includes additional type checking.
    """
    if isinstance(profile, list):
        return profile[0] if profile else {}
    elif isinstance(profile, dict):
        return profile
    else:
        return {}  # or raise TypeError("Profile must be list or dict")

def login(self, form):
        profile = self.get_profile_dict(self.get_user_profile())
        email_key = \
            [value for value in self.email_keys if value in profile.keys()]
        email = profile[email_key[0]] if (len(email_key) > 0) else None

This would give back the users primary email address (by default the primary email address is the first in github, I think most services are working this way).

Sample case
# Case 1: List containing dictionary
profile1 = [
    {
        "email": "[email protected]",
        "verified": true
    }
]

# Case 2: Just dictionary
profile2 = {
    "email": "[email protected]",
    "verified": true
}

# Both would work:
result1 = get_profile_dict(profile1)  # Returns the dictionary
result2 = get_profile_dict(profile2)  # Returns the dictionary

Describe alternatives you've considered

Well, I think there could be some more complex logic to handle the primary key part of this, but I think it won't really be necessary.

Additional context

Image

I would be happy, to create a PR about this.

@hauserkristof
Copy link
Contributor Author

I've created a PR for this: #8392

@akshay-joshi akshay-joshi moved this to 🏗 In Progress in Current Sprint (186) Jan 24, 2025
@akshay-joshi akshay-joshi moved this from 🏗 In Progress to In Review in Current Sprint (186) Jan 24, 2025
khushboovashi pushed a commit that referenced this issue Jan 28, 2025
@khushboovashi khushboovashi moved this from In Review to In Testing in Current Sprint (186) Jan 28, 2025
@akshay-joshi akshay-joshi added this to the 9.0 milestone Jan 28, 2025
@RohitBhati8269 RohitBhati8269 self-assigned this Jan 29, 2025
@RohitBhati8269
Copy link
Contributor

Working fine Tested and Verified on :- build
Package :- wheel
OS :- macOS Sequoia 15.0.1

@RohitBhati8269 RohitBhati8269 moved this from In Testing to ✅ Done in Current Sprint (186) Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants