Skip to content

fix: remove activation_key from account REST API response#832

Merged
Kelketek merged 1 commit intoviadanna/sumac/offlinefrom
fox/offline-branch-activation-patch
Apr 2, 2026
Merged

fix: remove activation_key from account REST API response#832
Kelketek merged 1 commit intoviadanna/sumac/offlinefrom
fox/offline-branch-activation-patch

Conversation

@Kelketek
Copy link
Copy Markdown
Member

@Kelketek Kelketek commented Mar 31, 2026

cherry-pick from upstream: openedx#38241

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.

Copy link
Copy Markdown
Member

@samuelallan72 samuelallan72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kelketek

This misses the change to openedx/core/djangoapps/user_api/accounts/views.py from the upstream commit ( openedx@ad342ae ) - see this line: https://github.com/open-craft/edx-platform/blob/ed8ca52fd49bc9812e3257c25079c49cb0e4878e/openedx/core/djangoapps/user_api/accounts/views.py#L174 . Not critical, but I think important to retain the patch as a whole to avoid confusing code drift.

Also, please show in the commit and PR description that this is a cherry pick of openedx@ad342ae

@Kelketek
Copy link
Copy Markdown
Member Author

Kelketek commented Apr 1, 2026

@samuelallan72 It does remove that change, but that change is applied after a massive rewrite of the docstring. I removed it to lower the amount of conflicting lines.

I'm not sure that adding the full docstring will apply cleaner, since the commit which adds that full documentation won't come til much later, causing a conflict down the line.

@samuelallan72
Copy link
Copy Markdown
Member

@Kelketek ah I see. In that case, dropping that docstring diff seems fine to me.

Please still update the PR descriptions and commit messages to show the cherry pick. :)

cherry-pick from upstream: openedx#38241

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>
@Kelketek Kelketek force-pushed the fox/offline-branch-activation-patch branch from ed8ca52 to 2528c4a Compare April 1, 2026 14:48
Copy link
Copy Markdown
Member

@samuelallan72 samuelallan72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kelketek 👍 Thanks!

  • I tested this: checked this matches the upstream patch, checked it appears to apply cleanly, checked that the failing tests aren't related
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@Kelketek Kelketek merged commit 8a16cc2 into viadanna/sumac/offline Apr 2, 2026
47 of 49 checks passed
@Kelketek Kelketek deleted the fox/offline-branch-activation-patch branch April 2, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants