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

Setting multiple field presets for role not working #10782

Closed
3 tasks done
NickSettler opened this issue Dec 31, 2021 · 8 comments · Fixed by #11423
Closed
3 tasks done

Setting multiple field presets for role not working #10782

NickSettler opened this issue Dec 31, 2021 · 8 comments · Fixed by #11423
Assignees
Labels
Milestone

Comments

@NickSettler
Copy link

NickSettler commented Dec 31, 2021

Preflight Checklist

Describe the Bug

I am trying to make something kind of register. So I gave custom create permissions for the public role on directus_users collection. But when trying to set multiple field presets it's not working. I'm setting the field presets in a such way

{
    "status": "active",
    "role": "54303c36-facc-4323-8eee-469f3afe2900"
}

But as the result, when I'm using endpoint for users creation, there's only status field set in a new user. Role field is empty. And it doesn't changes if I change the order of presets

To Reproduce

  1. Install a clean Directus
  2. Make a role like User
  3. Make users creation api endpoint public and set field presets including role and something else
  4. Try to create a user without authorization using endpoint
  5. Check the user's role and other fields should be filled by preset

Errors Shown

When I'm trying to login into this users right after I've created it using api, I see the following error from login endpoint. And as i realized this happens because the role is not set from the preset

{
    "errors": [
        {
            "message": "Invalid user credentials.",
            "extensions": {
                "code": "INVALID_CREDENTIALS"
            }
        }
    ]
}

What version of Directus are you using?

9.4.1

What version of Node.js are you using?

14.18.0

What database are you using?

Postgres 12

What browser are you using?

Chrome

What operating system are you using?

Ubuntu 20.04

How are you deploying Directus?

docker-compose

@NickSettler
Copy link
Author

I suppose that this issue #10576 and the related commited might be the reason why now it works in some incorrect way

@azrikahar
Copy link
Contributor

Hmm #10576 should just be concerning the App, not the API (when you are creating item with presets), so I believe it's not really related to it.

Seems like rather than that commit causing issue, it's more so it didn't resolve this completely, since the parseFilter wasn't removed on the API side over here:

permission.presets = parseFilter(permission.presets, accountability!, filterContext);

Definitely an oversight. I've just tested removing that line and your issue will be resolved 👍


That said, removing the parsing flat out will remove the possibility of using dynamic variables instead as stated here, so we'll probably need to tackle that aspect together when fixing this. Perhaps an offshoot of parseFilter, maybe parsePresets which uses parseDynamicVariables only? 🤔 That way we can add it to the App side as well. paging @rijkvanzanten @licitdev for thoughts on this

@rijkvanzanten
Copy link
Member

@azrikahar Yeah agreed on that. We should extract the logic that handles the actual replacement of those variables though, so we can re-use it between the two functions 👍🏻

@brandnewx
Copy link

I believe Directus need a lot more new unit tests to make sure that we won't see bug regressions. Unit tests will ensure that things that work will stay working, after new code are added and there are accidental oversights. Without unit tests, things will randomly break here and there.

@benhaynes
Copy link
Sponsor Member

We agree! We're actively working on tests... but it's slow going with our small team. We'd love an extra set of hands if you're interested in contributing!

@scouech
Copy link

scouech commented Jan 25, 2022

@rijkvanzanten no way around that at the moment? Right now users created don't get the right role added, so they don't get the right privileges which is quite a problem for a user based project 🤓

@RANSoftRA

This comment has been minimized.

@RANSoftRA
Copy link

RANSoftRA commented Jan 27, 2022

We were able to work around that issue, because our use case is rather simple:

In our case (when a user registers =public role, then the user should be assigned to the role "Customer") we were able to overcome this issue by creating a custom Hook on users.create action:

Example:

[directus-root]/extensions/hooks/custom/index.js

module.exports = function registerHooks({action}, {services, exceptions}) {

    const {UsersService} = services;

    action('users.create', async ({payload}, {schema}) => {

        // public role tries to create user and no payload role is set - set default role
        if (!context.accountability.role && !input.payload.role) {
            ...
            console.log('Setting role for ' + payload.email + ' to default role');
            const userService = new UsersService({schema: schema});

            await userService.updateOne(input.key, {
                    'role': '<your-role-id-here>'
                });
           ...
        }
    });
};

In this case we just assign a defaultRole, if no role was specified.

@rijkvanzanten rijkvanzanten added this to the v9-next milestone Jan 27, 2022
@rijkvanzanten rijkvanzanten self-assigned this Feb 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants