Skip to content

Commit 50131ab

Browse files
feanilclaude
authored andcommitted
fix: remove activation_key from account REST API response
The activation_key field was exposed in /api/user/v1/accounts/{username}, allowing an attacker to bypass email verification by combining two behaviors: 1. OAuth2 password grant issues tokens to inactive users (intentional) 2. activation_key returned in API response (the vulnerability) An attacker could register, get an OAuth2 token, read the activation_key from the API, then GET /activate/{key} to activate without email access. Fix: remove activation_key from UserReadOnlySerializer.to_representation() and from ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] (which controls the field whitelist in _filter_fields — listed fields default to None even if absent from the serializer data dict). Reported by Daniel Baillo via the Open edX security working group. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 36f199a commit 50131ab

File tree

5 files changed

+72
-19
lines changed

5 files changed

+72
-19
lines changed

lms/envs/common.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4314,7 +4314,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
43144314
"secondary_email_enabled",
43154315
"year_of_birth",
43164316
"phone_number",
4317-
"activation_key",
43184317
"pending_name_change",
43194318
]
43204319
)

openedx/core/djangoapps/user_api/accounts/serializers.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,6 @@ def to_representation(self, user): # lint-amnesty, pylint: disable=arguments-di
142142
except ObjectDoesNotExist:
143143
account_recovery = None
144144

