Skip to content

Bugfix/resend verification email link #7533

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

Closed
wants to merge 2 commits into from
Closed

Bugfix/resend verification email link #7533

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 30, 2021

New Pull Request Checklist

Issue Description

Currently, if you use a custom Public Server URL for your Parse Server setup, then the "Resend Link" button on the Invalid Verification Link page is broken. When pressing the button you get redirected to a page with a "error: unauthorized" error message.

This is because the button's form is always pointing at /apps/<appId>/resend_verification_email, and not at <public-server-url>/apps/<app-id>/resend_verification_email. I.e. it doesn't take public Server URL into consideration.

I've made a fix, where the page's client-side Javascripts checks its URL for whether it has a public Server Url. If so, then it's prepended to the form's action URL.

This already seems to be fixed for the next major release (5.0.0) of parse-server, where PagesRouter will replace the PublicApiRouter. For PagesRouter it uses completely new HTML templates (under public/), and publicServerUrl seems to be used correctly there.

Related issue:

Approach

TODOs before merging

@codecov
Copy link

codecov bot commented Aug 30, 2021

Codecov Report

Merging #7533 (b259d35) into master (1e0d408) will decrease coverage by 9.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7533      +/-   ##
==========================================
- Coverage   93.95%   84.70%   -9.25%     
==========================================
  Files         181      181              
  Lines       13273    13273              
==========================================
- Hits        12470    11243    -1227     
- Misses        803     2030    +1227     
Impacted Files Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js 2.47% <0.00%> (-92.90%) ⬇️
src/Adapters/Storage/Postgres/PostgresClient.js 5.00% <0.00%> (-65.00%) ⬇️
src/Controllers/UserController.js 95.34% <0.00%> (-2.33%) ⬇️
src/Controllers/FilesController.js 92.00% <0.00%> (-2.00%) ⬇️
src/ParseServerRESTController.js 97.01% <0.00%> (-1.50%) ⬇️
src/Controllers/index.js 96.66% <0.00%> (-1.12%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 79.50% <0.00%> (-0.82%) ⬇️
src/Routers/UsersRouter.js 93.82% <0.00%> (-0.57%) ⬇️
src/middlewares.js 96.92% <0.00%> (-0.44%) ⬇️
src/RestWrite.js 93.78% <0.00%> (-0.32%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e0d408...b259d35. Read the comment docs.

@mtrezza
Copy link
Member

mtrezza commented Aug 30, 2021

Thanks for opening this PR!

If this is in reference to #3857, could you please add this reference to the PR description?

@ghost
Copy link
Author

ghost commented Aug 31, 2021

Thanks for opening this PR!

If this is in reference to #3857, could you please add this reference to the PR description?

Sure, I've now added a reference to that issue.

@mtrezza
Copy link
Member

mtrezza commented Sep 2, 2021

Looking closer, I am hesitant to approve this PR, because:

  1. There may be a custom URL scenario in which the browser URL parsing in this PR breaks the button.
  2. These HTML pages are not Parse Server internals but merely example pages which are unlikely to be used in their original form, without branding or style customization. These pages are likely heavily adapted and any change would be unlikely to make it into the HTML pages actually in use.

The likeliness that this PR would either break something or it doesn't have any effect at all, maybe not be worth the risk. Considering that this change will be obsolete in 3-4 months, with the release of Parse Server 5.0 and the Pages Router, in which the button URL will be fully provided by the server:

<form method="POST" action="{{{publicServerUrl}}}/apps/{{{appId}}}/resend_verification_email">

The Pages Router is still flagged as "experiemental" in Parse Server 5.0 but in reality it has been used heavily in production systems since March 2021, without a single bug being reported. So this PR would really only be relevant if backported for Parse Server 4.x releases, which we are unlikely to do, because of the reasons above.

I still think that your solution in this PR could be helpful to others if they come across the same issue and the solution works for them.

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.

1 participant