Skip to content

Fix: Preserve OAuth referer when resending confirmation email #6714

Open
bhatganeshdarshan wants to merge 1 commit intoopenstreetmap:masterfrom
bhatganeshdarshan:fix-oauth-resend-confirmation
Open

Fix: Preserve OAuth referer when resending confirmation email #6714
bhatganeshdarshan wants to merge 1 commit intoopenstreetmap:masterfrom
bhatganeshdarshan:fix-oauth-resend-confirmation

Conversation

@bhatganeshdarshan
Copy link

Description

  • When users sign up through an OAuth2 authorization flow and click "Resend confirmation email", the OAuth referer information is lost. The resent confirmation email does NOT contain the referer parameter with the oauth_return_url, causing the "Continue authorization" button to disappear from the welcome page after email confirmation.

  • Note : After signup if user doesnt click on "resend confirmation" , the initial confirmation link which was sent to email properly redirects to welcome page with "Continue authorization" , the above issue occurs only when user clicks "Resend confirmation mail"

Related to #6699

Fix

  • Store referer in session during user signup
  • Pass stored referer when resending confirmation email
  • Include referer in redirect after resending confirmation

This ensures that users who sign up via OAuth2 and click 'Resend confirmation email' will still see the 'Continue authorization' button on the welcome page after confirming their account.

Added unit tests:

  • test_confirm_success_with_oauth_referer: Verifies initial signup confirmation preserves OAuth referer
  • test_confirm_resend_preserves_oauth_referer: Verifies resending confirmation also preserves OAuth referer (this test fails without the fix, proving the bug exists)

- Store referer in session during user signup
- Pass stored referer when resending confirmation email
- Include referer in redirect after resending confirmation

This ensures that users who sign up via OAuth2 and click 'Resend
confirmation email' will still see the 'Continue authorization' button
on the welcome page after confirming their account.

Added comprehensive unit tests:
- test_confirm_success_with_oauth_referer: Verifies initial signup
  confirmation preserves OAuth referer
- test_confirm_resend_preserves_oauth_referer: Verifies resending
  confirmation also preserves OAuth referer (this test fails without
  the fix, proving the bug exists)
@bhatganeshdarshan bhatganeshdarshan force-pushed the fix-oauth-resend-confirmation branch from 3def057 to 3ccbb1f Compare January 15, 2026 20:14
@bhatganeshdarshan
Copy link
Author

Hi! Just a gentle ping to check if someone has time to review this PR when convenient. Thanks!

@tomhughes
Copy link
Member

So this needs to delete :referer from the session in all the places that currently delete :pending_user from the session, or it will stay around forever.

I'm also not sure about the way the tests are written - they seem to be testing more than is probably necessary and they look more like integration or system tests than controller tests at the moment but I think that may mean they're doing more than they need to rather than that they should be moved.

The first test is probably redundant - we already have tests further up that check that the referer is preserved normally I think and OAuth is not special, it's just another example of a referer. It's also not truly following through each stop as demonstrated by the fact that at the end it just makes up the referer URL for the final confirm rather than extracting it from the email.

The second test is mostly modelled on the first one but oddly stops sooner without running the final test that the confirm redirects as expected.

The critical thing that should be tested is the thing that's being fixed here - that if you visit the signup page with a referer and then visit the resend confirmation page that the subsequent email preserves the referer and that visiting the confirmation link from the email redirects to the expected referer from the original signup.

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