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

[BUG] Access Control lower cases resources intermittently #6004

Closed
JoeGaffney opened this issue May 30, 2024 · 6 comments · Fixed by #6021, #6058 or #6105
Closed

[BUG] Access Control lower cases resources intermittently #6004

JoeGaffney opened this issue May 30, 2024 · 6 comments · Fixed by #6021, #6058 or #6105
Assignees
Labels
bug Something isn't working pr-ready
Milestone

Comments

@JoeGaffney
Copy link

JoeGaffney commented May 30, 2024

Describe the bug

Sometimes resource names can get changed to lower case even though they are Camel cased.

            {
              name: "auditLogs",
              list: "/administration/auditLogs",
              show: "/administration/auditLogs/show/:id",
              meta: {
                icon: <></>,
                parent: "administration",
                label: "Audit Logs",
              },
            },

Will then appear as auditlogs in the access control Can method

Steps To Reproduce

Create a camelCase resouce.

Log the resource name in the accessControl.

export const accessControlProvider = {{
can: async ({{ resource, action, params }}) => {{
console.log(resource)
console.log("Resource Name:", params?.resource?.name);
...
}},
}};

Expected behavior

Not to change the casing

Packages

  • @refine-dev/core

Additional Context

Was mentioned on discord to put a ticket in @aliemir said he has a solution.

Potentially somehow related to kbar?

@JoeGaffney JoeGaffney added the bug Something isn't working label May 30, 2024
@BatuhanW BatuhanW added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 3, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Jun 3, 2024

Hey @JoeGaffney thanks for the issue. We are open to contributions for this one.

@JayBhensdadia
Copy link
Contributor

JayBhensdadia commented Jun 4, 2024

Hey @BatuhanW
i would like to work on this issue.
can you please assign it to me?

@BatuhanW
Copy link
Member

BatuhanW commented Jun 5, 2024

Hey @JayBhensdadia assigned to you.

Please make sure to thoroughly read and follow our contribution guide. PRs that doesn't follow our contribution guide, will be closed.

https://refine.dev/docs/guides-concepts/contributing/

@JayBhensdadia
Copy link
Contributor

sure

@aliemir
Copy link
Member

aliemir commented Jun 6, 2024

This issue is related with our sider components (ThemedSiderV2 etc.). In the <CanAccess /> wrappers of the menu items we're sending the resource.name with toLowerCase(). Since we don't have this transform in any other usages of useCan or <CanAccess /> I don't think this is going to be a breaking change because user already needs to handle both lower cased and non-transformed resource names 🤔

@JoeGaffney
Copy link
Author

ah that makes sense with the pattern I was seeing ( some cases changes and others not)

I did have a resource defined with only a show and not a list in the resources that was camel and was not affected. So would not appear in the side panel and thus probably not go through the canAccess first from the sider

@BatuhanW BatuhanW added this to the July Release milestone Jun 10, 2024
@BatuhanW BatuhanW added pr-ready and removed good first issue Good for newcomers help wanted Extra attention is needed labels Jun 13, 2024
@BatuhanW BatuhanW linked a pull request Jul 5, 2024 that will close this issue
@BatuhanW BatuhanW closed this as completed Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment