8000 feat: add ability to return missing perms by rxri · Pull Request #2238 · better-auth/better-auth · GitHub
[go: up one dir, main page]
Skip to content

feat: add ability to return missing perms #2238

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

feat: add ability to return missing perms #2238

wants to merge 19 commits into from
+728 −118

Conversation

rxri
Copy link
Contributor
@rxri rxri commented Apr 12, 2025 •
edited
Loading

need some opinion about it

that would be a great feature for developers that wanna return the missing permissions or just see in the code themselves

all 8000 tests (existing & new ones) pass after modifying authorize fn
functions are backwards compatible (if returnMissingPermissions is false or undefined` they will return boolean)

@mrge-io Mrge IO
Copy link
mrge-io bot commented Apr 12, 2025

Review this PR on mrge.io

@vercel Vercel
Copy link
vercel bot commented Apr 12, 2025

@rxri is attempting to deploy a commit to the better-auth Team on Vercel.

A member of the Team first needs to authorize it.

@pkg-pr-new pkg.pr.new
Copy link
pkg-pr-new bot commented Apr 12, 2025 •
edited
Loading

Open in StackBlitz

npm i https://pkg.pr.new/better-auth/better-auth@2238

npm i https://pkg.pr.new/better-auth/better-auth/@better-auth/cli@2238

npm i https://pkg.pr.new/better-auth/better-auth/@better-auth/expo@2238

npm i https://pkg.pr.new/better-auth/better-auth/@better-auth/stripe@2238

commit: 659ba22

mrge-io[bot]
mrge-io bot reviewed Apr 12, 2025
View reviewed changes
Copy link
@mrge-io mrge-io bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mrge found 11 issues. View them in mrge.io

@rxri
Copy link
Contributor Author
rxri commented Apr 12, 2025 •
edited
Loading

btw I think this should also be added to api-key plugin

@rxri
Copy link
Contributor Author
rxri commented Apr 18, 2025

@Bekacru had any chance to look at this? Since you said you may have different idea for it ^^

I also added has-permission route to api-key plugin since it would be really useful to just check permissions of the api-key and getting the missing permissions back, without the need of verifying the api key

rxri added 6 commits April 19, 2025 01:07
@rxri
docs: close block
b9aae92
@rxri
fix(access): handle undefined statements
b4d4496 
If api key or user would have no permissions at all, the statements would be `null` or `undefined` leading to `authorize` function not returning anything at all. We need to check if `statements` is null/undefined and assign empty object so function can run properly and return missing permissions
@rxri
perf(access): optimize authorize, checkArrayActions & `checkObjec…
63b8e2c 
…tActions`
@rxri
feat(access): use Object.hasOwn
58e2ba6
@rxri
docs: wording
4540ffa
@rxri
docs: wording
5720fcb
@ping-maxwell ping-maxwell self-assigned this Apr 20, 2025
@ping-maxwell
Copy link
Collaborator

Cool idea.

@rxri rxri changed the title feat(admin/organization): add ability to return missing perms feat: add ability to return missing perms Apr 30, 2025
@Bekacru Bekacru added this to the 1.2.8 milestone May 9, 2025
@jazzberry-ai Jazzberry AI
Copy link
jazzberry-ai bot commented May 9, 2025 •
edited
Loading

Bug Report

Name Severity Example test case Description
API Key Permission Bypass Critical User A uses API key created for User B to gain permissions. The hasPermissionApiKey route in packages/better-auth/src/plugins/api-key/routes/has-permission-api-key.ts does not verify that the API key belongs to the user making the request, allowing any user to use any API key to bypass access controls.

Comments? Email us.

@rxri
Copy link
Contributor Author
rxri commented May 9, 2025

server only endpoint tho ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrge-io mrge-io[bot] mrge-io[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@ping-maxwell ping-maxwell

Labels
None yet
Projects
None yet
Milestone
1.2.8
Development

Successfully merging this pull request may close these issues.
3 participants
@rxri @ping-maxwell @Bekacru
0