Skip to content

temp: Add configuration option to redirect to external site when TPA account is unlinked#540

Merged
xitij2000 merged 1 commit intoopencraft-release/nutmeg.2from
kshitij/redirect-on-tpa-unlinked
May 29, 2023
Merged

temp: Add configuration option to redirect to external site when TPA account is unlinked#540
xitij2000 merged 1 commit intoopencraft-release/nutmeg.2from
kshitij/redirect-on-tpa-unlinked

Conversation

@xitij2000
Copy link
Member

@xitij2000 xitij2000 commented May 24, 2023

Testing instructions

  1. Add the dummy OAuth provider
  2. Try logging in using the dummy oauth provider
  3. You will see an error message about an unlinked TPA account, asking you to log in.
  4. Check out this branch
  5. Add a site config variable called OC_REDIRECT_ON_TPA_UNLINKED_ACCOUNT with the URL to redirect to.
  6. Clear cookies or us a fresh browser session, and try logging in with the dummy provider again.
  7. You should be redirected to the URl set above.

Private-ref: BB-7394

Copy link
Member

@kaustavb12 kaustavb12 left a 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.

Copy link
Member

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.

@xitij2000 xitij2000 force-pushed the kshitij/redirect-on-tpa-unlinked branch 2 times, most recently from 0bd8531 to 7a37621 Compare May 25, 2023 17:22
@xitij2000 xitij2000 force-pushed the kshitij/redirect-on-tpa-unlinked branch from 7a37621 to 91f8d36 Compare May 26, 2023 06:26
@xitij2000 xitij2000 merged commit b63af53 into opencraft-release/nutmeg.2 May 29, 2023
@xitij2000 xitij2000 deleted the kshitij/redirect-on-tpa-unlinked branch May 29, 2023 06:29
@Agrendalath
Copy link
Member

@xitij2000, please add a ticket number to the PR. Also, could you ensure we have a follow-up to create a permanent solution?

@0x29a
Copy link

0x29a commented Apr 10, 2024

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.

@kaustavb12
Copy link
Member

@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

@0x29a
Copy link

0x29a commented Apr 10, 2024

Thanks for letting me know, @kaustavb12. I'll mention this in the code drift project.

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.

4 participants