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

[v4] Error when logging in #5689

Closed
MikeHarrison opened this issue Sep 24, 2023 · 20 comments
Closed

[v4] Error when logging in #5689

MikeHarrison opened this issue Sep 24, 2023 · 20 comments
Milestone

Comments

@MikeHarrison
Copy link

Description

I have installed the latest Beta, and created a new account by going to /panel and entering my details. When I log out and try to log back in using those credentials I get an "Invalid Login" error message.

Expected behavior
A successful login

To reproduce

  1. Install Kirby V4 Beta 1
  2. Go to /panel and create a new admin account
  3. Log out
  4. Try and log in using the newly created account

This is the PHP error log of the login, it looks like the user email isn't being picked up:

[24-Sep-2023 19:35:31 UTC] Kirby\Exception\NotFoundException: The user "" cannot be found in /Users/mike/Dropbox/boilerplate/kirby/src/Cms/Auth.php:525
Stack trace:
#0 /Users/mike/Dropbox/boilerplate/kirby/src/Cms/Auth.php(392): Kirby\Cms\Auth->validatePassword('', 'password')
#1 /Users/mike/Dropbox/boilerplate/kirby/config/api/routes/auth.php(70): Kirby\Cms\Auth->login('', 'password', false)
#2 [internal function]: Kirby\Cms\Api->{closure}()
#3 /Users/mike/Dropbox/boilerplate/site/plugins/kirby-uniform/vendor/getkirby/cms/src/Api/Api.php(193): Closure->call(Object(Kirby\Cms\Api))
#4 /Users/mike/Dropbox/boilerplate/site/plugins/kirby-uniform/vendor/getkirby/cms/src/Cms/Api.php(49): Kirby\Api\Api->call('auth/login', 'POST', Array)
#5 /Users/mike/Dropbox/boilerplate/site/plugins/kirby-uniform/vendor/getkirby/cms/src/Api/Api.php(454): Kirby\Cms\Api->call('auth/login', 'POST', Array)
#6 /Users/mike/Dropbox/boilerplate/kirby/config/routes.php(47): Kirby\Api\Api->render('auth/login', 'POST', Array)
#7 [internal function]: Kirby\Http\Route->{closure}('auth/login')
#8 /Users/mike/Dropbox/boilerplate/site/plugins/kirby-uniform/vendor/getkirby/cms/src/Http/Router.php(122): Closure->call(Object(Kirby\Http\Route), 'auth/login')
#9 /Users/mike/Dropbox/boilerplate/kirby/src/Cms/App.php(337): Kirby\Http\Router->call('api/auth/login', 'POST')
#10 /Users/mike/Dropbox/boilerplate/kirby/src/Cms/App.php(1190): Kirby\Cms\App->call('api/auth/login', 'POST')
#11 /Users/mike/Dropbox/Boilerplate/index.php(5): Kirby\Cms\App->render()
#12 {main}

Your setup

Kirby Version
V4 Beta 1

Your system (please complete the following information)

  • Device: Mac Mini 2018
  • OS: MacOS Ventura 13.4
  • Browser: Firefox
  • Version: 117
@afbora afbora added needs: information ❓ Requires more information to proceed needs: replication 🔬 Requires a sample to reproduce the issue and removed needs: information ❓ Requires more information to proceed needs: replication 🔬 Requires a sample to reproduce the issue labels Sep 24, 2023
@afbora
Copy link
Member

afbora commented Sep 24, 2023

I can reproduce the issue only autofilled inputs in Firefox. Same for you as well?

@afbora
Copy link
Member

afbora commented Sep 24, 2023

I think this is Firefox bug that introduced in v110 and will be fixed v118 (currently v117): https://bugzilla.mozilla.org/show_bug.cgi?id=1817926

@lukaskleinschmidt
Copy link
Contributor

lukaskleinschmidt commented Sep 25, 2023

Sadly not fixed in 118. I have a few clients using Firefox having this problem and not sure how to proceed.
I did some testing and this is kind of my test setup:

panel.plugin('app/site', {
  components: {
    'k-email-input': {
      extends: 'k-email-input',
      mounted() {
        console.log('$props:', this.$props.value);
        console.log('$listeners:', this.$listeners);

        this.$refs.input.addEventListener('input', event => {
          console.log('input', event.target.value);
          this.onInput(event.target.value);
        });

        // for (let i = 0; i < 20; i++) {
        //   setTimeout(() => {
        //     console.log(i, this.$refs.input.value, this.$props.value);
        //   }, i);
        // }

        // setTimeout(() => {
        //   this.onInput(this.$refs.input.value);
        // }, 20);
      }
    }
  },
});

