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

debug and fix user hijacking. #1912

Merged
merged 7 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions galaxy_ng/social/pipeline/user.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
#!/usr/bin/env python3

import logging

from galaxy_ng.app.models.auth import User
from galaxy_ng.app.utils.galaxy import generate_unverified_email


logger = logging.getLogger(__name__)


USER_FIELDS = ['username', 'email']


Expand All @@ -14,10 +18,13 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs):

def create_user(strategy, details, backend, user=None, *args, **kwargs):

logger.info(f'create_user(1): user-kwarg:{user}')

if user:
if user.username != details.get('username'):
user.username = details.get('username')
user.save()
logger.info(f'create_user(2): returning user-kwarg {user}')
return {'is_new': False}

fields = dict(
Expand All @@ -26,6 +33,7 @@ def create_user(strategy, details, backend, user=None, *args, **kwargs):
)

if not fields:
logger.info(f'create_user(3): no fields for {user}')
return

# bypass the strange logic that can't find the user ... ?
Expand All @@ -34,25 +42,40 @@ def create_user(strategy, details, backend, user=None, *args, **kwargs):
github_id = details.get('id')
if not github_id:
github_id = kwargs['response']['id']
logger.info(
f'create_user(4): enumerate username:{username} email:{email} github_id:{github_id}'
)

# check for all possible emails ...
possible_emails = [generate_unverified_email(github_id), email]
found_email = None
for possible_email in possible_emails:

# if the email is null maybe that causes the user hijacking?
if not email:
continue

found_email = User.objects.filter(email=possible_email).first()
if found_email is not None:
logger.info(f'create_user(5): found user {found_email} via email {possible_email}')
break

if found_email is not None:

# fix the username if they've changed their login since last time
if found_email.username != username:
logger.info(f'create_user(6): set found user {found_email} username to {username}')
found_email.username = username
found_email.save()

if found_email.email != email:
logger.info(f'create_user(7): set found user {found_email} email to {email}')
found_email.email = email
found_email.save()

logger.info(
f'create_user(8): returning found user {found_email} via email {possible_email}'
)
return {
'is_new': False,
'user': found_email
Expand All @@ -62,17 +85,20 @@ def create_user(strategy, details, backend, user=None, *args, **kwargs):
if found_username is not None and found_username.email:
# we have an old user who's got the username but it's not the same person logging in ...
# so change that username? The email should be unique right?
logger.info(f'create_user(9): set {found_username} username to {found_username.email}')
found_username.username = found_username.email
found_username.save()

found_username = User.objects.filter(username=username).first()
if found_username is not None:
logger.info(f'create_user(10): {found_username}')
return {
'is_new': False,
'user': found_username
}

new_user = strategy.create_user(**fields)
logger.info(f'create_user(11): {new_user}')
return {
'is_new': True,
'user': new_user
Expand Down
94 changes: 94 additions & 0 deletions galaxy_ng/tests/integration/community/test_community_hijacking.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
"""test_community.py - Tests related to the community featureset.
"""

import pytest

from ..utils import (
get_client,
SocialGithubClient,
GithubAdminClient,
cleanup_namespace,
)
from ..utils.legacy import (
cleanup_social_user,
)


pytestmark = pytest.mark.qa # noqa: F821


def extract_default_config(ansible_config):
base_cfg = ansible_config('github_user_1')
cfg = {}
cfg['token'] = None
cfg['url'] = base_cfg.get('url')
cfg['auth_url'] = base_cfg.get('auth_url')
cfg['github_url'] = base_cfg.get('github_url')
cfg['github_api_url'] = base_cfg.get('github_api_url')
return cfg


@pytest.mark.deployment_community
def test_community_hijacking(ansible_config):

default_cfg = extract_default_config(ansible_config)
admin_config = ansible_config("admin")
admin_client = get_client(
config=admin_config,
request_token=False,
require_auth=True
)
ga = GithubAdminClient()

usermap = {
'jctannerTESTME': {
'uid': 1000,
'login': 'jctannerTESTME',
# 'email': '[email protected]',
'email': '',
},
'drod0258X': {
'uid': 1001,
'login': 'drod0258X',
# 'email': '[email protected]',
'email': ''
}
}

# clean up all traces of these users ...
for username, udata in usermap.items():
ga.delete_user(uid=udata['uid'])
for uname in [username, username.lower()]:
cleanup_social_user(uname, ansible_config)
cleanup_namespace(uname, api_client=admin_client)
ga.delete_user(login=uname)

# create api client configs
for username, udata in usermap.items():
ucfg = ga.create_user(**udata)
ucfg.update(default_cfg)
ucfg['username'] = username
usermap[username]['cfg'] = ucfg

# make clients and login
for username, udata in usermap.items():
sclient = SocialGithubClient(config=udata['cfg'])
usermap[username]['client'] = sclient
usermap[username]['client'].login()

# force logout
for username, udata in usermap.items():
usermap[username]['client'].logout()

# force login
for username, udata in usermap.items():
usermap[username]['client'].login()

# check me
for username, udata in usermap.items():
me_rr = usermap[username]['client'].get('_ui/v1/me/')
usermap[username]['me'] = me_rr.json()['username']

# ensure no shenanigens happened ...
for username, udata in usermap.items():
assert udata['me'] == username
3 changes: 3 additions & 0 deletions galaxy_ng/tests/integration/utils/client_social_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ def logout(self, expected_code=None):
if self._rs is None:
raise Exception('client is not authenticated')

rr = self.post(absolute_url='/api/_ui/v1/auth/logout/', data={})
assert rr.status_code == 204, rr.text

def get(self, relative_url: str = None, absolute_url: str = None) -> requests.models.Response:

pheaders = {
Expand Down
2 changes: 1 addition & 1 deletion profiles/community/github_mock/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ RUN pip install -r /app/requirements.txt
RUN mkdir -p /src
RUN test -d /src/werkzeug || git clone -b SAVE_THE_STREAMS https://github.com/jctanner/werkzeug /src/werkzeug
RUN pip install -e /src/werkzeug
CMD python3 /app/flaskapp.py
CMD PYTHONUNBUFFERED=1 python3 /app/flaskapp.py
6 changes: 3 additions & 3 deletions profiles/community/github_mock/flaskapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def create_tables():
CREATE TABLE IF NOT EXISTS users (
id INTEGER PRIMARY KEY AUTOINCREMENT,
login TEXT NOT NULL UNIQUE,
email TEXT NOT NULL UNIQUE,
email TEXT NOT NULL,
password TEXT NOT NULL
)
''')
Expand Down Expand Up @@ -421,8 +421,8 @@ def admin_add_user():
if password is None:
password = get_new_password()
email = ds.get('email', login + '@github.com')
if email is None or not email:
email = login + '@github.com'
# if email is None or not email:
# email = login + '@github.com'

conn = sqlite3.connect(db_name)
cursor = conn.cursor()
Expand Down
Loading