145-
try:
146-
activation_key = user.registration.activation_key
147-
except ObjectDoesNotExist:
148-
activation_key = None
149-
150145
data = {
151146
"username": user.username,
152147
"url": self.context.get('request').build_absolute_uri(
@@ -161,7 +156,6 @@ def to_representation(self, user): # lint-amnesty, pylint: disable=arguments-di
161156
"date_joined": user.date_joined.replace(microsecond=0),
162157
"last_login": user.last_login,
163158
"is_active": user.is_active,
164-
"activation_key": activation_key,
165159
"bio": None,
166160
"country": None,
167161
"state": None,

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,6 @@ def test_create_account(self):
635635
'id': user.id,
636636
'name': self.USERNAME,
637637
'verified_name': None,
638-
'activation_key': user.registration.activation_key,
639638
'gender': None, 'goals': '',
640639
'is_active': False,
641640
'level_of_education': None,

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ class TestAccountsAPI(FilteredQueryCountMixin, CacheIsolationTestCase, UserAPITe
358358
"""
359359

360360
ENABLED_CACHES = ['default']
361-
TOTAL_QUERY_COUNT = 26
362-
FULL_RESPONSE_FIELD_COUNT = 29
361+
TOTAL_QUERY_COUNT = 25
362+
FULL_RESPONSE_FIELD_COUNT = 28
363363

364364
def setUp(self):
365365
super().setUp()
@@ -488,19 +488,19 @@ def test_get_account_unknown_user(self, api_client, user):
488488
("client", "user"),
489489
)
490490
@ddt.unpack
491-
def test_regsitration_activation_key(self, api_client, user):
491+
def test_regsitration_activation_key_not_exposed(self, api_client, user):
492492
"""
493-
Test that registration activation key has a value.
493+
Test that activation_key is NOT returned in the account API response.
494494
495-
UserFactory does not auto-generate registration object for the test users.
496-
It is created only for users that signup via email/API. Therefore, activation key has to be tested manually.
495+
The activation_key is a secret used for email verification and must not be
496+
exposed via the API, as doing so allows bypassing email verification.
497497
"""
498498
self.create_user_registration(self.user)
499499

500500
client = self.login_client(api_client, user)
501501
response = self.send_get(client)
502502

503-
assert response.data["activation_key"] is not None
503+
assert "activation_key" not in response.data
504504

505505
def test_successful_get_account_by_email(self):
506506
"""
@@ -811,12 +811,12 @@ def verify_get_own_information(queries):
811811
assert data['time_zone'] is None
812812

813813
self.client.login(username=self.user.username, password=TEST_PASSWORD)
814-
verify_get_own_information(self._get_num_queries(24))
814+
verify_get_own_information(self._get_num_queries(23))
815815

816816
# Now make sure that the user can get the same information, even if not active
817817
self.user.is_active = False
818818
self.user.save()
819-
verify_get_own_information(self._get_num_queries(16))
819+
verify_get_own_information(self._get_num_queries(15))
820820

821821
def test_get_account_empty_string(self):
822822
"""
@@ -831,7 +831,7 @@ def test_get_account_empty_string(self):
831831
legacy_profile.save()
832832

833833
self.client.login(username=self.user.username, password=TEST_PASSWORD)
834-
with self.assertNumQueries(self._get_num_queries(24), table_ignorelist=WAFFLE_TABLES):
834+
with self.assertNumQueries(self._get_num_queries(23), table_ignorelist=WAFFLE_TABLES):
835835
response = self.send_get(self.client)
836836
for empty_field in ("level_of_education", "gender", "country", "state", "bio",):
837837
assert response.data[empty_field] is None

openedx/core/djangoapps/user_api/accounts/views.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,68 @@ def search_emails(self, request):
400400

401401
def retrieve(self, request, username):
402402
"""
403-
GET /api/user/v1/accounts/{username}/
403+
Retrieve a single detailed user object
404+
405+
**Example Requests**
406+
407+
GET /api/user/v1/accounts/{username}/
408+
409+
**Response**
410+
411+
If no user exists with the specified username, or email, an HTTP 404 "Not Found" response is returned.
412+
413+
If the user makes the request for her own account, or makes a request for another account and has "is_staff" access, an HTTP 200 "OK" response is returned. The response contains the following values.
414+
415+
* `id`: numerical lms user id in db
416+
* `bio`: null or textual representation of user biographical information ("about me").
417+
* `country`: An ISO 3166 country code or null.
418+
* `date_joined`: The date the account was created, in the string format provided by datetime. For example, "2014-08-26T17:52:11Z".
419+
* `last_login`: The latest date the user logged in, in the string datetime format.
420+
* `email`: Email address for the user. New email addresses must be confirmed via a confirmation email, so GET does not reflect the change until the address has been confirmed.
421+
* `secondary_email`: A secondary email address for the user. Unlike the email field, GET will reflect the latest update to this field even if changes have yet to be confirmed.
422+
* `verified_name`: Approved verified name of the learner present in name affirmation plugin
423+
* `extended_profile`: A list of objects with the keys `field_name` and `field_value`, returning any populated `extended_profile_fields` configured in the **Site Configuration**
424+
* `gender`: One of the following values:
425+
* null
426+
* "f"
427+
* "m"
428+
* "o"
429+
* `goals`: The textual representation of the user's goals, or null.
430+
* `is_active`: Boolean representation of whether a user is active.
431+
* `language`: The user's preferred language, or null.
432+
* `language_proficiencies`: Array of language preferences. Each preference is a JSON object with the following keys:
433+
* "code": string ISO 639-1 language code e.g. "en".
434+
* `level_of_education`: One of the following values:
435+
* "p": PhD or Doctorate
436+
* "m": Master's or professional degree
437+
* "b": Bachelor's degree
438+
* "a": Associate's degree
439+
* "hs": Secondary/high school
440+
* "jhs": Junior secondary/junior high/middle school
441+
* "el": Elementary/primary school
442+
* "none": None
443+
* "o": Other
444+
* null: The user did not enter a value
445+
* `mailing_address`: The textual representation of the user's mailing address, or null.
446+
* `name`: The full name of the user.
447+
* `profile_image`: A JSON representation of a user's profile image information. This representation has the following keys.
448+
* "has_image": Boolean indicating whether the user has a profile image.
449+
* "image_url_*": Absolute URL to various sizes of a user's profile image, where '*' matches a representation of the corresponding image size, such as 'small', 'medium', 'large', and 'full'. These are configurable via PROFILE_IMAGE_SIZES_MAP.
450+
* `requires_parental_consent`: True if the user is a minor requiring parental consent.
451+
* `social_links`: Array of social links, sorted alphabetically by "platform". Each preference is a JSON object with the following keys:
452+
* "platform": A particular social platform, ex: 'facebook'
453+
* "social_link": The link to the user's profile on the particular platform
454+
* `username`: The username associated with the account.
455+
* `year_of_birth`: The year the user was born, as an integer, or null.
456+
* `account_privacy`: The user's setting for sharing her personal profile. Possible values are "all_users", "private", or "custom". If "custom", the user has selectively chosen a subset of shareable fields to make visible to others via the User Preferences API.
457+
* `phone_number`: The phone number for the user. String of numbers with an optional `+` sign at the start.
458+
* `pending_name_change`: If the user has an active name change request, returns the requested name.
459+
460+
For all text fields, plain text instead of HTML is supported. The data is stored exactly as specified. Clients must HTML escape rendered values to avoid script injections.
461+
462+
If a user who does not have "is_staff" access requests account information for a different user, only a subset of these fields is returned. The returned fields depend on the `ACCOUNT_VISIBILITY_CONFIGURATION` configuration setting and the visibility preference of the user for whom data is requested.
463+
464+
A user can view which account fields they have shared with other users by requesting their own username and providing the "view=shared" URL parameter.
404465
"""
405466
try:
406467
account_settings = get_account_settings(request, [username], view=request.query_params.get("view"))

0 commit comments

Comments
 (0)