Review _user API mod

[dbauszus-glx]

The _user API methods should each individually check whether the ACL is provided and user admin rights are required.

The admin view method was pushed into it's own mod file user/admin.js

The user/delete API will now allow for a logged in user to remove themselves.

[AlexanderGeere]

Looks good to me

[RobAndrewHurst]

I am happy for this to be reviewed. @simon-leech Do you still want to write some tests for this? Or should I try give it a crack?

[simon-leech]

@RobAndrewHurst Happy for you to pick up the tests then I can review the PR and tests all in one go?

[RobAndrewHurst]

Sure!

[RobAndrewHurst]

@RobAndrewHurst Happy for you to pick up the tests then I can review the PR and tests all in one go?

The main problem with testing these modules is that we can't test them via the CLI. We would want to write a login_test similar to the test_view to the run on deployed instances. This will then fully test the ACL.

I don't see a point in mocking the ACL in the CLI.

Let me know what you think?

@AlexanderGeere @dbauszus-glx @simon-leech

[simon-leech]

@RobAndrewHurst Happy for you to pick up the tests then I can review the PR and tests all in one go?

The main problem with testing these modules is that we can't test them via the CLI. We would want to write a login_test similar to the test_view to the run on deployed instances. This will then fully test the ACL.

I don't see a point in mocking the ACL in the CLI.

Let me know what you think?

@AlexanderGeere @dbauszus-glx @simon-leech

@RobAndrewHurst Happy for you to pick up the tests then I can review the PR and tests all in one go?

The main problem with testing these modules is that we can't test them via the CLI. We would want to write a login_test similar to the test_view to the run on deployed instances. This will then fully test the ACL.

I don't see a point in mocking the ACL in the CLI.

Let me know what you think?

@AlexanderGeere @dbauszus-glx @simon-leech

@RobAndrewHurst Agreed - sounds quite tricky to mock, maybe we can add a login_test view as a separate PR? Once this one is in?

[RobAndrewHurst]

@RobAndrewHurst Agreed - sounds quite tricky to mock, maybe we can add a login_test view as a separate PR? Once this one is in?

Sounds good!

[dbauszus-glx]

This is back into draft. There was a couple of issues especially in regards to the auth mod, token and session check.

The session check did not work when no session field is available in ACL. This was failing silently with the request authenticating.

The session and token check are now individual documented methods.

[dbauszus-glx]

The update method as called from the admin panel will now do the verification bits.

  let verification_by_admin = ''
  if (req.params.field === 'verified' && req.params.value === true) {

    verification_by_admin = `
      , password = password_reset
      , password_reset = NULL
      , failedattempts = 0
      , verificationtoken = NULL
      , approved = true
      , approved_by = '${req.params.user.email}|${ISODate}'
    `
  }

  // Get user to update from ACL.
  const rows = await acl(`
    UPDATE acl_schema.acl_table
    SET
      ${req.params.field} = $2
      ${verification_by_admin}
      ${approved_by}
    WHERE lower(email) = lower($1);`,
    [email, req.params.value])

  if (rows instanceof Error) {
    return res.status(500).send('Failed to access ACL.')
  }

The verification can be called directly by an admin.

http://localhost:3000/api/user/update?email=dennis@mail.com&field=verified&value=true

[RobAndrewHurst]

Can we force the users verification flag be set to false when they reset their passwords? So that when an admin user goes to reset their password via the verification flag they don't have to toggle it from true to false to true.

[dbauszus-glx]

@RobAndrewHurst No, we cannot. Otherwise I could set your verification to false knowing your email address.

Anybody could... I can anyways being an administrator. But you get what I mean.

[RobAndrewHurst]

We then need a way for Admin's to easily see who needs to be verified again

[dbauszus-glx]

I added a column re_verification which is false if there user [record] has a verification token. If clicked on this the the verified true update query will be sent and the verified and approved columns will be toggled after successful update query.

[RobAndrewHurst]

Resetting a users email (roberthurstsa@gmail.com) I don't see a green tick in the outstanding password reset column.

But after re-verifying the user through the admin panel, I then see a green tick for the user. Seems as if the logic needs to be reversed?

^^ This is after re-verifying the user.

[dbauszus-glx]

Green tick means that the user is fully verified. Happy for you to reverse the logic.

[RobAndrewHurst]

Green tick means that the user is fully verified. Happy for you to reverse the logic.

Ah ok! If you put it like that I am happy with it!

[RobAndrewHurst]

Should we change the message returned from this to maybe "Your session has been terminated. Please login again."

[RobAndrewHurst]

Maybe something like this.

[RobAndrewHurst]

On vanilla email templates we don't see the host of the email that is being sent. In this case it should be localhost:300. I set the ALIAS in the .env and I don't see it coming through.

[dbauszus-glx]

@RobAndrewHurst The issue with missing host should be resolved in last commit.

The register request get's short circuited in the api module since it doesn't have user auth by definition.

Hence the reqHost util is required in the register module itself.

[RobAndrewHurst]

I have run through the ACL process. Looks all good!

[simon-leech]

I'm happy with this too, seems to be working from my testing!

[simon-leech]

It doesn't seem to want me to mark it as approved though idk why 😂