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

PADV-516 - Allow platform acount association by email. #104

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

Squirrel18
Copy link

@Squirrel18 Squirrel18 commented Jul 24, 2023

Description:

This PR allows to associate the edx-platform account with a SAML account under these conditions:

  • The platform user's email matches the SAML account email.
  • There are not multiple platform accounts with the same email.
  • The platform user is not staff or superuser.
  • The platform user is active.
    This is necessary to prevent users from logging into their edx-platform account to link the SAML IDP account and the edx-platform account.

How to test:

  • Set up a local SAML IDP, check this video about SAML and configuring a local IDP: https://youtu.be/1PoWSVtwheI
  • Create a test account on edx-platform.
  • Set this feature flag at site level: ASSOCIATE_ACCOUNT_WITH_SAML_IDP_USER
  • In a private window, log in using the local IDP and use an account with the same email address as the edx-platform account.
  • You should be logged in automatically and the accounts should be linked.

Notes:

Reviewers:

Comment on lines 870 to 886

# It is not allowed to be associated to a staff or superuser to prevent impersonation.
if users[0].is_staff or users[0].is_superuser:
raise AuthException(
backend,
f'It is not allowed to auto associate staff or superuser users {email}',
)

# It is not allowed to be associated to an inactive account to prevent impersonation.
if not users[0].is_active:
raise AuthException(
backend,
f'Platform user account is not active: {email} {backend.name}',
)

return {
'user': users[0],

Choose a reason for hiding this comment

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

Suggested change
# It is not allowed to be associated to a staff or superuser to prevent impersonation.
if users[0].is_staff or users[0].is_superuser:
raise AuthException(
backend,
f'It is not allowed to auto associate staff or superuser users {email}',
)
# It is not allowed to be associated to an inactive account to prevent impersonation.
if not users[0].is_active:
raise AuthException(
backend,
f'Platform user account is not active: {email} {backend.name}',
)
return {
'user': users[0],
user_account = users[0]
# It is not allowed to be associated to a staff or superuser to prevent impersonation.
if user_account.is_staff or user_account.is_superuser:
raise AuthException(
backend,
f'It is not allowed to auto associate staff or superuser users {email}',
)
# It is not allowed to be associated to an inactive account to prevent impersonation.
if not user_account.is_active:
raise AuthException(
backend,
f'Platform user account is not active: {email} {backend.name}',
)
return {
'user': user_account,

@Squirrel18 Squirrel18 merged commit c2428f8 into pearson-release/olive.stage Jul 25, 2023
30 of 34 checks passed
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.

2 participants