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.

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

feat(acl): support more JWT algorithms for ACL #8912

Merged
merged 2 commits into from
Aug 25, 2023
Merged

feat(acl): support more JWT algorithms for ACL #8912

merged 2 commits into from
Aug 25, 2023

Conversation

rderbier
Copy link
Member

@rderbier rderbier commented Jul 19, 2023 •
edited by mangalaman93
Loading

Description: Until now, we only supported HS256 algorithm for signing and verifying JWT algorithms. This PR adds support for longer keys (HS384, HS512) as well as asymmetric algorithms such as RS, RSS-PSS, ES for various key and shalengths.
Fixes: #8814
Closes: https://dgraph.atlassian.net/browse/DGRAPHCORE-95

@dgraph-bot dgraph-bot added area/testing Testing related issues area/enterprise Related to proprietary features area/core internal mechanisms go Pull requests that update Go code labels Jul 19, 2023
mangalaman93
mangalaman93 requested changes Jul 19, 2023
View reviewed changes
Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

Thanks Raphael for doing this. We should also add integration test for public-private key case. Happy to help if you need any.

dgraph/cmd/alpha/run.go Outdated Show resolved Hide resolved
edgraph/access_ee_test.go Outdated Show resolved Hide resolved
ee/keys_ee.go Outdated Show resolved Hide resolved
x/jwt_helper.go Outdated Show resolved Hide resolved
edgraph/access_ee_test.go Outdated Show resolved Hide resolved
@mangalaman93 mangalaman93 force-pushed the raphael/acl branch 3 times, most recently from 777be30 to e0d9a33 Compare August 7, 2023 19:30
@mangalaman93 mangalaman93 changed the title feat(acl): accept external RS256 signed JWT for ACL feat(acl): support more JWT algorithms for ACL Aug 7, 2023
@mangalaman93 mangalaman93 marked this pull request as ready for review August 7, 2023 19:34
@mangalaman93 mangalaman93 force-pushed the raphael/acl branch 9 times, most recently from ec83bcc to 68d1598 Compare August 9, 2023 17:13
@rderbier
Copy link
Member Author

rderbier commented Aug 10, 2023 •
edited
Loading

Note: This PR is a first step to fix #8814.
To really get to the point where we accept an external login, we need to be able to start the alpha with an async algo (RS256 for example) and a public validation key for which we don't have the private key.
That means that login in Dgraph can be done by the Dgraph private key.
But JWT decoding is done using graph public key ( so user going through Dgraph login will be accepted) and the external public key ( so user doing an external login but presenting a JWT we can decode/ verify and in which we have the userid and groups info will also be accepted).

@mangalaman93 mangalaman93 force-pushed the raphael/acl branch 4 times, most recently from a235dfc to c0d92bb Compare August 15, 2023 04:47
@mangalaman93 mangalaman93 force-pushed the raphael/acl branch 2 times, most recently from b8399e8 to 240235a Compare August 15, 2023 06:48
@mangalaman93 mangalaman93 self-requested a review August 15, 2023 07:44
mangalaman93
mangalaman93 previously approved these changes Aug 15, 2023
View reviewed changes
harshil-goel
harshil-goel previously approved these changes Aug 16, 2023
View reviewed changes
@mangalaman93 mangalaman93 self-assigned this Aug 18, 2023
@mangalaman93 mangalaman93 force-pushed the raphael/acl branch 5 times, most recently from 04c1427 to 174eba2 Compare August 22, 2023 17:38
mangalaman93
mangalaman93 previously approved these changes Aug 22, 2023
View reviewed changes
mangalaman93
mangalaman93 previously approved these changes Aug 24, 2023
View reviewed changes
@mangalaman93 mangalaman93 merged commit 4e89c4e into main Aug 25, 2023
@mangalaman93 mangalaman93 deleted the raphael/acl branch August 25, 2023 13:10
shivaji-kharse pushed a commit that referenced this pull request Mar 12, 2024
@rderbier @mangalaman93
feat(acl): support more JWT algorithms in ACL (#8912)
aad7343 
Description: Until now, we only supported HS256 algorithm for signing
and verifying JWT algorithms. This PR adds support for longer keys
(HS384, HS512) as well as asymmetric algorithms such as RS, RSS-PSS,
ES for various key and SHA lengths.

Partially Fixes: #8814
Closes: https://dgraph.atlassian.net/browse/DGRAPHCORE-95

---------

Co-authored-by: Aman Mangal <[email protected]>
This was referenced Jun 26, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshil-goel harshil-goel harshil-goel approved these changes

@mangalaman93 mangalaman93 mangalaman93 approved these changes

@meghalims meghalims Awaiting requested review from meghalims

@sanjayk-github-dev sanjayk-github-dev Awaiting requested review from sanjayk-github-dev

@all-seeing-code all-seeing-code Awaiting requested review from all-seeing-code

@billprovince billprovince Awaiting requested review from billprovince

@joshua-goldstein joshua-goldstein Awaiting requested review from joshua-goldstein

Assignees

@mangalaman93 mangalaman93

Labels
area/core internal mechanisms area/enterprise Related to proprietary features area/testing Testing related issues go Pull requests that update Go code
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] ACL accepts externally generated JWT token
4 participants
@rderbier @mangalaman93 @harshil-goel @dgraph-bot