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

Remove X-EdX-Api-Key usage #103

Closed
asadali145 opened this issue Feb 14, 2024 · 6 comments
Closed

Remove X-EdX-Api-Key usage #103

asadali145 opened this issue Feb 14, 2024 · 6 comments

Comments

@asadali145
Copy link
Contributor

asadali145 commented Feb 14, 2024

Description/Context

Open edX is going to remove the support of X-EdX-Api-Key header. Currently, we are using the X-EdX-Api-Key and token authentication both in our client. As per the initial R&D, we can safely remove the usage of X-EdX-Api-Key and our apps would still work as we use the token authentication in our Apps.

Usages:

Acceptance Criteria:

  • Usage of X-EdX-Api-Key is removed safely without an impact on the existing functionality.
  • Update docs if needed.

Related tickets

mitodl/mitxpro#2872
mitodl/mitxonline#2102

@arslanashraf7
Copy link
Contributor

@mudassir-hafeez Could you take a look at this before mitodl/mitxpro#2872? These two are associated issues and we should work on this one before the xPRO one.

@mudassir-hafeez
Copy link
Contributor

mudassir-hafeez commented May 15, 2024

@pdpinch
After conducting some R&D and testing various endpoints of the edx-api-client (along with xpro), I've discovered a potential impact on the deactivate_enrollment API for non-staff users if the EdX-Api-Key is removed.

  • It seems the edx-platform does not update enrollment (i.e. deactivation) for non-staff users without the X-EdX-Api-Key and raises error, even though it is deprecated, and allows them unless users have staff privileges or an EdX-Api-Key is set.
  • The following Xpro features/management commands would be impacted if EdX-Api-Key usage is removed:
    • defer_enrollment
    • refund_enrollment
    • transfer_enrollment
  • Some similar features exist in mitxonline, which are subject to the same impact.

I suggest we wait until EdX completely removes the usage of EdX-Api-Key from edx-platform. After that, we can address any necessary fixes or adjustments.

@pdpinch
Copy link
Member

pdpinch commented May 15, 2024 via email

@arslanashraf7
Copy link
Contributor

In addition to @pdpinch 's comment, Here are some thoughts from my end:

  1. I agree with Peter, We did shift the enrollments to invite only we now use staff tokens to enroll users through API.

Some questions:

  1. Do we still use EdX-Api-Key while enrolling users through the staff token?
  2. If Yes, Does the enrollment work If we remove it?
  3. If enrollments are working without EdX-Api-Key and the unenrollments are being performed on behalf of the user's token, can we use the staff token in unenrollment as well?

If we can perform unenroll with staff token and without EdX-Api-Key, I think we won't need to worry what edX doesn't with API key.

I'd also like to note that the impact of any changes we make in edx-api-client would not be limited to MIT xPRO, It would also touch MITx Online since that uses the same client for API calls to edX.

@asadali145
Copy link
Contributor Author

Update

We(Me, Muddassir, and Arslan) discussed the enrollments. Here are the notes of the discussion and steps forward:

  • We already moved to the forced enrollments using the staff user and enabled the invitation-only flag for the course enrollments in xPro and MITx Online.
  • Enrollments are created using the staff user but we found that unenrollment is using the EdX-Api-Key and the user token. Arslan has suggested that we use the staff user for the unenrollments as well (Which should be easy to do).

Next Steps

  • Use staff user for the unenrollments
  • Remove EdX-Api-Key

CC: @pdpinch @arslanashraf7 @mudassir-hafeez

@mudassir-hafeez
Copy link
Contributor

Closing #103 as PR #104 and related PRs (mitodl/mitxpro#2982 and mitodl/mitxonline#2217) have been deployed to production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants