feat: redirect to custom URL when third-party auth account is unlinked #1078

ArturGaspar · 2023-10-13T15:56:32Z

Settings

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG["TPA_UNLINKED_ACCOUNT_PROVISION_URL"] = "http://example.com/"

TUTOR_GROVE_LMS_ENV_FEATURES: |
  ENABLE_THIRD_PARTY_AUTH: True
  ENABLE_COMBINED_LOGIN_REGISTRATION: True

Description

Allow redirecting to a custom URL when signing in via third-party auth when the account is not linked.

How Has This Been Tested?

Set the following settings in the LMS:
- ENABLE_MFE_CONFIG_API = True
- ENABLE_COMBINED_LOGIN_REGISTRATION = True
- ENABLE_THIRD_PARTY_AUTH = True
- AUTHN_MICROFRONTEND_URL = 'http://localhost:1999'
- MFE_CONFIG = {"TPA_UNLINKED_ACCOUNT_PROVISION_URL": "http://example.com"}
Set 'ENABLE_THIRD_PARTY_AUTH': True and 'ENABLE_AUTHN_MICROFRONTEND': True in FEATURES dict in LMS settings (edx-platform/lms/envs/common.py).
Create a dummy backend Oauth2 provider at http://localhost:18000/admin/third_party_auth/oauth2providerconfig/ and enable its "Visible" setting.
Set MFE_CONFIG_API_URL='http://localhost:18000/api/mfe_config/v1' in frontend-app-authn (e.g. in .env.development).
Go to http://localhost:1999/login.
Sign in with the "Dummy" provider.
See that you are redirected to example.com

The login page will briefly show before redirect but I see no way to prevent this as the request to http://localhost:18000/api/mfe_context is made asynchronously and we don't know the third-party auth context until it is made, but the login page can be rendered before that.

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Sandbox, if applicable. - N/A
Is there adequate test coverage for your changes?

Post-merge Checklist

Deploy the changes to prod after verifying on stage or ask @openedx/vanguards to do it.
🎉 🙌 Celebrate! Thanks for your contribution.

openedx-webhooks · 2023-10-13T15:56:37Z

Thanks for the pull request, @ArturGaspar! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 · 2023-10-25T13:03:07Z

Hi @openedx/vanguards! This is ready for review.

tecoholic

@ArturGaspar 👍

I tested this: Followed the testing instructions and verified that on TPA unlinked state, the user is redirected to the external url set in MFE_CONFIG.
I read through the code
I checked for accessibility issues
Includes documentation

tecoholic · 2023-10-26T09:44:53Z

src/config/index.js

@@ -19,6 +19,7 @@ const configuration = {
  SEARCH_CATALOG_URL: process.env.SEARCH_CATALOG_URL || null,
  TOS_AND_HONOR_CODE: process.env.TOS_AND_HONOR_CODE || null,
  TOS_LINK: process.env.TOS_LINK || null,
+  TPA_UNLINKED_ACCOUNT_PROVISION_URL: process.env.TPA_UNLINKED_ACCOUNT_PROVISION_URL || null,


@ArturGaspar Is the TPA_UNLINKED_ACCOUNT_PROVISION_URL required here and in the .env? I tested without setting the value in the .env and the change works fine as long as the MFE_CONFIG has this information.

I guess it doesn't hurt to have a default value here, maybe we could document this better somewhere?

@tecoholic, here it is necessary because someone might want to enable this feature without the MFE_CONFIG API. In .env it is not necessary (as this file defaults it to null) but it seemed appropriate to have it there for reference.

tecoholic · 2023-10-26T10:08:07Z

Note for core reviewers: The typical use case for this is when an organization wants to do registration of a user across multiple services or use an external service to collect extra registration data that is not available from the TPA provider. So, when the user lands without an account, the get redirected to the external service specified in the TPA_UNLINKED_ACCOUNT_PROVISION_URL where they enter extra details and then get sent back to the login/registration page.

arbrandes

Alright, I got this working on a local Tutor dev environment (but not on the sandbox, likely because for some reason it doesn't install the "dummy" Oauth provider).

I have no objections to the code, but in order to accept this we'll need to review some documentation on the intended use case. Before calling Product in here, can the author(s) please add some documentation to the README, with a good example of how this can be taken advantage of?

open-craft-grove · 2024-03-14T10:35:26Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-14T10:35:38Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

ArturGaspar · 2024-03-14T11:23:26Z

@arbrandes I have added some documentation now, in the style of the documentation of other configuration options. Is this good enough?

open-craft-grove · 2024-03-14T12:06:19Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-14T12:08:21Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-15T07:50:18Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-15T07:50:49Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-27T19:21:11Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-03-27T19:21:41Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

ArturGaspar · 2024-04-24T16:22:37Z

@arbrandes Would you have an estimate of when do you think you will be able to review this? I have added documentation to the README as requested.

open-craft-grove · 2024-05-16T20:44:44Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

arbrandes · 2024-05-17T12:31:28Z

README.rst

@@ -119,6 +119,10 @@ The authentication micro-frontend also requires the following additional variabl
     - Enables the image layout feature within the authn. When set to True, this feature allows the inclusion of images in the base container layout. For more details on configuring this feature, please refer to the `Modifying base container <docs/how_tos/modifying_base_container.rst>`_.
     - ``true`` | ``''`` (empty strings are falsy)

+   * - ``TPA_UNLINKED_ACCOUNT_PROVISION_URL``
+     - URL to redirect to when the identity provided by third-party authentication is not yet linked to a platform account. This allows for redirecting to a custom sign-up flow handled by an external service to create the linked account. An empty string (the default) disables this feature.


Apologies for holding this up, but it's still not clear to me how an account can get in this situation. It would be a big help if you could describe the logistration flow that benefits from this (maybe with a diagram?), and why one of the existing flows is not sufficient.

On a more general note, shouldn't this be a per-provider thing, as opposed to a single URL for everything?

open-craft-grove · 2024-05-22T17:17:15Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-05-22T18:01:10Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 13, 2023

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch 2 times, most recently from 90eac4e to 1acae39 Compare October 16, 2023 10:55

ArturGaspar marked this pull request as ready for review October 16, 2023 10:57

ArturGaspar requested a review from a team as a code owner October 16, 2023 10:57

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch 4 times, most recently from 830c2bc to 43efc72 Compare October 16, 2023 17:18

ArturGaspar mentioned this pull request Oct 16, 2023

feat: redirect to custom URL when third-party auth account is unlinked openedx/edx-platform#33513

Closed

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch 3 times, most recently from 06687b9 to f4d1db5 Compare October 23, 2023 17:30

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 24, 2023

ArturGaspar mentioned this pull request Oct 25, 2023

feat: redirect to custom URL when third-party auth account is unlinked open-craft/frontend-app-authn#4

Open

4 tasks

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch from f4d1db5 to ba65290 Compare October 25, 2023 06:45

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 25, 2023

mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Oct 25, 2023

tecoholic approved these changes Oct 26, 2023

View reviewed changes

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 26, 2023

mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Oct 27, 2023

mphilbrick211 added needs test run Author's first PR to this repository, awaiting test authorization from Axim and removed waiting for eng review PR is ready for review. Review and merge it, or suggest changes. labels Nov 14, 2023

arbrandes requested changes Feb 28, 2024

View reviewed changes

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch from 4da8414 to 2f25d54 Compare March 14, 2024 09:59

ArturGaspar requested a review from a team as a code owner March 14, 2024 09:59

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch 2 times, most recently from 0fe344e to 308f697 Compare March 14, 2024 11:22

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch from 308f697 to 038ac15 Compare March 15, 2024 07:14

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch from 038ac15 to abad25c Compare March 27, 2024 18:31

ArturGaspar requested a review from arbrandes March 27, 2024 18:55

kaustavb12 mentioned this pull request Apr 10, 2024

temp: Add configuration option to redirect to external site when TPA account is unlinked open-craft/edx-platform#540

Merged

itsjeyd removed the request for review from a team April 10, 2024 11:12

itsjeyd added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Apr 10, 2024

kaustavb12 mentioned this pull request Apr 14, 2024

chore: port code drift to quince open-craft/frontend-app-authn#9

Merged

feat: redirect to custom URL when third-party auth account is unlinked

d813aea

ArturGaspar force-pushed the artur/redirect-on-tpa-unlinked branch from abad25c to d813aea Compare May 16, 2024 19:56

arbrandes reviewed May 17, 2024

View reviewed changes

mphilbrick211 removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label May 17, 2024

itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label May 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: redirect to custom URL when third-party auth account is unlinked #1078

feat: redirect to custom URL when third-party auth account is unlinked #1078

ArturGaspar commented Oct 13, 2023 •

edited by arbrandes

openedx-webhooks commented Oct 13, 2023 •

edited

mphilbrick211 commented Oct 25, 2023

tecoholic left a comment •

edited

tecoholic Oct 26, 2023

ArturGaspar Oct 26, 2023

tecoholic commented Oct 26, 2023 •

edited

arbrandes left a comment

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

ArturGaspar commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 15, 2024

open-craft-grove commented Mar 15, 2024

open-craft-grove commented Mar 27, 2024

open-craft-grove commented Mar 27, 2024

ArturGaspar commented Apr 24, 2024

open-craft-grove commented May 16, 2024

arbrandes May 17, 2024

arbrandes May 17, 2024

open-craft-grove commented May 22, 2024

open-craft-grove commented May 22, 2024

feat: redirect to custom URL when third-party auth account is unlinked #1078

Are you sure you want to change the base?

feat: redirect to custom URL when third-party auth account is unlinked #1078

Conversation

ArturGaspar commented Oct 13, 2023 • edited by arbrandes

Description

How Has This Been Tested?

Merge Checklist

Post-merge Checklist

openedx-webhooks commented Oct 13, 2023 • edited

mphilbrick211 commented Oct 25, 2023

tecoholic left a comment • edited

Choose a reason for hiding this comment

tecoholic Oct 26, 2023

Choose a reason for hiding this comment

ArturGaspar Oct 26, 2023

Choose a reason for hiding this comment

tecoholic commented Oct 26, 2023 • edited

arbrandes left a comment

Choose a reason for hiding this comment

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

ArturGaspar commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 14, 2024

open-craft-grove commented Mar 15, 2024

open-craft-grove commented Mar 15, 2024

open-craft-grove commented Mar 27, 2024

open-craft-grove commented Mar 27, 2024

ArturGaspar commented Apr 24, 2024

open-craft-grove commented May 16, 2024

arbrandes May 17, 2024

Choose a reason for hiding this comment

arbrandes May 17, 2024

Choose a reason for hiding this comment

open-craft-grove commented May 22, 2024

open-craft-grove commented May 22, 2024

ArturGaspar commented Oct 13, 2023 •

edited by arbrandes

openedx-webhooks commented Oct 13, 2023 •

edited

tecoholic left a comment •

edited

tecoholic commented Oct 26, 2023 •

edited