Skip to content
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

Require login with Account Activation doesn't work #19

Open
joehoyle opened this issue May 20, 2019 · 8 comments
Open

Require login with Account Activation doesn't work #19

joehoyle opened this issue May 20, 2019 · 8 comments
Labels
bug Existing functionality isn't behaving as expected could have Could be done, or nice to have, low priority for now

Comments

@joehoyle
Copy link
Member

If you click the wp-signup/activate link in the email, require-login will redirect you to login first.

@joehoyle joehoyle added the bug Existing functionality isn't behaving as expected label May 20, 2019
@rmccue rmccue added this to the 2.0 Beta milestone Jun 12, 2019
@joehoyle joehoyle removed this from the 2.0 Beta milestone Jul 17, 2019
@rmccue rmccue added this to the 3.0 milestone Aug 29, 2019
@roborourke
Copy link
Contributor

Needs RFC, @joehoyle to write up current thinking on this problem.

@roborourke roborourke removed this from the 3.0 milestone Nov 6, 2019
@roborourke
Copy link
Contributor

Any update / spec for this @joehoyle? I can't remember what we were discussing with regards to an RFC, perhaps rolling our own activate.php dropin?

@joehoyle
Copy link
Member Author

joehoyle commented Apr 1, 2020

So I delved into this a bit more a while back, writing up what I foudn at the time. I quite quickly found myself in a rabbit role of messy, convoluted code that is the wp-activate / sign-up flow.

Account Activations are quite tied in with site signups. That doesn't have to be the case -- Multisite has privision to allow users be added without a site, but there's a lot of overlap and inconsistnecy in how this is applied. It just makes working with all this code quite laborious.

But, what about the issue at hand? Basically the activate-account link sent in the activate email won't work, because the site thinks you need to be logged in. I tried adding excludes to require-login for wp-activate.php. I then found the activation link and flow was going down "the wrong path" of site creation, which was triggering different urls, and that's where I basically came to a holt.

Thinking about it now, it could have been I had something misconfigured that meant WP thought I was trying to sign up for a site, when I infact wasn't, I just wanted to "confirm" the user-creation... but it didn't end up being that simple.

So, from here we could have another crack at that -- try add the right exlcudes to the require-login plugin for the activation flow, or we could implement our own activate account endpoint and avoid a lot of that mess of WP. I don't think in this normal flow we ever want site-creation to be part of activation for example.

I think it would be worth having another go at the approach I took, with someone that has a bit more time to invest into this.

@rmccue
Copy link
Member

rmccue commented Apr 1, 2020

Just to note, I solved this in https://github.com/humanmade/vantage-auth/pull/37 (private repo) and it requires a decent amount of work (essentially, a custom login page; I used a theme because it was easier there, but in altis-security it should use a different setup).

@jennybeaumont
Copy link

What are our next steps here? This seems pretty stale.

@rmccue rmccue added the could have Could be done, or nice to have, low priority for now label Jan 26, 2021
@roborourke
Copy link
Contributor

Work in progress: humanmade/hm-require-login#10

Needs re-review and testing but should be good to go.

@yumito
Copy link
Contributor

yumito commented Oct 11, 2021

Why is this bug estimated at 3 points - do we plan to time box the fix?
If so, let's keep the estimate and add a time box period in the description. If not - let's remove it.

@roborourke
Copy link
Contributor

@yumito Might've been from before we stopped estimating bugs. Fine to remove it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Existing functionality isn't behaving as expected could have Could be done, or nice to have, low priority for now
Projects
None yet
Development

No branches or pull requests

5 participants