-
Notifications
You must be signed in to change notification settings - Fork 7
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
temp: Add configuration option to redirect to external site when TPA account is unlinked #540
temp: Add configuration option to redirect to external site when TPA account is unlinked #540
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Approved, subject to the resolution of the review comments
- I tested this: Tested in a sandbox.
- I read through the code
- I checked for accessibility issues
- Includes documentation
- I made sure any change in configuration variables is reflected in the corresponding client's
configuration-secure
repository.
@@ -99,7 +99,13 @@ def _do_third_party_auth(request): | |||
) | |||
) | |||
|
|||
raise AuthFailedError(message, error_code='third-party-auth-with-no-linked-account') # lint-amnesty, pylint: disable=raise-missing-from | |||
redirect_url = configuration_helpers.get_value('OC_REDIRECT_ON_TPA_UNLINKED_ACCOUNT', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should leave a comment here explaining the motivation for this change and that this is intended to be a temporary change. We should also indicate this in the PR commit message, for the benefit of the person preparing the next common branch.
0bd8531
to
7a37621
Compare
…account is unlinked
7a37621
to
91f8d36
Compare
@xitij2000, please add a ticket number to the PR. Also, could you ensure we have a follow-up to create a permanent solution? |
Hey @xitij2000, would you like me to create a follow-up ticket to create a permanent solution for this? Redwood isn't cut yet, so it's a good chance to get rid of this code drift altogether. |
@0x29a I believe this PR via BB-7742 is the permanent solution for this problem. We won't need the changes here once that is merged and we switch to AuthN MFE. cc. @xitij2000 |
Thanks for letting me know, @kaustavb12. I'll mention this in the code drift project. |
Testing instructions
OC_REDIRECT_ON_TPA_UNLINKED_ACCOUNT
with the URL to redirect to.Private-ref
: BB-7394