The listeners seem to be set correctly. I replicated the input event and the input value is actually correct.
But this.onInput(event.target.value) does not seem to work here (yet).

If you uncomment the last setTimeout block everything works as expectet. Not really sure where this problem comes from.
The middle part was just for testing when the browser autofill actually kicks in (for me around 9-16 ms) and it also correctly fires the input event.

@lukaskleinschmidt
Copy link
Contributor

Just saw the title. This issue is not exclusive to v4

@bastianallgeier bastianallgeier changed the title [V4] Error when logging in Error when logging in Sep 25, 2023
@MikeHarrison
Copy link
Author

I can reproduce the issue only autofilled inputs in Firefox. Same for you as well?

Hi, yes I didn't mention sorry. These are autofilled inputs, generated by NordPass Addon

@afbora
Copy link
Member

afbora commented Sep 25, 2023

@distantnative I think this issue seems to have arisen again: #4962

@afbora
Copy link
Member

afbora commented Sep 25, 2023

@lukaskleinschmidt I've tested on 3.9.6.1 and works as expected (seems already fixed in #4962 but I think came back to with v4). The issue happened only in v4 for me.

@lukaskleinschmidt
Copy link
Contributor

My bad. Just checked again. The version I tested on was the 4.0.0-alpha.3. So old design but still the v4 😅

@afbora afbora changed the title Error when logging in [v4] Error when logging in Sep 25, 2023
@afbora afbora added this to the 4.0.0 milestone Sep 25, 2023
@afbora
Copy link
Member

afbora commented Sep 25, 2023

@distantnative Here related commit that broke the #4962 PR.

@distantnative
Copy link
Member

Swapping v-model with value/@input should be quite the same. We need this also for Vue 3 so I don't think reverting it is an option. We'll need to find out where's the bug with Firefox.

@lukaskleinschmidt
Copy link
Contributor

@distantnative out of curiosity. Why would you want/need to remove v-model for Vue 3?

@distantnative
Copy link
Member

@lukaskleinschmidt Cause v-model undergoes a massive breaking change to my understanding - what events need to be emitted and to which prop the value gets passed.

@lukaskleinschmidt
Copy link
Contributor

You mean from v2 to v3? Or additional changes down the road in v3?
If it is the first one I get the intention behind the change.

But v-model actually does a few more things under the hood on native input elements. At least in my understanding. Perhaps it is just some order of execution where v-model has the edge over a custom implementation?

From the Vue 2/3 docs:

Note
v-model will ignore the initial value, checked or selected attributes found on any form elements. It will always treat the current bound JavaScript state as the source of truth. You should declare the initial value on the JavaScript side, using reactivity APIs.

So not sure what kind of additional magic is happening in the background that makes the v-model implementation work. My guess is just an appropriate debounce or delay that v-model takes into account already for such autofill cases?

@distantnative
Copy link
Member

v2 -> v3

Breaks completely any setup that allows external components (like plugin components) when using v-model

@distantnative
Copy link
Member

@bastianallgeier what's your take on this: return to v-model to just solve this for now and leave our struggles for the future?

@afbora
Copy link
Member

afbora commented Oct 10, 2023

@distantnative I've found source of issue and have a great clue about the issue. I'm pretty sure that this issue is related with merging values. I'm sure you have the correct fix for that.

Works great with the following change:

- this.values = { ...this.value, [name]: value };
+ this.values[name] = value;

https://github.com/getkirby/kirby/blob/4.0.0-beta.2/panel/src/components/Forms/Fieldset.vue#L126

@bastianallgeier
Copy link
Member

@afbora could you test the fix on v4/fix/fieldset-reactivity?

@afbora
Copy link
Member

afbora commented Oct 11, 2023

@bastianallgeier Login works great for me in Firefox 👍

@bastianallgeier
Copy link
Member

I'll create a PR

@mountbatt
Copy link

i found a workaround on beta 2. it has something todo with the autoprefill of the mail adress. if i add an blank at the and with my keyboard and delete it then the field gets "recognized" and the login works. but this only happens on a windows 10 machine in edge and firefox. on all macs i tested it works without any workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants