sqrldev / wordpress-sqrl-login

SQRL Login WordPress plugin
MIT License
21 stars 7 forks source link

Undefined variables #55

Closed cjhaas closed 4 years ago

cjhaas commented 4 years ago

Describe the bug There are a couple of undefined variables in the code that PhpStorm is picking up right away.

On line 676 there's $wp_users which isn't defined yet in that scope: https://github.com/sqrldev/wordpress-sqrl-login/blob/65531622453a2ed430f32aa118e44a7f0683ee21/sqrl-login.php#L676

On lines 1281 (and possibly 1287) there's $login_url: https://github.com/sqrldev/wordpress-sqrl-login/blob/65531622453a2ed430f32aa118e44a7f0683ee21/sqrl-login.php#L1281

To Reproduce Expected behavior Screenshots Desktop (please complete the following information): Smartphone (please complete the following information): n/a

Additional context The first one is also showing up in my debug log when enabled. It might be able to use a call to wp_get_current_user() but I'm not sure 100% if that plays into the login cycle correctly.

The second one I think can just have the following added before it: $login_url = site_url( 'wp-login.php', 'https' );

sanzeeb3 commented 4 years ago

$wp_users[0] should probably be $session['user'] and let login URL be site_url( 'wp-login.php', 'login' );

kalaspuffar commented 4 years ago

Hi @cjhaas

I don't have PHPStorm but if you want to do similar changes to the code PRs is welcome :) I think that would be more efficient than going back and forth with small changes.

Otherwise, I will make these changes as soon as I get some time. :)

Best regards Daniel

cjhaas commented 4 years ago

Thanks @sanzeeb3 and @kalaspuffar.

I created a PR with 8 commits for various things that PhpStorm picked up, hopefully splitting them up makes them easier to decide what you want to keep and what you want to leave as-is for now.

There's still a couple of other things, specifically in api_callback about variable typing, but I didn't want to introduce too many changes right now.

kalaspuffar commented 4 years ago

PR is merged.